Nugetgallery: Improve error message when nuget.org rejects package files with dependencies with 2.X client unsupported version

[airbreather]

This is related to NuGet/Home#6434 and was originally identified (to my knowledge) in a comment there.

I've learned that I should be specifying exclusive upper-bounds for dependencies as something like "2.0.0-0", instead of the naive* "2.0.0", in order to communicate that the dependent package may not be (and, in the real-world case that prompted this report, definitely is not) compatible with any version of the dependency whose major version is 2.

*"2.0.0" is naive because SemVer specifies that, e.g., "2.0.0-pre001" is actually a lower version than "2.0.0", so communicating that "2.0.0" is the upper-bound would not exclude pre-release versions whose major version is "2".

The NuGet application is perfectly happy with "2.0.0-0", and in local testing, it seems to do what I expected it to do. MyGet.org accepts such packages as well (though I haven't tested this exact minimal repro over there).

However, nuget.org rejects packages that follow this format.

When trying to publish through the command-line, I get a nondescript error (InternalServerError https://www.nuget.org/api/v2/package/). Publishing through the website gives me a nicer hint about what's going on:

The package file in question was built using the .NET Core SDK using the following two files. The expectation is that the system should raise red flags if a downstream package references version 2.0.0-pre001 (or at least 2.0.0-pre002, which includes an actual break) of AirBreather.NuGetReproPackages.Referenced, because breaking changes are allowed under SemVer when accompanied by a major version increment:

ClassFromReferencingPackage.cs

using AirBreather.NuGetReproPackages.Referenced;

namespace AirBreather.NuGetReproPackages.Referencing
{
    public static class ClassFromReferencingPackage
    {
        public static int ReferencedClassMethodCount =>
            typeof(ClassFromReferencedPackage).GetMethods().Length;
    }
}

AirBreather.NuGetReproPackages.Referencing.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <Authors>airbreather</Authors>
    <Company>airbreather</Company>
    <Description>Dependent package used as part of a minimal repro of a GitHub issue being opened in NuGet/NuGetGallery</Description>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="AirBreather.NuGetReproPackages.Referenced" Version="[1.0.0,2.0.0-0)" />
  </ItemGroup>

</Project>

Technically, I get the same errors when trying to publish a package whose actual version is "2.0.0-0", but perhaps it's appropriate to reject packages whose actual versions have numeric prerelease tags. I claim that it seems far less appropriate to reject otherwise valid packages that specify upper-bound dependency versions this way, at least until a more satisfying resolution to NuGet/Home#6434 becomes available.

[nkolev92]

This seems like it was a conscious decision in the past.

Found https://github.com/NuGet/NuGetGallery/blob/5276638fccf406a30f126bf8845dccc75045f0f4/src/NuGetGallery.Core/Packaging/ManifestValidator.cs#L173-L191 and https://github.com/NuGet/NuGetGallery/issues/3226.

This is a problem because it doesn't allow people to exclude all the prerelease versions of a certain major like in the linked issue.

However, If the version is semver 2 the validation is skipped, so that's one potential workaround.

Using -a isntead of -0 is another one.

[airbreather]

Using -a isntead of -0 is another one.

Side note, according to the SemVer spec, precedence is determined by lexical ASCII sort order, which I interpret to mean -A < -a :grin:

[nkolev92]

While that is correct, NuGet has implemented version in a case insensitive manner.

http://nugettoolsdev.azurewebsites.net/4.9.0-rtm.5658/version-comparison?versionA=1.0.0-a&versionB=1.0.0-A

Relevant https://github.com/semver/semver/pull/276

https://github.com/semver/semver/pull/276#issuecomment-145193945.
https://github.com/semver/semver/pull/276#issuecomment-276495875

As called out in that thread, neither nuget nor npm handle prereleases perfectly :D

As often said Impementors gonna implement :)

[joelverhagen]

As mentioned by @nkolev92, this was a conscious decision made on behalf of old clients. NuGet 2.13 and older break on 2.0.0-0. For example:
http://nugettoolsdev.azurewebsites.net/2.13.0/parse-version?version=2.0.0-0

There were over a million package downloads in the past 6 weeks from clients 2.13 or older:
https://www.nuget.org/stats

I agree this is not an ideal situation but we need to evaluate the impact of removing this nuget.org restriction. In other words: _How many people will we be affecting?_ _What kind of workaround do these users have?_ _Which specific scenarios are broken for these users? All? Just package discovery? What about restore?_ This bug is particularly bad because if one of these types of versions happens to show up in a list of package from an API response (e.g. package search), some old client versions completely error out. In other words, if a package of this bad format made its way to a common search path (top 20 packages or something), the entire experience could break down.

As a side note, we put a _lot_ of effort into solving this problem for SemVer 2.0.0 versions by introducing new protocol (e.g. search, see semVerLevel) to allow new clients to opt in to seeing SemVer 2.0.0 versions. This was a rather expensive effort and complicated the API surface area.

That being said, 2.13, 2.8, etc are very old clients. We can't support everything forever 😄.

Let's keep this issue open so that we can keep this decision on our radar.

[loic-sharma]

In the meantime, should we improve the error message to something like The package manifest contains a version that's incompatible with legacy clients: '{version}'?

[joelverhagen]

Yeah, sounds good to me.