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

Why is going from version ~x to version ~x~y an upgrade? #4272

Open
MSoegtropIMC opened this issue Jul 11, 2020 · 10 comments · May be fixed by #4996
Open

Why is going from version ~x to version ~x~y an upgrade? #4272

MSoegtropIMC opened this issue Jul 11, 2020 · 10 comments · May be fixed by #4996
Assignees
Projects

Comments

@MSoegtropIMC
Copy link

During upgrade I saw these updates:

  - upgrade   coq-compcert-64        3.7~coq-platform to 3.7~coq-platform~open-source
  - upgrade   coq-compcert           3.7~coq-platform to 3.7~coq-platform~open-source

I wonder why going from 3.7~coq-platform to 3.7~coq-platform~open-source is considered an upgrade. Shouldn't according to the documentation 3.7~coq-platform~open-source be smaller in the version ordering than 3.7~coq-platform since ~ is smaller than any character, including end of line?

See: (https://opam.ocaml.org/doc/Manual.html#Package-Formulas)

It looks like ~ is considered as smaller than end of line only at the beginning of a word, but not in the middle of a word.

@emillon
Copy link
Contributor

emillon commented Jul 23, 2020

Hi!

What's happening here is that the algorithm from Debian that opam is using is interpreting dashes in version names very early. These version strings are parsed like this:

  • version string: 3.7~coq-platform
  • upstream revision: 3.7~coq
  • local revision: platform

and

  • version string: 3.7~coq-platform~open-source
  • upstream revision: 3.7~coq-platform~open
  • local revision: source

To compare these, upstream revisions are first compared, and it amounts to comparing the end of string in the first one to the first dash in the second one. So, the second one is greater.

This is weird and confusing, but this is consistent with the behavior in dpkg. For example:

% dpkg --compare-versions '3.7~coq-platform' 'lt' '3.7~coq-platform~open-source' && echo upgrade
upgrade

So I think you can fix this issue by removing dashes in version parts:

% dpkg --compare-versions '3.7~coqplatform' 'lt' '3.7~coqplatform~opensource' && echo upgrade

@MSoegtropIMC
Copy link
Author

@emillon : thanks for shedding some light on this mystery - well actually a lot of light!

The opam manual explicitly directs to the Debian definition but also gives its own definition (see link in initial post). Would you say that this behavior matches the opam documentation? I don't think so because hyphens are not mentioned as having a special meaning. So I would still say that it is a documentation bug. Either the opam manual should give a correct description of the Debian algorithm or just refer to the Debian definition and give some illustrative typical examples.

@emillon
Copy link
Contributor

emillon commented Jul 23, 2020

The part in the opam manual seems correct to me, but it only describes how to compare two "revisions" (not an official name), not the whole strings with epoch and local revision. I agree that it's a documentation bug.

@MSoegtropIMC
Copy link
Author

I agree that it's a documentation bug.

Should I suggest a correction and you review it?

@emillon
Copy link
Contributor

emillon commented Jul 23, 2020

Sure!

@AltGr
Copy link
Member

AltGr commented Sep 2, 2020

I actually wasn't aware of this (or forgot 🙄), thanks @emillon. Maybe we should simplify the comparison to remove both the epoch and local revision parts, that would make more sense to me 🤔
That would basically mean calling OpamVersionCompare.compare_chunks directly instead of OpamVersionCompare.compare.

No opam package that I am aware of uses the epoch or revisions.

@MSoegtropIMC
Copy link
Author

Maybe we should simplify the comparison to remove both the epoch and local revision parts, that would make more sense to me.

I am fine either way but agree that as is it is more complicated than necessary without giving much of an advantage. Either way the documentation should state this explicitly.

Sorry for not following up on this as yet - I am quite busy with a largish opam based meta build project (https://github.com/MSoegtropIMC/coq-platform) and have quite a few loose threads right now.

@emillon
Copy link
Contributor

emillon commented Sep 2, 2020

No opam package that I am aware of uses the epoch or revisions.

At least nocrypto does, it was updated to -1 and then to -2 by changing adding patches without changing the upstream tarball (in other words, using this the same way Debian does it). I think it's an important mechanism.

As for epochs, no packages seem to use them right now, but this should be useful when we reach a scale when some packages change their version format.

@dra27
Copy link
Member

dra27 commented Jul 8, 2021

Cross-referencing #4566

@dra27 dra27 added this to To do in Opam 2.2.0 via automation Jul 8, 2021
@dra27
Copy link
Member

dra27 commented Jul 8, 2021

Put on the 2.2.0 spike for @AltGr - we should definitely sort the documentation but possibly also take the opportunity to simplify the version handling as suggested in #4272 (comment)

AltGr added a commit to AltGr/opam that referenced this issue Jan 14, 2022
As specified in the docs, and not as handled in Debian: we used to run the
Debian comparison from Dose, which handled epoch prefixes and revision suffixes
in an undocumented way (undocumented in opam, not blaming Dose here!).

Closes ocaml#4272
@AltGr AltGr linked a pull request Jan 14, 2022 that will close this issue
@rjbou rjbou moved this from To do to Bump to 2.3? in Opam 2.2.0 Jan 21, 2022
@dra27 dra27 added this to To do in Opam 2.3 via automation May 17, 2022
@dra27 dra27 removed this from Bump to 2.3? in Opam 2.2.0 May 17, 2022
@dra27 dra27 added this to To do in Opam 2.2.0 via automation May 17, 2022
@dra27 dra27 removed this from To do in Opam 2.3 May 17, 2022
@dra27 dra27 moved this from To do to In progress in Opam 2.2.0 May 17, 2022
@dra27 dra27 added this to To do in Opam 2.3 via automation Jan 23, 2023
@dra27 dra27 removed this from In progress in Opam 2.2.0 Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Opam 2.3
  
To do
Development

Successfully merging a pull request may close this issue.

5 participants