request: Use `pubspec_overrides.yaml` instead of modifying `pubspec.lock`

invertase / melos

🌋 A tool for managing Dart projects with multiple packages. With IntelliJ and Vscode IDE support. Supports automated versioning, changelogs & publishing via Conventional Commits.

https://melos.invertase.dev/~melos-latest

Apache License 2.0

1.19k stars 207 forks source link

request: Use `pubspec_overrides.yaml` instead of modifying `pubspec.lock` #263

Closed spydon closed 2 years ago

spydon commented 2 years ago

Is there an existing feature request for this?

[X] I have searched the existing issues.

Command

melos bootstrap

Description

https://github.com/dart-lang/pub/pull/3215 was recently merged which introduces a new file (pubspec_overrides.yaml) which makes it possibly to specify dependency overrides, and it is a perfect fit for Melos (I guess that is why @blaugold made it).

Reasoning

This would be very good to have since you wouldn't have to re-run melos bootstrap again when another application has run pub get or modifies the pubspec.lock file.

Additional context and comments

No response

spydon commented 2 years ago

Will solve #117 too

blaugold commented 2 years ago

You also won't have to run flutter pub get before melos bs to make sure Flutter generated files exist, and the VS Code setting dart.runPubGetOnPubspecChanges can be enabled again. Moving to pubspec_overrides.yaml should improve the user experience for melos quite a bit.

We'll have to wait for the feature to land, though. The stable Dart channel releases every ~3 months, so 2.17.0 could be available around the beginning of May.

spydon commented 2 years ago

You also won't have to run flutter pub get before melos bs to make sure Flutter generated files exist, and the VS Code setting dart.runPubGetOnPubspecChanges can be enabled again. Moving to pubspec_overrides.yaml should improve the user experience for melos quite a bit.

🙌

We'll have to wait for the feature to land, though. The stable Dart channel releases every ~3 months, so 2.17.0 could be available around the beginning of May.

The implementation in Melos could theoretically begin before it is released to stable though, since it will be quite a lot of changes internally? (Just not merging it until stable is out ofc)

blaugold commented 2 years ago

The implementation in Melos could theoretically begin before it is released to stable though, since it will be quite a lot if changes internally? (Just not merging it until stable is out ofc)

That's true. Maybe we should keep the current implementation of melos bs and make the new one opt in. Not everyone uses the latest Dart and Flutter versions immediately. Or alternatively make it part of a 2.0.0 release where breaking changes are expected.

Salakar commented 2 years ago

I don't think we need to change current behaviour either just yet and we can just add generation pubspec_overrides.yaml in and it should just work with existing melos bs behaviour.

The issue at the moment (based on my original debugging internals when I originally built Melos) that causes #117 etc is a check that checks if dependencies are sourcing same versions from the same locations, in package_config.json/pubspec.lock vs pubspec.yaml - and since the overrides file would make them both resolve to path dependencies then that check should no longer fail and cause #117 and friends. In a later version we can then do a breaking change and drop old melos bs behaviour as a breaking change when later Dart version has wider adoption

What about a flag to to enable support in melos.yaml or even just add it in as a feature release without a flag (users will just see a new overrides file in each of the packages and nothing else changing, they can either commit or ignore these).

Salakar commented 2 years ago

I can work on adding it in today and publishing a preview,

I'll work with following assumptions (not sure how to test using new Dart version yet, if someone can point in the right direction of best way to update that'd be good):

For each package, if overrides need to be done, generate a pubspec_overrides.yaml file in a format simalar to:

# Generated by Melos, do not edit or modify this file manually.
dependency_overrides:
some_package:
path: ../some_package

If pubspec_overrides.yaml already exists and not Generated by Melos - error out. We could merge into users overrides but no yaml libraries currently preserve styling or comment nodes when generating yaml output - and potentially there's other issues with merging that I can't think of right now.
Keep existing melos bs behaviour and generate modified locks/package_config files - but add a flag to opt-out of old bs behavior - if you know you're only going to use dart versions that support new overrides. Default on for reduced friction until removed at a later date.

Open to feedback and if my assumptions need changing or I'm missing something

blaugold commented 2 years ago

@Salakar I don't see anything wrong with the outlined approach, and I like the flag in melos.yaml to opt in. With the new behavior, we should run flutter/dart pub get in each package after generating pubspec_overrides.yaml.

According to the README, you can run pub from the cloned repository with dart bin/pub.dart.

Just FYI, there is yaml_edit which preserves comments, not sure about whitespace, though.

Salakar commented 2 years ago

Just FYI, there is yaml_edit which preserves comments, not sure about whitespace, though.

I was not aware of this one somehow 🙈 what would you think the behaviour should be for existing override files that were not generated by melos?

blaugold commented 2 years ago

We could mark each dependency override that was added by melos and only update/remove those.

dependency_overrides:
  some_package: # Generated by Melos
    path: ../some_package

If an override already exists for a local package that means the users wants to manage the override for that package themselves.

Side note: IIRC melos currently merges its own overrides with dependency_overrides from pubspec.yaml. I think the new behavior should ignore dependency_overrides and recommend to the user to move them to pubspec_overrides.yaml, if we allow editing of pubspec_overrides.yaml.

blaugold commented 2 years ago

An update on how to try out pubspec_overrides.yaml: The latest Dev channel release of the Dart SDK (2.17.0-266.0.dev) includes support for the new feature.

Salakar commented 2 years ago

Apologies I had a ton of things come up so I didn't get very far with this at all yet 🙈 I "should" have some time next week unless someone wants to get it done before me

blaugold commented 2 years ago

No worries. I might have some time in the next few days. pubspec_overrides.yaml has also landed on the Flutter master channel.