GetRestoreDotnetCliToolsTask fails when missing version metadata

[natemcmaster]

When a PackageReference is missing "Version", it fails gracefully with something like

warning NU1604: Project dependency xunit does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.

When DotNetCliToolReference is missing "Version", it fails completely with an ArgumentNullException. This breaks our ability to use the /t:GenerateRestoreGraphFile target to example a repo's restore graph for issues.

Repro

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

  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
     <DotNetCliToolReference Include="dotnet-xunit" />
  </ItemGroup>

</Project>

dotnet restore

Result

C:\Users\namc\.dotnet\x64\sdk\2.0.0-preview3-006915\NuGet.targets(331,5): error MSB4018: The "GetRestoreDotnetCliToolsTask" task failed unexpectedly.\r [C:\tmp\tst\classlib1\tst.csproj]
System.ArgumentNullException: Value cannot be null.\r [C:\tmp\tst\classlib1\tst.csproj]
Parameter name: value\r [C:\tmp\tst\classlib1\tst.csproj]
    at NuGet.Versioning.VersionRange.TryParse(String value, Boolean allowFloating, VersionRange& versionRange)\r [C:\tmp\tst\classlib1\tst.csproj]
    at NuGet.Versioning.VersionRange.Parse(String value, Boolean allowFloating)\r [C:\tmp\tst\classlib1\tst.csproj]
    at NuGet.Build.Tasks.GetRestoreDotnetCliToolsTask.Execute()\r [C:\tmp\tst\classlib1\tst.csproj]
    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()\r [C:\tmp\tst\classlib1\tst.csproj]
    at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__25.MoveNext() [C:\tmp\tst\classlib1\tst.csproj]

Details

NuGet product used: dotnet.exe
dotnet.exe --version: 2.0.0-preview3-006915

[natemcmaster]

cc @emgarten @nkolev92

[nkolev92]

@natemcmaster Thanks, taking a look now.

[nkolev92]

Double checked in VS, the behavior there is correct.
I can repro the issue with the CLI though.

[nkolev92]

This is a regression from 4.3.
I will work on a fix for 4.4, as I don't think this is a ship stopper.

[emgarten]

@natemcmaster is it common to have tool references without giving a version?

I would say the ideal behavior here is that msbuild would fail with an error giving the line number and helpful error about the project file being incorrect due to a missing version. This would stop MSBuild before the dg file is written.

I can see how it would be useful to make a best effort attempt at writing out the dg file also so I would like to understand how you are using it here.

[natemcmaster]

Our scenario is this: we are removing floating versions from all PackageRef and using a tool to generate an intermediate file containing <PackageReference Update="id" Version="x.y.z" /> that will be imported at the end of the csproj file. In order to generate this, we use /t:GenerateRestoreGraphFile to inspect the dependency graph. DG generation works fine if PackageRef has no version, but fails if DotnetCliToolRef has no version. I'm fairly sure this is an uncommon way to use MSBuild and PackageReferences.

[nkolev92]

@natemcmaster This is fixed now.
It will go into CLI/SDK with our next insertion.

[natemcmaster]

Awesome, thanks!