[Bug]: VersionRange [1.0.0, 2.0.0) matches version 2.0.0-alpha.1 which can break package graphs

[ramonsmits]

NuGet Product Used

NuGet SDK

Product Version

NuGet.Versioning 6.3.0

Worked before?

No response

Impact

It bothers me. A fix would be nice

Repro Steps & Context

The following code does output True which seems incorrect AFAIK prereleases should not match if the max version is not inclusive:

var range = VersionRange.Parse("[1.0.0, 2.0.0)");
var version = NuGetVersion.Parse("2.0.0-alpha.1")
Console.WriteLine(range.Satisfies(version));

I can workaround this by doing:

var comparer = new VersionComparer(VersionComparison.Version);
var isMatch = range.Satisfies(v) && (!comparer.Equals(range.MaxVersion, v) || range.IsMaxInclusive);

However, I cannot use .FindBestMatch with NuGetVersionFloatBehavior.AbsoluteLatest. For this I need to do:

IEnumerable<NuGetVersion> versions;

var bestVersion = versions
    .Where(v => r.Satisfies(v) && (!comparer.Equals(r.MaxVersion, v) || r.IsMaxInclusive))
    .OrderByDescending(v => v)
	.FirstOrDefault();

Verbose Logs

No response

[donnie-msft]

@nkolev92 Is the behavior intentional or would this be a DCR to change the behavior? Or is it really a bug?

[nkolev92]

This is working as expected. I don't think we'd want to consider changing it.

The following code does output True which seems incorrect AFAIK prereleases should not match if the max version is not inclusive:

That expectation is incorrect.

Given that 2.0.0-alpha.1 comes before 2.0.0, as all prerelease versions are considered to come before the stable one.

It's findbestmatch that uses context such as prerelease vs not prerelease.

For example, when checking for the best version, if both the lower bound and upper bound are stable, the stable versions are preferred.

These 3 examples should help clarify that:

var range = VersionRange.Parse("[1.0.*, 2.0.0)");

            var versions = new List<NuGetVersion>()
                {
                    NuGetVersion.Parse("1.1.0-alpha.2"),
                    NuGetVersion.Parse("1.5.0"),
                };

            range.FindBestMatch(versions).Should().Be(NuGetVersion.Parse("1.5.0"));

            range = VersionRange.Parse("[1.0.*, 2.0.0)");

            versions = new List<NuGetVersion>()
                {
                    NuGetVersion.Parse("1.6.0"),
                    NuGetVersion.Parse("1.5.0"),
                };

            range.FindBestMatch(versions).Should().Be(NuGetVersion.Parse("1.5.0"));

            range = VersionRange.Parse("[1.0.*, 2.0.0)");

            versions = new List<NuGetVersion>()
                {
                    NuGetVersion.Parse("1.6.0-beta.1"),
                    NuGetVersion.Parse("1.5.0-beta.1"),
                };

            range.FindBestMatch(versions).Should().Be(null);

However, I cannot use .FindBestMatch with NuGetVersionFloatBehavior.AbsoluteLatest. For this I need to do:

Can you elaborate on this?

tldr; Satisfies works as expected, FindBestMatch is what the csproj uses.

[ghost]

This issue has been automatically marked as stale because we have not received a response in 14 days. It will be closed if no further activity occurs within another 14 days of this comment.

[ramonsmits]

@nkolev92 That to me seems incorrect behavior. I know that 2.0.0-alpha.1 comes before 2.0.0 when comparing versions but that is not expected in a range comparison.

I'm pretty sure most if not all people don't want to upgrade to a prerelease of the next major when they specify such a version range. Why would then only want to upgrade to the last pre-release of the next major but not actually interested in a non-release.

In a range range I would expect:

[5,6) anything from 5.0.0 till but not include ANY 6.0.0 (pre)release
[5-beta,6) anything from 5.0.0-beta till but not include ANY 6.0.0 release
[5-beta,6-beta) anything from 5.0.0-beta till but only include 6.0.0 pre releases until 6.0.0-beta (although that doesn't make any sense I think).

I checked and indeed VS suggest the latest prerelease 6.0.0-rc.2.21480.5 of the next major which is really not expected as the compatible range by the packages states [5,6):

<PackageReference Include="System.Text.Json" Version="[5,6)" />

<PackageReference Include="System.Text.Json" Version="[5,6-a)" />

Specifying the max version like 6-a is very ugly. It does not make sense IMHO. You can't do [5,6-*) or [5,6-) but there does not seem to be another syntax.

That means that anywhere we use ranges we must add -a which is just insanely ugly so my tool will just apply the logic earlier that if no prerelease name is specified to ignore prereleases to in the max version.

[ramonsmits]

Take for example all mentioned ranges in the following issue. I'm pretty sure none of them expect that the pre-release package of those majors would be included:

• [Feature]: Allow to specify version range for packagereferences while using the latest / a specific version during build. #11842

[nkolev92]

I don't think you can look at ranges specified in your project file only focusing on what's within the range, you need to apply the context of what's appropriate for that range.
A few other things:

• NuGet's default resolution is lowest compatible.
• Floating version ranges are the only ones that break this and as such only the min range can "float" to a latest version.

I checked and indeed VS suggest the latest prerelease of the next major which is really not expected:

Not sure what you mean by suggests.

Take a look at the following example:

var range = VersionRange.Parse("[5.0.0, 6.0.0)");

            var versions = new List<NuGetVersion>()
                {
                    NuGetVersion.Parse("5.5.0"),
                    NuGetVersion.Parse("5.6.0"),
                };

            range.FindBestMatch(versions).Should().Be(NuGetVersion.Parse("5.5.0"));

            range = VersionRange.Parse("[5.0.0, 6.0.0)");

            versions = new List<NuGetVersion>()
                {
                    NuGetVersion.Parse("5.5.0-preview.2"),
                    NuGetVersion.Parse("5.6.0"),
                };

            range.FindBestMatch(versions).Should().Be(NuGetVersion.Parse("5.6.0"));

            range = VersionRange.Parse("[5.0.0, 6.0.0)");

            versions = new List<NuGetVersion>()
                {
                    NuGetVersion.Parse("5.5.0-preview.2"),
                    NuGetVersion.Parse("5.6.0-preview.6"),
                };

            range.FindBestMatch(versions).Should().Be(null);

A preview version would satisfy the range, but it's not what's preferred.
When choosing a version, if the range is stable, only stable versions are allowed.

It is basically a conscious decision that prerelease ranges allow both stable and prerelease versions and stable ranges allow stable versions only.

<PackageReference Include="System.Text.Json" Version="[5,6-a)" />

What's the goal with this range? 5.x versions and all their prereleases?

[ramonsmits]

• NuGet's default resolution is lowest compatible.

@nkolev92 Yes, but that applies to the whole version graph for all references. Meaning, if a transitive dependency pushes it to the next major then you can have an issue.

Not sure what you mean by suggests.

I included an image and the "compatible" version range. Adjusted the text for clarify in the previous comment.

Your latest sample focusses on MINORS. With minors its not an issue when packages follow Semver as then the newer pre-release MUST still have all the API's it is dependent on. With majors its a different story as it allows breaking changes and there is a very big change API's are gone.

Still IMHO, when I would target [5.0,5.5) then I would ONLY expect pre-release packages of 5.1, 5.2, 5.3 and 5.4 but NOT any pre-release packages of 5.5 .

It is basically a conscious decision that prerelease ranges allow both stable and prerelease versions and stable ranges allow stable versions only.

As a packages maintainer you cannot express if a range allows for pre-release packages WITHIN the range only at the EDGE of the range. Within the range is only in control of the user.

What's the goal with this range? 5.x versions and all their prereleases?

No, the goal of [5,6-a) is to state that it is only compatible with version 5.x and not with 6.x.

With only majors that is not a big issue but with minors it is. For example, take [5.2.0,6.0.0) would indicate I'm compatible with version 5.2.0 until but NOT include 6.0.0. However, due to the current login in VersionRange type it only states that its is only compatible with version 5.2.0 until but not include 6.0.0 but DO include pre-releases which is pretty flawed.

Interestingly we ran into this issue Today twice where the build was working fine because one of the project references was a prerelease package but after updating that its non-prerelease the build suddenly failed. The cause was that ANOTHER reference had a version range that allows for the prerelease but not for the actual release. IMHO that build should have failed as package restore should not have been able to resolve a compatible version graph.

In my opinion the range should have the following logic:

1. ❌ [5,6) 5.0-rc.1
2. ✔️ [5,6) 5.0
3. ✔️ [5,6) 5.1-rc.1
4. ✔️ [5,6) 5.1
5. ❌ [5,6) 6.0-rc.1
6. ❌ [5,6) 6.0

The fact that it currently passes for (5) is a flaw and already has bitten us several times and really not expected for most users. It allows for some builds to just pass and compiles successfully while at runtime fail because of missing APIs.

Can you give me an example of a use case where this actually does make sense?

Just stating that it is working as expected does not make the behavior right as it shouldn't.

Are you expecting all those packages maintainers that they must state their version ranges [5,6-a) instead of [5,6) as what most if not all maintainers do?

[nkolev92]

I included an image and the "compatible" version range. Adjusted the text for clarify in the previous comment.

The PM UI doesn't really filter the versions in the version list today. That's covered in #6566.

I think we're trying to discuss 2 different topics here. Range expression through the NuGetVersionRange type and what the resolvers do are separate things.

With only majors that is not a big issue but with minors it is. For example, take [5.2.0,6.0.0) would indicate I'm compatible with version 5.2.0 until but NOT include 6.0.0. However, due to the current login in VersionRange type it only states that its is only compatible with version 5.2.0 until but not include 6.0.0 but DO include pre-releases which is pretty flawed.

As I mentioned, prerelease versions are only going to be selected if other components explicitly request them. NuGet won't just use a version because it fits the range. It looks for the lowest compatible.

[ramonsmits]

The PM UI doesn't really filter the versions in the version list today. That's covered in #6566.

Not sure what you are referring to but the screenshot I added earlier clearly shows the package manager suggesting a RC of the next major.

Range expression through the NuGetVersionRange type and what the resolvers do are separate things.

The resolvers apply the same version range check. See sample below.

As I mentioned, prerelease versions are only going to be selected if other components explicitly request them. NuGet won't just use a version because it fits the range. It looks for the lowest compatible.

Unless a transient dependency pushes it into the next major. As that suddenly becomes the lowest compatible version. Take the following example:

Project:

Package A [5.0.0,6.0.0)
Package B [1.0.0,2.0.0)

Package A v 5.0.0:

Package B [2.0.0-rc.1,3.0.0)

Now Nuget will resolve version 2.0.0 prerelease packages as that is the only common compatible version in the version graph. That is clearly unwanted, this shouldn't even be allowed to compile as the intent of the [1.0.0,2.0.0) range it to not include v2, not even pre-release packages.

The reason for it so far is that VersionRange allows it. The only sane matches should be:

1. ❌ [5,6) 5.0-rc.1
2. ✔️ [5,6) 5.0
3. ✔️ [5,6) 5.1-rc.1
4. ✔️ [5,6) 5.1
5. ❌ [5,6) 6.0-rc.1
6. ❌ [5,6) 6.0

[ramonsmits]

@nkolev92 @donnie-msft Can this please be reopened? It really causes issues. I've now seen it so manny times due to dependabot upgrading to pre-releases that at runtime are not compatible.

[ramonsmits]

It seems this is more or less a duplicate of:

• NU1608 warning is not evaluating prerelease versions combined with exclusive upper bounds properly #6434

[zivkan]

I just created a proposal for a warning to help guide customers to use a syntax that achieves the desired outcome, without causing breaking behaviour (well, as long as you don't count customers who use TreatWarningsAsErrors as breaking): #12423. Feel free to add a 👍 reaction to the new issue to upvote it and increase its prioritization.