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

Automatically update version number by appending git hash #2724

Open
arporter opened this issue Sep 30, 2024 · 17 comments
Open

Automatically update version number by appending git hash #2724

arporter opened this issue Sep 30, 2024 · 17 comments
Labels
enhancement NG-ARCH Issues relevant to the GPU parallelisation of LFRic and other models expected to be used in NG-ARCH

Comments

@arporter
Copy link
Member

At the telco today, we agreed that the PSyclone version number (stored in src/psyclone/version.py and also in doc/reference_guide/doxygen.config) should have the git hash of the last commit (to master) appended to it. This will enable people to see when they are working with a version checked-out of GitHub, as opposed to a released version.

We could do this manually as part of the end-of-the-review process but it would be much better if we could automate it using a GitHub Action.

@hiker
Copy link
Collaborator

hiker commented Oct 1, 2024

I've found https://stackoverflow.com/questions/3442874/in-git-how-can-i-write-the-current-commit-hash-to-a-file-in-the-same-commit. In short, we can't add the hash before writing the file (since adding the hash will change the hash). One solution mentioned was to use the previous hash. Or we could use the post-checkout hook? Not sure how this works with updates, merge, cherry-pick etc.
git attributes are mentioned, but they apparently need to be installed each time manually(?)

On https://softwareengineering.stackexchange.com/questions/333680/how-to-version-when-using-trunk-based-development it was mentioned:

To differentiate "trunk-builds" from official releases, it is common to add a suffix like snapshot-DATETIME to the version number.

Not sure if this can be automated (i.e. modified automatically on commit) - I did a brief search, and there seems to be odd cases where modifying file might have unexpected side effects. I hope one of you knows more of git hooks than I do :)

I am getting the feeling that just adding "-alpha" to the version number after a release might be the easiest option (and would work for me, all I need is to distinguish current master from the last release). It would also follow the semantic versioning standard (https://semver.org/).

One other idea: version.py tries to run and interpret git branch (we can't use git branch --show-current, it provides no output in case of a tag or hash based checkout). Possible outcomes (not sure if this list is really complete)

  • (HEAD detached at 2.5.0) --> checked out tags/2.5.0 - identified by a regular expression of integers and dots
  • (HEAD detached at 2f4162581) --> checked out a hash - identified by not a version number as above, and a valid hex number
  • Any other non-error: we are on a branch - use the existing version number and append - with branch-name (2.5.0-257_verify_loop_fusion)
  • Error - not a git checkout, just use the existing version number.

@arporter
Copy link
Member Author

arporter commented Oct 1, 2024

Thanks for thoroughly investigating Joerg. I like the idea of sticking to SemVer but I'm worried we're going to have e.g. 'alpha' versions of releases that will never actually exist. e.g. at the moment we should probably be at 3.0-alpha which is fine as we will definitely release 3.0. As soon as that is out of the door, we bump the 'working' version to 3.1-alpha. Say we then discover a bug in 3.0 that requires an immediate fix so we need to release 3.0.1. Do we backport the bug fix to 3.0 and then release that or does it go into 3.1? I guess the answer will depend on how urgently the bug fix is needed?

@hiker
Copy link
Collaborator

hiker commented Oct 3, 2024

I would have said to use 3.0.0.1-alpha. But we could also just add ... hot or so - calling it 3.0-hot?

Maye not ideal, but KISS.

@arporter
Copy link
Member Author

arporter commented Oct 7, 2024

Incidentally, when we build the fparser documentation it gives us:
image
which at least makes it very clear you've not got '0.1.5'.

@hiker
Copy link
Collaborator

hiker commented Oct 9, 2024

While not ideal (the reg-exp becomes a bit too general, but I guess a sequence of int and dots, and then letters, digits, + would be ok). It looks a bit like the output of git describe:

$ git describe
0.0.15-447-g2d8cef7

The 447 seems to be the number of commits since ... the last tag or release or so?

But (I think) again the problem: the doc is not part of a commit, it can use a hash. We can't add it to a commit, since this by itself changes the hash (unless we would be happy to add a previous tag, which would be good enough ... or we would manually add 'something' in every time we merge).

@hiker
Copy link
Collaborator

hiker commented Oct 9, 2024

I just found:

>>> import pkg_resources
<stdin>:1: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
>>> pkg_resources.get_distribution("fparser").version
'0.1.5.dev42+g5f24ee2'
>>> pkg_resources.get_distribution("psyclone").version
'2.5.0'

So doesn't really help, since I am on current master ... but thought to mention this, just in case :)

I am getting more and more the feeling that just adding -dev to master after a release would be easiest?

@arporter
Copy link
Member Author

arporter commented Oct 9, 2024

I am getting more and more the feeling that just adding -dev to master after a release would be easiest?

I'm inclined to agree with you. KISS. No additional dependencies. Just requires the reviewer of the first PR after a release to update the version 'number' in the two locations. (One of which is the doxygen config file.) Would @sergisiso and @TeranIvy be happy with this proposal?

@hiker
Copy link
Collaborator

hiker commented Nov 22, 2024

In order to prepare Fab for the next release: what exactly will the version number be: 3.0? Or just 3 (which feels wrong), or ...?

And what are we going to do after the release. My suggestion (as kiss) would be to then change the version number to be 3.0.0.1-dev or so. Which would give us the ability to mark certain un-released staged (3.0.0.2-dev, ..., 3.0.0.27-dev, ...) if required.

@arporter
Copy link
Member Author

It's going to be 3.0. And yes, I agree with your suggestion. Probably it will be 3.0.1-dev and we'll change this to 3.1-dev as soon as anything significant gets merged. I thought SemVer only specified the first three digits as in x.y.z?

@hiker
Copy link
Collaborator

hiker commented Nov 23, 2024

Oh, thanks, I didn't check with SemVer. Three digits is fine ;) OK, then I'll update my scripts to support this.
I think I would actually leave it with only changing the minor version, even if something significant gets merged into dev. Reasoning: I believe we should always have:

version_last_release < any version_on dev < version_next_release

So if we have something significant and release 3.1, this condition will automatically be fulfilled.

I am still having second thoughts :( My best idea of handling 'dev' is to just ignore -dev when parsing version numbers, or adding a fourth digit, i.e. 3.0.1-dev would be 3.0.1.1 internally in Fab. But if we then need to release a 3.0.1, we would violate this condition (3,0,1,1) or (3,0,1) is not <(3,0,1).

The way I see it, we could either:

  • use 3.0.0-dev after the 3.0 release (--> I will use 3.0.0.1 internally, which works with a next 3.0.1 release, and the last released version 3.0)
  • use 3.0.1-dev after the 3.0 release, but then the next minor release would have to be 3.0.2 (to have dev_version < version_next_release.
  • Add a custom version comparison to Fab to ensure e.g. 3.0.1-dev < 3.0.1.

Am I missing something? 3.0.0-dev somehow sounds like it's the development of 3.0.0, so I don't think that's ideal? Or would it be understood as 'development based on 3.0.0?'.

Or is my 'rule' about versioning number comparison stupid/useless? I would prefer not to add custom comparison function for version numbers to Fab (since tuples pretty much do exactly what we need in version comparisons, i.e. ((3,2) > (3,1,99) etc))

@hiker hiker added the NG-ARCH Issues relevant to the GPU parallelisation of LFRic and other models expected to be used in NG-ARCH label Nov 27, 2024
@hiker
Copy link
Collaborator

hiker commented Nov 27, 2024

We agreed to change the version number of master after the release to be 3.0.1-dev. If we have bigger changes, that number might be changed without a release (e.g. if the next release should need to be 3.1, we will change it to 3.1-dev. If required, we might also do more frequent releases.

I've marked this as NG-ARCH, since it is a requirement for my Fab work in NG-ARCH.

@arporter
Copy link
Member Author

I just hit this again in fparser - I was about to say I'd update the version number thanks to your pytest fix but then discovered that we already have:

fparser]$ cat src/fparser/_version.py
# file generated by setuptools_scm
# don't change, don't track in version control
TYPE_CHECKING = False
if TYPE_CHECKING:
    from typing import Tuple, Union
    VERSION_TUPLE = Tuple[Union[int, str], ...]
else:
    VERSION_TUPLE = object

version: str
__version__: str
__version_tuple__: VERSION_TUPLE
version_tuple: VERSION_TUPLE

__version__ = version = '0.2.1.dev2+gf8a590b'
__version_tuple__ = version_tuple = (0, 2, 1, 'dev2', 'gf8a590b')

which, as you can see, has been automatically created for us by setuptools_scm. As the pool of people we have contributing and reviewing grows, automation is increasingly important. Therefore, is there a good reason why we shouldn't just copy what's been implemented in fparser?

(Sergi and I have a nasty feeling that setuptools_scm is the package that gave/gives @TeranIvy a headache on the Crays though?)

@hiker
Copy link
Collaborator

hiker commented Nov 29, 2024

Funny, we must have been looking at the same code. My issue was more that (for me) fparser 0.2 still reports version 0.1.5.dev75+g84a4b1d After some googling, my understanding is that this because I didn't install it again after pulling 0.2. So I did a pip install again, and now I get 0.2.1.dev2+gf8a590b.

So that means this will mean we devs needs to remember to either update the file or do a pip install again. But I have to agree that this is unlikely to ever be an issue for us. One question: if (say) we decide that the next release will not be 3.0.1, but 3.1. Previously at that time this important change comes in, we would update the version from 3.0.1-dev to 3.1.0-dev (so dependent scripts like Fab can be updated). Do we still have that option (without doing an 3.1.0 release??)?

hiker added a commit that referenced this issue Dec 7, 2024
@hiker
Copy link
Collaborator

hiker commented Dec 7, 2024

I have done #2818 to append -dev, while we can decide what to do with setuptool_scm.
FWIW, it appears that somewhere in the pip dependencies for PSyclone, setup_scm is required (didn't check where), I just noticed it in the list of dependencies being installed for PSyclone.

@hiker
Copy link
Collaborator

hiker commented Dec 7, 2024

I just noticed on https://spack.readthedocs.io/en/latest/packaging_guide.html, that they support --develop . Should we instead of -devel use -develop?

@sergisiso
Copy link
Collaborator

I would keep the -dev, spack has several tags develop > main > master > head > trunk > stable that can be used but they just point to a particular branch. The:

version("develop", branch="master")

in the spack package already works, we don't have to adopt their naming scheme

@hiker
Copy link
Collaborator

hiker commented Dec 9, 2024

I would keep the -dev, spack has several tags develop > main > master > head > trunk > stable that can be used but they just point to a particular branch. The:

version("develop", branch="master")

in the spack package already works, we don't have to adopt their naming scheme

You beat me to it, I realised as well that the spack version name is not derived from the package. So I'll leave it the way it is. Thanks!

sergisiso added a commit that referenced this issue Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement NG-ARCH Issues relevant to the GPU parallelisation of LFRic and other models expected to be used in NG-ARCH
Projects
None yet
Development

No branches or pull requests

3 participants