`Plugin.isDevDependency` if exclusively in `dev_dependencies` by matanlurey · Pull Request #157462 · flutter/flutter

matanlurey · 2024-10-23T19:42:33Z

Work towards #56591.

I explicitly want an LGTM from @andrewkolos @jmagman @jonahwilliams before merging.

After this PR, <Plugin>.isDevDependency is resolved based on the following logic, IFF:

The plugin comes from a package A listed in the app's package's dev_dependencies: ...
The package A is not a normal dependency of any transitive non-dev dependency of the app

See compute_dev_dependencies_test.dart for probably the best specification of this behavior.

We (still) do not write the property to disk (i.e. it never makes it to .flutter-plugins-dependencies), so there is no impact to build artifacts at this time; that would come in a follow-up PR (and then follow-up follow-up PRs for the various build systems in both Gradle and Xcode to actually use that value to omit dependencies).

Some tests had to be updated; for the most part it was updating the default ProcessManager because a call to dart pub deps --json is now made in code that computes what plugins are available, but there should be no change in behavior.

/cc @jonasfj @sigurdm for FYI only (we talked on an internal thread about this; see dart-lang/sdk#56968).

/cc @camsim99 @cbracken @johnmccutchan for visibility on the change.

…ncies

matanlurey · 2024-10-25T19:13:38Z

Shoot, there are still commands.shard tests that need updates, I'll work on those.

jonahwilliams

So i think the overall approach SGTM. I don't really have much to nit about the code, but I don't really work in the tool anymore.

LGTM

jonahwilliams · 2024-10-25T19:11:59Z

packages/flutter_tools/lib/src/compute_dev_dependencies.dart

+      return <String>{};
+    }
+
+    jsonResult = json.decode(stdout) as Map<String, Object?>;


I don't think this is likely, but you could also check that this is a map too...

matanlurey · 2024-10-25T19:36:06Z

Updated tests in commands.shard.

I'm slowed down by running into test failures that only occur locally but seem unrelated:

jmagman · 2024-10-25T20:44:40Z

packages/flutter_tools/lib/src/compute_dev_dependencies.dart

+    devDependencies.removeAll(directDependencies);
+
+    // And continue visiting.
+    directDependencies.forEach(visitPackage);


Is an infinite loop possible?

No, clarified in both the comment and with a local variable above.

matanlurey · 2024-10-29T18:12:41Z

Sigh, this is blocked by #102983 and #73870.

matanlurey · 2024-10-31T21:57:14Z

There's a lot of unintentional-looking formatting here :/

Yeah my mistake, I really messed up my default workspace - will fix that going forward.

matanlurey · 2024-11-01T18:01:02Z

Ping - looking for another once-over so I can unblock #158009.

Thanks!

jmagman

Sorry for the delay.

…ncies

jonahwilliams

LGTM

Andrew is OOO until Monday - I reached out to him on Tuesday afternoon asking if he had final feedback. Happy to consider changes in post.

…flutter/flutter#157462)

#158204 collided with #157462. Fortunately the change is 1-line in 1-test.

…flutter/flutter#157462)

flutter/flutter@73546b3...c8510f2 2024-11-08 [email protected] Roll Flutter Engine from 44d788f4651b to a08bd5a07c2a (3 revisions) (flutter/flutter#158375) 2024-11-08 [email protected] Add ability to override `NavigationDestination.label` padding for `NavigationBar` (flutter/flutter#158260) 2024-11-08 [email protected] Add flutter/package code generation instructions (flutter/flutter#158326) 2024-11-08 [email protected] Roll Flutter Engine from 8e19915c19fc to 44d788f4651b (3 revisions) (flutter/flutter#158362) 2024-11-08 [email protected] Roll Flutter Engine from bcb281cde579 to 8e19915c19fc (4 revisions) (flutter/flutter#158354) 2024-11-07 [email protected] Make `_SelectableRegionSelectionContainerDelegate` public (flutter/flutter#147080) 2024-11-07 [email protected] Manual roll Flutter Engine from 371c86fb6b49 to bcb281cde579 (flutter/flutter#158346) 2024-11-07 [email protected] Add clarification on review timelines in PR template (flutter/flutter#158345) 2024-11-07 [email protected] Increase Java heap limit to 8GB for plugin integration tests using deferred components (flutter/flutter#158330) 2024-11-07 [email protected] Roll pub packages (flutter/flutter#158337) 2024-11-07 [email protected] Roll Flutter Engine from ac50b20ae5c9 to 371c86fb6b49 (5 revisions) (flutter/flutter#158336) 2024-11-07 [email protected] Fix a breakage caused by the test being unskipped. (flutter/flutter#158335) 2024-11-07 [email protected] Roll Flutter Engine from 8a963cfc134c to ac50b20ae5c9 (1 revision) (flutter/flutter#158308) 2024-11-07 [email protected] `Plugin.isDevDependency` if exclusively in `dev_dependencies` (flutter/flutter#157462) 2024-11-07 [email protected] Add recently imported packages to issue template (flutter/flutter#158324) 2024-11-07 [email protected] Roll Flutter Engine from 076688d95818 to 8a963cfc134c (1 revision) (flutter/flutter#158304) 2024-11-07 [email protected] Roll Flutter Engine from 94dac953a95f to 076688d95818 (2 revisions) (flutter/flutter#158303) 2024-11-07 [email protected] Make leak tracking bots blocking. (flutter/flutter#157866) 2024-11-07 [email protected] Roll Flutter Engine from b36ca3319825 to 94dac953a95f (1 revision) (flutter/flutter#158297) 2024-11-06 [email protected] Roll Flutter Engine from 58ac1dadd69d to b36ca3319825 (9 revisions) (flutter/flutter#158295) 2024-11-06 [email protected] Added cusor control properties to CupertinoSearchTextField and tests (flutter/flutter#158240) 2024-11-06 [email protected] Fix flakiness in hot_reload_test.dart (flutter/flutter#158271) 2024-11-06 [email protected] Fix use of deprecated `buildDir` in Android templates/tests/examples (flutter/flutter#157560) 2024-11-06 [email protected] Roll Flutter Engine from f03f11300a9d to 58ac1dadd69d (5 revisions) (flutter/flutter#158283) 2024-11-06 [email protected] Roll pub packages (flutter/flutter#158281) 2024-11-06 [email protected] Delete firebase_android_embedding_v2_smoke_test (flutter/flutter#158223) 2024-11-06 [email protected] [web] fix --ab option for web benchmarks (flutter/flutter#154574) 2024-11-06 [email protected] excluding website-cms from critical pr triage (flutter/flutter#158220) 2024-11-06 [email protected] Add test for `image.frame_builder.0.dart` (flutter/flutter#158247) 2024-11-06 [email protected] Roll Packages from 7219431 to bb5a258 (6 revisions) (flutter/flutter#158267) 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

…flutter/flutter#157462)

WIP: computeDirectDevDependencies.

adeae19

matanlurey mentioned this pull request Oct 23, 2024

Strip plugins created from dev_dependencies from release-mode apps #56591

Closed

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

matanlurey added 8 commits October 24, 2024 09:10

++

bee240d

Merge remote-tracking branch 'upstream/main' into compute-dev-depende…

c042d5c

…ncies

++

8711919

Merge remote-tracking branch 'upstream/main' into compute-dev-depende…

f07ae4b

…ncies

Implement.

85c72c3

Just give up.

540c213

++

3e22cff

All tests passing.

4f4d86a

github-actions bot added the a: desktop Running on desktop label Oct 25, 2024

Merge remote-tracking branch 'upstream/main' into compute-dev-depende…

5167609

…ncies

matanlurey changed the title ~~WIP: computeDirectDevDependencies.~~ Plugin.isDevDependency if exclusively in dev_dependencies Oct 25, 2024

matanlurey requested review from andrewkolos, jmagman and jonahwilliams October 25, 2024 19:06

matanlurey marked this pull request as ready for review October 25, 2024 19:07

matanlurey mentioned this pull request Oct 25, 2024

Flutter tool needs to traverse/walk pub dependencies, should it (long-term)? dart-lang/sdk#56968

Open

Use approved dependency.

51d3e2f

jonahwilliams approved these changes Oct 25, 2024

View reviewed changes

Update commands.shard tests with process allowance.

bb3b723

++

544dee0

jmagman reviewed Oct 25, 2024

View reviewed changes

Use base/io, clarify.

2b2bccf

jmagman approved these changes Oct 25, 2024

View reviewed changes

matanlurey added the blocked Issue is blocked by another issue label Oct 29, 2024

matanlurey added 2 commits October 31, 2024 14:52

Reformat.

b665720

++

3c31507

matanlurey requested a review from andrewkolos October 31, 2024 21:57

Fix test.

ac5093a

camsim99 mentioned this pull request Nov 1, 2024

Add dev_dependency attribute to plugins in .flutter-plugins-dependencies #158009

Merged

9 tasks

This was referenced Nov 1, 2024

Move --no-implicit-pubspec-resolution to a Feature #158012

Closed

Move explicit package dependencies to a feature flag #158016

Merged

jmagman approved these changes Nov 5, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/main' into compute-dev-depende…

45f430f

…ncies

jonahwilliams approved these changes Nov 5, 2024

View reviewed changes

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

auto-submit bot merged commit 78cfc1a into flutter:master Nov 7, 2024

engine-flutter-autoroll mentioned this pull request Nov 7, 2024

Roll Flutter from 73546b3b71a7 to 78cfc1ae9b70 (17 revisions) flutter/packages#8034

Closed

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

2e3b766

…flutter/flutter#157462)

matanlurey mentioned this pull request Nov 7, 2024

Fix a breakage caused by the test being unskipped. #158335

Merged

matanlurey added a commit that referenced this pull request Nov 7, 2024

Fix a breakage caused by the test being unskipped. (#158335)

3c689ce

#158204 collided with #157462. Fortunately the change is 1-line in 1-test.

engine-flutter-autoroll mentioned this pull request Nov 8, 2024

Roll Flutter from 73546b3b71a7 to 7abb083ae2ad (29 revisions) flutter/packages#8039

Closed

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

2f42273

…flutter/flutter#157462)

engine-flutter-autoroll mentioned this pull request Nov 8, 2024

Roll Flutter from 73546b3b71a7 to c8510f29ccc2 (30 revisions) flutter/packages#8042

Merged

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

30d9bb0

…flutter/flutter#157462)

huycozy mentioned this pull request Nov 11, 2024

[tool_crash] StateError: Bad state: dart pub deps --json had unexpected output: Expected field "devDependencies" to be a list, got "null" #158441

Closed

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

2636009

…flutter/flutter#157462)

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

317e571

…flutter/flutter#157462)

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

93ad6a6

…flutter/flutter#157462)

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

4906fb9

…flutter/flutter#157462)

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

78cfc1a Plugin.isDevDependency if exclusively in dev_dependencies (…

0be887f

…flutter/flutter#157462)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Plugin.isDevDependency` if exclusively in `dev_dependencies`#157462

`Plugin.isDevDependency` if exclusively in `dev_dependencies`#157462
auto-submit[bot] merged 25 commits intoflutter:masterfrom
matanlurey:compute-dev-dependencies

matanlurey commented Oct 23, 2024 •

edited

Loading

Uh oh!

matanlurey commented Oct 25, 2024

Uh oh!

jonahwilliams left a comment

Uh oh!

jonahwilliams Oct 25, 2024

Uh oh!

matanlurey commented Oct 25, 2024

Uh oh!

jmagman Oct 25, 2024

Uh oh!

matanlurey Oct 25, 2024

Uh oh!

matanlurey commented Oct 29, 2024

Uh oh!

matanlurey commented Oct 31, 2024

Uh oh!

matanlurey commented Nov 1, 2024

Uh oh!

jmagman left a comment

Uh oh!

jonahwilliams left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

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

Uh oh!

Uh oh!

matanlurey commented Oct 25, 2024

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

matanlurey commented Oct 25, 2024

Uh oh!

jmagman Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

matanlurey Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

matanlurey commented Oct 29, 2024

Uh oh!

matanlurey commented Oct 31, 2024

Uh oh!

matanlurey commented Nov 1, 2024

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matanlurey commented Oct 23, 2024 •

edited

Loading