Why does git height start at 1 instead of 0?

[asherber]

I'm curious about this. It looks like if I change the version in version.json to something like 2.4, and then commit and build, the version number I get is actually 2.4.1 rather than 2.4.0. From the source, it looks like the package is reserving 0 for a changed but uncommitted version.json, and always returning a positive (rather than non-negative) git height once the change is committed.

There are some unfortunate implications here. First, it means that git height for this library is calculated in a different manner from the other libraries I've seen, in which the commit where the change was introduced is 0. (It's also different from what is implied by the README, and what I think a user would expect to be the case.) Second, it means that I can never release a version of my assembly which is x.y.0. Among other things, I think this might run counter to the SemVer spec, which indicates that patch must be reset to 0 when minor or major increments.

Can you clarify why the git height is calculated this way?

[asherber]

It looks like I can get what I expect if I set buildNumberOffset to -1, except that if I change version.json and build before I commit, I get:

Version's parameters must be greater than or equal to zero.
Parameter name: build

I think the issue is in GitExtensions.GetIdAsVersionHelper():

int build = versionHeight.Value + (versionOptions?.BuildNumberOffset ?? 0);
...
return new Version(baseVersion.Major, baseVersion.Minor, build, revision);

This would throw an exception any time there's a negative offset which has greater absolute value than the version height.

It might make sense to either do:

int build = versionHeight.Value + (versionOptions?.BuildNumberOffset ?? 0);
build = Math.Min(build, 0);

which would guarantee a non-negative build number, or:

int build = versionHeight.Value == 0 ? 0 
    : versionHeight.Value + (versionOptions?.BuildNumberOffset ?? 0);

which would prevent the exception with an uncommitted version.json but would leave the exception in other cases (which may have been desired behavior).

[AArnott]

Thanks for the feedback and the time you've taken to investigate your options and report the problem you found. I'll answer you at a high level below, after first responding to a couple points you make:

I think this might run counter to the SemVer spec, which indicates that patch must be reset to 0 when minor or major increments.

I think the point of that spec requirement is that when 1.2.3 is rev'd to 1.3, it shouldn't be 1.3.4 or 1.3.3, but 1.3.0 (the 3rd digit is reset). It doesn't require that you ship the 1.3.0 version precisely. In fact SemVer would insist (IMO) that if you think you'll ship 1.3.0 and then you realize in testing you need to patch something, you'll end up shipping 1.3.1 and never ship 1.3.0, and in such an event, someone pointing out that "you should have shipped 1.3.0 because of semver" would be missing one of the points of semver.

It looks like I can get what I expect if I set buildNumberOffset to -1

That's right. buildNumberOffset is available for at least a couple use cases, including folks who really want to ship a release with revision=0. It also help people migrate when they want to start with an existing major.minor version but need the revision to exceed some number.
Your error report about "Version's parameters must be greater than or equal to zero." being thrown before the commit is something I'm sure we can fix.

To answer your question about git height "starting" with 1 instead of 0, it's simply because I count commits, including the commit that set the version, so the minimum it could ever count is 1. This also has (IMO) a satisfying side effect of discouraging folks from getting hooked on shipping x.y.0 releases. Releasing x.y.0 when the revision number is git height is you could only achieve it by incrementing the version as the very last commit in a release. There are two problems with this:

1. It completely breaks when you realize after that version incrementing commit that you have a bug that requires a newer commit. Now you end up with x.y.1 or more due to the fixes you had to take after bumping the version. In practice, I find this happens a lot. So reaching 0 is already an unlikely scenario.
2. It means although you may have been working on a new x.y release for weeks or perhaps months, you only increment the version after all your changes instead of before them. Think about it: if you ship a prerelease of 4.0, you'd want it to be 4.0-beta, right? You wouldn't want it to be 3.5-beta because you'd already shipped 3.5. So you'll want to bump it to 4.0-beta right after you ship 3.5 as you get to work on your next great release. So you'll be building betas with such versions as 4.0.15-beta, 4.0.20-beta, etc. And when you are ready to go stable, you'll remove the -beta suffix, leaving your version as 4.0.35 as your first stable release in the 4.0 series. So you won't get 4.0.0 anyway.

Does that make sense?

[asherber]

Thanks much for the detailed reply! I don't fully agree with all of your points, but I get where you're coming from and I appreciate your reasoning.

I do think the issue with the build number that I reported above should be addressed in some way. I also think it would be good to tweak the graf on git height in the README to make it clearer what's going on. In particular, it's the word 'between' that I think gives the wrong impression. Maybe something like this (italics indicates changed text):

Git 'height' is the number of commits in the longest path that includes HEAD (the code you're building) and some origin point. In this case the origin is the commit that set the major.minor version number to the values found in HEAD.

For example, if the version specified at HEAD is 3.4 and the longest path in git history from HEAD to where the version file was changed to 3.4 includes 15 commits, then the git height is "15". Note that the first commit to include a changed version file has a git height of 1; this is different from other versioning systems which start counting from 0.

(That last sentence isn't strictly needed, but I think it helps to be explicit about what's going on.)

[AArnott]

Good feedback. I'll fix both the docs and the exception thrown before closing this issue.

[asherber]

Thanks!