Coverlet: Prerelease versions in nightly feed should use semver 2.0

[gitfool]

The current version scheme is causing an issue with Dependabot:

Bump coverlet.collector from 1.3.1-preview.24.g6479f627e4 to 1.3.1-preview.8.ge848786b0d

Prerelease versions in nightly feed should use semver 2.0 (NuGet support). See coverlet package history. Specifically, the git sha should be added as metadata; e.g. 1.3.1-preview.24.g6479f627e4 should be 1.3.1-preview.24+g6479f627e4 so it's ignored in version precedence.

That said, there is another Dependabot issue that will be relevant but don't let that stop you using proper semver 2.0 versions.

[MarcoRossignoli]

Thanks for reporting this, we're using nerdbank for versioning the nightly, I need to check deeper.

https://github.com/coverlet-coverage/coverlet/blob/master/src/coverlet.collector/version.json
https://github.com/coverlet-coverage/coverlet/blob/master/eng/nightly.ps1#L16

[gitfool]

@MarcoRossignoli This difference in behavior can be seen in the build logs for the same commit 6479f62 (standard) (nightly) and is due to the following:
https://github.com/coverlet-coverage/coverlet/blob/6479f627e4dc89d8ef13f3143ab85e4240719e4b/eng/nightly.ps1#L16
This is overriding the PublicRelease property which causes Nerdbank.GitVersioning to append the git sha suffix.

Given the "height" is specified after a prerelease label:
https://github.com/coverlet-coverage/coverlet/blob/6479f627e4dc89d8ef13f3143ab85e4240719e4b/src/coverlet.collector/version.json#L3
... the nightly build could be treated as a public release by either removing the override above or explicitly setting it to true.

Note: the git sha is still embedded in the NuGet package by SourceLink, so no information is lost.

[MarcoRossignoli]

I'd like the idea to have sha on version for fast visual check/select related to repo commit, if I understood well we'll lose that, right?

[gitfool]

Yes, the git sha would be removed from the visible NuGet package version for a "public release". It's not in the standard (non-nightly) package versions though, which is inconsistent. Also, given Dependabot has an issue with such suffixes, it would be better to avoid it.

[MarcoRossignoli]

Got it, we could public like a release /p:PublicRelease=true and have a sem 2 conformance coverlet.console.3.0.0-preview.0.nupkg https://semver.org/#spec-item-9

cc: @tonerdo @petli

[MarcoRossignoli]

@AArnott is possible have + instead of . on non public release version?

"version": "3.0.0-preview.{height}", generates something like 1.3.1-preview.24.g6479f627e4 and to be semVer 2 should be 1.3.1-preview.24+g6479f627e4 for sha metadata

[AArnott]

@MarcoRossignoli Yes, you can make NB.GV generate semver 2-compliant NuGet package versions by adding this to your version.json file:

"nugetPackageVersion": {
     "semVer": 2
  },

[AArnott]

Oh, I misunderstood the question. I think the answer is no.
The reason being that PublicRelease=false is meant to be used on topic branches -- not your default or servicing branches. As such, there can be no assumption of linear versioning as topic branches may have the same height but different content. If we were to include the git commit ID as +buildMetadata then both topic branches may build a package with exactly the same version information (since semver2 forbids build metadata being used to distinguish between versions).
So while build metadata is allowed for _public_ releases, for non-public releases we put it in the -prerelease portion as a prerelease identifier (which is also semver2 compliant). This way each package built is guaranteed to have a unique version for a given commit.

Bump coverlet.collector from 1.3.1-preview.24.g6479f627e4 to 1.3.1-preview.8.ge848786b0d

The opening issue here is wrong. According to semver v2, a prerelease is divided into identifiers using . and a numeric identifier should be ordered as such.:

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows:
Identifiers consisting of only digits are compared numerically.
...

So you see, this is actually a bug in dependabot in that it is sorting 8 as higher than 24 incorrectly.

[MarcoRossignoli]

@AArnott thanks a lot for the clarification, @gitfool do you agree with Andrew?
If so feel free to close this, pre-release versioning seems ok as is(missed the two pre-release info on docs)

[gitfool]

@MarcoRossignoli yes, I linked to the Dependabot issues above, but for some reason thought switching to using metadata for the git sha would help as it would be stripped. However, a quick look at the relevant Dependabot source confirms that the prerelease part of the version is still compared _lexically_, without considering dot separated identifiers consisting of only digits that should be compared _numerically_:

prerelease_string.<=>(other_prerelease_string)

😞

[AArnott]

@gitfool The linked Dependabot issue is about build metadata. I don't see in the issue description any mention of the bug in its -prerelease identifier sorting. Is there a separate issue for that or are you interested in filing one?

[gitfool]

@AArnott yes, the first issue I referred to was semver 2.0 versions not handled correctly for nuget pre-release dependencies.