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

Output either release version or commit hash #483

Merged
merged 6 commits into from
Dec 8, 2021
Merged

Conversation

vesalvojdani
Copy link
Member

This should be fairly non-controversial, but it needs review because I'm not 100% this would actually do what I want for a release version... Also, I started to doubt the idea of either showing the proper release version or the git hash (never both): is there a need somewhere (e.g. sv-comp) where displaying both would be good to precisely identify the competing version?

@sim642 sim642 self-requested a review December 6, 2021 17:35
@sim642 sim642 added the setup Dependencies, CI, releasing label Dec 6, 2021
@sim642
Copy link
Member

sim642 commented Dec 7, 2021

Also, I started to doubt the idea of either showing the proper release version or the git hash (never both): is there a need somewhere (e.g. sv-comp) where displaying both would be good to precisely identify the competing version?

Having both for any kind of release is very much useful to avoid mixups. For example, all the SV-COMP submissions are locally tagged with svcomp22 such that it would be contained in the version output, but if there's need for an updated SV-COMP submission, then that tag is moved. Without the commit hash anywhere in the version output, it would be much harder to verify which binary was compiled from which sources, since both just report as svcomp22.
The same goes for opam releases where you might locally tag it multiple times only to find out problems in the release process that need to be fixed and tagged again.

@vesalvojdani
Copy link
Member Author

Okay, I agree with svcomp having a precise version id printed is useful, but having the hashes in our opam releases is like admitting we don't trust our release process.

@sim642
Copy link
Member

sim642 commented Dec 7, 2021

I suspect installing directly from opam without out repository won't even have the commit hash because it doesn't use the git repository for getting the sources, but a source archive, and using our git describe script stuff to fill that in. I tried at some point in a fresh switch, but I don't remember anymore, I think it said something like 1.1.1 (unknown).

This could be avoided if we didn't use our script (or even a shell command in a dune rule), but also another substitution like %%VCS_COMMIT_ID%%. But that of course does nothing during development strictly in the repo and not installing (ocaml/dune#1539).

There's also another difference. Our script uses git describe --all --long --dirty while there's a substitution VERSION, but that uses git describe --always --dirty. The difference is that our custom thing includes branch names, while the normal describe doesn't. The ugly downside of ours is that it prefixes everything with heads/ or tags/.

I've found a third way of doing this: https://dune.readthedocs.io/en/stable/dune-libs.html#build-info. According to that description, it works for both releases (just containing the version number, no commit hash) and development (git describe --always --dirty). But again, that differs from our script.
There's a bunch of open dune issues related to having more control over dune-build-info: ocaml/dune#4453, ocaml/dune#3856 and ocaml/dune#3523.
Also, it doesn't work for Gobview until dune 3.0 is released: ocaml/dune#4444.

In conclusion, there's no good way to do it exactly as we might want right now. I'll have to rethink this for 2.0 by maybe combining some of the above to get everything to work in all the possible build and install scenarios.

@sim642 sim642 merged commit dc894e3 into master Dec 8, 2021
@sim642 sim642 deleted the version-cleanup branch December 8, 2021 11:25
@sim642 sim642 added this to the v2.0.0 milestone Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
setup Dependencies, CI, releasing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants