search

ocaml / opam-repository

Main public package repository for opam, the source package manager of OCaml.
https://opam.ocaml.org
Creative Commons Zero v1.0 Universal
517 stars 1.14k forks source link

Possible breakage in opam update #25961

Open mseri opened 5 months ago

mseri commented 5 months ago

Due to the removal of some files, your opam repository may fail to update properly (on macos or bsd, if you don't have gpatch installed). If this is the case please install gpatch or delete $OPAMROOT/repo/default{,.tar.gz} and re-do opam update apologies for the inconvenience.

kit-ty-kate commented 5 months ago

There is a pretty big problem though: the breakage is silent. The patch command from macOS/BSDs makes a file empty instead of removing it so the files that have been removed by https://github.com/ocaml/opam-repository/pull/25960 will be there but empty and they will overwrite the file of the same name added by extra-source.

This should not be advertise as "possible breakage" but Giant Warning to anyone still not using GNU patch on macOS instead and it might not be "breaking"

mseri commented 5 months ago

What do you reckon we should do? Should we revert the change? Ping also @hannesm

hannesm commented 5 months ago

My point of view is:

There's a workaround for those affected: rm -rf ~/.opam/repo/default{,.tar.gz} && opam update default. So, we should carry on (and not revert), also with the opam-archive in mind we definitively want the freedom to remove packages.

hannesm commented 5 months ago

And finally, I don't see a way to have a failsafe and smooth transition. Of course we could wait (forever?) until everyone updated to 2.1.6. We could also push for a release of opam where "if extra_source is present, a extra_file doesn't overwrite it" - but that also would mean to wait for an unknown time until everybody upgraded to that new opam release. Given that we don't have any statistics about "opam version usage", I see no point in delaying.

kit-ty-kate commented 5 months ago

The discuss post announces this as a policy change, which i think is fair to understand this way. However one thing that hasn't been done is to codify it in https://github.com/ocaml/opam-repository/wiki/Policies, could anyone write an amendment for it?

hannesm commented 5 months ago

I was wondering whether such a patch would be sensible (and would work):

diff --git a/repo b/repo
index ff07c1813e..8e7eb9533c 100644
--- a/repo
+++ b/repo
@@ -8,4 +8,7 @@ announce: [
 """
 [INFO] opam 2.1 includes many performance improvements over 2.0; please consider upgrading (https://opam.ocaml.org/doc/Install.html)
 """ {opam-version >= "2.0.10" & opam-version < "2.1.0~~"}
+"""
+[WARNING] please ensure to have GNU patch installed as `patch`. Otherwise update may fail silently (since it can't remove files).
+""" {os = "macos" & opam-version < "2.1.6"}
 ]
hannesm commented 5 months ago

The discuss post announces this as a policy change, which i think is fair to understand this way. However one thing that hasn't been done is to codify it in https://github.com/ocaml/opam-repository/wiki/Policies, could anyone write an amendment for it?

I did in https://github.com/ocaml/opam-repository/wiki/Policies/_compare/75e8f008da518f1ba586f577f2bd9b48955bda18...a5ffe6f5b1975857f6f4613c469c31ced2af3c47

I also opened https://github.com/ocaml/opam-repository/pull/25963 to guide contributors how to use extra-source.

kit-ty-kate commented 5 months ago

I was wondering whether such a patch would be sensible (and would work):

yes i think it would be nice, however currently users of the default opam-repository (via opam.ocaml.org) use a different repo file generated here: https://github.com/ocaml-opam/opam2web/blob/d3d36b81174cbbb9df9dfe525cd2cfe77fd86206/bin/opam-web.sh#L26 so this one would probably also need to be modified.

I'm not sure if there is a clean way to merge both (use opam-repository's repo file as a base and modify it as needed), I'll open a ticket at least to track this.

hannesm commented 5 months ago

yes i think it would be nice, however currently users of the default opam-repository (via opam.ocaml.org) use a different repo file generated here: https://github.com/ocaml-opam/opam2web/blob/d3d36b81174cbbb9df9dfe525cd2cfe77fd86206/bin/opam-web.sh#L26 so this one would probably also need to be modified.

I opened https://github.com/ocaml-opam/opam2web/pull/234 which should work smoothly (at least with my knowledge and tests of shell) -- of course a review would be welcome.

See https://github.com/ocaml/opam-repository/pull/25967 for the update above.

mseri commented 5 months ago

I have also checked the dependency on gpatch for some macos package managers

shonfeder commented 3 weeks ago

I think this has been worked out of the system by this point. Are we OK to close it?

mseri commented 2 weeks ago

I suggest we make an entry on the FAQ in the Wiki with the specific error for this issue and the command to fix it, and then close this issue