Support PackageReference without version

[hangy]

With ASP.NET Core 2.1, the main package reference for the core projects is changed from

<PackageReference Include="Microsoft.AspNetCore.All" Version="2.0.8" />

to

<PackageReference Include="Microsoft.AspNetCore.App" />

I think the actual version is determined by the SDK. Running NuKeeper on a csproj that contains this PackageReference displays an ArgumentNullException. This package could be skipped, so that the error is not shown.

Could not read package from <PackageReference Include="Microsoft.AspNetCore.App" /> in file C:\Users\me\Source\Repos\Foo\Bar\Bar.csproj ArgumentException : Value cannot be null or an empty string.
Parameter name: value

Probably caused by the NuGetVersion constructor at

NuKeeper/NuKeeper.Inspection/RepositoryInspection/PackageInProject.cs

Line 15 in ec27e14

this(new PackageIdentity(id, new NuGetVersion(version)), path)

[skolima]

That's... odd :-) this is still a package listed normally on NuGet https://www.nuget.org/packages/Microsoft.AspNetCore.App/ and I definitely missed that "no version" option when upgrading my projects.

[skolima]

Simple repro: dotnet new webapi; nukeeper log=Verbose

call stack:

Could not read package from <PackageReference Include="Microsoft.AspNetCore.App" /> in file c:\carnect\test\test.csproj ArgumentException : Value cannot be null or an empty string.
Parameter name: value
   at NuGet.Versioning.NuGetVersion.Parse(String value)
   at NuGet.Versioning.NuGetVersion..ctor(String version)
   at NuKeeper.Inspection.RepositoryInspection.PackageInProject..ctor(String id, String version, PackagePath path) in C:\Code\NuKeeper\NuKeeper.Inspection\RepositoryInspection\PackageInProject.cs:line 15
   at NuKeeper.Inspection.RepositoryInspection.ProjectFileReader.XmlToPackage(XElement el, PackagePath path) in C:\Code\NuKeeper\NuKeeper.Inspection\RepositoryInspection\ProjectFileReader.cs:line 70```

[AnthonySteele]

Ack! yes, we should just skip this one ref and not crash.
"don't crash" is a good rule for almost any scenario.

[skolima]

Played a bit with other packages without a version - NuGet will just install the lowest version published. Ack. But it's valid, just not very useful. Agreed - best behaviour here is to skip.

[AnthonySteele]

I am not surprised that nuget picks the minimum version; within other constraints like platform support and other packages that could depend on it, with version ranges. I am seeing similar at work where we get an old version of a transitive dependency. Which is sometimes not ideal.

[skolima]

Oh, no, for normal packages, it ignores the target platform constraints completely in this case. Just the oldest version published, end of story :-D

[skolima]

Grabbing ideas from the Big League: dotnet-outdated filters out "auto-references" based on how NuGet parsing reports them ( https://github.com/jerriep/dotnet-outdated/blob/master/src/DotNetOutdated/Services/ProjectAnalysisService.cs#L69 ) but reports references with missing constraints with the lowest-possible version, as this is also coming from NuGet own parsing ( https://github.com/jerriep/dotnet-outdated/blob/master/src/DotNetOutdated/Program.cs#L183 ). It would be nice to get this behaviour (including version range handling and multiple target framework handling), but it's a big piece of work. For now, skipping those dependencies where we can't read the version seems an acceptable workaround.

[skolima]

Ah, we do skip those packages and continue with the rest of analysis. See https://github.com/NuKeeperDotNet/NuKeeper/blob/master/NuKeeper.Inspection/RepositoryInspection/ProjectFileReader.cs#L63 (and the implementations for other 2 formats). @AnthonySteele perhaps the solution here is to not log this as _logger.Error but _logger.Verbose ?

[hangy]

Yup, it does not cause the process to crash, but this being reported as an Error is weird. It might be better to detect the empty version before passing it to the NuGet version, so that it can be skipped correctly (and other Exceptions would still cause an Error).

[AnthonySteele]

The PR above should stop this being reported as an error.

In another codebase, I now have imported versions with a range, e.g. <PackageReference Include="AWSSDK.Core" Version="3.3.*" />

It would be good to detect and work with these.

Looking at outdated again, I see types like NuGet.LibraryModel.LibraryDependency which might be useful?

[skolima]

outdated uses NuGet version range handling. Which is great, but it's going to be a bigger piece of work to move towards it.

[AnthonySteele]

The bug is fixed, what remains is a missing feature.

[skolima]

Those references might be actually going away; issue to track is dotnet/aspnetcore#3307

[skolima]

With #443 in place this issue is most likely ready to be closed?