Remove Guardrails - Restore of "runtimes" section should only apply to the current project #3768

Closed
jgoshi opened this Issue Oct 25, 2016 · 13 comments

Comments

Projects
None yet
5 participants
@jgoshi

jgoshi commented Oct 25, 2016

NuGet product used: dotnet.exe
NuGet version (x.x.x.xxx):
dotnet.exe --version: 1.0.0-preview3-003892
OS version: Windows Server 2012 R2

In the project.json world, the runtimes section would only apply to the current project and not and P2P projects. For csproj files it looks like it tries to do this for all projects.

Repro steps (in the dotnet cli repo)

  1. Go to tools\Archiver
  2. Run dotnet restore
  3. Success
  4. Go to src\Microsoft.Dotnet.Archive (which is a dependent project for tools\Archiver)
  5. Add the "runtimes" section to the project.json file
  6. dotnet restore fails

The above implies that dotnet restore only applies the runtimes section to the current project.

If we try the same thing in the csproj world.

  1. Go to tools\Archiver
  2. Run dotnet migrate
  3. Run dotnet restore3 .\Archiver.csproj /p:RuntimeIdentifiers=win7-x64

We get a failure (and it appears to be because the runtime identifier is carried across P2P):

"C:\cli\tools\Archiver\Archiver.csproj" (Restore target) (1) ->
(Restore target) ->
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.AppContext 4.1.0 provides a co
mpile-time reference assembly for System.AppContext on .NETStandard,Version=v1.3, but there is no run-time assembly comp
atible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Diagnostics.Tracing 4.1.0 prov
ides a compile-time reference assembly for System.Diagnostics.Tracing on .NETStandard,Version=v1.3, but there is no run-
time assembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.IO 4.1.0 provides a compile-ti
me reference assembly for System.IO on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win8
1-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Linq 4.1.0 provides a compile-
time reference assembly for System.Linq on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with
win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Linq.Expressions 4.1.0 provide
s a compile-time reference assembly for System.Linq.Expressions on .NETStandard,Version=v1.3, but there is no run-time a
ssembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Reflection 4.1.0 provides a co
mpile-time reference assembly for System.Reflection on .NETStandard,Version=v1.3, but there is no run-time assembly comp
atible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Runtime 4.1.0 provides a compi
le-time reference assembly for System.Runtime on .NETStandard,Version=v1.3, but there is no run-time assembly compatible
with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Runtime.Extensions 4.1.0 provi
des a compile-time reference assembly for System.Runtime.Extensions on .NETStandard,Version=v1.3, but there is no run-ti
me assembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Runtime.InteropServices 4.1.0
provides a compile-time reference assembly for System.Runtime.InteropServices on .NETStandard,Version=v1.3, but there is
no run-time assembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Security.Cryptography.Algorith
ms 4.2.0 provides a compile-time reference assembly for System.Security.Cryptography.Algorithms on .NETStandard,Version=
v1.3, but there is no run-time assembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Security.Cryptography.X509Cert
ificates 4.1.0 provides a compile-time reference assembly for System.Security.Cryptography.X509Certificates on .NETStand
ard,Version=v1.3, but there is no run-time assembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : System.Text.RegularExpressions 4.1.0
provides a compile-time reference assembly for System.Text.RegularExpressions on .NETStandard,Version=v1.3, but there is
no run-time assembly compatible with win81-x64. [C:\cli\tools\Archiver\Archiver.csproj]
C:\cli.dotnet_stage0\x64\sdk\1.0.0-preview3-003892\NuGet.targets(70,5): error : One or more packages are incompatible
with .NETStandard,Version=v1.3 (win81-x64). [C:\cli\tools\Archiver\Archiver.csproj]

0 Warning(s)
13 Error(s)
@TheRealPiotrP

This comment has been minimized.

Show comment
Hide comment
@TheRealPiotrP

TheRealPiotrP Oct 25, 2016

@rrelyea this is preventing migrating CLI to msbuild

/cc @eerhardt @emgarten FYI

@rrelyea this is preventing migrating CLI to msbuild

/cc @eerhardt @emgarten FYI

@rrelyea rrelyea added this to the 4.0 RC milestone Oct 25, 2016

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 25, 2016

Contributor

If you put rid1;rid2 in the csproj, and stopped setting that property from the command line, does it work?

Contributor

rrelyea commented Oct 25, 2016

If you put rid1;rid2 in the csproj, and stopped setting that property from the command line, does it work?

@jgoshi

This comment has been minimized.

Show comment
Hide comment
@jgoshi

jgoshi Oct 25, 2016

If I add this property to the csproj and stop passing it from the command line, then restore3 works:
win7-x64

Seems like restore3 expects the property to be names RuntimeIdentifiers with an s while publish3 expects it without the s. But that's probably a separate issue.

jgoshi commented Oct 25, 2016

If I add this property to the csproj and stop passing it from the command line, then restore3 works:
win7-x64

Seems like restore3 expects the property to be names RuntimeIdentifiers with an s while publish3 expects it without the s. But that's probably a separate issue.

@eerhardt

This comment has been minimized.

Show comment
Hide comment
@eerhardt

eerhardt Oct 25, 2016

I think the best thing we can do here for Dev15 RC is the following plan:

  1. RuntimeIdentifiers gets specified in the .csprojs as necessary.
  2. dotnet migrate takes the "runtimes" section and puts it in RuntimeIdentifiers in the .csproj.

Now restore will work correctly.

  1. We fix dotnet publish --runtime to no longer issue a restore. It will be expected that the project is already restored for the RID you specify. If it hasn't an error is raised telling you to add the RID to the .csproj and run restore again.

Thoughts?

I think the best thing we can do here for Dev15 RC is the following plan:

  1. RuntimeIdentifiers gets specified in the .csprojs as necessary.
  2. dotnet migrate takes the "runtimes" section and puts it in RuntimeIdentifiers in the .csproj.

Now restore will work correctly.

  1. We fix dotnet publish --runtime to no longer issue a restore. It will be expected that the project is already restored for the RID you specify. If it hasn't an error is raised telling you to add the RID to the .csproj and run restore again.

Thoughts?

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 25, 2016

Contributor

@eerhardt - like it!
@jgoshi - we support singular or plural forms of "RuntimeIdentifier(s)"

Contributor

rrelyea commented Oct 25, 2016

@eerhardt - like it!
@jgoshi - we support singular or plural forms of "RuntimeIdentifier(s)"

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 25, 2016

Contributor

another way of working around this issue is restore each project directly with "--no-dependencies" -- it will only create one assets file.

Contributor

rrelyea commented Oct 25, 2016

another way of working around this issue is restore each project directly with "--no-dependencies" -- it will only create one assets file.

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 25, 2016

Contributor

can we close this issue?

Contributor

rrelyea commented Oct 25, 2016

can we close this issue?

@jgoshi

This comment has been minimized.

Show comment
Hide comment
@jgoshi

jgoshi Oct 25, 2016

If we work around the issue, isn't this still a bug though? Passing through the command line is supposed to work right?

jgoshi commented Oct 25, 2016

If we work around the issue, isn't this still a bug though? Passing through the command line is supposed to work right?

@emgarten

This comment has been minimized.

Show comment
Hide comment
@emgarten

emgarten Oct 25, 2016

Contributor

The real cause of the failure here is the guardrails check for runtimes. This needs to be fixed: #1623

Without guardrails the restore would have completed without errors, and publish could then decided which assemblies were needed.

Applying the RIDs to all projects seems like the correct behavior to me. If a child project needed RIDs and was already restored correctly this would end breaking the child project.

Contributor

emgarten commented Oct 25, 2016

The real cause of the failure here is the guardrails check for runtimes. This needs to be fixed: #1623

Without guardrails the restore would have completed without errors, and publish could then decided which assemblies were needed.

Applying the RIDs to all projects seems like the correct behavior to me. If a child project needed RIDs and was already restored correctly this would end breaking the child project.

@jgoshi

This comment has been minimized.

Show comment
Hide comment
@jgoshi

jgoshi Oct 25, 2016

Ok, in that case sure go ahead and close this issue.

jgoshi commented Oct 25, 2016

Ok, in that case sure go ahead and close this issue.

@eerhardt

This comment has been minimized.

Show comment
Hide comment
@eerhardt

eerhardt Oct 27, 2016

Applying the RIDs to all projects seems like the correct behavior to me. If a child project needed RIDs and was already restored correctly this would end breaking the child project.

If that is the correct behavior, please give us a way to only apply the RID to a single project, and still allow referenced projects to be restored correctly.

See dotnet/cli#4294 (comment). It is a goal of the CLI's to allow publishing self-contained apps to be a "publish time" decision. Basically, the user shouldn't have to change the project at all to be able to publish portable or self-contained. They should be able to just dotnet new, dotnet restore, dotnet publish --runtime RID without needing to edit the .csproj.

/cc @blackdwarf

Applying the RIDs to all projects seems like the correct behavior to me. If a child project needed RIDs and was already restored correctly this would end breaking the child project.

If that is the correct behavior, please give us a way to only apply the RID to a single project, and still allow referenced projects to be restored correctly.

See dotnet/cli#4294 (comment). It is a goal of the CLI's to allow publishing self-contained apps to be a "publish time" decision. Basically, the user shouldn't have to change the project at all to be able to publish portable or self-contained. They should be able to just dotnet new, dotnet restore, dotnet publish --runtime RID without needing to edit the .csproj.

/cc @blackdwarf

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 28, 2016

Contributor

@eerhardt - if users want to control nested projects with different RIDs, for now, they should turn off recursive restore, and restore the main project and the dependency project with different RIDs.
Otherwise, use the project file.

Contributor

rrelyea commented Oct 28, 2016

@eerhardt - if users want to control nested projects with different RIDs, for now, they should turn off recursive restore, and restore the main project and the dependency project with different RIDs.
Otherwise, use the project file.

@rrelyea rrelyea modified the milestones: 4.0 RC2, 4.0 RC Oct 28, 2016

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 28, 2016

Contributor

we'll continue discussion, if any, next milestone.

Contributor

rrelyea commented Oct 28, 2016

we'll continue discussion, if any, next milestone.

@rrelyea rrelyea changed the title from Restore of "runtimes" section should only apply to the current project to Remove Guardrails - Restore of "runtimes" section should only apply to the current project Nov 11, 2016

@rrelyea rrelyea modified the milestones: 4.0 RC3, 4.0 RC2 Nov 11, 2016

emgarten added a commit to NuGet/NuGet.Client that referenced this issue Dec 20, 2016

emgarten added a commit to NuGet/NuGet.Client that referenced this issue Dec 20, 2016

@emgarten emgarten referenced this issue in NuGet/NuGet.Client Dec 20, 2016

Closed

Remove runtime guardrails check #1068

emgarten added a commit to NuGet/NuGet.Client that referenced this issue Dec 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment