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

Pack includes content brought in through dependency packages into the current project's package (SDK based projects only) #8867

Closed
Tracked by #6285
sungam3r opened this issue Dec 2, 2019 · 14 comments · Fixed by NuGet/NuGet.Client#3798
Assignees
Labels
Area:ContentFiles PackageReference contentFiles folder Functionality:Pack help wanted Considered good issues for community contributions. Priority:2 Issues for the current backlog. Product:dotnet.exe Type:Bug

Comments

@sungam3r
Copy link

sungam3r commented Dec 2, 2019

Details about Problem

NuGet product used (NuGet.exe | VS UI | Package Manager Console | dotnet.exe): VS 2019

NuGet version (x.x.x.xxx):

dotnet.exe --version (if appropriate): 3.0.100

VS version (if appropriate): 16.3.8

OS version (i.e. win10 v1607 (14393.321)): Windows 10.0.14393 x64

Worked before? If so, with which NuGet version: -

Detailed repro steps so we can see the same problem

I have been using the PrivateAssets="all" option for a long time. It works great for PackageReference. Now I needed to use some content file in my project and I could not achieve the desired behavior with PrivateAssets="all". I want to build "base" package with one file data.json which should be consumed in "top" package but should not be exposed further to consumers of the "top" package.

My "base" package:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <IncludeBuildOutput>false</IncludeBuildOutput>
    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
  </PropertyGroup>
  <ItemGroup>
    <Content Include="data.json">
      <PackagePath>contentFiles/any/any/data.json</PackagePath>
    </Content>
  </ItemGroup>
</Project>

The result seems to be good for me:
image

My "top" package:

<Project>
  <ItemGroup>
    <PackageReference Include="BasePackage" Version="1.0.0">
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>
</Project>

The Result is unexpected for me:
image

Why is the file again in the content section? This file is a development dependency, some sort of settings file. The "base" package is a package with Roslyn analyzers and the settings file - data.json. I do not present the analyzers used for clarity. For them, PrivateAssets works as it should - they do not fall into the dependencies of the "base" package. But for the settings file - data.json, I can’t get the same behavior.

I spent a lot of time resolving this issue and it seems to me that I do not see any obvious solution, which is somewhere nearby. I will be very grateful for the advice.

@donnie-msft
Copy link
Contributor

I'm not exactly following your goal, here. You want the first consumer of your package to see the content (data.json), but anyone who consumes that new package should not see the content of data.json?

This MSBuild reference shows examples of how the folder structures work for Content Files: https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#including-content-in-a-package
You may particularly be interested in the ContentTargetFolders and $(IncludeContentInPack) properties.

@donnie-msft donnie-msft added Area:ContentFiles PackageReference contentFiles folder WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Dec 7, 2019
@sungam3r
Copy link
Author

sungam3r commented Dec 7, 2019

You want the first consumer of your package to see the content (data.json), but anyone who consumes that new package should not see the content of data.json?

Exactly, I do not want to include this file transitively.

@sungam3r
Copy link
Author

Both ContentTargetFolders and $(IncludeContentInPack) will not help. ContentTargetFolders just changes destination folder. If $(IncludeContentInPack) is set to false on any project, then the content from that project are not included in the nuget package. This setting applies to all project content, not specific dependency. I have not yet figured out how to achieve the desired behavior.

@nkolev92 nkolev92 changed the title How to suppress contentfiles flow? Pack includes content brought in through dependency packages into the current project's package Dec 30, 2019
@nkolev92 nkolev92 changed the title Pack includes content brought in through dependency packages into the current project's package Pack includes content brought in through dependency packages into the current project's package (SDK based projects only) Dec 30, 2019
@nkolev92
Copy link
Member

nkolev92 commented Dec 30, 2019

The short explanation:

  • This is a bug in pack. The mechanics for packing content files inside a package overlap the mechanics for flowing content files into the project they need to go to.

The long explanation:

  • NuGet identifies several types of asset groups, defined here: https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets. contentFiles are one of those groups. When a consuming project installs a package, it allows said project to control which assets from the package in question get consumed.

  • PrivateAssets controls whether the assets from the package flow to the dependants of the consuming project (or package if this project gets packaged).
    PrivateAssets=All, tells NuGet that the package in question is a development time dependency and should not be included as part of the dependencies if the consuming project gets packed.

  • IncludeAssets/ExcludeAssets controls which asset groups get consumed by the current project.

  • In the scenario in question, the base package is correctly created. By putting metadata such as PrivateAssets=All, NuGet understands that this package should not be flown to the dependants of the current project. When the consumer project gets packed, the BasePackage is not included in the dependency list (check the nuspec to verify that, I did that on my end).
    What does get included "accidentally" is the content that the current project is consuming. That's because of the way the content files are consumed in the current project.

  • NuGet generates a nuget.g.props file to include the content files from packages into the consuming project. (In SDK based projects). If you look at the nuget.g.props file you would see:

<?xml version="1.0" encoding="utf-8" standalone="no"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <RestoreSuccess Condition=" '$(RestoreSuccess)' == '' ">True</RestoreSuccess>
    <RestoreTool Condition=" '$(RestoreTool)' == '' ">NuGet</RestoreTool>
    <ProjectAssetsFile Condition=" '$(ProjectAssetsFile)' == '' ">$(MSBuildThisFileDirectory)project.assets.json</ProjectAssetsFile>
    <NuGetPackageRoot Condition=" '$(NuGetPackageRoot)' == '' ">$(UserProfile)\Documents\Code\Test\8867\gpf</NuGetPackageRoot>
    <NuGetPackageFolders Condition=" '$(NuGetPackageFolders)' == '' ">C:\Users\Nikolche\Documents\Code\Test\8867\gpf;C:\Program Files\dotnet\sdk\NuGetFallbackFolder</NuGetPackageFolders>
    <NuGetProjectStyle Condition=" '$(NuGetProjectStyle)' == '' ">PackageReference</NuGetProjectStyle>
    <NuGetToolVersion Condition=" '$(NuGetToolVersion)' == '' ">5.4.0</NuGetToolVersion>
  </PropertyGroup>
  <PropertyGroup>
    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
  </PropertyGroup>
  <ItemGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <Content Include="$(NuGetPackageRoot)\basepackage\1.0.0\contentFiles\any\any\data.json" Condition="Exists('$(NuGetPackageRoot)\basepackage\1.0.0\contentFiles\any\any\data.json')">
      <NuGetPackageId>BasePackage</NuGetPackageId>
      <NuGetPackageVersion>1.0.0</NuGetPackageVersion>
      <NuGetItemType>Content</NuGetItemType>
      <Private>False</Private>
      <Link>data.json</Link>
    </Content>
  </ItemGroup>
</Project>

Note the conditional inclusion.
The pack implementation is not consider this condition when evaluating the content to be included.

So the technical fix here is:
Pack/Restore should be updated so pack can exclude content files when evaluating the files to be included into the current package.

  • Note that the PrivateAssets part is "correctly" considered by NuGet. The content files will not be flown to consumers of the project consuming base package (If curious, create project that consumes that to check it out, I did :) ). But if you end up packing the project in question, that's when issues occur.

Workaround:

As far as workarounds by the time this gets fixed go, unfortunately I can only think of the big hammer. Set IncludeContentInPack="false" in the project consuming BasePackage.

@nkolev92 nkolev92 added Product:dotnet.exe Functionality:Pack Type:Bug and removed WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Dec 30, 2019
@nkolev92 nkolev92 added this to the Backlog milestone Dec 30, 2019
@nkolev92 nkolev92 added Triage:NeedsTriageDiscussion help wanted Considered good issues for community contributions. labels Dec 30, 2019
@sungam3r
Copy link
Author

@nkolev92 Thanks for the detailed answer. I guessed that it could be a bug. But I did not understand this comment:

Set IncludeContentInPack="true" in the project consuming BasePackage.

Did you mean set IncludeContentInPack="false" in "top" package to disable packing any content from all dependencies?

As far as workarounds by the time this gets fixed go

Could you say something about when this will be fixed? In the near future is this real?

@nkolev92
Copy link
Member

Did you mean set IncludeContentInPack="false" in "top" package to disable packing any content from all dependencies?

Yes, I meant to write that.

Could you say something about when this will be fixed? In the near future is this real?

Not sure at this point. We have lots of issue that we prioritize based on many factors.
However, we'd be happy to take a PR.

@sungam3r
Copy link
Author

Thanks.

We have lots of issue that we prioritize based on many factors.

Yes, I looked at the current list of bugs and guessed that there was enough work there even without my problem.

However, we'd be happy to take a PR.

Unfortunately, this is not an area where I can effectively help. Actually your advice to use IncludeContentInPack="false" suits me. I write a lot of library projects, there is no content and I can use IncludeContentInPack="false" completely safely. I have already taken this approach and verified that the final behavior is what I needed.

@sungam3r
Copy link
Author

One more note. IncludeContentInPack="false" conflicts with PackageIcon.

Warning if you use PackageIconUrl:

NU5048 The 'PackageIconUrl'/'iconUrl' element is deprecated. Consider using the 'PackageIcon'/'icon' element instead. Learn more at https://aka.ms/deprecateIconUrl

After you use PackageIcon with IncludeContentInPack="false":

NU5046 The icon file 'icon.png' does not exist in the package.

So you are locked to use PackageIconUrl yet.

@nkolev92 nkolev92 added Resolution:Duplicate This issue appears to be a Duplicate of another issue and removed Triage:NeedsTriageDiscussion Pipeline:New Issues labels Apr 16, 2020
@nkolev92
Copy link
Member

Duplicate of #8867.

@jakubmisek
Copy link

Duplicate of #8867.

Isn't it closed accidentally? Issue #8867: Duplicate of #8867.

We are still having this issue, when pack creates content folder with flattened content of dependencies. Isn't there a different workaround?

@sungam3r
Copy link
Author

sungam3r commented Oct 9, 2020

Wow! I didn't even notice.

@zivkan zivkan added Triage:NeedsTriageDiscussion and removed Resolution:Duplicate This issue appears to be a Duplicate of another issue labels Oct 12, 2020
@zivkan
Copy link
Member

zivkan commented Oct 12, 2020

Good catch!

@zivkan zivkan reopened this Oct 12, 2020
@zkat zkat removed this from the Backlog milestone Oct 12, 2020
@zkat zkat added Priority:2 Issues for the current backlog. and removed Triage:NeedsTriageDiscussion labels Oct 12, 2020
@nkolev92
Copy link
Member

Meant to say duplicate of #6138 :)

@dominoFire
Copy link
Contributor

Reopening since this is a bug in PackageReference project. Related bug applies to packages.config projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:ContentFiles PackageReference contentFiles folder Functionality:Pack help wanted Considered good issues for community contributions. Priority:2 Issues for the current backlog. Product:dotnet.exe Type:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants