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 should add pack files to the compile/runtime sections of assets.json #4514

Open
livarcocc opened this issue Feb 6, 2017 · 4 comments
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Status:Excluded from icebox cleanup Status:Inactive Icebox issues not updated for a specific long time Triage:Investigate
Milestone

Comments

@livarcocc
Copy link

Have a project A and a Project B.

Project A has a ProjectReference to Project B.

And ProjectB has something like this in its csproj:

<ItemGroup>
    <Content Include="runtimes\win7-x86\native\**\*">
      <PackagePath>lib\$(TargetFramework)</PackagePath>
      <Pack>true</Pack>
    </Content>
  </ItemGroup>

The assemblies under native won't show up in the compile/runtime sections of A's ProjectReference to B in the assets.json file. While, if A referenced B as a nupkg, it would. This forces people to reference projects as packages when they could just reference them as projects.

Once this gets fixed in NuGet, then additional changes will need to be made in the SDK as well.

See dotnet/sdk#765.

@rrelyea
Copy link
Contributor

rrelyea commented Feb 6, 2017

@emgarten - please investigate and understand impact if we take for 4.0 or 4.0.1.

@emgarten
Copy link
Member

emgarten commented Feb 6, 2017

We discussed this for restore/pack, it is a major feature that we decided not to take for RTM due to the size.

DNX supported this by defining all additional files in project.json under pack includes. Restore then added these into the lock file.

With MSBuild finding all assets is more involved than just reading a static json file.

To make this work we will need:

  1. Restore runs the pack target against every project to find all assets. This needs to happen both in MSBuild and in Visual Studio (unclear how this can work in Visual Studio currently)
  2. Restore runs nearest TFM asset selection against the result of pack (already in place from the p2p transitive changes)
  3. Dotnet CLI, MSbuild, and Visual Studio need to be updated to rely on the assets.json file for all project outputs instead of reading the project directly to find them. Without this we will always have inconsistent results.

Additional complications/open questions:

  • Packing several projects into one package with ref and runtime assemblies changes the depth of the graph
  • Flags such as IncludeReferenceProjects also change the depth of the graph which changes the resolver result
  • Finding p2p references is already the majority of restore time. Finding all assets from projects also would impact perf also.
  • Is it possible to find all assets that would be in the packed nupkg without building first?

@rrelyea rrelyea modified the milestones: 4.0.1, 4.0 RTM Feb 8, 2017
@rrelyea
Copy link
Contributor

rrelyea commented Feb 8, 2017

Moving to 4.0.1 - just trying to confirm in linked issue that they don't absolutely need it in rtm.

@emgarten
Copy link
Member

emgarten commented Nov 9, 2017

This would still be useful, however we have not heard a lot of feedback on this so far.

@emgarten emgarten removed their assignment Feb 14, 2018
@ghost ghost added the Status:Inactive Icebox issues not updated for a specific long time label Sep 1, 2022
@jeffkl jeffkl added Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. and removed Pipeline:Icebox labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Status:Excluded from icebox cleanup Status:Inactive Icebox issues not updated for a specific long time Triage:Investigate
Projects
None yet
Development

No branches or pull requests

6 participants