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

Set the priority of user-set archive-mirrors higher than the repositories' #4830

Merged
merged 1 commit into from
Dec 1, 2021

Conversation

kit-ty-kate
Copy link
Member

Fixes #4411

I can't find any good reason someone would have a custom cache but want the repository one (even a custom local one) to be taken first.

The one annoying issue with this is that this changes behaviour based on the version of opam used by the user, however we can always announce it before the release (or rather before the change in opam-repository), so it should be ok.

@kit-ty-kate kit-ty-kate added this to PR in Progress in Opam 2.1.x via automation Sep 7, 2021
@kit-ty-kate kit-ty-kate added this to the 2.2.0~alpha milestone Sep 7, 2021
@kit-ty-kate kit-ty-kate removed this from PR in Progress in Opam 2.1.x Sep 7, 2021
@kit-ty-kate kit-ty-kate added this to PR in progress in Opam 2.2.0 via automation Sep 7, 2021
@dra27
Copy link
Member

dra27 commented Sep 17, 2021

The reasoning for the original behaviour is mentioned in #4411 (comment). Discussing this morning, we think the only downside is the roundtrip for a 404 when trying a global cache which doesn't have a file where the repo cache then does (so the downside is small).

Sadly archive-mirrors isn't itself filtered, but the (negative) feedback when archive-mirrors was added to git+https://github.com/ocaml/opam-repository.git in ocaml/opam-repository#17355 (and was reverted in ocaml/opam-repository#17552) was for CI systems, which should more readily be in a position to update to the required version of opam if the change causes problems. I agree that an announcement is probably sufficient on the opam-repository PR which makes the change.

@rjbou rjbou moved this from PR in progress to PR to review in Opam 2.2.0 Nov 15, 2021
@kit-ty-kate
Copy link
Member Author

The one annoying issue with this is that this changes behaviour based on the version of opam used by the user, however we can always announce it before the release (or rather before the change in opam-repository), so it should be ok.

Actually there is a way to do it (or not) based on the opam version: ocaml/opam-repository#20130
So this isn’t an issue anymore.

@kit-ty-kate
Copy link
Member Author

@AltGr approved via DM.

@kit-ty-kate kit-ty-kate merged commit f40e44d into ocaml:master Dec 1, 2021
Opam 2.2.0 automation moved this from PR to review to Done Dec 1, 2021
@kit-ty-kate kit-ty-kate deleted the swap-cache-priority branch December 1, 2021 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

archive-mirrors in the repo file takes over archive-mirrors from the opam config
2 participants