Skip to content
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

Packages under pkg in this repo should override deps using pubspec_overrides.yaml #51952

Open
jakemac53 opened this issue Apr 4, 2023 · 5 comments
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes.

Comments

@jakemac53
Copy link
Contributor

jakemac53 commented Apr 4, 2023

Today SDK packages (and all SDK code) rely on the top level package config from the SDK for package resolution. Accidentally running a dart pub get can either fail, or cause other random problems. At best it will give you versions of dependencies that are not kept in step with the SDK versions of those dependencies.

We can resolve this by committing pubspec_overrides.yaml files next to our pubspec.yaml files. These only affect local development. See an example from the build mono_repo here https://github.com/dart-lang/build/blob/master/build/pubspec_overrides.yaml.

This would eliminate confusion around running dart pub get by accident - it would now be supported and load the correct version of the packages (it would be a different, smaller package_config.json than the normal SDK one, but the result would be the same outside of some very weird situations).

This would also alleviate concerns around dart run <script>, dart test, etc doing a pub get, which causes issues in the SDK (whether that change sticks is largely irrelevant to this change though).

cc @bkonyi @mit-mit @natebosch @leafpetersen @jonasfj @sigurdm

@jakemac53
Copy link
Contributor Author

Just to clarify, we would add all path dependencies, which would be relative paths (mostly into third_party)

@lrhn lrhn added the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Apr 4, 2023
@jakemac53
Copy link
Contributor Author

cc @athomas

@sigurdm
Copy link
Contributor

sigurdm commented Apr 11, 2023

  • dependency_overrides will work just as well. not sure what is better.

  • If you do a pub resolution inside a pkg/ folder you will get a .dart_tool/packages disjoint from the sdk-wide one, and while the packages are the same, it feels like a bit of a hack that might introduce some subtle bugs (I cannot think of exact cases though). It would probably be better to have a mechanism that prevented the local solve altogether (not sure exactly how...)

@jakemac53
Copy link
Contributor Author

  • dependency_overrides will work just as well. not sure what is better.

Well, until recently the pubspec_overrides.yaml file worked differently in that it wouldn't complain at publish time. I don't remember if we reverted back to that behavior or not, but it was the main value proposition for mono_repo authors.

  • It would probably be better to have a mechanism that prevented the local solve altogether (not sure exactly how...)

Yeah this is just a simple solution in lieu of a better one.

@jakemac53
Copy link
Contributor Author

In a separate thread we were discussing that these could be automatically generated when we are generating the package_config.json file to ensure they are fully in sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes.
Projects
None yet
Development

No branches or pull requests

3 participants