Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feature request: do not rebuild packages when only unimportant constraint has been changed #4647

Closed
kit-ty-kate opened this issue Apr 27, 2021 · 6 comments · Fixed by #5118
Closed

Comments

@kit-ty-kate
Copy link
Member

Currently opam uses OpamFile.OPAM.effectively_equal to detect whether or not a package should be rebuilt.
However in most cases, rebuilding will not change anything but the installed opam file in $OPAMROOT/<switch>/.opam-switch/packages.

My proposal is that if only depends, depopts, conflicts, conflict-class, available or remove changes then:

  • if none of the related dependencies change then we can ignore this change and only change the opam file for internal use.

I'm not sure exactly how to implement this, my guess is that it will cost an extra run of the solver, but maybe there is a simpler way to do this by just changing how opam requests rebuilds.

@kit-ty-kate
Copy link
Member Author

depexts can also be added to this list, though it is implemented slightly different from the others as I understand it.

@dra27
Copy link
Member

dra27 commented Apr 14, 2022

Having just discussed this some more, there's a chance that this is really simple! At this point when the switch state is loaded:

let changed =
OpamPackage.Map.merge (fun _ opam_new opam_installed ->
match opam_new, opam_installed with
| Some r, Some i when not (OpamFile.OPAM.effectively_equal i r) ->
Some ()
| _ -> None)
opams installed_opams
|> OpamPackage.keys

If we replace OpamFile.OPAM.effectively_equal with an alternate version which also ignores the fields above, then what should happen is that the package would no longer be marked for compulsory reinstall. The solver still runs at opam upgrade regardless, so if the dependency formula change actually affects the switch then everything happens as normal. At present, because the package is marked for reinstall, we see the "upstream changes" message and opam reinstalls it anyway.

@hannesm
Copy link
Member

hannesm commented Apr 14, 2022

My 2 cents: if the src changes, or some patches have been added/modified/removed, a rebuild should be done.

@kit-ty-kate
Copy link
Member Author

My 2 cents: if the src changes, or some patches have been added/modified/removed, a rebuild should be done.

of course. See the proposal above

@dra27
Copy link
Member

dra27 commented Apr 14, 2022

My 2 cents: if the src changes, or some patches have been added/modified/removed, a rebuild should be done.

Good grief, yes! The checks already in opam skip rebuilding if, say, you change the maintainer field of the opam package.

This proposal is just about extending that to dependency fields, which are necessarily updated in existing packages in opam-repository. For example, you may have foo.1 which depends on "ocaml" {> "4.08"} and is installed in your switch with OCaml 4.14.0. This package it turns out is not OCaml 5-ready, so the constraint needs to be changed to "ocaml" {> "4.08" & < "5.0"}. At present, the next opam upgrade in your switch would recompile foo.1 even though nothing has changed.

@emillon
Copy link
Contributor

emillon commented Nov 16, 2022

The checks already in opam skip rebuilding if, say, you change the maintainer field of the opam package

I don't think it's an issue in practice, but note that dune subst interprets some opam fields: version, maintainer, authors, homepage, issues, doc, license, repo. But this only applies when the package is pinned so it's not an issue when opam-repository is updated.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants