-
Notifications
You must be signed in to change notification settings - Fork 229
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.
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
Proposal for resolution: external
as support for mono-repositories
#4022
Comments
I think the core of this approach is to enable tooling like So if the root I suspect that when Regarding, tools that modify
I don't know what we should do. If To be clear: the idea is to let mono-repo tooling disable |
I’m working in a mono-repo with more than 300 packages, so I’m very curious about this proposal.
I.e. I have
Unfortunately, I don’t think I understand the real benefit of this change. I can see how the proposed changes fix the analyzer performance, which is IMO the main pain for mono-repos now, but it seems like we are solving this problem of a tool at the expense of the developer’s experience. Even if the proposed functionality already existed, I would prefer keeping the «Only Analyze Projects With Open Files» flag on in my IDE (which fixes the analyzer performance, not without drawbacks ofc) and keep the project simpler. |
This proposal looks really promising. Analyzer performance in large mono-repos is an issue I have heard of from multiple people, so reducing analyzer contexts would be a big win. Having to only resolve one package instead of 10s or 100s also has the potential to speed up CI jobs. Potential issues
|
Thanks for the feedback! Some initial responses:
Interesting point. But I still think that even though the packages are internal, the resolution is external, and thus I think the term makes some sense. But we are not fixed on the naming if you have better suggestions.
Well I cannot promise what these packages will implement. But I could imagine that these packages could help generate the repo-wide pubspec.yaml. Perhaps they could also help making repo-wide upgrades (somehow).
Yeah, I think your example should be supported, as these will all be included via dependency overrides.
I don't think this is as much an open question, as tasks that will have to happen for this proposal to be useful.
Again, it is not the packages that are external, but the resolution mechanism. The idea is to keep the proposal minimal for now.
I think the main point is ensuring that all the sub-packages in a mono-repo share a single resolution (a single pubspec.lock), and thus reduces the potential for the confusion that could occur if two sub-packages end up with different versions of a dependency - causing surprising behavior. Also there is potentially some time to save by only running |
Also thanks for feedback!
IMO if a shared resolution cannot be found, it is an advantage to find out early and solve that issue somehow, rather than developing with different versions of dependencies in different sub-projects.
Good point - though it would perhaps be a confusing state of the mono-repo, but it shows there are ways out if really needed.
yeah - they probably need to recognize this pattern. Really good point.
Good question! I guess that depends what pubspec that lint is going to look into - @pq do you have input here?
Another good question. Ideally it 'should just work' (from the definition of package config), but there will probably be stuff breaking. @christopherfujino can you foresee stuff that would work poorly in this model? |
Good question. @jakemac53? 📟 |
No it would not work. Is there any particular reason the proposal here says not to correctly specify dev_dependencies per-pubspec? I would suggest just requiring them to be there for each package still. If we don't have a standard that requires a root level pubspec, I don't see how this could be supported either. Unless we just wanted the lint to ignore entirely non-lib dirs in packages with
This should work fine as long as they still have valid pubspecs as well. This would need to include dev dependencies though, similarly to the
It should error imo. And pub get/upgrade should also error. |
There is a potential separate issue here - there might be tools which just assume a file at exactly |
Yeah, I'm pretty sure many things would break in the tool. Worse, it seems like it would be more difficult to reason about where to find this information? Obviously, this is something we could overcome, but I suspect it would be painful. |
What about if pub still wrote the package_config.json, but perhaps if its contents contained the absolute path to the root file? |
Fwiw, the behavior is fully specified https://github.com/dart-lang/language/blob/main/accepted/2.8/language-versioning/package-config-file-v2.md#finding-the-file. But, that doesn't mean all tools actually do that lookup logic given they don't really have to for most external use cases. |
Also, reading the specification it actually does currently talk a lot about Pub specifically, and its current behavior of putting a file in each package root. So we would want to probably treat this as a breaking change and go through the normal breaking change processes. |
That might be a nice way to work around things, although very likely it would still be breaking for users of Although possibly it could be a symlink? I am not sure this would work on windows. |
Please no symlinks :) |
Because dev-dependencies are only resolved for the root pubspec. |
I would argue that this is not breaking. No existing code should be affected. The root pubspec will be the one resolved, and where the .dart_tool/package_config.json is placed exactly as specified in https://github.com/dart-lang/language/blob/main/accepted/2.8/language-versioning/package-config-file-v2.md#finding-the-file . So from that perspective nothing changes. What is new is that we have to teach all our tools to not expect being run in the root project. And the specification explicitly talks about searching parent directories. And in that sense we also tap in to an existing (but perhaps not very used) mechanism. |
What makes dev dependencies different from regular dependencies in this respect? The tools will still be used at the package level, so I think it is reasonable to specify them. You should at least be allowed to specify them, as it will be required for build_runner and the |
Regarding breakage, the spec also has wording like this As a user, if I have a structure like this:
And I am currently in the The description of "root_uri" for a package even explicitly calls this out More concretely, if this is known to break existing packages, then regardless of what the spec says or if you agree its breaking according to the spec, it is in fact breaking. |
@olexale, I think you're touching on something really important here. Specifically, the question easily becomes are mono-repository developers willing to use a single resolution? Meaning picking a single-version of each dependency for all apps/packages in the mono-repository -- this is not necessarily a small price, but it doesn't have many other upsides.
If dependencies for
If If you have a special case where
But this quickly gets messy. For this proposal ( |
@jonasfj I agree that having the single version solve is important and the discipline really pays off in the long run. But I also think there are times when the discipline of committing small changes butts heads with the single version policy, and it's important to have an escape hatch. And I think this proposal supports that (correct me). So let's say I have a mono-repo with 30 packages, and 4 of them are apps, which use If you were forced to update all 4 apps in one commit, this could easily be a bridge too far. Maybe you don't know the business logic in the other 3, or even just 1 of the 3. Maybe after a Herculean effort, you commit a migration of all 4, but it's got bugs revealed in manual testing, and must be rolled back a few days after it lands, but in the meantime the 4 apps have received other commits. There are even more mundane reasons like App 1 has a high priority feature, relying on super_widget v4.0.0, shipping in Q1, but the other 3 apps have other priorities and cannot commit engineering time to bump super_widget in their apps this for Q1. It's easy to imagine disaster when some infra requirement prevents small commits. So I think what you would do with
In the interim time, you do not get all of the benefits of the mono-repo: For example, analyzer would run with 2 analysis contexts, then 3, 4, and 5, before dropping back to 1. But remember, the mono-repo has 30 packages, so you're still in a wildly better place in terms of analysis contexts. In terms of the hopes and dreams that your 30 packages have a single (or minimal) version solve, your much closer than 30. Again, you would have 2 version solves (package config files), then 3, 4, 5, then back to 1. This scenario is actually pretty rosy, and there can be situations that will require more effort or coordination, but it's a good starting place. For example, maybe one of the 30 packages is a notifications widget that 3 of the apps use. If that notifications widget depends on super_widget v3.0.0, and one or some of the 3 apps use super_widget v3.0.0, it won't be as easy to bump super_widget to v4.0.0 incrementally. The idea is that developers should be empowered with some escape hatch to avoid at all costs a massive all-packages breaking-change pub dep version bump. |
@sigurdm wrote up a slightly different proposal that accomplishes the same goal. |
How would that |
It wouldn't affect it. When a package using that resolution is published, the |
Closing this as we are implementing workspaces #4127 instead. |
NOTE: This proposal has been partially succeeded by a proposal with a proper workspace file. See https://docs.google.com/document/d/1UEEAGdWIgVf0X7o8WPQCPmL4LSzaGCfJTM0pURoDLfE/edit?resourcekey=0-c5CMaOoc_pg3ZwJKMAM0og . We probably want to support
resolution: external
as well asresolution: workspace
for cases such as the dart sdk where the package-config is not created by pub.This issue suggests a simple mechanism in the pub client for better supporting a shared resolution between the packages of a mono-repo. Namely adding
to a pubspec.yaml to indicate that pub should not treat it as a root package.
The main parts of this proposal does not contain much support in the pub client for how to manage multiple packages together in a single project. We instead suggest pushing more advanced support to external tools such as
package:mono_repo
orpackage:melos
.Motivation
Mono-repositories are repositories with tightly related packages that are often developed and released together.
The dart team is maintaining a number of mono-repos Eg:
dart-lang/test
google/protobuf.dart
dart-lang/ecosystem
Another special case of mono-repos is the dart SDK where a common resolution is used, but not maintained by pub, even though many of the subpackages have a
pubspec.yaml
, and some are published to pub.dev individually...Externally we know of several customers using mono-repositories.
When developing in a mono-repo it is often desirable to use a shared version resolution for ensuring consistent behavior.
By having a shared resolution, tools such as the analyzer can also share context between analyzing each package, with big potential memory savings.
Some packages are in a mono-repo for organizational purposes, and might not want to share the same resolution (and would not benefit from this proposal). One such repo could be: flutter/packages.
Some tutorials are split in multiple "chapters" each with a single
pubspec.yaml
, and the analyzer has been known to choke on those because they each have a separate context. They could perhaps benefit from this proposal (if developing thepubspec.yaml
is not part of the tutorial).Existing support for mono-repositories
There exists tooling to support handling of mono-repos, such as mono_repo.dart and melos. These tools can help running the same command for all sub-projects
They, however, don't allow a shared resolution between the packages. These could be updated to allow for generating a shared root
pubspec.yaml
.Proposed mechanism
A new section in
pubspec.yaml
would mark a pubspec that it should not be used for a "local resolution", but instead rely on the Dart VM's mechanism for searching parent directories for a.dart_tool/package_config.json
containing a resolution to use.Given this,
dart pub get
would not generate adart_tool/package_config.json
file in the same directory. Instead it could either (still open):pubspec.yaml
in parent directories and resolve that instead.dart run
anddart test
needs to ignorepubspec.yaml
s withresolution: external
when resolving packages.dart pub publish
should however consider thispubspec.yaml
when doing validations.We could create a lint discouraging from having
dev_dependencies
anddependency_overrides
in apubspec.yaml
withresolution: external
.Creating a mono-repo
Now to create a mono-repo, one would create a "root"
pubspec.yaml
withdependency_overrides
with path-dependencies on all the sub-projects.All
dev_dependencies
should be collected in the rootpubspec.yaml
, and thus shared between the subprojects.Running
dart pub get
in this root-directory will create a sharedpubspec.lock
(that can be checked into source control if desired) and a shareddart_tool/package_config.json
for consumption by the Dart VM and analyzer.We now rely on the
dart run
resolution mechanism to look up through parent directories for finding thedart_tool/package_config.json
file, and providing the package resolution. This should enabledart test
to still work from the sub-project folder (we need to test that this works).One could think of mechanisms for "linking" the sub-projects into the root-project, such that they could be operated on as a whole (eg. when doing upgrades). We propose to (at least as a first step) leave such functionality out of the pub client, and for example
The Dart SDK
In the Dart SDK we can mark all the in-sdk-repo packages as
resolution: external
.There will not (at least for now) be a root
pubspec.yaml
, but we will instead rely on the current alternative tooling to generate a shareddart_tool/package_config.json
.Open questions:
dependency_overrides
good enough?build_runner
would work in such a setup. Can each sub-project contain its ownbuild.yaml
?dart pub add
? Can you add to a sub-package, and should that resolve together with other packages?dart pub add --dev
find the rootpubspec.yaml
?dart pub upgrade --major-versions
? Is there a way to upgrade all thepubspec.yaml
's of a project in sync? (Could perhaps be added later)We might need a way to force resolution of the single package, e.g. for deployment of a sub-package. (Could we piggy back on
dart pub get --enforce-lockfile
- then we should have called itdart pub get --deploy
😐)Potential downsides
dependencies_overrides
can cause some issuesDependency constraints among sub-projects are not checked.
When publishing you might have constraints not matching sibling dependencies
This can to some extent be helped by doing a sub-package-only resolution in
dart pub publish
.pubspec.yaml
might be missing thedependency_override
that should include a child.The text was updated successfully, but these errors were encountered: