Home: Nomination errors, such as bad data (version, tfm etc), are not propagated to Visual Studio.

[nkolev92]

Take this project;

<Project>
  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net472</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="NuGet.Common" Version="blabla" />
  </ItemGroup>


  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
</Project>

Restore from the commandline.
The output contains something like the below:

Microsoft (R) Build Engine version 16.0.360-preview+g9781d96883 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 1/22/2019 2:23:03 PM.
Project "C:\Users\nikolev.REDMOND\Source\Repos\ConsoleApp51\ConsoleApp51\ConsoleApp51.csproj" on node 1 (Restore target(s)).
C:\Program Files (x86)\Microsoft Visual Studio\2019\IntPreview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets(119,5): error : 'blabla' is not a valid version string. [C:\Users\nikolev.REDMOND\Source\Repos\ConsoleApp51\ConsoleApp51\ConsoleApp51.csproj]
Done Building Project "C:\Users\nikolev.REDMOND\Source\Repos\ConsoleApp51\ConsoleApp51\ConsoleApp51.csproj" (Restore target(s)) -- FAILED.


Build FAILED.

"C:\Users\nikolev.REDMOND\Source\Repos\ConsoleApp51\ConsoleApp51\ConsoleApp51.csproj" (Restore target) (1) ->
(Restore target) ->
  C:\Program Files (x86)\Microsoft Visual Studio\2019\IntPreview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets(119,5): error : 'blabla' is not a valid version string. [C:\Users\nikolev.REDMOND\Source\Repos\ConsoleApp51\ConsoleApp51\ConsoleApp51.csproj]

    0 Warning(s)
    1 Error(s)

Restore in VS.

Nothing is seen in the error list.

The output is something like:

All packages are already installed and there is nothing to restore.
Time Elapsed: 00:00:00.1668614
========== Finished ==========

What's happening here is that the restore is being skipped because the package spec could not be generated properly.

//cc @rrelyea
This will become even more apparent when we add the exact version range checks for PackageDownload

[nkolev92]

The reason for this is that the faulty data comes through the nomination API, which means the caller is not NuGet, rather project-system.

[nkolev92]

Note that this does not happen with non-CPS based PackageReference because there is no nomination data that was attempted to be cached, rather we get it on the fly.

[dominoFire]

This also happens with an invalid <TargetFramework> in a .csproj file, for example:

1. Open VS
2. Create a new project, sdk style
3. Open the project file (.csproj) and edit:
   
   
   
   1. Remove <TargetFramework /> tag/node
   
   
4. Save the project file (Ctrl + S)

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/893946

In Visual Studio, you will see an error with the following log:

=====================
5/29/2019 2:44:41 PM
Recoverable
System.AggregateException: One or more errors occurred. ---> NuGet.Frameworks.FrameworkException: Invalid framework identifier ''.
   at NuGet.Frameworks.NuGetFramework.GetShortFolderName(IFrameworkNameProvider mappings)
   at NuGet.Frameworks.NuGetFramework.GetShortFolderName()
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.<>c.<ToPackageSpec>b__11_0(TargetFrameworkInformation tfi)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToPackageSpec(ProjectNames projectNames, IEnumerable TargetFrameworks, String originalTargetFrameworkstr, String msbuildProjectExtensionsPath)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToDependencyGraphSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo, IVsProjectRestoreInfo2 projectRestoreInfo2)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo projectRestoreInfo, IVsProjectRestoreInfo2 projectRestoreInfo2, CancellationToken token)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo2 projectRestoreInfo, CancellationToken token)
   at Microsoft.VisualStudio.ProjectSystem.VS.PackageRestore.PackageRestoreInitiator.PackageRestoreInitiatorInstance.<NominateProjectRestoreAsync>d__12.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.Threading.JoinableTask.<JoinAsync>d__68.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.ProjectSystem.VS.PackageRestore.PackageRestoreInitiator.PackageRestoreInitiatorInstance.<OnRestoreInfoChangedAsync>d__11.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.ProjectSystem.DataReceivingBlockSlim`1.<ProcessInputQueueAsync>d__5.MoveNext()
   --- End of inner exception stack trace ---
---> (Inner Exception #0) NuGet.Frameworks.FrameworkException: Invalid framework identifier ''.
   at NuGet.Frameworks.NuGetFramework.GetShortFolderName(IFrameworkNameProvider mappings)
   at NuGet.Frameworks.NuGetFramework.GetShortFolderName()
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.<>c.<ToPackageSpec>b__11_0(TargetFrameworkInformation tfi)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToPackageSpec(ProjectNames projectNames, IEnumerable TargetFrameworks, String originalTargetFrameworkstr, String msbuildProjectExtensionsPath)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToDependencyGraphSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo, IVsProjectRestoreInfo2 projectRestoreInfo2)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo projectRestoreInfo, IVsProjectRestoreInfo2 projectRestoreInfo2, CancellationToken token)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo2 projectRestoreInfo, CancellationToken token)
   at Microsoft.VisualStudio.ProjectSystem.VS.PackageRestore.PackageRestoreInitiator.PackageRestoreInitiatorInstance.<NominateProjectRestoreAsync>d__12.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.Threading.JoinableTask.<JoinAsync>d__68.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.ProjectSystem.VS.PackageRestore.PackageRestoreInitiator.PackageRestoreInitiatorInstance.<OnRestoreInfoChangedAsync>d__11.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.ProjectSystem.DataReceivingBlockSlim`1.<ProcessInputQueueAsync>d__5.MoveNext()<---

===================

For the other hand, if you dot dotnet.exe run in the same project, you'll see the following:

C:\Program Files\dotnet\sdk\3.0.100-preview5-011568\NuGet.targets(121,5): error : Invalid framework identifier ''. [C:\Users\FEAGUILA\source\repos\no.tfm.v6\no.tfm.v6\no.tfm.v6.csproj]

The build failed. Please fix the build errors and run again.

[davkean]

This has got worse in 16.1 because we're reporting these errors via faults, so you get a giant yellow bar instead of a nice error in the error list:

One thing I noticed that with a large set of TFMs such as:

<TargetFrameworks>netstandard1.0;netstandard2.0;Xamarin.iOS10;MonoAndroid81;uap10.0.16299;net461</TargetFrameworks>

If one of them is wrong you get the above fault but the _error mesage_ doesn't list which TFM is _wrong_ it just says:

NuGet.Frameworks.FrameworkException: Invalid framework identifier ''

[nkolev92]

That's a good point.

I have created https://github.com/NuGet/Home/issues/8174 to fix the Framework Parsing message. We will do that in 5.2/16.2

[zivkan]

I still need to debug the issue with package versions not being parsable, but with regards to a project targeting an invalid TFM, there is a change in behaviour from the project system between 15.9 and 16.2.

In 15.9 when project system calls IVsSolutionResoreService.NominateProjectAsync, IVsProjectRestoreInfo.TargetFrameworks[x].TargetFrameworkMoniker is equal to Unsupported,Version=v0.0, which matches what we return from IVsFrameworkParser.ParseFrameworkName.

In 16.2, we still return Unsupported,Version=v0.0 from IVsFrameworkParser.ParseFrameworkName, but now project system is passing _,Version=v2.0 (where the version number comes from the target framework, I tested with standard2.0) into the nomination API. NuGet should be more resilient, but this currently breaks us because only the string Unsupported is considered unsupported.

@davkean do you know why this was changed?

I'm looking at ways at improving NuGet's handling of this situation while minimising risk of adding breaking changes .

[davkean]

We are now passing the _TargetFrameworkMoniker_ value instead of _TargetFramework_. The former being the actual target framework not what the project says it is. Regardless of what values we pass though, they should be validated and put as errors in the assets files.

[nkolev92]

Regardless of what values we pass though, they should be validated and put as errors in the assets files.

I agree with that. It is just significantly bigger change.

The fix up for the specific code path that @zivkan is investigating is a cheaper win.

[zivkan]

@davkean thanks for the insight into what changed, but do you know why?

We're evaluating a few possible fixes in NuGet, but the ones that feel most "correct" also have the highest chance of breaking customers who use our packages directly.

I know the nomination API contract isn't well defined/formal, but if the value of TargetFrameworkMoniker is only values returned from IVsFrameworkParser.ParseFrameworkName, then the exception that is causing the yellow bar and watson reports wouldn't happen. I don't know where the value "_,Version=x,y" comes from, but if there's a good reason to use that instead of the return value of IVsFrameworkParser.ParseFrameworkName, or just using "Unsupported,Version=0.0", then that's fine. But if this change wasn't to fix a bug elsewhere, perhaps changing this in the project system instead of nuget introduces less risk. NuGet still needs to handle the scenario better and report errors in the assets file, but stopping the watson reports and the yellow bar in VS can be done more quickly.

[davkean]

We need to focus on the underlying cause, not the symptoms that you are hitting this this exact repro. Like PackageReference, and other properties, this data comes from a project file - it can and will contain bad data, whether that's the _TargetFramework_ or the _TargetFrameworkMoniker_ properties. Per the design of the SDK, users are allowed to put anything they want in <TargetFramework> as long as they correctly set <TargetFrameworkMoniker>. There's no guarantee that its parsable or well formed string. We could, for example, go back to passing TargetFramework and they could put the value of _, Version=x.y in this string and we'd be back to the same bug.

If the conclusion from this bug isn't "we're going to put parsing errors in the assets file," then can we meet next week to discuss? @rrelyea

[davkean]

Oh I understand the confusion.

Think of the <TargetFramework> property in the project as a user-provided string for a project config dimension (similar to Configuration|Platform). In the same way that I can have a called <Configuration>ThisIsMyCustomDebugString</Configuration>, users are allowed to have <TargetFramework>ThisIsMyCustomTFM</TargetFramework>. In the default case we translate <TargetFramework> to some well known <TargetFrameworkMoniker> but that's not required. As long as the user has set the <TargetFrameworkMoniker> the build and VS features should _just work_.

I think there's additional confusion because the NuGet code base itself likes to treat these values in <TargetFramework> as canonical handshakes between components. Throughout VS and the build, the only canonical handshake is the "real" moniker; ie _.NETFramework, Version=v4.0_ or _.NETCoreApp, Version=v2.0_ The reason we moved from passing TargetFramework -> TargetFrameworkMoniker is because the latter is the _real_ target that the project system and other components use, whereas previous it could result in VS thinking the moniker is one thing but NuGet thinking it was something else. Passing _TargetFramework_ in this exact case would seemingly fix the problem, but it doesn't resolve the underlying issue; NuGet shouldn't be throwing parsing errors back to the project-system - it should go straight into the Error List.

The relationship between the project-system and NuGet is that we have a bunch of properties that we pass through to NuGet with zero validation. Said valiation is supposed to occur in the same way that restore on command-line does with errors pushed into the assets file we we take and push to the Error List. That way, you get errors on the command-line in the same way you get errors in VS.

[nkolev92]

If the conclusion from this bug isn't "we're going to put parsing errors in the assets file," then can we meet next week to discuss?

That's conclusion from this bug, but it's a significantly bigger change.
We've never written an assets file for an invalid project before, so it's something we need to analyze. But don't get us wrong, we want and should fix this asap. Just not sure where it fits yet (whether it fits in 16.3, while the other watson failures are being treated as high pri) :)

Referring to your explanation.
Would you ever set TargetFrameworkMoniker when multi targeting?

There's no moniker equivalent to TargetFrameworks?

[davkean]

We pass _TargetFrameworks_ solely for you to burn into the props/targets. It's not supposed to be used to figure out the moniker for a project, that's what Frameworks.Select(f => f.TargetFrameworkMoniker) is.

[davkean]

Ignoring errors in assets file, what exact bug are you trying to fix?

[nkolev92]

We pass TargetFrameworks solely for you to burn into the props/targets. It's not supposed to be used to figure out the moniker for a project, that's what Frameworks.Select(f => f.TargetFrameworkMoniker) is.

As a project author of a multi targeting library, how would I declare that in the csproj?

<TargetFrameworks>net472;netcoreapp2.2</TargetFrameworks>

What other ways are there?

Ignoring errors in assets file, what exact bug are you trying to fix?

Specifically https://github.com/NuGet/Home/issues/8385, which is created in response to the watson hits.

[davkean]

In multi-target projects there's effectively two projects; outer project where the <TargetFrameworks> live and an inner project for each value where the <TargetFramework> lives. Only in the inner is the TargetFramework translated -> TargetFrameworkMoniker.

We don't use the canonical versions in the outer project, so we never created it. VS itself doesn't ever see that outer project.

[davkean]

For https://github.com/NuGet/Home/issues/8385, it says we used to call with "Unsupported" as the value of TargetFrameorkMoniker. That does not match what we did prior in 16.1, we didn't translate this value, we pulled it directly from project file: https://github.com/dotnet/project-system/blob/dev16.0.x/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/NuGet/ProjectRestoreInfoBuilder.cs#L53.

[nkolev92]

That aligns to my understanding.

How about the following:

I know it's a badly authored project, but what should NuGet restore for and how should the props/targets conditions be written?
Should NuGet error?

Basically what if the TargetFrameworkMoniker is out of sync from TargetFrameworks?

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

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net472</TargetFrameworks>
  </PropertyGroup>

  <PropertyGroup Condition="'$(TargetFramework)' == 'net472'">
    <TargetFrameworkMoniker>.NETFramework, Version=4.7.2</TargetFrameworkMoniker>
  </PropertyGroup>

    <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
    <TargetFrameworkMoniker>.NETStandard,Version=v2.1</TargetFrameworkMoniker>
  </PropertyGroup>

</Project>

edit

I created an issue specifically to eliminate any inconsistencies in how NuGet treats vs how the rest of build treats them. So we can continue TFM vs TF vs TFs there.

[davkean]

Note that I can repo this existing bug in 15.9 by putting a bad value in the moniker:

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>_,Version=v2.0</TargetFramework>
  </PropertyGroup>

</Project>

System.AggregateException: One or more errors occurred. ---> NuGet.Frameworks.FrameworkException: Invalid framework identifier ''.
   at NuGet.Frameworks.NuGetFramework.GetShortFolderName(IFrameworkNameProvider mappings)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.<>c.<ToPackageSpec>b__34_0(TargetFrameworkInformation tfi)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToPackageSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToDependencyGraphSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo projectRestoreInfo, CancellationToken token)
   at Microsoft.VisualStudio.ProjectSystem.VS.NuGet.PackageRestoreInitiator.PackageRestoreInitiatorInstance.<>c__DisplayClass15_0.<<NominateProject>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.ProjectSystem.CommonProjectSystemTools.Rethrow(Exception ex)
   at Microsoft.VisualStudio.ProjectSystem.ProjectErrorReporting.<>c__DisplayClass6_0.<SubmitErrorReport>b__0()
   at Microsoft.VisualStudio.ProjectSystem.ExceptionFilter.<>c__DisplayClass2_0.<Guard>g__action|0()
   at GuardMethodClass.GuardMethod(Func`1 , Func`2 , Func`2 )
   --- End of inner exception stack trace ---
---> (Inner Exception #0) NuGet.Frameworks.FrameworkException: Invalid framework identifier ''.
   at NuGet.Frameworks.NuGetFramework.GetShortFolderName(IFrameworkNameProvider mappings)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.<>c.<ToPackageSpec>b__34_0(TargetFrameworkInformation tfi)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToPackageSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToDependencyGraphSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo)
   at NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo projectRestoreInfo, CancellationToken token)
   at Microsoft.VisualStudio.ProjectSystem.VS.NuGet.PackageRestoreInitiator.PackageRestoreInitiatorInstance.<>c__DisplayClass15_0.<<NominateProject>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.ProjectSystem.CommonProjectSystemTools.Rethrow(Exception ex)
   at Microsoft.VisualStudio.ProjectSystem.ProjectErrorReporting.<>c__DisplayClass6_0.<SubmitErrorReport>b__0()
   at Microsoft.VisualStudio.ProjectSystem.ExceptionFilter.<>c__DisplayClass2_0.<Guard>g__action|0()
   at GuardMethodClass.GuardMethod(Func`1 , Func`2 , Func`2 )<---

[davkean]

@nkolev92 In that case that project should be considered as targeting ".NETStandard, v2.1" despite whats its <TargetFramework> says.

I think it makes more sense if you look at it like this:

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

  <PropertyGroup>
    <TargetFrameworks>latestnetstandard;net472</TargetFrameworks>
  </PropertyGroup>

  <PropertyGroup Condition="'$(TargetFramework)' == 'net472'">
    <TargetFrameworkMoniker>.NETFramework, Version=4.7.2</TargetFrameworkMoniker>
  </PropertyGroup>

    <PropertyGroup Condition="'$(TargetFramework)' == 'latestnetstandard'">
    <TargetFrameworkMoniker>.NETStandard,Version=v2.1</TargetFrameworkMoniker>
  </PropertyGroup>

</Project>

[nkolev92]

Thanks for the explanation @davkean.
We'll investigate in https://github.com/NuGet/Home/issues/8388.
Even commandline, we treat TargetFrameworks as the source of truth.
We'll need to reconsider that behavior.

Specifically:

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

  <PropertyGroup>
    <TargetFrameworks>latestnetstandard;net472</TargetFrameworks>
  </PropertyGroup>

  <PropertyGroup Condition="'$(TargetFramework)' == 'net472'">
    <TargetFrameworkMoniker>.NETFramework, Version=4.7.2</TargetFrameworkMoniker>
  </PropertyGroup>

    <PropertyGroup Condition="'$(TargetFramework)' == 'latestnetstandard'">
    <TargetFrameworkMoniker>.NETStandard,Version=v2.1</TargetFrameworkMoniker>
  </PropertyGroup>

</Project>

would fail saying latestnetstandard is an unsupported framework.

[davkean]

Agreed. We have the same problem in some components throughout the project-system, I'm slowly weeding them out. The NuGet nomination code was one I tackled in 16.1.

[nkolev92]

Better late than never! :)

[nkolev92]

Related: https://github.com/NuGet/Home/issues/8456