[CI] Update deps script - #12427
Conversation
fe706da to
1285be8
Compare
There was a problem hiding this comment.
Code Review
This pull request updates MakeDepsPathBasedCommand to recursively resolve and include local in-repo dependencies of target packages using a queue-based approach, preventing pub version solving conflicts. It also ensures a package's own directory is included in the overrides and updates the corresponding tests. The review feedback suggests refining the federated package matching logic to prevent overly broad matches and using package.parsePubspec().name instead of package.directory.basename for more robust package name retrieval.
justinmc
left a comment
There was a problem hiding this comment.
LGTM, thanks for the detailed PR description. @stuartmorgan-g should also review.
I really don't think we should do this as a general approach; instead I would highly recommend that this portion of the walk be restricted to dev dependencies. From a high level, here's the thing that the pathified check was added to avoid (which happened not infrequently before):
If you over-pathify, it's very easy for someone to accidentally bypass the safety added by the pathified check and publish something that breaks clients, because you are no longer testing the same combination of packages that clients will run when B is published. For dev dependencies it should be fine since those don't affect clients, but changing non-dev dependencies would regress the safety net. |
|
Ok thank you. This is very helpful. I will update to only cover dev dependencies. |
|
Ok, I have updated this. I also made sure we have test coverage for both aspects, one to validate that example apps receive path overrides for both the target dependency (material_ui) and their own parent package (cupertino_ui), resolving pub solver dependencies cleanly. And the other to ensure it does not recursively override dependencies of target packages to preserve that safety net. PTAL. :) I'll update the PR description as well. |
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM with one nit
| <String, RepositoryPackage>{...localDependencies, parentPackageName: package}, | ||
| versions, | ||
| additionalPackagesToOverride: packagesToOverride, | ||
| additionalPackagesToOverride: <String>{...packagesToOverride, parentPackageName}, |
There was a problem hiding this comment.
Could you add an inline comment on parentPackgaeName being here? Since foo/example always depends on foo by path, it's really non-obvious why this would be necessary outside the context of someone reviewing this PR :)
Maybe something like:
// Add an override to the parent package in case a transitive dependency has a dependency on it,
// since that (non-path) dependency would conflict with the path-based dependency in the example.There was a problem hiding this comment.
Excellent! On the way. Thank you!
|
autosubmit label was removed for flutter/packages/12427, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
autosubmit label was removed for flutter/packages/12427, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…r#191008) flutter/packages@aaaf246...94485f1 2026-08-12 22373191+Hari-07@users.noreply.github.com [in_app_purchase_storekit] Group purchases into a single event in storekit2 (flutter/packages#12237) 2026-08-12 fluttergithubbot@gmail.com Sync release-material_ui-0.0.3+1 to main (flutter/packages#12439) 2026-08-11 katelovett@google.com [CI] Update deps script (flutter/packages#12427) 2026-08-11 fluttergithubbot@gmail.com Sync release-go_router-17.5.0 to main (flutter/packages#12417) 2026-08-11 piyushanand.1221@gmail.com [webview_flutter_android] Set support for web authentication (flutter/packages#11681) 2026-08-11 nateshmbhat1@gmail.com [video_player] : Add video track selection support for Android and iOS (flutter/packages#10688) 2026-08-11 sigurdm@google.com Inline error ignores in analysis_options (flutter/packages#12430) 2026-08-11 fluttergithubbot@gmail.com Sync release-cupertino_ui-0.0.3+1 to main (flutter/packages#12426) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
Fixes a Pub version solver failure during release PR testing (
make-deps-path-based) for co-dependent packages (specificallymaterial_uiandcupertino_ui).When creating a release PR for
material_ui(e.g.,release-material_ui-0.0.3+1), themake-deps-path-basedCI step failed duringflutter testonpackages/cupertino_uiwith the following Pub version solver error:material_uiandcupertino_uishare an asymmetric co-dependency in the repository:material_uihas a direct dependency oncupertino_ui(dependencies: cupertino_ui: ^0.0.3).cupertino_uihas a dev dependency onmaterial_ui(dev_dependencies: material_ui: ^0.0.2).When
make-deps-path-based --target-dependencies=material_uiruns:make-deps-path-basedaddsdependency_overrides: material_ui: {path: ../material_ui}tocupertino_ui/pubspec.yamlandcupertino_ui/example/pubspec.yaml.cupertino_ui/exampledepends directly oncupertino_ui(path: ..).material_ui(frompath) depends oncupertino_ui: ^0.0.3(from hostedpub.dev).cupertino_uiincupertino_ui/example'sdependency_overrides, Pub detectscupertino_uisourced frompath(incupertino_ui_examples) andhosted(inmaterial_ui), causing Pub's version solver to reject the resolution graph.Updated
make-deps-path-based(script/tool/lib/src/make_deps_path_based_command.dart):When
_addDependencyOverridesIfNecessaryrecursively updates example apps of a package (for (final RepositoryPackage example in package.getExamples())), it now explicitly includes the parentpackageitself in the local package mapping andadditionalPackagesToOverride.This ensures
cupertino_ui/example/pubspec.yamlreceives a path override for its parent package (dependency_overrides: cupertino_ui: {path: ...}) alongside the target package override (material_ui: {path: ...}). Consequently, Pub solver resolves both local path dependencies cleanly without requiring recursive dependency expansion (thereby preserving the safety net of--target-dependencies).Why
cupertino_uiHas adev_dependencyonmaterial_uicupertino_uideclaresmaterial_uiin itsdev_dependenciesbecause multiple files acrosscupertino_ui/lib/src/(e.g.,text_field.dart,dialog.dart,scrollbar.dart,theme.dart) use Dart@docImportdirectives referencing Material design widgets in public API documentation:/// @docImport 'package:material_ui/material_ui.dart';Per Dart doc guidelines, packages referenced in
@docImportmust be declared indev_dependenciesso static analysis anddartdoccan resolve the cross-referenced symbols.Didn't we fix this last week?
material_ui 0.0.3release Sync release-material_ui-0.0.3 to main #12352): Both packages still used Dart Workspaces (resolution: workspace). Under Dart Workspaces, Pub dependency resolution was handled at the top-level workspace root, hiding the Pub solver conflict.cupertino_ui 0.0.3by removing Dart workspaces fromcupertino_uiandmaterial_ui.cupertino_ui(targetDependencies=cupertino_ui),material_uigot path overrides forcupertino_ui. Sincecupertino_uididn't depend on a third package pulling hostedcupertino_ui,cupertino_ui's release succeeded.material_ui(targetDependencies=material_ui),material_ui's direct dependency oncupertino_uicausedcupertino_ui/exampleto fail.material_ui 0.0.3+1release [material_ui] Batch release #12423): This was the first release attempt ofmaterial_uisince Dart workspaces were removed, exposing the non-workspace Pub solver requirement.Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2