PrivateAssets and ExcludeAssets should be independent

[AArnott]

Details about Problem

NuGet product used (NuGet.exe | VS UI | Package Manager Console | dotnet.exe): I've tried dotnet.exe and nuget.exe and msbuild.exe as it ships with my VS.

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

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

VS version (if appropriate): 27713.00.d15.8

OS version (i.e. win10 v1607 (14393.321)): Windows 10 RS4

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

Detailed repro steps so we can see the same problem

In a .NET SDK style project, reference a package while specifying PrivateAssets and ExcludeAssets attributes, like this:

<PackageReference Include="CodeGeneration.Roslyn.BuildTime" PrivateAssets="none" ExcludeAssets="Build" />

Expected result

I expect that my project will not consume the build elements of the package, but that my own package's respective consumers will consume the build elements of the referenced package.

In other words, I don't want my project importing the build folder .targets file from that package, but I do want this expressed in my package's .nuspec file:

<dependency id="CodeGeneration.Roslyn.BuildTime" version="0.4.42" include="all" />

Actual result

<dependency id="CodeGeneration.Roslyn.BuildTime" version="0.4.42" include="Runtime,Compile,Native,ContentFiles,Analyzers" />

Additional discussion

The fact that I can specify PrivateAssets (which is documented to influence how the dependency is expressed to others), and I can specify ExcludeAssets/IncludeAssets (which influences how I get my own dependency), is fantastic, except that when I use the two together, they interact such that I might as well not set PrivateAssets because it seems to be ignored.

In another related case to this, I actually wanted these two attributes on my PackageReference: PrivateAssets="none" IncludeAssets="none" because I wanted to express a dependency downstream that I myself didn't share. But that simply suppresses the dependency in the .nuspec file. 😦

Sample Project

refissue.zip

[jainaashish]

I have my reservations about this requirement and It seems very specific to work around something else. Why would someone allow flowing some assets from it's own dependency without even consuming itself?

Since NuGet has these explicit properties PrivateAssets or ExcludeAssets/IncludeAssets, we're even thinking about this scenario but most other package managers will have a uber scope attribute to define the scope of a package which is implicitly applied to rest of it's dependencies as well, which seems like a right thing to do.

[AArnott]

This has come up for a few of my packages.

In some cases, the scenario is that I'm creating a specialized package that helps create a development environment for the consuming project (it usually includes build authoring). Part of that environment requires that the consumer also reference other packages that the build authoring offered by my package requires. But building my own package itself does not require those packages.

In another (perhaps more common) case, it's a library with analyzers. I have a library package, and an analyzers package. Whenever someone consumes my library I want them to get the analyzers for it too, so my library package depends on the analyzers package. But my library itself does not need or want the analyzers applied to itself.

If NuGet didn't want to support this, I don't understand why there are distinct attributes to track downstream dependencies vs. my own dependencies. Being distinct is super useful -- or would be if they worked. At the moment I don't even know how to reason about these attributes when used together.

[0liver]

This example in the current documentation looks a lot like the case you're describing.

Is it possible I was hoping that your scenario does now work ? but I've made my own painful experience with it not working, documented here: NuGet/docs.microsoft.com-nuget#2716.

[AArnott]

@0liver your example has ExcludeAssets set to a subset of PrivateAssets, so the bug doesn't show up there. The issue appears where ExcludeAssets includes elements that PrivateAssets doesn't. In such a case, I've found (as of when I filed the bug anyway) that PrivateAssets acts as if it is the union of what I set across both attributes.

[0liver]

A lot of information has been gathered in #11717 as to how the three attributes play together, including links to the concrete places in code that are responsible for the behavior. The current documentation is not correct in all aspects.

The gist of all of this is (from this comment):

LibraryIncludeFlags effectiveInclude = IncludeAssets & ~ExcludeAssets & ~PrivateAssets;

Looks like your issue is still valid.

~

Edit: I had to remove the rest of this comment, because it was incorrect due to a mistake on my end.

[AArnott]

Thanks, @0liver. You reference #11717 but that didn't turn into a hyperlink. Was it perhaps a typo?

[0liver]

I don't know why, but it's the same as linked higher up: #11717.

[erdembayar]

@AArnott
I'm trying to tackle this issue. But I was not able to reproduce this issue on my local. Could you be able to verify if I'm testing correct scenario?

1. When I pack the refissue.zip project then resulting nuspec file looks like this. Isn't it expected result already?

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
  <metadata>
    <id>refissue</id>
    <version>1.0.0</version>
    <authors>refissue</authors>
    <description>Package Description</description>
    <dependencies>
      <group targetFramework=".NETStandard2.0">
        <dependency id="CodeGeneration.Roslyn.BuildTime" version="0.4.42" include="All" />
      </group>
    </dependencies>
  </metadata>
</package>

2. I tried the related case you mentioned in description too:

In another related case to this, I actually wanted these two attributes on my PackageReference: PrivateAssets="none" IncludeAssets="none" because I wanted to express a dependency downstream that I myself didn't share. But that simply suppresses the dependency in the .nuspec file. 😦

  <ItemGroup>
    <PackageReference Include="CodeGeneration.Roslyn.BuildTime" Version="0.4.42" PrivateAssets="none" IncludeAssets="none" />
  </ItemGroup>

Here is nuspec generated on my local. Is it expected output or not? If not, what would be the expected output this case?

    <dependencies>
      <group targetFramework=".NETStandard2.0">
        <dependency id="CodeGeneration.Roslyn.BuildTime" version="0.4.42" include="All" />
      </group>
    </dependencies>

[AArnott]

Thank you for looking, @erdembayar. I don't know why, but the original repro seems to not happen with the current SDKs. But the problem is still there. It (for whatever reason) just requires a different dependency package.

I can repro with this: refissue.zip

I can also modify it to the second case and as per the bug, the dependency disappears.

I hope this helps.

[erdembayar]

Thank you for looking, @erdembayar. I don't know why, but the original repro seems to not happen with the current SDKs. But the problem is still there. It (for whatever reason) just requires a different dependency package.

I can repro with this: refissue.zip

I can also modify it to the second case and as per the bug, the dependency disappears.

I hope this helps.

I just tried with your new refissue.zip, still no repro. Do you have
nupkg file with issue? Here is one generated on my local (I changed extension from nupkg to zip):
refissue.1.0.0-beta2.zip

[AArnott]

Using the 7.0.203 SDK:

Using ExcludeAssets instead of IncludeAssets metadata on the PackageReference seems to impact the result in unpredictable ways.

For instance this:

<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.2.138-beta" PrivateAssets="compile" IncludeAssets="build" />

produces this:

<dependency id="Microsoft.Windows.CsWin32" version="0.2.138-beta" exclude="Runtime,Compile,Native,Analyzers,BuildTransitive" />

which excludes everything except build, so only build is expressed as a nuspec dependency. Why, well we might think it's because no asset dependency is expressed in nuspec that isn't a direct dependency of the project as well.

But now consider this:

<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.2.138-beta" PrivateAssets="compile" ExcludeAssets="build" />

which produces this:

<dependency id="Microsoft.Windows.CsWin32" version="0.2.138-beta" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />

Well now that includes everything but compile, which is what I wanted. It even includes build in the nuspec dependency although build is excluded from the project directly.

So... it would seem then that assets can be expressed as nuspec dependencies even without direct consumption provided I use ExcludeAssets metadata instead of IncludeAssets metadata.
But wait, it gets crazier...

Consider this case:

<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.2.138-beta" PrivateAssets="compile" ExcludeAssets="compile;runtime;contentFiles;build;buildMultitargeting;buildTransitive;analyzers" />

which produces:

<dependency id="Microsoft.Windows.CsWin32" version="0.2.138-beta" exclude="Runtime,Compile,Build,Analyzers,BuildTransitive" />

Note the nuspec dependency is back to using the exclude attribute, and it excludes far more than I wanted (I only wanted compile included), even though I'm using ExcludeAssets metadata which in the 2nd example didn't seem to have any impact.

So what do we have? 2 refissue.zip repros that no longer repro even on the same SDK version they were originally created with. We almost have a workaround of using ExcludeAssets instead of IncludeAssets on PackageReference, except that it works inconsistenly and I can't find any explanation for it to predict whether it'll work or not.

[EdLichtman]

Hi all, late to the party but here's another prime example of how I plan to use this:

I, as a package developer have a dependency on Nuke.Common.

Nuke.Common doesn't package certain build targets as transitive, on as build.

Therefore, I'd like to tell my other inheriting libraries "you need a dependency on Nuke.Common, bit not just like an implicit one. You want everything from them"

Another example is that I'd like to build a central package library with json, and that library is useless without a different library with source generators. So I'd like to have my configuration say "this library will add AdditionalFiles to your project and will then cause your project to depend upon the source generator library analyzers", but the only way to do that right now is by sneaking it in buildTransitive assets.

[EdLichtman]

@erdembayar, Here is my example that shows what's going on:

https://github.com/EdLichtman/UniformPackagesDemo/tree/master/DependencyHellExample

You'll see that there is the Recipient. The Recipient references Parent.
The Parent says that "None of my assets are private, so everything should flow down to whatever references me"
The parent ALSO says "But I don't want the BuildTransitive to apply to me."

The reason for it is that the "most internal library", that is, "DependencyHell", requires that a certain GlobalProperty is configured. DependencyHell doesn't care that Parent configures it. All DependencyHell cares about is, "does the entry assembly configure it".

So when you build DependencyHellRecipient, it should error, but it doesn't.

[EdLichtman]

@jainaashish

Here's a thought to add -- I was writing up an issue I have with Licensing and Security concerns over how we package up analyzers, and came up with an idea that may help work towards resolving this.
#12941

[erdembayar]

@nkolev92
Last time I synced with @AArnott, he's not blocked on this anymore. I'm not sure if it has changed since then. After much deeper discussion, it seems that we may not need to make any code changes. It could be solved with a combination of current assets, but the combination result is not consistent with our expectations. We need to answer questions mentioned in #6938 (comment).

[jzabroski]

Just wondering if it's a best practice to unassign oneself from a ticket while mentioning the ticket contains items to follow up on, and not create a new ticket to track that follow up.

[EdLichtman]

@erdembayar, @nkolev92 I still have some outstanding concerns on this matter. While @AArnott may not be blocked, I am, and the reason I did not open a new ticket and dilute your escalation queue is because I was relying on this ticket to be discussed and worked upon.

[nkolev92]

@EdLichtman @jzabroski This issue is still something we want to address.

Erick was just providing the status since he's not able to continue working on it himself at this point.

[EdLichtman]

oh, ok. Thank you for the clearing up the misunderstanding.

[erdembayar]

I will share my findings with the next person. There is a possibility that the calculation of assets may have changed between SDK versions. This issue was first filed in 2018, and based on the latest SDK testing, it seems that we may not need to make any code changes. Of course, code changes can be done to explicitly specify this; that was my initial intention, but it would require a lot of changes. Instead, we could possibly use the correct combination of asset flags. For that, we still need to address the question raised in #6938 (comment) and document the appropriate asset flag combinations to use.