Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NuGet restore in Visual Studio restores for multiple rids #1474

Closed
dsplaisted opened this issue Feb 5, 2017 · 4 comments · Fixed by dotnet/sdk#870
Closed

NuGet restore in Visual Studio restores for multiple rids #1474

dsplaisted opened this issue Feb 5, 2017 · 4 comments · Fixed by dotnet/sdk#870
Assignees
Labels
Resolution-Fixed The bug has been fixed, refer to the milestone to see in which release it was fixed.
Milestone

Comments

@dsplaisted
Copy link
Member

dsplaisted commented Feb 5, 2017

  • In VS, create a new Console App (.NET Core)
  • Wait for the NuGet restore to complete
  • Inspect the targets in the obj\project.assets.json file

EXPECTED: A single section under targets for .NETCoreApp,Version=v1.0
ACTUAL: The following targets sections are also present: .NETCoreApp,Version=v1.0/win, .NETCoreApp,Version=v1.0/win-x64, and .NETCoreApp,Version=v1.0/win-x86

The main reason this is a problem is because it is different behavior than what happens when you do a command-line restore restore with the .NET CLI. Particularly, if you reference the System.ValueTuple package from a project targeting .NET Core 1.0, you will get package downgrade warnings in Visual Studio, but not if you do a command-line restore.

@natidea @emgarten @rrelyea

@srivatsn
Copy link
Contributor

srivatsn commented Feb 5, 2017

@emgarten should this move to the NuGet repo? I assume the PS is just nominating passing in the TF as NetCoreApp, version=1.0.

@emgarten
Copy link
Member

emgarten commented Feb 5, 2017

I get the same warnings when using desktop MSBuild /t:Restore, and in that scenario the RIDs are coming from:
C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\Microsoft\NuGet\15.0\Microsoft.NuGet.targets

Is this build task suppose to be used for .NETCore projects? I was under the impression that it was only used for project.json and legacy csproj PackageReferences.

I don't know of anywhere that NuGet assumes RIDs during restore, the PS is most likely getting these RIDs from the target above and passing them along during the nomination.

Also, I'm not sure that the RIDs are the problem here. Leaving them off may cause this to fail at runtime.

@srivatsn srivatsn added the Bug label Feb 5, 2017
@srivatsn srivatsn added this to the 1.0 RTM milestone Feb 5, 2017
@dsplaisted
Copy link
Member Author

Also, I'm not sure that the RIDs are the problem here. Leaving them off may cause this to fail at runtime.

If the RIDs aren't added by default when using the .NET CLI, then that's the scenario we have the most test coverage of, so I wouldn't expect there to runtime failures if RuntimeIdentifiers is no longer defaulted.

@natidea
Copy link
Contributor

natidea commented Feb 8, 2017

I recall that back in October, the CLI removed it dependency on Microsoft.NuGet.targets which caused us to react with this PR to pull in properties that were missing as a result: dotnet/sdk#218

However, I don't think we ever removed it from VS/msbuild situations. Isn't this targets file authored in such a way that it is always available to all project types (i.e. because of the corresponding ImportAfter targets file)?

I'll inspect to confirm this is what is actually passing the RIDs to NuGet. I don't see NuGetRuntimeIdentifier or BaseNuGetRuntimeIdentifier in any of our Rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution-Fixed The bug has been fixed, refer to the milestone to see in which release it was fixed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants