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

Embed version numbers independently from VCS info #1363

Closed
emillon opened this issue May 14, 2020 · 9 comments
Closed

Embed version numbers independently from VCS info #1363

emillon opened this issue May 14, 2020 · 9 comments

Comments

@emillon
Copy link
Collaborator

emillon commented May 14, 2020

Hi,

At the moment version numbers are used for two things:

  • what is displayed by ocamlformat --version.
  • what is considered the "current" version by the check for the version= field in .ocamlformat files.

Comparing to dune, the first one corresponds to the dune version, and the second one to the dune lang version. We require tighter coupling between these two. It's actually a 1:1 mapping so they are not considered two different things. This piece of data is sourced from the VCS using gen_version or, after #1133, by dune-build-info.

This approach of using the same source for both has some limitations:

  • it does not work if ocamlformat sources are embedded in a different repository (such as a different workspace or duniverse)
  • it does not work outside a git repository
  • it is hard to relax the version checks since it is a simple string.

I suggest that instead of having the lang version to be derived from the tool version, there's a current_version embedded somewhere in the code and that is bumped manually at each release.

We already embed some version numbers in the source code to say that this option has been deprecated in version X, or that it has been removed in version Y. That mechanism would be ported to this new version type.

As a side note, the usage of dune-build-info requires special care because dev binaries do not have proper version info. This solves this problem, since it would affect only the output of ocamlformat --version (which is mostly cosmetic), and not version checks.

@jberdine
Copy link
Collaborator

No, this has come up several times, but every commit needs a distinct version that can be reported by ocamlformat --version or similar and checked using the version config.

@jberdine
Copy link
Collaborator

jberdine commented May 14, 2020 via email

@jberdine
Copy link
Collaborator

I don't think that I understand the proposed approach where there are two version numbers, in particular how they can be different. One thing that is needed is for a version number that comes as close to being an exact indicator of behavior as implementable, which can be set in config files. A second thing that is needed is a version number that obeys the "newer than" order that opam uses, to make sure that e.g. opam update chooses newer versions when it thinks it is. Currently, we use one version string to play both these roles, basically by using manually maintained numbers for versions that are released as opam packages, and otherwise falling back on VCS info as the closest approximation to a unique identifier as possible (AFAIU we can't e.g. hash the binary since the version needs to go into it). Is the proposal to keep both of these notions of version at all times, rather than to choose one or the other depending on whether the sources correspond to a released package? Would that be better than using more of the info from git describe --always --dirty, which shows the last tag, the number of commits since then, and the hash. So e.g. 0.14.1-13-g000da99 could be parsed into (0.14.1, 13, 000da99) in order to use the first component of the triple for things like unifying the situation with deprecation warnings. Is this similar to what you have in mind?

@jberdine
Copy link
Collaborator

For the situation when e.g. the sources of ocamlformat are checked out in duniverse, I presume that the checked out source files can be changed relative to the repo, and so just using a fixed version analogous to what can be done for pinning doesn't work. Is there a way to detect if the source files have been changed? If so, we could perhaps do something like use a manually declared version if the sources are unchanged, and otherwise use the VCS hash from the duniverse. Or maybe just manually md5sum the sources ourselves no matter the context.

@gpetiot
Copy link
Collaborator

gpetiot commented May 27, 2020

Could we just declare a version = 0.14.2-34 that is manually updated each time a PR is merged? would we be losing some features? what is the purpose of having the hash embedded in the version number?

@gpetiot gpetiot added this to To do in OCaml Platform via automation Jul 10, 2020
@gpetiot
Copy link
Collaborator

gpetiot commented Sep 7, 2021

ping @emillon

@jberdine
Copy link
Collaborator

jberdine commented Sep 8, 2021

The goal of having git describe --always --dirty in the version is to get a close approximation to a unique identifier for the code that was compiled into the executable. Manually maintaining a version that is a released number with a number of commits is easy to be wrong when building from anything but a checkout of some ancestor of main. In particular, distinguishing between different branches is not possible.

If the goal is to make the version string independent of VCS, could we ask dune for the list of source files and md5sum them ourselves?

@emillon
Copy link
Collaborator Author

emillon commented Sep 10, 2021

I think my point was that some ocamlformat versions behave the same way but are built from a different commit. For example, ocamlformat 0.15.0 and ocamlformat 0.15.1. When checking version= in .ocamlformat we don't care about that much detail, we just want to record the fact that the associated files were formatted with a binary that uses these formatting rules.

This feature request is not blocking anything so I'm fine with closing it too.

@gpetiot
Copy link
Collaborator

gpetiot commented Sep 30, 2021

I think my point was that some ocamlformat versions behave the same way but are built from a different commit. For example, ocamlformat 0.15.0 and ocamlformat 0.15.1. When checking version= in .ocamlformat we don't care about that much detail, we just want to record the fact that the associated files were formatted with a binary that uses these formatting rules.

This feature request is not blocking anything so I'm fine with closing it too.

I think it can make sense to accept some pattern matching on the version number, just having a * for the minor version so 0.15.* could accept 0.15.0 and 0.15.1.
But we would need to modify the archives of all published versions to add this feature and I don't know if that's worth it. Maybe the day ocamlformat is managed by dune (and so the version check) or something else, this tool will just have to disable the version check with the option.

This feature looks distant from the original issue though, so I'm closing it.

@gpetiot gpetiot closed this as completed Sep 30, 2021
OCaml Platform automation moved this from To do to Done Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

3 participants