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

Proposal: Adding System.SemanticVersion #19317

Open
NickCraver opened this issue Nov 9, 2016 · 93 comments
Open

Proposal: Adding System.SemanticVersion #19317

NickCraver opened this issue Nov 9, 2016 · 93 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime design-discussion Ongoing discussion about design without consensus
Milestone

Comments

@NickCraver
Copy link
Member

NickCraver commented Nov 9, 2016

AB#1115515
One of the issues I keep running into with the "default open" world: prereleases. As more and more projects move to semantic versioning, it'd be nice to have a built-in type to handle these. Today, this fails:

Version.Parse("1.0.0-beta1");

Because System.Version has no concept of pre-release suffixes. I'm not arguing it should be added (that's maybe a little to breaking).

Instead how about a new type? System.SemanticVersion The main property most people would be interested in is the prerelease label, but other tenants of semantic versioning should be included. Comparisons being a big one. A lot of devs get this wrong on corner cases. I was looking for how to do best structure this and noticed: NuGet is already doing it. Here's their SemanticVersion class: (part 1, part2).

The API likely needs a little refinement for general use in the BCL, but is there a desire for this from others? I have use cases from parsing package versions to determining which version of Elasticsearch a client is connected to. There's a broad spectrum of semantic versioning use cases now.

Suggestions from others (collating here):

  • It should be comparable to Version (additional operator overloads)

API

namespace System 
{
    public class SemanticVersion : IFormattable, IComparable, IComparable<SemanticVersion>, IEquatable<SemanticVersion>, ICloneable
    {
        public SemanticVersion(int major, int minor, int patch);
        public SemanticVersion(int major, int minor, int patch, string prereleaseLabel);
        public SemanticVersion(int major, int minor, int patch, string prereleaseLabel, string metadata);
        public SemanticVersion(int major, int minor, int patch, IEnumerable<string> prereleaseLabel, IEnumerable<string> metadata);
        public SemanticVersion(string version);

        protected SemanticVersion(Version version, string prereleaseLabel = null, string metadata = null);
        protected SemanticVersion(Version version, IEnumerable<string> prereleaseLabels, IEnumerable<string> metadata);

        public int Major { get; }
        public int Minor { get; }
        public int Patch { get; }
        public virtual IEnumerable<string> PrereleaseLabels { get; }
        public virtual bool IsPrerelease { get; }
        public virtual IEnumerable<string> Metadata { get; }
        public virtual bool HasMetadata { get; }

        public virtual string ToString(string format, IFormatProvider formatProvider);

        public override string ToString();
        public override int GetHashCode();

        public virtual SemanticVersion Clone();
        object ICloneable.Clone() => Clone();

        public virtual bool Equals(SemanticVersion other);
        public virtual bool Equals(Version other);

        public virtual int CompareTo(object obj);
        public virtual int CompareTo(SemanticVersion other);
        public virtual int CompareTo(Version other);

        public static SemanticVersion Parse(string versionString);
        public static bool TryParse(string versionString, out SemanticVersion version);

        public static bool operator ==(SemanticVersion left, SemanticVersion right);
        public static bool operator !=(SemanticVersion left, SemanticVersion right);
        public static bool operator <(SemanticVersion left, SemanticVersion right);
        public static bool operator <=(SemanticVersion left, SemanticVersion right);
        public static bool operator >(SemanticVersion left, SemanticVersion right);
        public static bool operator >=(SemanticVersion left, SemanticVersion right);
 
        public static bool operator ==(Version left, SemanticVersion right);
        public static bool operator !=(Version left, SemanticVersion right);
        public static bool operator <(Version left, SemanticVersion right);
        public static bool operator <=(Version left, SemanticVersion right);
        public static bool operator >(Version left, SemanticVersion right);
        public static bool operator >=(Version left, SemanticVersion right);

        public static explicit operator SemanticVersion(Version version); 

        public static bool operator ==(SemanticVersion left, Version right);
        public static bool operator !=(SemanticVersion left, Version right);
        public static bool operator <(SemanticVersion left, Version right);
        public static bool operator <=(SemanticVersion left, Version right);
        public static bool operator >(SemanticVersion left, Version right);
        public static bool operator >=(SemanticVersion left, Version right);
    }
}
@somewhatabstract
Copy link

As an addition, I'd want to see an associated comparer that could properly sort SemanticVersion and Version values together using semantic versioning rules.

@karelz
Copy link
Member

karelz commented Nov 10, 2016

Sounds like a good idea. Also quite a few votes (15) in just 5 hours ...

Side question out of curiosity: Are all those 15 folks watching the whole repo or was the issue promoted somewhere (Twitter, MVP summit)?

@karelz
Copy link
Member

karelz commented Nov 10, 2016

We need formal API proposal ...

@NickCraver
Copy link
Member Author

I'm talking with the NuGet team tomorrow, I'll try and follow-up on this after. They obviously have some good insights here that are excellent use cases and considerations. I'll try to pitch a formal API (or provide more info) as soon as time allows afterwards.

@cdrnet
Copy link

cdrnet commented Nov 10, 2016

Since a major aspects of the semantics of semantic versioning is about compatibility, it could be worth to also provide basic functionality to check whether an actual semantic version is expected to be compatible to a known version (simply put: same major version, same or larger minor, patch)

@karelz
Copy link
Member

karelz commented Nov 10, 2016

Sounds good, let us know how that goes - assigning to you for now as you're working on it. Let me know if that changes.

@Structed
Copy link

The issue was indeed shared via Twitter by @NickCraver

@karelz
Copy link
Member

karelz commented Nov 11, 2016

@Structed that explains it, thanks!

@NickCraver
Copy link
Member Author

NickCraver commented Nov 12, 2016

I talked with @yishaigalatzer and @rrelyea at the summit a little about this. I'm not sure they're confident NuGet can use a SemanticVersion in the BCL and I'm not sure either - but let's find out!

Starting with NuGet's current implementation (linked in the issue), here's a first pass at what System.SemanticVersion could look like. The changes are including Version comparison operators and conversions, renaming the comparison enum (this will be something we'll have to find common ground on for NuGet to use those overloads, or add methods). I left updated comments on members I thought may be a little confusing.

My hope is that NuGet's SemanticVersion could simply inherit from this at some point. I'm not sure if that's an achievable goal.

namespace Sysem 
{
    public class SemanticVersion : IFormattable, IComparable, IComparable<SemanticVersion>, IEquatable<SemanticVersion>, IComparable<Version>, IEquatable<Version>
    {
        public SemanticVersion(SemanticVersion version); // Copy
        public SemanticVersion(int major, int minor, int patch);
        public SemanticVersion(int major, int minor, int patch, string releaseLabel);
        public SemanticVersion(int major, int minor, int patch, string releaseLabel, string metadata);
        public SemanticVersion(int major, int minor, int patch, IEnumerable<string> releaseLabels, string metadata);

        protected SemanticVersion(int major, int minor, int patch, int revision, string releaseLabel, string metadata);
        protected SemanticVersion(int major, int minor, int patch, int revision, IEnumerable<string> releaseLabels, string metadata);

        protected SemanticVersion(Version version, string releaseLabel = null, string metadata = null);
        protected SemanticVersion(Version version, IEnumerable<string> releaseLabels, string metadata);

        public int Major { get; }
        public int Minor { get; }
        public int Patch { get; }
        public IEnumerable<string> ReleaseLabels { get; }
        public virtual bool IsPrerelease { get; }
        public virtual string Metadata { get; }
        public virtual bool HasMetadata { get; }

        /// <summary>
        /// Gives a normalized representation of the version, excluding metadata.
        /// </summary>
        public virtual string ToNormalizedString();

        /// <summary>
        /// Gives a full representation of the version including metadata.
        /// </summary>
        public virtual string ToFullString();
        public virtual string ToString(string format, IFormatProvider formatProvider);

        public virtual bool Equals(SemanticVersion other);
        public virtual bool Equals(SemanticVersion other, SemanticVersionComparison versionComparison);
        public virtual bool Equals(Version other);

        public virtual int CompareTo(object obj);
        public virtual int CompareTo(SemanticVersion other);
        public virtual int CompareTo(SemanticVersion other, SemanticVersionComparison versionComparison);
        public virtual int CompareTo(Version other);

        public static bool operator ==(SemanticVersion version1, SemanticVersion version2);
        public static bool operator !=(SemanticVersion version1, SemanticVersion version2);
        public static bool operator <(SemanticVersion version1, SemanticVersion version2);
        public static bool operator <=(SemanticVersion version1, SemanticVersion version2);
        public static bool operator >(SemanticVersion version1, SemanticVersion version2);
        public static bool operator >=(SemanticVersion version1, SemanticVersion version2);

        public static explicit operator SemanticVersion(Version version); 

        public static bool operator ==(SemanticVersion version1, Version version2);
        public static bool operator !=(SemanticVersion version1, Version version2);
        public static bool operator <(SemanticVersion version1, Version version2);
        public static bool operator <=(SemanticVersion version1, Version version2);
        public static bool operator >(SemanticVersion version1, Version version2);
        public static bool operator >=(SemanticVersion version1, Version version2);
    }

    public enum SemanticVersionComparison
    {
        /// <summary>
        /// Semantic version 2.0.1-rc comparison.
        /// </summary>
        Default = 0,
        /// <summary>
        /// Compares only the version numbers.
        /// </summary>
        Version = 1,
        /// <summary>
        /// Include Version number and Release labels in the compare.
        /// </summary>
        VersionRelease = 2,
        /// <summary>
        /// Include all metadata during the compare.
        /// </summary>
        VersionReleaseMetadata = 3
    }
}

Some questions:

  • Obvious: what's missing?
  • Overloads: NuGet goes above and beyond this on comparisons, do we need more extension points?
  • Should an implicit operator to Version be included? I'd think this does more harm than good.
  • Should conversions to Version fail if prerelease labels are present? If so, how?
  • Version ranges are a thing, should that be handled in another class? I'd learn towards yes.
  • What variety of formats should be allowed? I think NuGet's implementation has this pretty well covered already, that may be a lift, functionality wise. There are some allocations we can eliminate though.

Thoughts?

@jcolebrand
Copy link

So anything that doesn't match the original Version is now a release-label?

Can't we establish some commonalities around that? alpha, beta, pre, etc. By providing a text enum we start to solidify common business practices and get people on the same stage. You can still have populated release labels but how often are those strings plural?

If you have Version 1.0.9-1pre1 what does that parse into? Are we assuming that 1pre1 is by itself a single release label?

I think that if we're gonna handle sem-ver, let's go ahead and do ranges. Those could be nullable LowerMajor, LowerMinor, LowerPatch as additional fields, and then we could incorporate those into the operator methods. This is because we would assume that your two options are "upper - lower" when you parse things, and that you won't have an array of versions. That would need to be handled with an array of Version or SemVer.

@justinvp
Copy link
Contributor

  1. What are the scenarios for subclassing SemanticVersion? Why not make it sealed like Version and be fully immutable like Version?
  2. Consider a strongly-typed Clone() method that returns SemanticVersion instead of having a copy constructor. SemanticVersion could also implement ICloneable explicitly (privately) with it's ICloneable.Clone() implementation returning the result of calling the strongly-typed Clone(). This would be more consistent with Version which has a Clone() (albeit, not strongly typed) instead of a copy constructor.
  3. Override GetHashCode() (since it overrides Equals() and implements IEquatable<SemanticVersion>)?
  4. Override ToString()?
  5. The operator parameter names should be left and right instead of version1 and version2 according to the framework design guidelines.
  6. What about Parse/TryParse?

@jnm2
Copy link
Contributor

jnm2 commented Nov 12, 2016

I agree that this is a BCL must-have. I am a huge proponent of minimalism as long as people's needs aren't blocked.

@jnm2
Copy link
Contributor

jnm2 commented Nov 12, 2016

@karelz

Side question out of curiosity: Are all those 15 folks watching the whole repo or was the issue promoted somewhere (Twitter, MVP summit)?

I came from Twitter but now that issues that I care about like this have been brought to my attention, I will be watching the repo from now on.

@NickCraver
Copy link
Member Author

NickCraver commented Nov 12, 2016

@jcolebrand SemanticVersion is not constrained to those things, so this has to be more flexible per the rules at http://semver.org/ to be generally useful.

I think that if we're gonna handle sem-ver, let's go ahead and do ranges.

I don't disagree...but what's that look like, in API form? Pitch out some ideas?

@justinvp Responses:

What are the scenarios for subclassing SemanticVersion? Why not make it sealed like Version and be fully immutable like Version?

NuGet does additional comparisons in NuGetVersion, you can find it here. Whether it's best for them to subclass this or not is of course up for debate. Maybe sealing it is better, I'm not sure.

Consider a strongly-typed Clone() method that returns SemanticVersion instead of having a copy constructor.

Good idea, that's a better approach. Updated.

Override GetHashCode() (since it overrides Equals() and implements IEquatable<SemanticVersion>)?
Override ToString()?

Yes, absolutely. I just left these off for conciseness and left them as assumptions - I'll add them back to be explicit this is happening.

The operator parameter names should be left and right instead of version1 and version2 according to the framework design guidelines.

IMO, neither of those are really clear, but if that's consistent elsewhere in the framework of course we should stay consistent. I'll update.

What about Parse/TryParse?

Well now I just look like an idiot for leaving that off. Adding.

Here's a new revision (should we track this in one place at the top, or does that make comments confusing as it's edited? I can see it both ways):

namespace Sysem 
{
    public class SemanticVersion : IFormattable, IComparable, IComparable<SemanticVersion>, IEquatable<SemanticVersion>, ICloneable
    {
        public SemanticVersion(int major, int minor, int patch);
        public SemanticVersion(int major, int minor, int patch, string releaseLabel);
        public SemanticVersion(int major, int minor, int patch, string releaseLabel, string metadata);
        public SemanticVersion(int major, int minor, int patch, IEnumerable<string> releaseLabels, string metadata);

        protected SemanticVersion(int major, int minor, int patch, int revision, string releaseLabel, string metadata);
        protected SemanticVersion(int major, int minor, int patch, int revision, IEnumerable<string> releaseLabels, string metadata);

        protected SemanticVersion(Version version, string releaseLabel = null, string metadata = null);
        protected SemanticVersion(Version version, IEnumerable<string> releaseLabels, string metadata);

        public int Major { get; }
        public int Minor { get; }
        public int Patch { get; }
        public IEnumerable<string> ReleaseLabels { get; }
        public virtual bool IsPrerelease { get; }
        public virtual string Metadata { get; }
        public virtual bool HasMetadata { get; }

        /// <summary>
        /// Gives a normalized representation of the version, excluding metadata.
        /// </summary>
        public virtual string ToNormalizedString();

        /// <summary>
        /// Gives a full representation of the version including metadata.
        /// </summary>
        public virtual string ToFullString();
        public virtual string ToString(string format, IFormatProvider formatProvider);

        public override string ToString();
        public override int GetHashCode();

        public virtual SemanticVersion Clone();
        object ICloneable.Clone() => Clone();

        public virtual bool Equals(SemanticVersion other);
        public virtual bool Equals(SemanticVersion other, SemanticVersionComparison versionComparison);
        public virtual bool Equals(Version other);

        public virtual int CompareTo(object obj);
        public virtual int CompareTo(SemanticVersion other);
        public virtual int CompareTo(SemanticVersion other, SemanticVersionComparison versionComparison);
        public virtual int CompareTo(Version other);

        public static SemanticVersion Parse(string versionString);
        public static bool TryParse(string versionString, out SemanticVersion version);

        public static bool operator ==(SemanticVersion left, SemanticVersion right);
        public static bool operator !=(SemanticVersion left, SemanticVersion right);
        public static bool operator <(SemanticVersion left, SemanticVersion right);
        public static bool operator <=(SemanticVersion left, SemanticVersion right);
        public static bool operator >(SemanticVersion left, SemanticVersion right);
        public static bool operator >=(SemanticVersion left, SemanticVersion right);

        public static explicit operator SemanticVersion(Version version); 

        public static bool operator ==(SemanticVersion left, Version right);
        public static bool operator !=(SemanticVersion left, Version right);
        public static bool operator <(SemanticVersion left, Version right);
        public static bool operator <=(SemanticVersion left, Version right);
        public static bool operator >(SemanticVersion left, Version right);
        public static bool operator >=(SemanticVersion left, Version right);
    }

    public enum SemanticVersionComparison
    {
        /// <summary>
        /// Semantic version 2.0.1-rc comparison.
        /// </summary>
        Default = 0,
        /// <summary>
        /// Compares only the version numbers.
        /// </summary>
        Version = 1,
        /// <summary>
        /// Include Version number and Release labels in the compare.
        /// </summary>
        VersionRelease = 2,
        /// <summary>
        /// Include all metadata during the compare.
        /// </summary>
        VersionReleaseMetadata = 3
    }
}

@clairernovotny
Copy link
Member

@NickCraver The public SemanticVersion Clone() method would need to be virtual based on the current pattern.

@NickCraver
Copy link
Member Author

Oh yes, updating it that way.

I bet we end up sealed here, but I can't be the one to do it, @davidfowl won't talk to me again.

@clairernovotny
Copy link
Member

I think the ReleaseLabels needs a way to be set from a derived class as well. Not sure if it should just be an IList<string> or be virtual?

@yishaigalatzer
Copy link

Please don't ever seal, that's just an unnecessary constraint.

Version ranges do not belong in dotnet core, they are a nuget feature that we plan to keep expanding. Either all of these go to a package from nuget we can keep revving without being tied to dotnet releases as we add new features or stick with just version.

Metadata should not be part of the comparison nor should imho the enum be there. If you want to compare the core version just get the version component out of it. Keep it simple.

Semver only supports 3 levels of version, but nuget always allowed 4 to match backward compatibility

A semver1 vs semver2 class or distinction is required.

@vcsjones
Copy link
Member

vcsjones commented Nov 13, 2016

One issue with the equality operators is this scenario:

Version version = new Version(1, 0, 0);
SemanticVersion semver = new SemanticVersion(1, 0, 0);

var equal2 = semver == version; //Compiles
var equal1 = version == semver; //Does not compile

I'm not a big fan of having equality operators having left and right be in a specific order, by type.

This can be solved in a few ways.

  1. Implement the same operators on Version.

  2. Implement the operators on SemanticVersion with left and right swapped, e.g.:

    public static bool operator ==(SemanticVersion left, Version right);
    public static bool operator ==(Version left, SemanticVersion right);
    public static bool operator !=(SemanticVersion left, Version right);
    public static bool operator !=(Version left, SemanticVersion right);
    //etc...
  3. Decide it is not a problem that needs to be solved.

Personally I like the idea of Version also having matching operators. I suppose there is probably precedent somewhere in the framework that should be followed for how this might be solved.

This also opens up the discussion that currently a SemanticVersion knows how to compare itself to a Version, but a Version doesn't know how to compare itself to a SemanticVersion.

I think it would make sense for that to all be a single proposal, otherwise it's difficult to see how all of the pieces fit together.

@NickCraver
Copy link
Member Author

NickCraver commented Nov 13, 2016

@yishaigalatzer Good thoughts, much appreciated.

Version ranges do not belong in dotnet core, they are a nuget feature that we plan to keep expanding. Either all of these go to a package from nuget we can keep revving without being tied to dotnet releases as we add new features or stick with just version.

I agree on ranges (and didn't include them). If they are only NuGet specific then yes, we should leave them out and hopefully leave this extensible where NuGet can use the base SemanticVersion, if that can be reasonably done. I'm not sure I follow on the second half. Many more people than NuGet have a need for SemanticVersion (hence this proposal), it should be more widely available that that, and IMO, a BCL include as Version is. I'm not 100% against it being in another library, e.g. System.SemanticVersioning, but can you provide some examples of how NuGet has evolved this?

A few followup questions:

  1. A concern: will anyone find it? Or will we have many developers creating it? I hope tooling solves this and makes it a non-issue.
  2. The arguments here are that some pieces are NuGet specific and have no place in a base version. Totally valid. What would be in this package then? A base version that NuGet can inherit in Nuget.Versioning? If so, have the bits in this base version revved any, or just the inheritor NuGet would maintain? I'm trying to ascertain the split of where the changes are happening.
  3. Does the trend towards fewer packages in .NET 2.0 not go against this approach of smaller ones for everything? Maybe @terrajobst can comment on suggestions there?

Metadata should not be part of the comparison nor should imho the enum be there. If you want to compare the core version just get the version component out of it. Keep it simple.

After reading this, I agree. Thoughts from others on nuking the enum and overloads?

A semver1 vs semver2 class or distinction is required.

Perhaps a return of the version in enum form (which can be added to in a non-breaking way) would be the best option here? e.g.:

    public SemanticVersionSpecification VersionSpecification { get; };
...
public enum SemanticVersionSpecification {
    Version1,
    Version2
}

...these are terrible names, but you get the idea. We could determine the minimum (or maximum) level it's compatible with similar to how NuGet does this today.

@vcsjones I looked at the first example I could think of being added later: BigInterger, and it implements both directions, though it's in another library so I think it's viewable to do either way given the inclusion situation.

@vcsjones
Copy link
Member

@NickCraver

I looked at the first example I could think of being added later: BigInterger, and it implements both directions

That seems the best way to go. The more I think on it, the more I dislike the idea of Version knowing anything about SemanticVersion. I would then suggest that the following be added to the proposal:

public static bool operator ==(Version left, SemanticVersion right);
public static bool operator !=(Version left, SemanticVersion right);
public static bool operator <(Version left, SemanticVersion right);
public static bool operator <=(Version left, SemanticVersion right);
public static bool operator >(Version left, SemanticVersion right);
public static bool operator >=(Version left, SemanticVersion right);

NickCraver referenced this issue in opserver/Opserver Nov 14, 2016
Looking at SemanticVersion as a long-term here, but until something like
https://github.com/dotnet/corefx/issues/13526 is in place, reverting to
strings to fix #220.
@dasMulli
Copy link
Contributor

@NickCraver

  1. I believe the metadata should not be a single string field but rather an IEnumerable<string> just like ReleaseLabels. The semver "spec" has specific requirements about how these have to be constructed so it would seem like a good place to put the logic and not require users to look up and implement it themselves.
  2. Does the type have to be class or could it be a struct? That way, no Clone() is needed.
    I don't really see a benefit for using a class here since it is meant to be an immutable structure judging from the signatures. One would loose the ability to override the virtual members though (so it's just like sealed)
  3. On the sealed topic: If i build an API with it, i would expect the behaviour to be the one defined by the BCL and not from a different user's subclass - having an overwritten CompareTo() on an instance passed to an API method, this can potentially blow up. Or a subclass that returns a different metadata string on every call because someone really tries to break my code :trollface: .
    I do believe that there are benefits for implementing custom comparisons but then the user should make use of custom comparers and be aware of the context of the custom logic.
    The only use case i can think of is when a new SemVer version is released and there is a class SemanticVersion3 : SemanticVersion. But that would break any expectation about the behaviour just as much as a custom subclasses would.

Addition:
I would also very much like to see an AssemblySemanticVersionAttribute that makes use of the proposed SemanticVersion. Would be a piece of 🍰 to generate using the new msbuild system and be of great use for diagnostic / logging purposes.

@jnm2
Copy link
Contributor

jnm2 commented Nov 15, 2016

I would also very much like to see an AssemblySemanticVersionAttribute that makes use of the proposed SemanticVersion. Would be a piece of 🍰 to generate using the new msbuild system and be of great use for diagnostic / logging purposes.

It would be super nice to be able to do this in one attribute instead of two:

[assembly: AssemblyVersion("2.0.10.0")]
[assembly: AssemblyInformationalVersion("2.0.10")] // Or sometimes "2.0.10-rc.1"

@tjrobinson
Copy link

This implementation of SemVer might be worth a look: https://github.com/AndyPook/SemVer There are some unit tests based on the spec.

@sharwell
Copy link
Member

sharwell commented Dec 2, 2016

❓ Why is the operator to convert Version to SemanticVersion an explicit operator instead of implicit? I'm generally a fan of avoiding implicit operators, but this seems like a case where the conversion would always succeed without loss of information or broken behavior.

❓ Is there ever a case where semVer {operator} (SemanticVersion)ver is not the same as semVer {operator} ver (where ver is a Version)?

@vcsjones
Copy link
Member

vcsjones commented Dec 2, 2016

Another question:

SemVer does not have 4 components in its version number, it has major, minor, patch. However, this API proposal allows creating an instance from a Version, either by cast or through the constructor. There are also constructor overloads that accept revision. However, it isn't exposed as a property like the other components.

What is the purpose of this 4th component? Are we just ignoring it? If so, then @sharwell's suggestion of an implicit cast is a no-go since the 4th component is ignored. If we aren't ignoring it, are we OK deviating from the SemVer spec?

@JonHanna
Copy link
Contributor

JonHanna commented Dec 2, 2016

I'm not sure about the comparisons. Given 0.9, 0.9.1 and 1.0-beta, surely one always considers 1.0-beta the highest, but vary one whether or not one filters it out of consideration?

@yishaigalatzer
Copy link

yishaigalatzer commented Dec 2, 2016 via email

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@ericstj ericstj moved this from Triage Needed P1 (Requested for 3.1 or 5.0) to Triaged - Future in .NET Core impacting internal partners May 19, 2020
@ericstj ericstj modified the milestones: 5.0, Future May 19, 2020
@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label May 19, 2020
@iSazonov
Copy link
Contributor

iSazonov commented Nov 30, 2020

The last previous 5 comments seem to be the most helpful and explain why this story has been going on for many years without progress.


What do we see?

  1. There are many ecosystems.
  2. Each ecosystem uses the Semantic Version standard in some way.
  3. No ecosystem has a reference implementation of the standard as first class support in their BCL.

What conclusion could be drawn from this?

We could close this discussion and do nothing - we (.Net ecosystem) have been living this way for the last 4 years and nothing terrible has happened, which means it will not happen.

But a reasonable question arises - can we still do something useful for .Net ecosystem?
Since .Net developers use Semantic Version standard and communicate with other ecosystems which use Semantic Version then the reasonable answer is yes.
But that would take us 4 years back to the beginning of this discussion - and this begs the next question -

what are we doing wrong if we can't move forward?

The reasonable answer is: System.SemanticVersion class is not what we need.

This answer seems counterintuitive, but it explains a lot. Let's see what all ecosystems have in common.

what all ecosystems have in common (in the versioning context)?

  1. They use some many years traditions for versioning
  2. Now they try to use Semantic Version standard
  3. They use some Semantic Version rules for versioning - we can say they use _ specific for the ecosystem a subset of Semantic Version standard_.
  4. Whole ecosystem (all projects, services and so on) depends on the rules.
  5. They embed the Semantic Version in their traditional versioning in some way - they defines specific rules for mapping Semantic Version and traditional versioning.
  6. They interact with other ecosystems.

The same for the .Net ecosystem

  1. Uses System.Version class and AssemblyVersion attribute and the old school versioning schema
  2. Uses Semantic Version in some places.
  3. Has specific rules. Ex. for .Net Core https://docs.microsoft.com/en-us/dotnet/core/versions/
  4. Yes, changing something in .Net Core versioning could break all third-party projects and services.
  5. Yes, see p.3 for .Net Core, PowerShell also has simple mapping rules.
  6. Yes, thus PowerShell distributes packages for same Linux distributives with slightly different rules for versioning.

Now we know why here is no progress

  1. Whole ecosystem (all projects, services and so on) depends on the rules.
  2. Yes, changing something in .Net Core versioning could break all third-party projects and services.

Let's look PowerShell ecosystem. If we move PowerShellGet to Semantic Version we should:

  • implement new PowerShellGet site
  • create new PowerShellGet PowerShell module
  • enhance PowerShell itself
  • all PoweShell module developers should update their modules.

So moving to Semantic Version causes an avalanche of changes - this is a catastrophe. We need a lot of effort not to ruin everything and find workarounds and tradeoffs for backward compatibility.

What is the main conclusion?

The solution that does not make breaking changes will take root.
Then it becomes obvious that the only solution that will make progress is to enhancement the System.Version class itself.

What are the requirements for enhancing System.Version class?

  1. No breaking changes
    Really this class is so simple that I hope it is not difficult to add new functionality without breaking changes
  2. Obviously we need to add Semantic Version specific properties.
  3. Support generic behavior
    Since every ecosystem can have specific rules for versioning (parsing, mapping, comparing and so on) the new design should support these customizations.
  4. Support defaults and changing defaults
    Since every ecosystem can have specific rules for versioning they could implement these rules and share its for whole ecosystem.
    So the new design should support implementations of the rules out of the System.Version class, the class should accept the implementations by arguments and has global defaults.
  5. Design and implement defaults for .Net ecosystem. I believe most of .Net projects could benefit from this as a best practice.
  6. As implementation detail, we could use the same trick as DateTimeKind to distinguish "version" and "sematic version" states of Version class.

@StephenBonikowsky StephenBonikowsky moved this from Triaged - Future to Triage Needed P2 in .NET Core impacting internal partners Dec 8, 2020
@jnm2
Copy link
Contributor

jnm2 commented Feb 13, 2021

@iSazonov What if an application has logic that depends on this?

var isTwoToFourPartDottedNumericVersion = Version.TryParse(someString, out _);

@iSazonov
Copy link
Contributor

iSazonov commented Feb 14, 2021

@iSazonov What if an application has logic that depends on this?

var isTwoToFourPartDottedNumericVersion = Version.TryParse(someString, out _);

First point in my proposal is "no breaking changes". My proposal is (1) to enhance Version class with new functionality, (2) to add a mapping of a classic versions to/from semantic versions based on a policies which can be standardized for some communities and can be custom for specific projects. This allows flexible and transparent adoptions to the semantic version standard.

I see MSFT team moved this to P2 and I see no reason why not assign this to 6.0 and brainstorm now.

@jnm2
Copy link
Contributor

jnm2 commented Feb 14, 2021

I don't see how you avoid breaking this scenario if Version.TryParse begins returning true for things like 1.2.3-beta.

attributeSourceFile.Write("[assembly: AssemblyVersion(\"");

var isTwoToFourPartDottedNumericVersion = Version.TryParse(someString, out _);
if (isTwoToFourPartDottedNumericVersion)
{
    attributeSourceFile.Write(someString);
}
else
{
    // Do some special handling to reformat `someString` to make it acceptable to AssemblyVersionAttribute
}

attributeSourceFile.WriteLine("\")]");

@iSazonov
Copy link
Contributor

I don't see how you avoid breaking this scenario if Version.TryParse begins returning true for things like 1.2.3-beta.

A key in the proposal is that we don't change default behavior and you should explicitly enable new feature by assigning a semantic version policy in some way (global flag, specific build, new overload, config option and so on).
Your example just shows that it is impossible in all cases to transparently migrate to Semantic version and developers should have flexibility in this API.

@jnm2
Copy link
Contributor

jnm2 commented Feb 15, 2021

I'm not seeing that in your proposal. Maybe you could spell it out there?

@danmoseley danmoseley moved this from Triage Needed P2 to Triaged - Future (.NET 7) in .NET Core impacting internal partners Jul 21, 2021
@YohanSciubukgian
Copy link

Do you have any update to share on this proposal ?
It could be great to have it out-of-the-box as this semantic versioning format becomes more and more standard over time.

@baronfel
Copy link
Member

I'd love to see SemVer in a place that we could use it in MSBuild if nothing else. Doing comparisons on SDK versions and feature bands is so painful right now.

@richlander
Copy link
Member

@jeffhandley -- Can you re-assess this feature for .NET 8?

@Thaina
Copy link

Thaina commented Dec 19, 2022

I wish semantic versioning should be support in dotnet core. I think we should only pick 2-3 best and most widely used semantic versioning standard and only support that

Maybe we could expand support in the future if we consider making arbitrary pattern in similar way as datetime parsing

@zivkan
Copy link
Member

zivkan commented Mar 1, 2023

This proposal was shown to me yesterday, and I spent some time to read all the comments today. I have some feedback, in case it helps.

  • PrereleaseLabels: IEnumerable<string> or IReadOnlyList<string>

The second blog post I ever wrote was about invetigating a perf difference between a "clean room" NuGet version implementation, and NuGet's own NuGetVersion implementation. I found that switching from NuGetVersion using IEnumerable<string> for prerelease labels, to string[] improved perf by about 50% (or maybe 35%, maybe I calculated the improvement wrong). Having said that, my benchmark had an unreasonably large number of version strings in it (every unique version string on nuget.org). But any app that needs to sort versions (or frequently compare equivilence) will need to enumerate the prerelease labels. I assume that string[] is not important, and anything that can be cast to IList<string> (or IReadOnlyList<T>?) should also have the same perf, as long as the compare/equals implementation does not call GetEnumerator(). I assume the perf benefit is entirely due to the lack of heap allocations in what is a very hot method during sort.

While the class (or struct, if you all decide to do that) can "cheat" and use the backing field for the public property directly, but that would mean that anyone writing a custom comparer can't benefit (well, they could try casting, but that doesn't feel very safe).

  • prerelease segments that are parsable as int

The first blog post I ever wrote was about different implementations of a version class, and how quickly it can sort. The key here is that SemVer says that any pre-release segment (part of the string separated by .) that is only digits, should be sorted as a number. Maybe there's some efficient way to sort two strings of different lengths as numbers without parsing the string as int, but the only way I can think of doing so is to parse the string as int, and then compare the ints.

If we don't care about perf of people writing custom IComparer<SemanticVersion>, then public IReadOnlyList<string> PrereleaseLabels is fine, and we can consider cached pre-parsed int? as an implementation internal detail. But if we want to enable others to be able to compare at maximum performance, then my benchmarks showed that making the pre-parsed int? public, so the comparer doesn't have to parse, can yield about 10% perf improvement, but only if the string and int? are in a class or struct, so that the IComparer<T> doesn't have to do more than 1 array lookup per compare. Interestingly, when there are separate string[] and int?[], then the perf was the same as needing to parse the string as an int. Well, unless there's something wrong with my benchmark code.

I don't know how perf sensitive the runtime is. It does make the public API less clean, but it's something I wanted the relevant people to be aware about, so they can make an informed decision. But 10% might be small enough to ignore the perf improvement to keep a nicer public API.

  • class or struct

A SemanticVersion without either prerelease labels or metadata labels can be entirely stack allocated. But as soon as a version string has either prerelease or metadata, you need some kind of collection (array, or list) to store the parsed segments. Perhaps I just don't know enough about .NET perf, but I don't think it's possible to have a struct with a stack allocated collection as a field or property, is it?

Maybe it doesn't matter, because the collection can be allocated once and as a struct SemanticVersion is copied around it just keeps referencing the same collection, so it can be allocated just once. People more knowledgeable than me can make a decision, but I thought it's another thing that should be known by people making decisions.

  • case sensitive or insensitive

The SemVer spec doesn't explicitly state whether comparison of prerelease labels should be case sensitive or not, only that it should be "compared lexically in ASCII sort order". I assume this to mean case sensitive. However, I imagine that NuGet isn't the only tool that uses the version string in directory names, and therefore "needs" to treat prerelease labels as case insensitive to work as expected on case insensitive file systems.

This is another reason why prerelease labels being IReadOnlyList<T> instead of IEnumerable<T> is a good idea, so that the BCL can either ship a case insensitive SemVer comparer, similar to how there are multiple StringComparer implementations, or at least allow customers to write one of their own, without needing to do allocations and take a perf hit due to GC.

  • Ways NuGetVersion is different

Just an FYI, but a while ago I updated NuGet's docs to explicitly list all the ways I could determine that NuGet's versions differ from SemanticVersion. Probably not very relevant here, but these are the ways that NuGet would need to be capable of extending System.SemanticVersion, otherwise NuGet can't ever adopt the BCL implementation. But I see that this has more or less been mentioned in this issue already.

@jeffhandley
Copy link
Member

@ericstj and I discussed this topic again recently in response to the recent comments and activity here. We're glad to see folks from the NuGet team sharing more notes on if/how something in the core libraries could be used by NuGet (and other packaging systems build on .NET). Nonetheless, a viable path forward hasn't yet surfaced.

The possible approaches that we discussed were:

  1. Extend Version to permit extensibility through storing additional parts and providing a Comparer

    • Existing methods for constructing versions behave as today with only numeric comparisons
    • Define new Parse methods for parsing versions with the extra parts, and provide default comparer handling for them
    • This introduces very high risk that a derived Version could flow through systems that do not anticipate it, and where the consumer has logic very strictly tied to the 4-part version number concept
  2. Unseal Version to permit derived implementations

    • This could theoretically unblock others from deriving from Version to integrate into their flavors of SemanticVersion or custom version implementations (like what NuGet had to do)
    • But the high risk of a derived Version reaching a component that expected a traditional System.Version is still very high
  3. Add a SemanticVersion type into the core libraries

    • This would be what was described as a "clean room" semantic versioning implementation
    • It would be implemented exclusively to the semver spec, with no extensibility
    • Much like was found in the specification itself (see notes below), versioning of Semantic Versioning (in the meta sense) is not feasible, as adding any new parts or changing behavior is bound to break consumers of the versions
    • The type would need to be sealed (for the same reasons as mentioned above)
    • This combination of conditions likely precludes systems like NuGet from being able to use it

Thus far, a clear path toward a viable yet reusable SemanticVersion is not revealing itself. I suspect we're stuck with the status quo where each system that deals with semantic versions carries its own domain-specific implementation where that system's quirks can be handled.

Notes from the SemVer spec struggling with versioning itself:

@KrzysztofCwalina
Copy link
Member

If a new type is added, please don't name it SemanticVersion. There is so much confusion around this term ("semantic versioning") in the .NET ecosystem. We in the Azure SDK team run into many users who write multi-language apps and they have quite specific mental model of what semantic versioning means and does when they come for background of languages that actually do support semantic versioning. .NET does not! Semantic versioning depends on the ability to load more than one version of a package into the a process/context, and on the package/manger using the major/minor version numbers to decide whether to load side-by-side or not. We don't (and cannot) do it in .NET.

Of course, we should (if possible) add the ability for the existing Version type to parse and store more complex version information. I would just not add a new type and especially I would not call it SemanticVersion

@iSazonov
Copy link
Contributor

iSazonov commented Mar 9, 2023

As for Version enhancement, as implementation detail, we could use the same trick as DateTimeKind to distinguish "version" and "sematic version" states of Version class. It could eliminate changes in the behavior of existing code.

@NickCraver
Copy link
Member Author

NickCraver commented Mar 9, 2023

@KrzysztofCwalina could you please expound on this part?

Semantic versioning depends on the ability to load more than one version of a package into the a process/context, and on the package/manger using the major/minor version numbers to decide whether to load side-by-side or not. We don't (and cannot) do it in .NET.

This is honestly the first time I'm ever hearing anything like that. As far as I've ever seen, the semantic versioning is purely for comparison and resolution (e.g. determining which NuGet package version to use), but nothing about side-by-side loading. FWIW, I totally agree this isn't right to land here, all of the comments above make great points. I'm just super curious to learn about that side-by-side requirement and where it comes from :)

@KrzysztofCwalina
Copy link
Member

determining which NuGet package version to use

And this is at the root of the issue. In .NET, the algorithm which package managers use to select package versions is very simple: the latest out of the set of known dependencies. The reason is that CLR can only load one version and so loading any version but the latest would cause program failures. In other languages/runtimes, where the runtimes can load more than one version of a package, the algorithm is more elaborate, where the runtime might load one or more versions depending on whether the versions are compatible or not. And so, the whole semantic versioning scheme (minor/major/revision parts) is to allow packages to communicate to package managers how compatible they [think they] are. Since .NET picks the latest, there is nothing "semantic" (behavioral) about the version parts as the don't affect semantics/behavior.

But, by us using "semantic versioning" in docs, APIs, etc. we create an impression that .NET versioning works like JS/Python versioning does, which is does not. In .NET, only one versioning approach works: be 100% API compatible (i.e. additions only).

@somewhatabstract
Copy link

somewhatabstract commented Mar 14, 2023

Should this therefore be split between a simple semantic version type that contains all the fields, and then separate comparers that implement the various different rules that consumers might require? It seems like that would fit far better than trying to make a single base class that suits all circumstances.

@aayjaychan
Copy link

Semantic versioning depends on the ability to load more than one version of a package into the a process/context, and on the package/manger using the major/minor version numbers to decide whether to load side-by-side or not.

This is one way of using semantic version, but the versioning scheme doesn't require this kind of behaviour.

Semantic versioning is (to me at least) mainly a documentation scheme that describes the relationships between versions, and the urgency and risk of an upgrade. Patch update? Apply ASAP. Major upgrade? Better plan it properly and test it thoroughly. Tools that can parse and compare the version numbers, and workflows that can be built on top, are just nice-to-haves.

I'm surprised anyone would conflate semantic versioning and side-by-side loading. I'm curious where the confusion comes from; the official spec certainly doesn't mention anything about the latter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime design-discussion Ongoing discussion about design without consensus
Projects
No open projects
.NET Core impacting internal partners
  
Triaged - Future (.NET 7)
Development

No branches or pull requests