Home: installing a devDependency PackageReference should default to excludeassets=compile

[vsfeedback]

There is a problem with installation of PostSharp build-time packages in the Visual Studio 15.8 Preview 3, due to a change that makes PackageReference to have some specific metadata when installed through the UI.

Let's consider two packages:

PostSharp provides build tools.
PostSharp.Redist provides compile-time and runtime libraries.

PostSharp references PostSharp.Redist.

When installing the package in VS 15.7 the following is added to the project file:

<PackageReference Include="PostSharp" Version="6.0.16-rc" />

In VS 15.8 Preview 3 the same results in:

<PackageReference Include="PostSharp" Version="6.0.16-rc">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>

The default without metadata works fine (15.7), the new combination of assets (15.8) does not include "compile", which means that compilation behaves as if libraries from "PostSharp.Redist" package were not referenced.

It seems that this behavior happens only if there are no libraries in the package, otherwise a PackageReference without metadata is produced.

I have also found that Microsoft.SourceLink.Vsts.Git package also has this behavior.

It seems to me that these asset sets are transitively applied along package dependencies.

Attached is a simple project that should build after adding "PostSharp" package through "Manage NuGet Packages..." or Package Manager Console.

_This issue has been moved from https://developercommunity.visualstudio.com/content/problem/284136/nuget-package-manager-uses-incomplete-includeasset.html
VSTS ticketId: 640679_
_These are the original issue comments:_
(no comments)
_These are the original issue solutions:_
(no solutions)

[nkolev92]

/cc @jainaashish @rohit21agrawal @rrelyea

This is a fall-off from https://github.com/NuGet/Home/issues/4125.

Ideally we need this resolved before 15.8 ships.

[jainaashish]

This should be discussed before making any judgement.

I don't follow why a build-time package has a compile-time dependency? Package PostSharp clearly says that it's build time dependency only so the package should be updated to avoid any compile-time dependency. So I don't think we should resolve it, instead should encourage package author to fix their packages.

[rohit21agrawal]

i agree, this is not a bug, rather a flaw in the package. The behavior is correct since the package is marked to be a development dependency. if someone wants to compile against PostSharp.Redist, they should reference it directly.

[nkolev92]

I didn't mean to imply that it has to change, merely that we need to provide clarity to the customers on this before 15.8 ships.

[gfraiteur]

Let me clarify our usage at PostSharp:

The PostSharp package includes the PostSharp post-compiler, which is introduced into build chain of a project through PostSharp.targets. The PostSharp package also references PostSharp.Redist, which contains the runtime libraries. Therefore the content of the PostSharp package is needed only at build time, while the content of PostSharp.Redist is needed both at run and build time.

Suppose we're building a project named MyLibrary that references the PostSharp package, and MyLibrary is distributed as a NuGet package. Projects that reference the MyLibrary package MAY OR MAY NOT need to execute the PostSharp post-compiler included in the PostSharp package. That depends on how PostSharp is being used in the project. For instance, MyLibrary may contain an unsealed class MyBaseClass enhanced with a [NotifyPropertyChanged] aspect; any class derived from MyBaseClass would need to be post-compiled by PostSharp. But it can also happen that MyLibrary does not contain anything that would require a referencing project to be post-processed by PostSharp.

Looking at https://docs.microsoft.com/en-us/nuget/reference/nuspec, the documentation says:

"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."

There are a few points worth attention here:

1. From the documentation, it seems that the real meaning of this flag is that a dependency is non-transitive. That seems a misunderstanding from our side, as we believed that developmentDependency meant that the package content is used for development only (not run time), which definitively is the case. Based on this we may remove the developmentDependency flag from our packages. Then we would need to find a way for users to explicitly exclude the development-time PostSharp package and include PostSharp.Redist.

2. In our case, the fact that the PostSharp package is a development dependency does not mean that dependencies of this package (e.g. PostSharp.Redist) are development dependencies too. This behavior was neither specified nor implemented.

3. As a matter of facts and regardless of which interpretation is correct, VS 15.8 introduces a breaking changes by interpreting the developmentDependency differently than previous versions. Even if we change the manifest of our packages, some users who are stuck to some old version will experience issues just by upgrading Visual Studio.

Therefore we respectfully suggest to consider to:

1. Make the developmentDependency obsolete and have nuget pack emit a warning when it is used (since we understand the specification was never implemented until VS 15.8), and

2. Define a new flag, document it carefully according to the actual implementation.

Thank you.

[jainaashish]

@gfraiteur I see a basic flow with how these packages are structured. You've a packageA which depends on packageB and the expectation is any consumer of packageA should exclude packageA as dependency but instead should be able to include packageB as dependency. Don't you see an issue with this statement itself?

I understand there is confusion around the name of developmentDependency flag and it's meaning. And we'll work to clarify that and update the docs to clearly mention the behavior. But coming back to this flag, until 15.8 it was not even supported with PackageReference which was a gap and restricting NuGet customers to switch from packages.config to PackageReference who were already using packages with this flag ON. So 15.8 preview bits introduce the support for this flag with PackageReference which was not there in earlier versions.

Regarding your suggestions, we had a detailed discussion internal as well external (parts of which you can find in issue# https://github.com/NuGet/Home/issues/4125) before bringing this developmentDependency support with PackageReference and decided to go ahead with it for multiple reasons:

• There are already packages with developmentDependency which are being used with packages.config which NuGet authors understand. Obsoleting this flag means all those packages will need to publish a new version which is not an easy task to complete. Even after multiple efforts, we wouldn't be able to get all package authors to publish new versions.
• Still lot of NuGet customers uses packages.config to manage their NuGet dependencies, introducing a new flag will require to support that with packages.config as well which is not the direction we want to go into, we want more and more customers to move from packages.config to PackageReference for all the obvious reasons.
• There was an existing flag which customers knows and supported with older system, so it was efficient to get that supported with new system as well instead of introducing a new concept for the same purpose.
  .....................

At the end, I'd suggest to have the correct behavior of the flag as it's today and encourage package authors and consumers to correct their packages and usage. This is the first case we've seen where a dev dependency depends on compile-time dependency, and wouldn't expect too many (not even 100s). On contrary if there are many such cases, then we can re-evaluate our strategy and discuss more.

[rohit21agrawal]

1. As a matter of facts and regardless of which interpretation is correct, VS 15.8 introduces a breaking changes by interpreting the developmentDependency differently than previous versions. Even if we change the manifest of our packages, some users who are stuck to some old version will experience issues just by upgrading Visual Studio.

I do not believe this is a breaking change - people who have already taken a dependency on PostSharp will continue to see the same behavior between 15.7 and 15.8 (since the PackageReference with its metadata has already been written out to the project file, and the VS upgrade won't change it). The only change in behavior is when you install the package from the UI or dotnet.exe - 15.8 ends up writing additional metadata to respect the developmentDependency flag in the package.

[nkolev92]

Not a breaking change in existing projects ofc.
The issue would with new projects and new installations only.

[gfraiteur]

The change is implemented on our side.
We're contacting our users.

http://www.postsharp.net/blog/post/Breaking-change-expected-in-Visual-Studio-2017-Update-8

[rohit21agrawal]

thanks @gfraiteur . we are closing this issue on our side.

[pamidur]

I have similar but slightly different case.

I have an only package which contains _lib/dll_ with attributes and _build/targets_. These attributes are removed after compilation by included _build/targets_. So no runtime dependencies are produced. That's why I always used developmentDependency=true. This makes those attributes accessible for user, but after compilation there are no traces left. Well, it used to work like this until v15.8

Now I have options:

• Remove developmentDependency from package = so this redundant dependency will leak to subsequent packages
• Force my users to go to every their project and add _compile_ to IncludeAssets

Am I missing proper way to do it?

[pamidur]

@gfraiteur , idk if it is still relevant for you, but I found a workaround - so basically if your *.targets is included into a project, you may adjust PackageReference options from there. So I added this to my targets file while developmentDependency still on:

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

works like a charm so far

[gfraiteur]

Thanks. Yes, we knew that, and forgot to write it in the blog post.

[chm-tm]

I have a similar package like pamidur's.
When I add it to the project in 15.8.4, it writes the IncludeAssets metadata without the Compile asset, too. Interestingly, the generated project.assets.json file has the compile time dependency, and Roslyn is happy! However, upon the first restore, the project.assets.json file is regenerated according the IncludeAssets metadata and from that point on, I receive compile time errors.

@rohit21agrawal, should I file a new issue for this behavior which seems to me like an inconsistency?

Pamidur's workaround works for me, too.

[pamidur]

@chm-tm , it doesn't work with command line tools
Apparently dotnet cli and visual studio work differently ->

• vs reads targets from package references before compilation
• dotnet cli reads targets from package references on compilation.

[chm-tm]

@pamidur, thanks for the warning. msbuild /restore doesn't pick up the update either.

[jainaashish]

When I add it to the project in 15.8.4, it writes the IncludeAssets metadata without the Compile asset, too. Interestingly, the generated project.assets.json file has the compile time dependency, and Roslyn is happy!
@chm-tm can you tell me how you add the package in project? VS install or dotnet add package or manually...??

I'd expect if there is IncludeAssets metadata defined, then all restore code paths (VS, dotnet restore, and msbuild /restore or /t:restore) should work consistently and generates the same project.assets.json file. If that's not the case for you, then can you give me simple repro for your issue?

[chm-tm]

@jainaashish, I have reproed it here.
Note that it's an old-style .NET 4.6 project w/o Sdk reference.

And please note pamidur's and my comment which already show the evidence of an(other) inconsistency regarding item evaluation order.

On a related note, I find it quite difficult to engineer a build system with MSBuild when a couple of include files come into play. Property and item evaluation order is easy to get wrong. I'd be lost without MSBuild Structured Log Viewer. Any guidance of how you tackle this complexity the right way is highly appreciated.

[pamidur]

There is similar discussion here #7285
I'll dub my comment from there
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!

[jainaashish]

thanks @chm-tm for providing the repro. I'll look into it.

About MSBuild, then I'm not the right person to comment, may be you can start a conversation with MSBuild folks about evaluation order. But personally I've also find Structured log to be very useful to understand MSBuild items or property evaluations.

[jainaashish]

Merged to dev. But re-opening to consider for 4.9

[jainaashish]

Merged to 4.9.0 with commit# 0081f51f7ea120ca551b226c8b665fa5b9b441fd