ENH: Use commit count as working copy revision

[lassoan]

When using git, revision is a sha1 hash, which is not as user-friendly as a revision counter as in SVN.

Added an option (enabled by default) to use commit count on the current branch as revision: Slicer_MAIN_PROJECT_WC_REVISION_FROM_COMMIT_COUNT.
Slicer_MAIN_PROJECT_WC_COMMIT_COUNT_BASE offset value can be specified (by deafult=0) to allow adjustment of the revision number.

These options are available to custom applications (read from application properties).

[hjmjohnson]

This seems like a reasonable approach.

[jcfr]

I think we should instead use something like this:

git describe --tags

See https://stackoverflow.com/questions/8595391/how-to-show-git-commit-using-number-of-commits-since-a-tag/22899881

This would include the last release and the number of commit since last release.

[pieper]

Is there a way to get git describe to leave off the object hash? It seems redundant to me for this purpose. I tried --abbrev=0 but it cuts off the commit count too for some reason.

The most logical and readable to me would be something like most recent tag plus number of commits (e.g. v4.10.0+1070, like git describe but with a + not - and without the object hash) which we could create with some small code if there's no git command for that.

My second choice would be just the raw commit count from what Andras proposed (git rev-list HEAD --count).

[jcfr]

just the raw commit count from what Andras proposed

Issue with that approach is that the commit count wouldn't allow to distinguish a minor release done from a branch and a preview (aka nightly) release. The commit count would be the same.

to leave off the object hash?

Since the output of the command is always the same, it would be easy to strip out the hash.

This is what the following python package is doing: https://github.com/warner/python-versioneer

[lassoan]

A global, monotonously increasing revision number served us well and all infrastructure (CDash, Slicer download site, download statistics, etc.) assume this.

If we don't have a strong reason to change to more complicated revisions, I would keep this current method.

Issue with that approach is that the commit count wouldn't allow to distinguish a minor release done from a branch and a preview (aka nightly) release. The commit count would be the same.

We would report the number of commits on the branch, which is actually a very good indicator of how "old" is that version is.

If a stable release is created at commit count 1000, then revision that has commit count 1003 may be slightly different on master and stable branches, but since we don't commit much into stable branches, I don't expect this slight divergence to be confusing at all.

Also not that while commit count in itself is not a unique identifier, but commit count + branch name is unique.

I know that this solution is not universal, but Slicer project is well positioned for this to work well, for a few reasons:

• one official Slicer core repository (no parallel forks)
• no significant parallel development happens on stable branch after stable release
• only one active stable branch
• no change to existing tools and processes, as it is about the same numbering scheme that we had before

The only downside I see is that commit count is not directly visible in GitHub web UI.

[jcfr]

If a stable release is created at commit count 1000, then revision that has commit count 1003 may be slightly different on master and stable branches, but since we don't commit much into stable branches

Currently slicer extensions are retrieved based on revision number. This is particularly important to support the auto-updated of extensions.

After we switch to use commit numbers as described in this PR for extension revision, extension for a preview (aka nightly) version could be confused with the one for a stable release. At least for few days until more commits are added to the master branch.

Ultimately, I think it will be done .. but I wanted to clarify.

In the future, if we have a long running maintenance branch for release, this would become a more prevalent issue.

[lassoan]

We could increment the revision offset in the master branch when we release a new stable. For example, we could add 100 to the revision offset (revision = commit_count+revision_offset) to ensure the same revision number will never be assigned to a stable and master version.

An very nice side-effect is that the simple comparison of revision can always tell which version is newer, even across different major/minor/patch versions (5.0.x versions will always have smaller revision numbers than 5.1.x versions).

[pieper]

Issue with that approach is that the commit count wouldn't allow to distinguish a minor release done from a branch and a preview (aka nightly) release. The commit count would be the same.

Right, but if we always tagged every minor release then tag+count would be unique, easy to read, and informative. I think this would help when communicating with users.

It would make the release history somewhat easier to parse in conversation, like 'preview v4.11.0+1090 was tagged as v5.0.0' or 'you need to get a preview more recent than v5.1.0+45'.

[lassoan]

Ok, so we say almost the same thing. The only small difference is what number we start to count from when we release a new major version.

One option is to restart from 0, the other option is to restart from a safe base number that is comfortably larger than the previous stable release's revision.

Restarting revision from 0 would feel more natural if we used it directly as patch version (4.11.1090 instead of 4.11.0+1090). Is "4.11.0+1090" a common convention? Is it supported by CMake, Python, etc. version comparison macros?

Starting from an increasing offset has the advantage is that a newer Slicer version will always have a larger revision (simple integer comparison works) and we remain compatible with all the existing infrastructure that relied on simple integer revision number.

[pieper]

Is "4.11.0+1090" a common convention?

Probably not, it's just nicer to my eye. But yes, 4.1.0.1090 would also work fine and should be good with most version comparison macros.

[lassoan]

I meant replacing the patch version with the revision (4.11.1090). Then we would follow semantic versioning scheme and thus be compatible with all the version comparison functions.

[pieper]

Using the revision count as the patch would mean we couldn't be able to issue "stable" patch releases, only incremental previews. I guess I could live with that. It would just mean incrementing the major and minor numbers more often, which we should probably be doing anyway.

[pieper]

On mac I get this build error. Adding qPrintable fixes it, but think I can't push to @lassoan 's fork so maybe you can just add it (just make it match the other parameters to the method call).

/Users/pieper/slicer/latest/Slicer/Base/QTGUI/qSlicerApplication.cxx:880:10: error: cannot pass non-trivial object of type 'QString' to variadic method; expected type
      from format string was 'char *' [-Wnon-pod-varargs]
         this->revision(),
         ^~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [Base/QTGUI/CMakeFiles/qSlicerBaseQTGUI.dir/qSlicerApplication.cxx.o] Error 1
make[1]: *** [Base/QTGUI/CMakeFiles/qSlicerBaseQTGUI.dir/all] Error 2
make: *** [all] Error 2

Once it builds it makes this revision which looks right to me.

I'm building a package now.

[lassoan]

I can't push to @lassoan 's fork

I've enabled "Allow edits from maintainers.", so you should be able to push to lassoan:use-commit-count-as-wc-revision branch. Could you try if it works?

[pieper]

Can't push now because of conflict. Looks like what happened is that when I did hub pr checkout 4731 during our meeting I got the version before you force pushed your clean ups. The current one shouldn't have the build error and can be merged IMO.

FWIW, the version with qPrintable package and runs correctly from my mac release build.

[lassoan]

Thanks for testing. If I don't hear anything from @jcfr then I'll merge this before the next nightly build.