Update of a Package should not modify PrivateAssets of a PackageReference

[SimonCropp]

before update

  <ItemGroup>
    <PackageReference Include="Fody" Version="3.0.0" PrivateAssets="None" />
    <PackageReference Include="FodyPackaging" Version="3.0.0" PrivateAssets="All" />
  </ItemGroup>

after update

  <ItemGroup>
    <PackageReference Include="Fody" Version="3.0.1" PrivateAssets="all">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
    </PackageReference>
    <PackageReference Include="FodyPackaging" Version="3.0.1" PrivateAssets="All" />
  </ItemGroup>

the problems is the first one results in the resultant nuget having a dependency on fody. the second one does not.

[rohit21agrawal]

is Fody version 3.0.1 marked as a developmentDependency? If so, then this is the intended behavior and introduced as part of 15.8 update of visual studio

[SimonCropp]

But that is a breaking change

[SimonCropp]

also i think this is still a bug. someone building a fody addin explicitly wants

  <PackageReference Include="Fody" Version="3.0.0" PrivateAssets="None" />

an upgrade should not change that explicit decision.

[pamidur]

This change brakes every single package where build and lib are somehow connected!

Example:
I have a package with lib and build. build alters IL when types from lib are used.

Then I have a solution which use the package. It has dependencies like the following:
project2->project1->package

Case: developmentDependency=true
Result: you cannot compile project1 because IncludeAssests don't include compile

Case: developmentDependency=false
Result: now your lib leaks even to project2, but not your build. So project1 is altered and project2 is not

[jainaashish]

@SimonCropp When you already had PrivateAssets set to None then updating this package will not change it. We respect the existing values set for these flags. I was not even able to repro your issue. Can you give me complete repro steps?

@pamidur The idea behind setting developmentDependency to true is when the author doesn't want consumer to take a compile time dependency on that package. That's why it exclude compile but consumer still have the option to decide for himself by overriding it which is always being persisted.

Your another case of build not flowing transitively is another issue which we're working on and hopefully will have some better end to end experience there.

[pamidur]

@jainaashish I'm cool with transient build, is there issue I can subscribe to?

I my case the order of actions:

• project is compiled with lib into dll
• build replaces all entries from lib in dll to something else
• build removes all references to lib from dll and in fact removes it from output like it never existed

What's wrong with compile in IncludeAssests ? If author doesn't want consumer have any compile time dependency, he could simply leave lib empty. I mean I don't understand such limitations.

[jainaashish]

build transitive issue# #6091

I don't understand why your build alter IL type from lib... that doesn't sound right.

And excluding compile isn't a limitation instead the right implementation for this developmentDependency flag. If the ask is that the author wants a compile time dependency on his package, then he don't need to set this flag. Empty lib wont work when there is a dll in developmentDependency to be consumed at runtime.

[pamidur]

Thanks,

This sounds totally inconsistent:

This flag includes build and analyzers, and exludes compile. But then it makes private everything. And even doesn't include this package to dependency on pack.

I'd expect this flag either means dependency to this package is not transient or this package doesn't have dlls to reference. But not both!

This weird combination is the limitation and ruins tons of use cases.

As for my case. My lib contains attributes, after compilation they are replaced with some IL instructions in marked methods.

[SimonCropp]

for a repro download this https://github.com/Fody/Caseless/archive/97f648181221fe333f9a660604b1dd50ed091f66.zip and do a nuget update

[jainaashish]

@SimonCropp updating from NuGet manager UI worked fine but I could repro it when I updated from NuGet powershell command. Are you also reproing it through PMC?

[rohit21agrawal]

@pamidur if a Project (ProjectA) consumes a Package (PackageB) and references a DLL in it, then how can you have it not be transient and expect consumers of ProjectA to be not broken?

[pamidur]

@rohit21agrawal , attributes and references are removed from ProjectA after compilation by supplied build.targets in ProjectB package. That's why I rather consider it to be "developmentDependency"

[SimonCropp]

@jainaashish no, i can repro through the ui

[iskiselev]

At least excluding compile assets should be mentioned in documentation. Currently it states that developmentDependency (2.8+) A Boolean value specifying whether the package is be marked as a development-only-dependency, which prevents the package from being included as a dependency in other packages.
It does not give any hints that it will exclude compile assets from compilation.

[SimonCropp]

how can this not be considered a breaking change?

[jainaashish]

@SimonCropp I still can't repro it from UI, can you make a repro video and share? or tell me EXACT repro steps to repro it locally... So far I can only repro it in PMC.

@iskiselev good point I'll get the doc updated accordingly.

[SimonCropp]

@jainaashish i dont think it matters "how u repro it". just that u can. arguably if the PMC is behaving differently from the UI i would consider that another bug

[pamidur]

I guess this is the similar issue #7084

In fact I suggest developmentDependency do this:

 <ItemGroup>
    <PackageReference Update="DevelopmentDependencyPackage">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>all</IncludeAssets>
    </PackageReference>
  </ItemGroup>

[jainaashish]

@SimonCropp I'm not saying this isn't a bug with PMC, just trying to understand all scenarios hence its important to know where all this repros and how...

[jainaashish]

Merged to 4.9.0 with 6d77978a8dd2b3f07851cc953e85333bb4950deb

[SimonCropp]

can this be re-opened. it is still happening