Add and plumb `useImplicitPubspecResolution` across `flutter_tools`. by matanlurey · Pull Request #157879 · flutter/flutter

matanlurey · 2024-10-30T19:57:24Z

Work towards #157819. No behavior changes as a result of this PR.

Based on a proof of concept by @jonahwilliams (#157818).

The existence of this flag (which for the time being, defaults to true) implies the following:

The (legacy, deprecated) .flutter-plugins file is not generated:
https://docs.flutter.dev/release/breaking-changes/flutter-plugins-configuration
The (legacy, deprecated) package:flutter_gen is not synthetically generated:
Declare package:flutter_gen to be deprecated website#11343
(awaiting website approvers, but owners approve this change)

This change creates useImplicitPubspecResolution and plumbs it through as a required variable, parsing it from a FlutterCommand.globalResults where able. In tests, I've defaulted the value to true 100% of the time - except for places where the value itself is acted on directly, in which case there are true and false test-cases (e.g. localization and i10n based classes and functions).

I'm not extremely happy this needed to change 50+ files, but is sort of a result of how inter-connected many of the elements of the tools are. I believe keeping this as an explicit (flagged) argument will be our best way to ensure the default behavior changes consistently and that tests are running as expected.

jonahwilliams

LGTM

packages/flutter_tools/lib/src/run_hot.dart

packages/flutter_tools/lib/src/resident_runner.dart

andrewkolos · 2024-10-30T23:38:29Z

packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart

+    expect(l10nDirectory.existsSync(), true);
+    expect(l10nDirectory.childFile('app_localizations_en.dart').existsSync(),
+        true);
+    expect(
+        l10nDirectory.childFile('app_localizations.dart').existsSync(), true);


I think using the isTrue matcher instead of true would give a slightly better failure message if any of these expects were to fail.

I think if anything isFalse/isTrue will possibly be deprecated in the future: dart-lang/test#2358.

andrewkolos · 2024-10-30T23:39:54Z

packages/flutter_tools/test/general.shard/cold_test.dart

+  testUsingContext(
+      'Exits with code 2 when HttpException is thrown '
+      'during VM service connection', () async {


Unintentional formatting change

andrewkolos · 2024-10-30T23:40:10Z

packages/flutter_tools/test/general.shard/cold_test.dart

+        exception: const HttpException(
+            'Connection closed before full header was received, '


Unintentional formatting change

andrewkolos · 2024-10-30T23:40:39Z

packages/flutter_tools/test/general.shard/cold_test.dart

+      final List<FlutterDevice> devices = <FlutterDevice>[
+        flutterDevice1,
+        flutterDevice2
+      ];


Unintentional formatting change?

andrewkolos · 2024-10-30T23:41:21Z

packages/flutter_tools/test/general.shard/cold_test.dart

+  }) : super(device,
+            buildInfo: BuildInfo.debug,
+            generator: generator,
+            developmentShaderCompiler: const FakeShaderCompiler());


I think there are still unintentional formatting changes in this file

andrewkolos · 2024-10-30T23:43:53Z

I'm not extremely happy this needed to change 50+ files

I am not terribly worried about this since we won't have this for more than one release

andrewkolos

LGTM with nits

auto-submit · 2024-10-31T03:12:15Z

auto label is removed for flutter/flutter/157879, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-10-31T03:48:24Z

auto label is removed for flutter/flutter/157879, due to - The status or check suite Linux tool_integration_tests_5_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.

…tools`. (flutter/flutter#157879)

Manual roll requested by [email protected] flutter/flutter@fe71cad...0fe6153 2024-10-31 [email protected] Roll Flutter Engine from c40b0b602822 to f2154ef3e31c (8 revisions) (flutter/flutter#157926) 2024-10-31 [email protected] Hides added routes before top-most route finishes pushing (flutter/flutter#156104) 2024-10-31 [email protected] Fix cursor on hover expand/collapse icon (#155207) (flutter/flutter#155209) 2024-10-31 [email protected] Add test for `media_query_data.system_gesture_insets.0.dart` (flutter/flutter#157854) 2024-10-31 [email protected] Add and plumb `useImplicitPubspecResolution` across `flutter_tools`. (flutter/flutter#157879) 2024-10-31 [email protected] Roll Flutter Engine from 090c33aeae83 to c40b0b602822 (1 revision) (flutter/flutter#157896) 2024-10-31 [email protected] Roll Flutter Engine from 9295eeb6d3ce to 090c33aeae83 (4 revisions) (flutter/flutter#157893) 2024-10-30 [email protected] Roll Flutter Engine from 2bd854e23b61 to 9295eeb6d3ce (5 revisions) (flutter/flutter#157882) 2024-10-30 [email protected] Adds a new helpful tool exit message for SocketExceptions thrown during mdns discovery (flutter/flutter#157638) 2024-10-30 [email protected] Fix `GlowingOverscrollIndicator` examples (flutter/flutter#155203) 2024-10-30 [email protected] Roll Flutter Engine from 906a7ad88052 to 2bd854e23b61 (1 revision) (flutter/flutter#157878) 2024-10-30 [email protected] Upgrade templates to AGP 8.7/Gradle 8.10.2 (flutter/flutter#157872) 2024-10-30 [email protected] Roll Flutter Engine from 57ed5d338e7e to 906a7ad88052 (13 revisions) (flutter/flutter#157875) 2024-10-30 [email protected] Update Material 3 `LinearProgressIndicator` for new visual style (flutter/flutter#154817) 2024-10-30 [email protected] Roll Flutter Engine from 999797a2f690 to 57ed5d338e7e (5 revisions) (flutter/flutter#157833) 2024-10-30 [email protected] Add hidden `--no-implicit-pubspec-resolution` flag for one stable release. (flutter/flutter#157635) 2024-10-30 [email protected] Roll Packages from 028027e to 7cc1caa (5 revisions) (flutter/flutter#157864) 2024-10-30 [email protected] Mention partial PRs in the contributing docs (flutter/flutter#157863) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

auto-submit · 2024-11-03T14:32:27Z

Time to revert pull request flutter/flutter/157879 has elapsed.
You need to open the revert manually and process as a regular pull request.

…tools`." (#158076) Reverts #157879 to unblock flutter roll. Prerequisite reverts: #157934 Reason: b/377107864

…tools`. (flutter/flutter#157879)

matanlurey added 4 commits October 30, 2024 12:42

Add and plumb useImplicitPubspecResolution across flutter_tools.

e875c5c

++

5a32723

++

8d94fa1

++

9229b71

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 30, 2024

Unformat.

116442b

matanlurey requested review from andrewkolos and jonahwilliams October 30, 2024 20:10

matanlurey marked this pull request as ready for review October 30, 2024 20:13

jonahwilliams approved these changes Oct 30, 2024

View reviewed changes

andrewkolos reviewed Oct 30, 2024

View reviewed changes

packages/flutter_tools/lib/src/run_hot.dart Outdated Show resolved Hide resolved

packages/flutter_tools/lib/src/run_hot.dart Outdated Show resolved Hide resolved

packages/flutter_tools/lib/src/resident_runner.dart Outdated Show resolved Hide resolved

Unformat.

1bb3daa

matanlurey requested a review from andrewkolos October 30, 2024 22:32

andrewkolos reviewed Oct 30, 2024

View reviewed changes

andrewkolos approved these changes Oct 30, 2024

View reviewed changes

Unformat.

69db724

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2024

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2024

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2024

auto-submit bot merged commit fb02229 into flutter:master Oct 31, 2024

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Roll Flutter from fe71cadf1959 to 16622e67a051 (15 revisions) flutter/packages#7971

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2024

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

ae7d651

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Roll Flutter from fe71cadf1959 to b01558c1cd70 (16 revisions) flutter/packages#7972

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2024

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

895c02a

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Roll Flutter from fe71cadf1959 to 0fe615343bd9 (18 revisions) flutter/packages#7973

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2024

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

07b9ce3

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Manual roll Flutter from fe71cadf1959 to 0fe615343bd9 (18 revisions) flutter/packages#7989

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 31, 2024

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

4195af6

…tools`. (flutter/flutter#157879)

matanlurey mentioned this pull request Oct 31, 2024

Plugin.isDevDependency if exclusively in dev_dependencies #157462

Merged

polina-c added the revert Autorevert PR (with "Reason for revert:" comment) label Nov 3, 2024

auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Nov 3, 2024

polina-c mentioned this pull request Nov 3, 2024

Revert "Add and plumb useImplicitPubspecResolution across flutter_tools." #158076

Merged

auto-submit bot pushed a commit that referenced this pull request Nov 3, 2024

Revert "Add and plumb useImplicitPubspecResolution across `flutter_…

0505176

…tools`." (#158076) Reverts #157879 to unblock flutter roll. Prerequisite reverts: #157934 Reason: b/377107864

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

3bbb52c

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

95475d0

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

1444cce

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

0832481

…tools`. (flutter/flutter#157879)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

fb02229 Add and plumb useImplicitPubspecResolution across `flutter_…

b905ed9

…tools`. (flutter/flutter#157879)

		exception: const HttpException(
		'Connection closed before full header was received, '

Conversation

matanlurey commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewkolos commented Oct 30, 2024

Uh oh!

andrewkolos left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Oct 31, 2024

Uh oh!

auto-submit bot commented Oct 31, 2024

Uh oh!

auto-submit bot commented Nov 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matanlurey commented Oct 30, 2024 •

edited

Loading