Repository navigation
[packages] Migrate package groupings to use workspaces #161385
Description
Now that the workspace feature is stable, it makes sense to start using it. Sub-groupings of packages in the packages repo should be migrated to workspaces.
This migration will enable the following benefits:
- Dependency overrides will no longer be necessary during development of package updates
- Only one pub get will be necessary per workspace
- More efficient code analysis
Justification:
- Other Google-owned packages have already been migrated. See json_serializable
These are the groupings that should be consolidated into workspaces:
- camera
- file_selector
- google_maps_flutter
- google_sign_in
- image_picker
- in_app_purchase
- local_auth
- path_provider
- pointer_interceptor
- quick_actions
- shared_preferences
- url_launcher
- video_player
- webview_flutter
Activity
stuartmorgan-g commentedon Jan 9, 2025
Do you have a design doc with a proposal for how this would be used and how it would interact with the various CI processes and repo tool options? Just adding workspaces unconditionally won't work for flutter/packages.
Other repos have different processes, so the fact that a workflow works elsewhere doesn't mean it will work here.
Rexios80 commentedon Jan 9, 2025
Tooling might be more efficient if we updated it to support workspaces, but migrating to workspaces shouldn't break any existing tooling since CLI commands behave the same in any workspace package
stuartmorgan-g commentedon Jan 9, 2025
Could you describe how the draft PR does not regress our CI's system of ensuring that updates to a package are compatible with both the cross-package updates and with the currently published packages? A system that was intentionally designed to avoid situations like melos-based CI systems have had where people accidentally published breaking changes as non-breaking without any CI catching it?
Rexios80 commentedon Jan 9, 2025
You are correct that the draft PR makes that check not work. We would need to update the tooling to insert a
pubspec_overrides.yamlfile with an emptyresolutionkey as described here: https://dart.dev/tools/pub/workspaces#temporarily-resolving-a-package-outside-its-workspaceRexios80 commentedon Jan 9, 2025
@stuartmorgan do you think this is worth pursuing further? Workspaces would make development a lot easier in most cases, but they would probably need to be disabled before any analysis is done.
stuartmorgan-g commentedon Jan 9, 2025
I'm not sure what exactly "this" is, which makes that hard to answer. If it's some kind of adoption of workspaces as part of flutter/packages tooling and processes, then sure, and the next step is a design document. If it's the draft PR's approach of just unconditionally enabling it without regard for how it will affect the existing systems, then no, because I'm not going to approve a PR without a doc that gives me confidence that the effects have been sufficiently evaluated (and certainly not a PR that I'm confident regresses critical safety features of our current system).
stuartmorgan-g commentedon Jan 9, 2025
(Also, even ignoring CI, the assertion that the change as written will not break any existing tooling is almost certainly incorrect; the repo tooling makes inferences about what kinds of packages things are based on detection of what is a package and what is an enclosing folder, and adding
puspec.yamlto directories that did not previous have them makes the assumptions several parts of the tooling are built on incorrect.)Rexios80 commentedon Jan 9, 2025
I meant the adoption of workspaces in the
flutter/packagesrepositoryI didn't intend for that PR to land as-is. I wanted to see how much, if any, of the existing CI infrastructure would be unhappy with the change.
Yes we would need to handle the new workspace pubspecs, as is evidenced by CI failures in the draft PR
This is a process I have not done before, and I am not sure when or if I will be able to create a design document for this. I will close the draft PR since I got the information I needed from it, and leave this issue for someone else to tackle if they wish.
stuartmorgan-g commentedon Jun 4, 2025
Some other wrinkles:
If we do a whole-repo workspace:
foo: >= 0.1.0 < 0.30and another unrelated package allowsfoo: ^0.1.0, we'll still always analyze the first package withfoo 0.1.xeven though many of its clients would get0.2.x.make-deps-path-basedhandles this, in contrast.)If we don't do a whole-repo workspace, then we can't do pathified analysis of our cross-package dependencies unless we maintain the current system in addition, adding a bunch of complexity to the repo tool. That analysis has occasionally prevented us from post-publish OOB breaking in our example apps (usually due to analyzer issues that we treat as errors).