Allow prerelease versions when parsing release entries

[maxbrunsfeld]

Fixes #449
Fixes #93

@paulcbetts I'm opening this now so that you can 🔫 it down sooner rather than later.

Overview

My strategy here is to avoid changing the format of RELEASES. The -delta and -full suffixes are still used to denote delta vs full packages, but semver pre-release strings are also allowed.

Unlike @GeertvanHorrik's proposed solution, I'm just relying on NuGet.SemanticVersion and its built-in ordering relation (prerelease versions are sorted alphabetically). No hard-coded orderings of specific prerelease strings.

Status

• AFAICT, there are twenty tests failing on master, and the same twenty fail against this branch.
• I haven't done any manual testing yet. I'll test some scenarios using Atom if you think this approach is viable.

[maxbrunsfeld]

/cc @kevinsawicki @nathansobo - This is trying to address Atom's issues w/ Squirrel on our Beta channel.

[anaisbetts]

You don't usually need the private keyword in C#, the default visibility is "as private as possible"

[maxbrunsfeld]

Noted. I was copying NuGet's style here with these static Regex instances.

[GeertvanHorrik]

Good to hear someone is picking this up again, well done. However... The string replacements were there for a reason.

In your case, it means 1.0-unstable123 is higher than 1.0-beta1. Is that really what you want? How are you handling unstable, alpha and beta? I think stuff like "unstable" and "nightly" should never be considered higher than alpha or beta. And since alpha is the first letter, it's hard to find something that comes in before that that makes sense to end users.

From what I remember, NuGet's logic also is case sensitive, so "1.0.0-unstable15" is lower than "1.0.0-Unstable14". So please be very careful when using something that is written for NuGet...

Note that all situations were covered with unit tests in my PR as well, so not sure why Paul didn't accept it. Hopefully yours will be accepted.

And now we are at it. I also added a feature (to my custom branch) that adds the release date. This is very useful in lots of scenarios (showing how long a release has been out and the user is still not on that release, or for license checking scenarios). It's a simple (non-breaking) addition since the parser can just assume null when a release date is unknown (old behavior).

[maxbrunsfeld]

In your case, it means 1.0-unstable123 is higher than 1.0-beta1

Yeah, I can't see any reason why unstable123 should be lower than beta1. From the specification on semver.org (emphasis mine):

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 and identifiers with letters or hyphens are compared lexically in ASCII sort order.

Here is what NuGet.SemanticVersion does to compare pre-release strings:

return StringComparer.OrdinalIgnoreCase.Compare(SpecialVersion, other.SpecialVersion);

Admittedly, they aren't exactly the same. NuGet's logic is case-insensitive, whereas semver stipulates ASCII-order (capitals first). Also, NuGet requires that pre-release names start with letters and don't contain any dots.

Overall though, I think that matching NuGet's behavior is an appropriate way for Squirrel to deal with pre-releases.

[anaisbetts]

Note that all situations were covered with unit tests in my PR as well, so not sure why Paul didn't accept it. Hopefully yours will be accepted.

I'm just as apprehensive now as I was then but this is a blocking bug for Atom's beta program so we're going to try it and see who gets broken. To be clear, I've never been apprehensive about the approach, only the unintended consequences that it could have. Updating software seems easy but is Very Tricky.

[GeertvanHorrik]

Then I am still wondering how atom will handle nightlies and betas. What names will you be using?

[nathansobo]

If we ever do nightlies, they'll always be for the next version. So if the latest beta is 1.2.0-beta-0, then the latest nightly will be 1.3.0-dev. This version states that that nightly will eventually be incorporated into what becomes Atom 1.3.0.

[GeertvanHorrik]

Yes, and how are you going to release 1.3.0-beta-1? Anyway, I solved this by using alpha as nightly builds.

[nathansobo]

Yes, and how are you going to release 1.3.0-beta-1

If I understand your question correctly, as soon as 1.3.0-beta-0 was released, nightlies would start being 1.4.0-dev-SHA, indicating the final stable release version that would contain the code in the nightly.

[GeertvanHorrik]

But according to semver, 1.3.0-beta-1 is smaller than 1.3.0-unstable-1 or 1.3.0-dev-1

[haacked]

But according to semver, 1.3.0-beta-1 is smaller than 1.3.0-unstable-1 or 1.3.0-dev-1

That is correct. If you need metadata in the version that doesn't affect precedence, you need to use the build metadata hash #. For example 1.3.0-beta-1#unstable.

[haacked]

Admittedly, they aren't exactly the same. NuGet's logic is case-insensitive, whereas semver stipulates ASCII-order (capitals first). Also, NuGet requires that pre-release names start with letters and don't contain any dots.

Overall though, I think that matching NuGet's behavior is an appropriate way for Squirrel to deal with pre-releases.

Keep in mind that there's an open PR to change the comparison of labels to be case insensitive.

I'm inclined to accept this PR but the person who submitted is no longer on the NuGet team and I've been very busy with other stuff.

[anaisbetts]

Thanks @maxbrunsfeld!

[maxbrunsfeld]

⚡ Thanks for throwing me a 🍖 on this.

[anaisbetts]

I'll put out a release on Monday or so