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

Possible breakage in opam update #25961

Open
mseri opened this issue May 29, 2024 · 12 comments
Open

Possible breakage in opam update #25961

mseri opened this issue May 29, 2024 · 12 comments

Comments

@mseri
Copy link
Member

mseri commented May 29, 2024

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.

@mseri mseri pinned this issue May 29, 2024
@mseri mseri changed the title Possible breakage Possible breakage in opam update May 29, 2024
@kit-ty-kate
Copy link
Member

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 #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
Copy link
Member Author

mseri commented May 29, 2024 via email

@hannesm
Copy link
Member

hannesm commented May 29, 2024

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
Copy link
Member

hannesm commented May 29, 2024

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
Copy link
Member

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
Copy link
Member

hannesm commented May 29, 2024

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
Copy link
Member

hannesm commented May 29, 2024

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 #25963 to guide contributors how to use extra-source.

@kit-ty-kate
Copy link
Member

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
Copy link
Member

hannesm commented May 29, 2024

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 ocaml-opam/opam2web#234 which should work smoothly (at least with my knowledge and tests of shell) -- of course a review would be welcome.

See #25967 for the update above.

mseri added a commit to mseri/macports-ports that referenced this issue May 30, 2024
Updating the opam package repository can silently fail
when using macos patch whenever files are removed upstream.
As of opam 2.1.6 there is a warning on install to
manually install gpatch to avoid any possible issue
and the corruption of the local switches. See also
ocaml/opam-repository#25961

This commit adds a dependency to gpatch to prevent
any issue and avoid unnecessary manual intervention.

Signed-off-by: Marcello Seri <[email protected]>
@mseri
Copy link
Member Author

mseri commented May 30, 2024

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

pmetzger pushed a commit to macports/macports-ports that referenced this issue May 31, 2024
Updating the opam package repository can silently fail
when using macos patch whenever files are removed upstream.
As of opam 2.1.6 there is a warning on install to
manually install gpatch to avoid any possible issue
and the corruption of the local switches. See also
ocaml/opam-repository#25961

This commit adds a dependency to gpatch to prevent
any issue and avoid unnecessary manual intervention.

Signed-off-by: Marcello Seri <[email protected]>
@shonfeder
Copy link
Contributor

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

@mseri
Copy link
Member Author

mseri commented Oct 28, 2024

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

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

No branches or pull requests

4 participants