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

Minimal update of lock file #305

Merged
merged 8 commits into from
May 23, 2022
Merged

Conversation

NathanReb
Copy link
Contributor

This PR adds a --minimal-update flag to lock which allows for updating the lock file, aiming at minimum changes in the dependencies.

When the flag is passed, the target lockfile (i.e. the one that it will write the generated lock file to) must already exists. It parsed and the list of packages extracted from it. It builds a map name -> version that is then passed to the solver. The map is used by the context to promote those version to the top of the preference sorted list of candidates for the package.
This result in different versions only getting picked if no solution exist with the version form the previous lock file. New packages are picked as they normally would and package that are not required anymore are also dropped without issues.

In short the preference order allows us to do everything we wanted from that feature making this a very easy to implement yet extremely useful feature!

TDD style

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
CHANGES.md Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@NathanReb
Copy link
Contributor Author

Note that the documentation page I added is obviously incomplete but I think it's better to improve it iteratively than to wait to have a perfection version of it before merging it. I'm happy to add to it in a separate PR!

Copy link
Member

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also pleasantly surprised how little changes (at least from the core logic part) it is. Hence I mostly looked at that to suggest some improvements. Glad to see that the implementation matches how my hunch it could be implemented, which is great since that means that the complexity increase of the code is very manageable 👍

The flags & parsing and tests look good. It's a bunch of code but essentially not-exciting code and the compiler does a good job of making sure this checks out.

test/bin/minimal-update.t/run.t Outdated Show resolved Hide resolved
lib/opam_solve.ml Outdated Show resolved Hide resolved
lib/opam_solve.ml Outdated Show resolved Hide resolved
lib/opam_solve.ml Outdated Show resolved Hide resolved
lib/opam_solve.ml Outdated Show resolved Hide resolved
lib/opam_solve.ml Outdated Show resolved Hide resolved
lib/opam_solve.ml Outdated Show resolved Hide resolved
cli/lock.ml Outdated Show resolved Hide resolved
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Copy link
Member

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, let's merge it!

@NathanReb NathanReb merged commit b57bd9f into tarides:main May 23, 2022
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Jun 9, 2022
CHANGES:

### Added

- Add a `--minimal-update` flag to `lock` to generate a lockfile
  with minimum dependency changes from a previous lockfile. (tarides/opam-monorepo#305,
  @NathanReb)
- Add command line options to complement or overwrite `x-opam-monorepo-*`
  fields. (tarides/opam-monorepo#307, @NathanReb)
- Save the `lock` CLI arguments in `x-opam-monorepo-cli-args` when generating a
  lock file. (tarides/opam-monorepo#309, @NathanReb)
@NathanReb NathanReb deleted the minimal-upgrade-lock branch June 10, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants