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

dotnet restore's "-r" parameter should set RuntimeIdentifier instead of RuntimeIdentifiers if only one is specified #9518

Open
dsplaisted opened this issue Jun 19, 2018 · 12 comments
Milestone

Comments

@dsplaisted
Copy link
Member

From @mikeharder on June 19, 2018 2:11

Repro Steps

  1. dotnet new console --no-restore
  2. dotnet restore -r win-x64
  3. dotnet build -r win-x64 --no-restore

These steps work as expected in CLI 2.1.300. However, in CLI 2.1.301 the build step fails:

C:\Temp\foo\foo.csproj : error : The project was restored using Microsoft.NETCore.App version 2.1.0, but with current settings,
version 2.1.1 would be used instead.  To resolve this issue, make sure the same settings are used for restore and for subsequent
operations such as build or publish.  Typically this issue can occur if the RuntimeIdentifier property is set during build or publish
but not during restore.

The error message is misleading, since the RuntimeIdentifier (-r) is set during both restore and build.

A workaround is to remove the --no-restore from the dotnet build command:

  Restoring packages for C:\Temp\foo\foo.csproj...
  Installing runtime.win-x64.Microsoft.NETCore.DotNetAppHost 2.1.1.
  Installing runtime.win-x64.Microsoft.NETCore.DotNetHostResolver 2.1.1.
  Installing runtime.win-x64.Microsoft.NETCore.DotNetHostPolicy 2.1.1.
  Installing runtime.win-x64.Microsoft.NETCore.App 2.1.1.
  Generating MSBuild file C:\Temp\foo\obj\foo.csproj.nuget.g.props.
  Generating MSBuild file C:\Temp\foo\obj\foo.csproj.nuget.g.targets.
  Restore completed in 24.89 sec for C:\Temp\foo\foo.csproj.
  foo -> C:\Temp\foo\bin\Debug\netcoreapp2.1\win-x64\foo.dll

Is this a bug or by design? Is it expected that dotnet build -r runtime --no-restore should always work after dotnet restore -r runtime?

CC: @JunTaoLuo

Copied from original issue: #2344

@dsplaisted
Copy link
Member Author

From @dasMulli on June 19, 2018 5:51

Looks like a duplicate of #2312

@dsplaisted
Copy link
Member Author

From @livarcocc on June 19, 2018 15:17

@dsplaisted to comment and close.

@dsplaisted
Copy link
Member Author

Here's the documentation of this behavior: https://docs.microsoft.com/en-us/dotnet/core/deploying/runtime-patch-selection

@livarcocc @nguerrera Should we consider setting the RuntimeIdentifier property (instead of the plural RuntimeIdentifiers) when only one RID is passed to restore via -r? It seems like that would make a lot more sense from a user perspective.

@dsplaisted
Copy link
Member Author

Also @KathleenDollard for discussion on whether we should change the behavior of the -r parameter to dotnet restore.

@dsplaisted
Copy link
Member Author

From @dasMulli on June 19, 2018 17:17

Maybe it would help if NuGet actually re-evaluated the project file for each RID, since putting

<RuntimeIdentifiers>osx-x64;win-x64</RuntimeIdentifiers>

no longer helps to fetch all RID-specific packages in advance (in fact, it will even go ahead and download old versions that dotnet publish won't use).
Could also help reduce the number of restore operations since the no-op restore is pretty much useless since the assets file will need updating for each build / publish operation with a difference in -r.

@dsplaisted
Copy link
Member Author

@dasMulli When we were designing the runtime roll-forward for 2.1.300, we considered having separate restores for each RID. We may do so eventually but it would be a significant amount of work. The design we were thinking of was that there would be a separate assets file per combination of Configuration, TargetFramework, and RuntimeIdentifier.

Note that if you want to do self-contained publish for different RIDs, you can set the TargetLatestRuntimePatch to true along with setting the RuntimeIdentifiers property. Then the default restore will restore the right packages. The possible downside is that if you do a Framework-dependent deployment, it will required the latest patch version of the shared framework to run on.

@dsplaisted dsplaisted changed the title dotnet build -r runtime --no-restore fails in 2.1.301 dotnet restore's -r parameter should set RuntimeIdentifier instead of RuntimeIdentifiers if only one is specified Jun 19, 2018
@dsplaisted dsplaisted changed the title dotnet restore's -r parameter should set RuntimeIdentifier instead of RuntimeIdentifiers if only one is specified dotnet restore's "-r" parameter should set RuntimeIdentifier instead of RuntimeIdentifiers if only one is specified Jun 19, 2018
@nguerrera
Copy link
Contributor

Technically, the single assets file has separate entries for distinct RIDs (since they have their own graphs). So it would be possible for NuGet to evaluate the project for each RID. I think there was even a NuGet bug tracking that possibility at one point, though I am not finding it now. My concern with that is perf of evaluating N times. True that you won't get no-op if you're changing RIDs, but I think even no-op would have to do N evaluations with the multiple evaluation approach.

@KathleenDollard
Copy link

@richlander Do you see any downside in storing the RID in RuntimeIdentifier instead of RuntimeIdentifiers

It is a functional change, but it fixes a regression.

We are considering this for 2.2.1xx

@dsplaisted
Copy link
Member Author

@eerhardt @natemcmaster Do you remember whether there was a specific reason that the -r option to dotnet restore sets the RuntimeIdentifiers property instead of RuntimeIdentifier even when only one is specified? Would there be a reason not to use RuntimeIdentifier in the case where only one RID was specified?

@eerhardt
Copy link
Member

It's been like that since that option was added: dotnet/cli#5079. I'm not sure it was considered to switch based on how many were specified.

@nguerrera
Copy link
Contributor

So I think it is a good change, but there is a potential break to dotnet restore -r 'rid1;rid2' I suppose that could be fixed by splitting on semicolon, but feels hacky.

@mcm-ham
Copy link

mcm-ham commented Aug 20, 2019

Is this still an issue since it doesn't look like this issue is targetting the 2.1 or 2.2 milestone any longer and according to ASP.NET Core 2.2 to 3.0 migration guide Microsoft.AspNetCore.App is now removed from csproj?

cgranade referenced this issue in microsoft/qsharp-compiler Sep 18, 2019
@msftgits msftgits transferred this issue from dotnet/cli Jan 31, 2020
@msftgits msftgits added this to the 5.0.1xx milestone Jan 31, 2020
@marcpopMSFT marcpopMSFT added the untriaged Request triage from a team member label Apr 16, 2020
@sfoslund sfoslund added needs team triage Requires a full team discussion and removed untriaged Request triage from a team member labels Apr 20, 2020
@sfoslund sfoslund removed their assignment Apr 20, 2020
@marcpopMSFT marcpopMSFT modified the milestones: 5.0.1xx, Backlog Apr 22, 2020
@marcpopMSFT marcpopMSFT removed the needs team triage Requires a full team discussion label Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants