Skip to content

Roll packages #74671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dnfield merged 12 commits into from
Jan 26, 2021
Merged

Roll packages #74671

dnfield merged 12 commits into from
Jan 26, 2021

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Jan 25, 2021 •
edited
Loading

This updates vm_service and package:test* in particular.

I have not run all tests on this yet.

I've added some analysis ignores for files that will be rolled into g3 to hopefully make rolls easier.

@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Jan 25, 2021
@google-cla google-cla bot added the cla: yes label Jan 25, 2021
@dnfield dnfield marked this pull request as ready for review January 25, 2021 20:18
@dnfield dnfield requested a review from jonahwilliams January 25, 2021 20:18
jonahwilliams
jonahwilliams approved these changes Jan 25, 2021
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

dnfield
dnfield commented Jan 25, 2021
View reviewed changes
packages/flutter_tools/lib/src/isolated/devfs_web.dart
Future<void> initialize({bool soundNullSafety}) async {}

@override
Future<bool> updateDependencies(Map<String, ModuleInfo> modules) async => true;
Copy link
Contributor Author

@dnfield dnfield Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is not used in the monorepo and should be perfectly safe to roll.

Copy link
Contributor

@jonahwilliams jonahwilliams Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this something the tool needs to implement? @grouma

Copy link
Member

@grouma grouma Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Since you guys use the Frontend Server your dependencies are always up to date and it is always initialized with the correct null safety support.

packages/flutter_driver/test/flutter_driver_test.dart

@override
void dispose() {
Future<void> dispose() async {
Copy link
Contributor Author

@dnfield dnfield Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code won't cause problems in g3 but it potentially could.

Copy link
Contributor

@jonahwilliams jonahwilliams Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, that is what frob is for :)

@dnfield dnfield mentioned this pull request Jan 25, 2021
Merged
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jan 25, 2021

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux customer_testing has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac customer_testing has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Windows customer_testing has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite customer_testing-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield
Copy link
Contributor Author

dnfield commented Jan 25, 2021

This is failing due to some package conflicts with the new gallery.

This was referenced Jan 25, 2021
Merged
Merged
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jan 25, 2021

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux flutter_plugins has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield dnfield mentioned this pull request Jan 26, 2021
Merged
@dnfield
Copy link
Contributor Author

dnfield commented Jan 26, 2021

This is now blocked on Mockito.

Currently, there is no version 4.x of Mockito that supports a new enough analyzer to use a null-safe package:crypto.

Checking with @srawlins to see if we can do anything about that. If not, we might want to wait on this until after stable release.

@dnfield
Copy link
Contributor Author

dnfield commented Jan 26, 2021

Full error text, which is not showing up in CI logs unfortunately, is:

Because mockito >=4.1.3 <5.0.0-nullsafety.0 depends on analyzer >=0.39.15 <0.41.0 which depends on crypto ^2.0.0, mockito >=4.1.3 <5.0.0-nullsafety.0 requires crypto ^2.0.0.
And because every version of flutter_driver from sdk depends on crypto 3.0.0-nullsafety.0, mockito >=4.1.3 <5.0.0-nullsafety.0 is incompatible with flutter_driver from sdk.
So, because camera depends on both flutter_driver any from sdk and mockito ^4.1.3, version solving failed.
Running "flutter pub get" in camera...                                  
pub get failed (1; So, because camera depends on both flutter_driver any from sdk and mockito ^4.1.3, version solving failed.)

@dnfield
Copy link
Contributor Author

dnfield commented Jan 26, 2021

Mockito 4.1.4 was published to help with this.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jan 26, 2021

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux flutter_plugins has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield dnfield mentioned this pull request Jan 26, 2021
Merged
@dnfield
Copy link
Contributor Author

dnfield commented Jan 26, 2021

Tree is actually green

@dnfield dnfield merged commit d153832 into flutter:master Jan 26, 2021
@dnfield dnfield deleted the roll_pkg branch January 26, 2021 17:16
christopherfujino added a commit to chris-forks/flutter that referenced this pull request Jan 28, 2021
@christopherfujino
Revert "Roll packages to null safe (vm_service, test, etc.) (flutter#…
2ea11af 
…74671)"

This reverts commit d153832.
christopherfujino added a commit that referenced this pull request Jan 29, 2021
@christopherfujino
Revert "Roll packages to null safe (vm_service, test, etc.) (#74671)" (…
c40e529 
…#74937)

This reverts commit d153832.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grouma grouma grouma left review comments

+1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: tests "flutter test", flutter_test, or one of our tests c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dnfield @fluttergithubbot @grouma @jonahwilliams