Skip to content

Conversation

@blaugold
Copy link
Contributor

@blaugold blaugold commented Oct 30, 2021

Tools like melos need to be able to resolve dependencies, to link up local packages for development.

With this change, up-to-date checks of package_config.json, .packages and pubspec.lock are skipped if package_config.json has been generated by a tool other than pub. This behavior is limited to pub run and dart run. pub get does the normal checks and overrides third-party modifications.

This change also eliminates an unnecessary call of Entrypoint.assertUpToDate. Just a few lines above, Entrypoint.resolveExecutable is called, which calls assertUpToDate in the process.

@google-cla google-cla bot added the cla: yes label Oct 30, 2021
@jonasfj
Copy link
Member

jonasfj commented Oct 31, 2021

Hmm, should we consider solving this problem by adding first class support for local dependency overrides with something like a pubspec-overrides.yaml sitting next to pubspec.yaml.

Maybe, this isn't a problem in reality, but if we encourage third-party tools to generate package-config.json, might we not risk that adding new features, build steps, etc. to dart pub get will be harder in the future.


On the flip side: we could also simplify our logic and only check the generator when timestamps don't align.

@jonasfj
Copy link
Member

jonasfj commented Oct 31, 2021

For the record, I'm not principally opposed to this change, just considering/brainstorming possible future implications :)

@blaugold
Copy link
Contributor Author

I think your right @jonasfj, that a cleaner, more universal approach is to support something like pubspec-overrides.yaml.
In issue #2161, a few possible solutions have been discussed, and it seems to me that a pubspec-overrides.yaml file provides the best trade-off between usability and flexibility. Users can write them manually, or have a tool generate them and still see what the current configuration is. It also works well with other tools running pub get for the user, such as flutter and IDE extensions.

Currently, melos is working around pub not having something like pubspec-overrides.yaml. It recreates the workspace with all of its packages in a temporary directory, but with only the files required to run pub get. It then calculates the required dependency-overrides, modifies the pubspec.yaml files and runs pub get in the temporary directory. The last step is to copy some of the files pub generated (e.g. package_config.json), back to the actual packages.

@blaugold
Copy link
Contributor Author

blaugold commented Nov 2, 2021

Following up on my last comment, I'm closing this PR since it's an attempt to enable a solution which is already a workaround.

@blaugold blaugold closed this Nov 2, 2021
@sigurdm
Copy link
Contributor

sigurdm commented Nov 2, 2021

In any case - thanks for the contribution. I think it highlights an area where we need to find a better solution!

@Salakar
Copy link

Salakar commented Nov 2, 2021

I would note there are places (at least in Flutter tooling) where 'third-party tool detection and doing something different' is already a thing, e.g. https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart#L49

I'm not against having a pubspec-overrides.yaml file but my initial thoughts are that this sounds like a user editable file that is VCS tracked - having tools edit/generate into it could be problematic - especially where the file already exists in a project. I'd personally rather avoid having Melos modify user editable & VCS tracked files 😅

Maybe, this isn't a problem in reality, but if we encourage third-party tools to generate package-config.json, might we not risk that adding new features, build steps, etc. to dart pub get will be harder in the future.

I'd note that Melos doesn't generate the package-config.json - we rely on pub directly to do this so all generated files are actually from existing pub tooling (we only replace the tool name in the output after pub generates it). I think in the Melos instance there isn't a risk of missing out on features - but that's just one tool 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants