Need a performace and unified way to compare versions

[wli3]

In msbuild condition. msbuild build will try to parse it as dec or hex first. So 2.11 will be smaller than 2.2.

<Project ToolsVersion="15.0" DefaultTargets="Try">
  <Target Name="Try">
   <Error Condition=" '2.11' &lt; 2.2 "
           Text='unexpected' />
  </Target>
</Project>

At the same time. If appending 0 at the end as in 2.2.0, it will cause parse as number fail and consider it as Version. However, Version class's comparison does not consider 2.1 and 2.1.0 to be equal. In fact 2.1 < 2.1.0. This will also occur in Property Function.

<Project ToolsVersion="15.0" DefaultTargets="Try">
  <Target Name="Try">
   <Error Condition=" '2.1' &lt; 2.1.0 "
           Text='unexpected' />
  </Target>
</Project>

It is different behavior of NuGet version.

Also there is performance concern.

[wli3]

cc @dsplaisted @nguerrera @rainersigwald I hope we can discuss it on next Monday's sync meeting

[rainersigwald]

What's the performance concern? Something you've measured, or just theoretical?

[wli3]

@nguerrera do you have concrete example?

[nguerrera]

Before we can say what the performance issue is (if any), I think we have to say what the code we would write would be.

1. Is there an evaluation-friendly way to compare versions without falling prey to 2.1 < 2.1.0 (System.Version annoyance) or 2.11 < 2.2 (Version interpreted as float).
2. If it exists, is (1) reasonable to maintain?
3. If it exists, is (1) reasonably fast compared to what we have now.

[dsplaisted]

@wli3 This is on the agenda for Monday's meeting

[wli3]

(edit description according to above)

[rainersigwald]

The answer @wli3 figured out to 1 is:

https://github.com/dotnet/sdk/issues/2158#issuecomment-382610785

$([System.Version]::Parse("2.11").CompareTo($([System.Version]::Parse("2.2")))) < 0

This seems pretty reasonable to me, and I have no reason to suspect it'd be a significant performance bottleneck--that's why I was asking "measured or theoretical?".

[nguerrera]

If the answers to my questions: are:

1. yes, use this...
2. yes
3. yes

Then, great we'll change our code to that. If there's a no in there, then we may need an msbuild feature.

[nguerrera]

Try

$([System.Version]::Parse("2.1").CompareTo($([System.Version]::Parse("2.1.0")))) < 0

[nguerrera]

I think we first have to normalize both sides to four parts.

[nguerrera]

Somewhat separate, but we would also like there to be a way to compare semantic/nuget versions. We set $(NETCoreSdkVersion) currently based on that and we would like customers to be able to reason about it with minimum versions. Currently, Roslyn has code that is enforcing it as an exact version, and I'd like to get them to change it to a min version, but it occurs to me that I don't know how to write that.

[rainersigwald]

Can we get NuGet's SemanticVersion put into System? I don't want to add another dependency or reimplement semver in MSBuild :(

[rainersigwald]

Ah, already proposed: dotnet/corefx#13526.

[cdmihai]

If we had System.SemanticVersion, would we want to hardcode it in, as general enough? Or provide a way in which different package managers could plugin their own version comparers? I think there's little chance of having other package managers (using something else other than semver) with tight msbuild integration right? :)

[rainersigwald]

I'd say we expose the ".NET Framework way". That's what we've done historically with System.Version but it doesn't behave according to modern expectations (witness 2.1 vs 2.1.0). If we get System.SemanticVersion, that would just be a continuation. We'd probably have to add some syntax to invoke it directly since we can't hijack the existing direct-comparison syntax, since 4-part versions aren't allowed in semver.

[nguerrera]

Pragmatically, I'd want a single type / msbuild expression that supports 4 parts as an extension to semver. But I am not inclined to get into that debate. :(

[rainersigwald]

For 16.0, I propose

$([MSBuild]::CompareVersions($(left), $(right)))

That:

• Version.Parse()s each side
• Pads the resulting object with .0 to reach 4 parts
• Returns System.Version.CompareTo() on the results.

Think this would be totally useless without support for prerelease semvers? Speak up, please!

[clairernovotny]

I think this would be helpful without the prerelease semvers, but adding prerelease semvers is even better. Otherwise, we have to split, and if it's a property that might be, we always have to copy/test for it. Think .NET Core SDK version, which has prereleases and not.

[dsplaisted]

I think it would be helpful if CompareVersions would first drop the prerelease specifiers before parsing versions. That way all prereleases of a given release are treated as equivalent in version to the RTM, but that may actually be a good thing. It means you can't write logic testing if you are on a given prerelease or later, but it also means that you can test that the version is greater than or equal to the RTM version, and still have that logic work when that version is in prerelease.

[rainersigwald]

How about CompareVersions(string left, string right, bool stripAfterHypen = false)?

But maybe it's sufficient to always do that, and doc the behavior.

[nguerrera]

But maybe it's sufficient to always do that, and doc the behavior.

I think I'd prefer that or something that will read better than a literal true in code review. CompareVersionsIgnoringPrerelease or something.

[nguerrera]

Do we also need to worry about versions with '+' and no '-', such as 15.8.166+gd4e8d81a88

Could we get away with stripping after first non-digit-or-dot?

Also, can you allow leading v so that we can compare directly against TargetFrameworkVersion without our ugly _TargetFrameworkVersionWithoutV?

[dsplaisted]

Also, can you allow leading v so that we can compare directly against TargetFrameworkVersion without our ugly _TargetFrameworkVersionWithoutV?

👍

[nguerrera]

Putting a note here that we will support $(_TargetFrameworkVersionWithoutV) outside the SDK. People use it already. But when we do a feature that compare against $(TargetFrameworkVersion) directly, we should encourage folks to do that.

[nguerrera]

(Putting that note here because I just approved a PR that uses it and points here.)

[rainersigwald]

@nguerrera ~was frustrated by this again~ volunteered to take a look at this.

[EdwardBlair]

All components of the version must be integers greater than or equal to 0. Metadata restricts the major, minor, build, and revision components for an assembly to a maximum value of UInt16.MaxValue - 1. If a component exceeds this value, a compilation error occurs.

This implementation does not have such restrictions and can be used to compare, e.g. AssemblyInformationalVersion semvers.