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

No DisableTransitiveProjectReferences analog for package references? #11803

Open
mdrexel opened this issue May 28, 2020 · 57 comments
Open

No DisableTransitiveProjectReferences analog for package references? #11803

mdrexel opened this issue May 28, 2020 · 57 comments

Comments

@mdrexel
Copy link

mdrexel commented May 28, 2020

Issue #1750 introduced the <DisableTransitiveProjectReferences> property so that an SDK-style csproj project can opt out of the new implicit transitive references feature. Originally, the proposed name for the property was <DisableImplicitTransitiveReferences>; however, during Pull Request #1751, the name was changed to DisableTransitiveProjectReferences to explicitly indicate that only project references would exhibit a behavior change; package references would not be impacted by setting this property.

(My understanding of the meanings of these phrases is as follows: package reference refers to NuGet package references, while project reference refers to a reference to another project in the same solution.)

There doesn't seem to be a mechanism to disable the transitive reference behavior for package dependencies. This makes it so that, for example, if Project A has a project reference to Project B, and Project B has a package reference to ex. Newtonsoft.Json, Project A can utilize Newtonsoft.Json without explicitly adding a package reference to it. When PrivateAssets is used, Project B's dependency on Newtonsoft.Json fails at runtime when called from Project A, because Newtonsoft.Json.dll is not copied to the output directory of Project A. (I'm just using Newtonsoft.Json as an example here because it's well-known and has no external dependencies when used with recent Framework/Core/Standard TFMs - this issue applies to any NuGet package dependency.)

What is the motivation for this behavior? Could an analogous property be added (named like <DisableTransitivePackageReferences>) so that in situations like the one described above, Project A would not be able to utilize Newtonsoft.Json without explicitly adding a package reference, but Project A could still utilize Project B (which relies on the dependency directly) without encountering runtime errors due to missing assemblies?

@pzwara
Copy link

pzwara commented Jun 6, 2020

DisableTransitivePackageReferences would be very useful

@mdrexel
Copy link
Author

mdrexel commented Jun 11, 2020

Hello again, it's been a couple weeks without any indication that this was seen, so I'm pinging again for visibility. I apologize in advance if this is a faux pas; I see that many other open issues were marked for triage and assigned, so I just wanted to try to keep this question from falling between the cracks.

@mdrexel
Copy link
Author

mdrexel commented Jul 2, 2020

Hello, it's me again 😊. It's been another few weeks without any signs so I'm just checking in to see if anyone has seen this issue.

rainersigwald pushed a commit that referenced this issue Jul 20, 2020
@mdrexel
Copy link
Author

mdrexel commented Aug 3, 2020

Hello, it's been another month without any activity. I noticed that a pull request referenced this issue, but it appears that it was unrelated. Has anyone seen this issue? Is there somewhere else we're supposed to submit things like this?

@mdrexel
Copy link
Author

mdrexel commented Sep 4, 2020

Just making my monthly comment to show that I am still hoping for a reply 😄.

@mdrexel
Copy link
Author

mdrexel commented Oct 5, 2020

I've come to leave my monthly comment. I see that some other issues have been marked as "Needs Triage", so I'm still hoping this will eventually be noticed.

@mdrexel
Copy link
Author

mdrexel commented Nov 6, 2020

Another month, another comment :). I'm still hoping to hear back on the viability of this feature request.

@mdrexel
Copy link
Author

mdrexel commented Dec 8, 2020

It's been a little over half a year with no indication that this question has been noticed. @jaredpar, as the developer who introduced the <DisableTransitiveProjectReferences> property, do you know why it was decided that that the setting would not influence package references? I see in the PR feedback that @nguerrera recommended changing the property name to indicate that package references would not be impacted, but the reasoning behind the design decision was omitted.

I see that other developers are hacking around the lack of an "official" solution; for example, in this NuGet issue, @KirillOsenkov explicitly copies the DLL that the project indirectly depends on into the output folder to resolve the missing-assembly-at-runtime issue.

Even if the outcome of this issue is simply "working as intended, no intent to change", I would appreciate a response :)

@KirillOsenkov
Copy link
Member

@KirillOsenkov
Copy link
Member

One way of preventing a PackageReference from flowing to referencing projects is setting PrivateAssets=“all” on the PackageReference itself. This gives you finer grained control and I think is a suitable workaround?

@mdrexel
Copy link
Author

mdrexel commented Dec 8, 2020

@KirillOsenkov unfortunately, using PrivateAssets causes the transitively referenced assemblies to not be copied to the build output; as you found in your workaround, you're able to explicitly mark the .DLL files as needing to be copied to the output folder, but this requires:

  • Manually modifying your .csproj file for each dependency you want to copy
  • Knowing some of the path information (ex. the TFM)

This is better than nothing, but given how convenient <DisableTransitiveProjectReferences> is, it would be really nice for there to be an equivalent feature for package references :)

@jaredpar
Copy link
Member

jaredpar commented Dec 8, 2020

@mdrexel

do you know why it was decided that that the setting would not influence package references?

The problem we were hitting was specific to project references and hence we limited the fix to project references.

@mdrexel
Copy link
Author

mdrexel commented Dec 8, 2020

@jaredpar that makes sense. I was wondering if there was any specific reasoning why the ability to disable transitive package references would be explicitly unwanted. For example, I could imagine a project that is published to a NuGet package (call it "Package A"), where opting out of transitive package dependencies could mean that Package A does not declare an explicit dependency on Package B in the .nuspec file, but consumes it transitively via Package C. I imagine this could be concerning because the dependency would be controlled by whatever Package C depends on, and so upgrading Package C could cause runtime errors in Package A due to breaking changes in Package B.

How would you feel about a property such as <DisableTransitivePackageReferences>?

@jaredpar
Copy link
Member

jaredpar commented Dec 8, 2020

I was wondering if there was any specific reasoning why the ability to disable transitive package references would be explicitly unwanted.

Nope that was not the case.

Think it may help a bit to explain the context of this change. This change occurred out of need as the dotnet/roslyn repository was adopting the .NET SDK and new project file format. At the time roslyn did this we were easily the largest and most complex solution that tried to ingest the SDK. This exposed a number of bugs in the SDK, VS project system, NuGet restore, etc ... Not unexpected after all this was brand new at the time and we were stressing it harder than it had been stressed before.

Up until this point we had been focusing hard on not changing the scenarios and / or features of the SDK. Instead we were focusing on getting roslyn to adapt to the SDK way of doing things. The reasoning was that part of the benefit of the SDK was having a more opinionated way of doing builds which in turn would allow us to have more aggressive defaults / features that minimized the amount of build code customers had to write.

This was essentially the last bug that was holding roslyn back from adopting the SDK and merging into master. We maintained the mentality of getting roslyn to have the SDK way of doing things. After a lot of discussion though we eventually decided to add this because the case was compelling. But we wanted to implement a fix specific to the scenario and not instead reset the idea of transitive dependencies. Basically wanted to keep the principles in place as much as possible.

@mdrexel
Copy link
Author

mdrexel commented Dec 8, 2020

That all seems very reasonable to me. Just to make sure I have understood you correctly, the reasons that <DisableTransitiveProjectReferences> was introduced without a corresponding <DisableTransitivePackageReferences> were:

  • The inability to disable transitive project references was a blocker for Roslyn adopting the SDK
  • The purpose of adopting the SDK was to benefit from the SDK having already "decided" things
  • In the spirit of conforming to the SDK, as minimal changes as necessary were to be introduced to the SDK during the adoption process

Please let me know if I have misinterpreted.

Now that we are far removed from this transition, would you foresee any reasons not to introduce a property like <DisableTransitivePackageReferences>?

@jaredpar
Copy link
Member

jaredpar commented Dec 8, 2020

The inability to disable transitive project references was a blocker for Roslyn adopting the SDK

Correct but a bit broader. The Roslyn solution is generally seen as a good proxy for customer solutions in size and complexity. Hence the rational is generally that if Roslyn can't adopt the SDK because of problem X then it's reasonable to expect a non-trivial amount of customers will also be unable to do so.

The purpose of adopting the SDK was to benefit from the SDK having already "decided" things

Generally yes but I'd word it differently. Essentially the benefit of the SDK style projects over traditional MSBuild projects is the SDK was a lot more opinionated and had a larger set of defaults based off those opinions. That in turn meant we could simplify the amount of code that was put into a project file and get to the nice minimal format we have today.

In the spirit of conforming to the SDK, as minimal changes as necessary were to be introduced to the SDK during the adoption process

Yes for Roslyn. We took the mentality that most of the decisions we had made which differed from the SDK decisions were essentially arbitrary (and they were). Hence rather than change the SDK we flipped to match the SDK way of doing things.

Note: to add context on top of context i want to emphasize that while I did do the bulk of the work to convert Roslyn to the new SDK format I work for the C# compiler team, not the SDK. Hence these views here are best read as the interpretation of a knowledgeable customer, not the product team. Possible SDK team may comment later and say "well actually". Please take their word, not mine 😄

@mdrexel
Copy link
Author

mdrexel commented Dec 8, 2020

Understood, I look forward to hopefully hearing from the SDK team 😜. Thank you for taking the time to explain the situation to me, I truly appreciate it. I would also like to thank you for your fine work on Roslyn!

@dsplaisted
Copy link
Member

Disabling transitive project references would have to involve changes to NuGet, I think.

@nkolev92 @zkat Have you seen other requests to allow disabling transitive package references? Should we move this issue to NuGet?

@nkolev92
Copy link
Contributor

nkolev92 commented Dec 10, 2020

No concrete asks on NuGet side that I am aware of. The idea needs work, since NuGet has to consider how this would affect packages, a concern that does not exist with ProjectReferences.

@mdrexel
Copy link
Author

mdrexel commented Dec 11, 2020

Hi @dsplaisted and @nkolev92, could you explain what impact a feature like this would have on NuGet? I am admittedly unfamiliar with how package references work under-the-hood. My naive expectation was that a DisableTransitivePackageReferences feature would be cause package references to have default behavior similar to a "reversed" PrivateAssets property.

The documentation for PrivateAssets says: "These assets will be consumed but won't flow to the parent project". By "reversed", I imagine behavior something like: "These assets will flow to the parent project, but will not be consumed by it". (I have understood "consumed" here to mean "referenced by the project"; please correct me if I am mistaken.) The behavior that I am looking for is for a project to be able to consume package references like normal, but other projects do not gain the ability to "use" these packages when adding a project reference.

Are you considering a different mechanism?

@nkolev92
Copy link
Contributor

PrivateAssets = Transitivity flow, parents won't consume.
ExcludeAssets/IncludeAssets are what handles what gets consumed.

What's use in this case? Preventing compilation against those packages? Because if you don't get them copied in the output folder, you'd run into runtime problems.

@mdrexel
Copy link
Author

mdrexel commented Dec 11, 2020

Because if you don't get them copied in the output folder, you'd run into runtime problems.

That is the behavior that I encountered when using PrivateAssets - if Project A references Package X, and Project B references Project A, when Project B is compiled, Package X was not copied to the output directory. This then caused runtime errors.

When I say "use", I mean whatever controls which packages are allowed to be referenced by code in a given project (barring the use of reflection or other tricks). I would like a setting like DisableTransitivePackageReferences to raise a CS0246 error when using a transitively referenced package.

Here is a small code example. I have a project named ProjectA. ProjectA.csproj includes a reference that looks like:

<ItemGroup>
  <PackageReference Include="Newtonsoft.Json" Version="12.0.3"/>
</ItemGroup>

In ProjectA, I have a class that looks like:

public class Example
{
    public static string Serialize(object value)
    {
        return JsonConvert.SerializeObject(value, Formatting.Indented);
    }
}

I have a different project named ProjectB. ProjectB.csproj includes a project reference that looks like:

<ItemGroup>
  <ProjectReference Include="..\ProjectA\ProjectA.csproj" />
</ItemGroup>

In ProjectB, I have a class that looks like:

using System;
using System.Collections.Generic;
using ProjectA;

namespace ProjectB
{
    public class Program
    {
        public static void Main(string[] args)
        {
            Console.WriteLine(Example.Serialize(new Dictionary<string, string>() { ["Hello"] = "World" }));
            Console.ReadLine();
        }
    }
}

Right now, there is nothing stopping me from writing something like this in ProjectB:

using System;
using System.Collections.Generic;
using ProjectA;
using Newtonsoft; // ProjectB transitively references Newtonsoft through ProjectA

If I go back to ProjectA.csproj and change my reference to look like:

<ItemGroup>
  <PackageReference Include="Newtonsoft.Json" Version="12.0.3">
    <PrivateAssets>all</PrivateAssets>
  </PackageReference>
</ItemGroup>

Now, trying to write using Newtonsoft; in ProjectB raises a CS0246 error, which is good. But, trying to run ProjectB results in:
image
This is because the Newtonsoft assembly was not copied to the output directory of ProjectB.

@nkolev92
Copy link
Contributor

This is because the Newtonsoft assembly was not copied to the output directory of ProjectB.

That would be expected in this case because newtonsoft.json is not visible at all to ProjectB.

Does <PrivateAssets>compile</PrivateAssets> do it?
Note that there are other defaults that are excluded for simplicity.

@mdrexel
Copy link
Author

mdrexel commented Dec 14, 2020

<PrivateAssets>compile</PrivateAssets> seems to behave how I would expect something like DisableTransitivePackageReferences to; running ProjectB works, but trying to add using Newtonsoft from ProjectB raises a CS0246 error. This gives me a few questions:

  • Does <PrivateAssets>compile</PrivateAssets> have any impact on NuGet packages created by packing the .csproj?
  • Looking at the documentation, I see other legal values like analyzers. I would like to totally isolate the package dependencies of ProjectA from ProjectB, including things like analyzers. I tried something like:
<IncludeAssets>runtime</IncludeAssets>
<PrivateAssets>all</PrivateAssets>

but that results in compile errors in ProjectA. Is it possible to totally isolate ProjectB from ProjectA's package dependencies?

  • It looks like controlling the behavior using PrivateAssets would require manually editing the .csproj every time a dependency is added, which I feel developers will forget to do :). Right now, we are using DisableTransitiveProjectReferences in a top-level Directory.Build.props so that the behavior is automatically applied to all projects in the solution. Is there some mechanism for defining the "default" PrivateAssets behavior? If so, would it be possible to alias such behavior to DisableTransitivePackageReferences for consistency with the project references property?

Thank you for your help @nkolev92, I really appreciate it :)

@nkolev92
Copy link
Contributor

nkolev92 commented Dec 16, 2020

Does compile have any impact on NuGet packages created by packing the .csproj?

Yes it does. The reference to the package will exist, but will be listed with exclude assets compile.

Is it possible to totally isolate ProjectB from ProjectA's package dependencies?

Every dependency of A would need to attach the metadata. What assets are included from a package is impacted

Is there some mechanism for defining the "default" PrivateAssets behavior?

You can use MSBuild item Update to achieve this. I understand that it's not exactly the most discoverable behavior though.

If so, would it be possible to alias such behavior to DisableTransitivePackageReferences for consistency with the project references property?

Given that Private/Include/Exclude assets provides a lot of customizability already, I think the argument that would need to be made is that this scenario is common enough to warrant a special switch. The concern would be that having many different switches could make the behavior pretty difficult to understand.

@mdrexel
Copy link
Author

mdrexel commented Dec 16, 2020

The reference to the package will exist, but will be listed with exclude assets compile.

What does this mean from the perspective of consumers of the package? That they themselves will not have transitive access to the dependencies of the package? That would be a disadvantage; my goal isn't to change the behavior for external package consumers, but rather only for internal developers.

I think the argument that would need to be made is that this scenario is common enough to warrant a special switch.

I think the most significant argument would be that this was a change in behavior when migrating from the old .csproj format to the new .csproj format, and restoring the old behavior is non-trivial. If you go through my earlier example using the old format rather than the new format, attempting to add using Newtonsoft in ProjectB raises the CS0246 error by default. As you yourself describe:

I understand that it's not exactly the most discoverable behavior though.

and

Every dependency of A would need to attach the metadata.

it sounds like the behavior I desire (an option akin to DisableTransitiveProjectReferences that would restore the behavior of the old .csproj format) would have significant usability gains.

Your suggestion to use an MSBuild item Update is interesting, but according to the documentation, update is "Available only for .NET Core projects in Visual Studio 2017 or later". My projects that were migrated from the old .csproj format to the new format are still on net48, and are likely to remain on it for the lifetime of the product. Additionally, I was unable to find an example of using the update option in a manner like this; could you show how one would be used to change the default PrivateAssets (even if I cannot leverage this technique, other readers may find it useful)?

My main goal is to avoid having to manually attach the <PrivateAssets>compile</PrivateAssets> property and having to watch other developers like a hawk to make sure they don't forget to :). That's better than needing to watch every file for the introduction of a transitively-referenced using, but it would be much more sustainable to have something like:

<PropertyGroup>
  <DisableTransitiveProjectReferences>true</DisableTransitiveProjectReferences>
  <DisableTransitivePackageReferences>true</DisableTransitivePackageReferences>
</PropertyGroup>

I recognize that adding additional switches like this incurs a maintenance burden, and increases the cognitive load on a reader unfamiliar with what the switches do. My counterpoint would be that there is already an existing DisableTransitiveProjectReferences that achieves half of restoring the old behavior, and that it would be natural for there to be a mirror DisableTransitivePackageReferences to deal with the remainder.

@nkolev92
Copy link
Contributor

What does this mean from the perspective of consumers of the package? That they themselves will not have transitive access to the dependencies of the package? That would be a disadvantage; my goal isn't to change the behavior for external package consumers, but rather only for internal developers.

They wouldn't be able to compile against it, but it will be part of their runtime.

Your suggestion to use an MSBuild item Update is interesting, but according to the documentation, update is "Available only for .NET Core projects in Visual Studio 2017 or later". My projects that were migrated from the old .csproj format to the new format are still on net48, and are likely to remain on it for the lifetime of the product.

Update should be supported in VS2017 for legacy projects as well.
I am not the expert there though, so I'll defer to @rainersigwald and @dsplaisted
In docs .NET Core is often used a distinguisher for sdk based projects.

@rainersigwald
Copy link
Member

Update is supported in all projects that use the dotnet/project-system managed project system, so your new SDK projects that target net48 will be fine. It also mostly works even in fully legacy projects, but some project changes might cause VS to mangle the project file if it's used (because the older project system isn't aware of Update it sometimes issues a sequence of MSBuild project-manipulation operations that cause it to get flattened).

@mdrexel
Copy link
Author

mdrexel commented Dec 17, 2020

They wouldn't be able to compile against it, but it will be part of their runtime.

This means that if a project defines <PrivateAssets>compile</PrivateAssets>, and the project is packed/published, consumers of the package can no longer transitively reference the dependencies of the project in their own code at compile time? Ideally, a mechanism to disable transitive package references would apply only during my local development, and not impact consumers of the package - just because I don't want transitive package references, doesn't mean I should get to make that call for others :)

@rainersigwald, could you show a small example of how to use update? I've thus far been unable to find an example that makes it clear how it would be leveraged to set the default PrivateAssets value.

@mdrexel
Copy link
Author

mdrexel commented Mar 11, 2021

Commenting to show I'm still interested in a solution which does not impact assembly binding redirect generation.

@mdrexel
Copy link
Author

mdrexel commented Apr 12, 2021

Hello again, commenting to show that I am still interested in solving this.

@MichaeIDietrich
Copy link

I'm happy I stumbled over this issue. We are looking for exactly the same feature, since we migrated over to SDK style projects for almost two years now. Adding such a feature would help to better highlight and manage direct dependencies for a far more cleaner project structure.

@RomanHubyak
Copy link

I am interested in having such possibility too. This would allow me to have cleaner dependencies tree and prevent it from accidental modifications, when projects at higher levels of hierarchy become dependent on dependencies of projects at lower levels.

@mdrexel
Copy link
Author

mdrexel commented May 13, 2021

Hello, checking in to show that I am still interested in finding a solution to this.

@mmarinchenko
Copy link

mmarinchenko commented May 14, 2021

Hi! I'm also interested in having such possibility)

We already have

DisableTransitiveProjectReferences="$(DisableTransitiveProjectReferences)"
and
DisableTransitiveFrameworkReferences="$(DisableTransitiveFrameworkReferences)"

Therefore it feels natural to have <DisableTransitiveXYZReferences> property for each <XYZReference> item. This can even be included in the dev checklist for adding a new item of the form <XYZReference> 😊

@mdrexel
Copy link
Author

mdrexel commented Aug 5, 2021

Hello, it's been a while since I last checked in. Just commenting to show that I am still interested in finding a solution for this.

@RomanHubyak
Copy link

RomanHubyak commented Aug 28, 2021

@mdrexel Hi! I am using the following in my Directory.Build.props in the root of the solution, so I don't have to specify it for each package in each project:

<Project>

  <PropertyGroup>
    <DisableTransitiveProjectReferences>true</DisableTransitiveProjectReferences>
  </PropertyGroup>

  <ItemDefinitionGroup>
    <PackageReference>
      <PrivateAssets>compile</PrivateAssets>
    </PackageReference>
  </ItemDefinitionGroup>

</Project>

@znakeeye
Copy link

Now it's been two years. Come on guys! This one is super important!

The DisableTransitiveProjectReferences setting is simply great! 👍 It allows you to almost disable the new, rookie-developer friendly way of declaring dependencies. For a plugin based app, we had a team of .NET masters who spent weeks on version conflicts that could only be fixed by setting this flag. So far so good.

Now, we see that transitive nuget packages are not covered by this setting. Woah, a ticking bomb for sure. To begin with, we cannot easily see (e.g. search in .csproj) where unwanted nuget packages are being referenced. (Sorry, I just discovered this glitch and I already have a headache...)

DisableTransitivePackageReferences to the rescue?
Please give us the DisableTransitivePackageReferences setting yesterday! It will serve .NET 6 developers well.

@JustinSchneiderPBI
Copy link

JustinSchneiderPBI commented Feb 17, 2023

This is super important, and very difficult for developers to work around, if at all. PrivateAssets\ExcludeAssets don't cut it, when the problem is pulling in the package itself. Some nugets have critical functionality, but are way overloaded on dependencies. And those dependencies take time to download, can be unavailable on secure nuget feeds, or introduce version and target framework complications. This is the biggest issue we still have in our repo, after 3 years.

@jez9999
Copy link

jez9999 commented Mar 18, 2023

Yeah, I'll add my vote for this setting (for what it's worth). Transitive references may work well for onion architecture, but plenty of projects don't use that (and nor should they IMHO, it's often unnecessarily complex). The default transitive reference behaviour - both for projects and for packages - is undesirable when you're specifically trying to stop projects referencing things transitively like with an n-tier architecture. A presentation layer shouldn't be able to directly reference EF stuff because the data layer references it, or ImageSharp because the BLL layer references it.

@JuliusMikkela
Copy link

Adding my vote as well, the lack of this is causing undue headaches in library construction and compliance. I fear if this is not implemented in .NET soon, the unbroken dependency chains will create real issues in our stack.

@KirillOsenkov
Copy link
Member

@jeffkl @JanKrivanek @rokonec @nkolev92 @rainersigwald

@kygagner
Copy link

kygagner commented Nov 6, 2023

Adding my vote here that this feature is necessary! .NET is much more broadly applicable than, as others have put it, the "developer-friendly" way. I know the majority use-case these days is copy-deployable apps, but industry came to depend on .NET Framework features like the GAC. We have pluggable components, not just implementations but shared contracts too. We use custom assembly resolvers. We need to be able to pull just the assets we want to support our transition to .NET 6+. I am doing my best with the tools available but it could be much easier with this feature.

@JustinSchneiderPBI
Copy link

I'd prefer the fix for PackageReferences transitive behavior, but I think we're able to work around some of these issues with PackageDownload. Does this work for others? https://learn.microsoft.com/en-us/nuget/consume-packages/packagedownload-functionality

@kygagner
Copy link

kygagner commented Nov 6, 2023

I'm grateful you pointed this feature out. It would be more convenient to be able to disable transitive package references. However, this will solve some of my immediate problems for sure.

@DalekBaldwin
Copy link

@mdrexel Hi! I am using the following in my Directory.Build.props in the root of the solution, so I don't have to specify it for each package in each project:

<Project>

  <PropertyGroup>
    <DisableTransitiveProjectReferences>true</DisableTransitiveProjectReferences>
  </PropertyGroup>

  <ItemDefinitionGroup>
    <PackageReference>
      <PrivateAssets>compile</PrivateAssets>
    </PackageReference>
  </ItemDefinitionGroup>

</Project>

This is a good start, but unfortunately not a complete replacement for DisableTransitivePackageReferences. If some of a project's dependencies refer to Nuget packages for things like System.Text.Json, and other dependencies simply take on the SDK's default version of System.Text.Json automatically for the corresponding TFM, you will see a lot of DLL hell warnings like "warning MSB3277: Found conflicts between different versions..." and "Consider app.config remapping of assembly...". Adding a redundant <PackageReference Include="System.Text.Json" /> to each project that gives a warning is probably the least verbose way to quash it, but it adds a lot of noise, and also hurts the design intent -- we want each ProjectReference and PackageReference to denote something that is actually utilized within the project. The new Directory.Packages.props facility won't help this -- if the Nuget package is not referenced, a project will not receive the pinned version.

What is really wanted is a full separation of: (1) Allowing/disallowing a project's code to refer at compile-time to things defined in particular projects/packages, and (2) All other analysis, artifact copying, runtime resolution, etc. of the dependency supply chain.

When my team transitioned from Framework to Core, we didn't even realize transitive project references had automatically become the new default. I wondered why design/architectural regressions seemed to happen so much more easily these days, and I just chalked it up to the simplified project files being less intimidating to edit by hand and thus edited more often and more carelessly. I only discovered DisableTransitiveProjectReferences a few weeks ago, enabled it, and saw massive improvements in the clarity of the structure of our primary git repo.

@Hixon10
Copy link

Hixon10 commented Jan 31, 2024

I also would be happy to get such setting, as @mdrexel requested.

Meanwhile, it looks like that I've managed to exclude not needed dependency like this:

<GlobalPackageReference ExcludeAssets="build" Include="Newtonsoft.Json">
      <IncludeAssets>runtime; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
</GlobalPackageReference>

It allows to have Newtonsoft.Json as a transitive dependency, but you cannot use it in your code anymore.

@stan-sz
Copy link
Contributor

stan-sz commented Feb 1, 2024

This seems to be solved with CPM's Transitive Pinning.

@mdrexel
Copy link
Author

mdrexel commented Feb 1, 2024

I checked out the Central Package Management tooling. Using the GlobalPackageReference element @Hixon10 provided, this does cause usages in projects that do not explicitly reference the package to fail:
image

If I remove that usage of Newtonsoft.Json from ProjectB, it builds and runs:
image

Packing ProjectA produces a normal-looking .nuspec file, no strange Include/Exclude attributes like there were when trying to use the Update attribute:
image

However, a side effect is it also causes all projects to reference the package, even if they do not include a PackageReference element for that package. (That makes sense, we declared a GlobalPackageReference element.) I added a new project to the solution, ProjectC, which is just an empty console application. Trying to use Newtonsoft.Json in this project fails as expected, but the reference still exists, so it shows up in places like the Visual Studio dependency tree:
image
and in the build output directory:
image

Packing ProjectC doesn't include a reference to Newtonsoft.Json in the .nuspec (which is good):
image
and it doesn't accidentally include the Newtonsoft.Json.dll in the package or anything (also good):
image

This is definitely a lot closer to the experience I would like from a hypothetical DisableTransitivePackageReferences, though I do observe a few issues:

  • All projects are inheriting the global package reference. I won't be surprised if this trips up tooling at some point - ex. the package appears in the .deps.json, which doesn't know about the Include/Exclude attributes:
    image
  • The package content gets copied to the output directory of all projects, because they inherit the global package reference. This could cause unexpected differences in behavior due to accidentally relying on the file, such as if the project dynamically loads assemblies (ex. plugin model or something).
    • (I didn't test it, but I bet this could also cause problems with packages that include things like build targets, since those could execute unexpectedly.)
  • It forces you into using the Central Package Management model. If I understand correctly, that means all projects in your solution need to use the exact same package versions (unless you maintain multiple Directory.Packages.props). And if you use multiple NuGet feeds, you get warning messages like warning NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source.

@KirillOsenkov
Copy link
Member

I just wish PackageDownload supported GeneratePathProperty

@kygagner
Copy link

kygagner commented Feb 2, 2024

Transitive pinning doesn't address the issue - I don't need to pin / override a transitive dependency, I need to exclude all dependencies. Also, to note on the GlobaclPackageReference - this is a similar strategy to excluding assets from an unwanted package in that it is a version specific solution. I want to be able to update a package without pulling any new transitive dependencies, whether I knew about them previously or not. And to the point about GeneratePathProperty for PackageDownload, this is also a good thing but different than installing the assets from a package and none of its transitive dependencies.

@csharper2010
Copy link

csharper2010 commented May 31, 2024

I wonder why there is so few attention on this issue.

If I understand the current behaviour correctly, there is no good solution to encapsulate and abstract away external packages in a larger application.

We converted big parts of our application still running on .NET Framework 4.8 to SDK-style projects as a first step towards .NET 8 which seemed fine in the beginning but with the transitive dependencies

  • the IDE suggestions while typing contained all the stuff from transitively referenced packages making it hard to find what I was actially looking for within our own code base (and there are a lot of classes with similar names in a lot of packages)
  • we unintentionally brought in extension methods from implementation details of the packages

I thought we had solved the problems using PrivateAssets="all" on many of the dependencies we tried to encapsulate with an API defined by us.

But now I found that PrivateAssets="all" also breaks runtime behaviour on net8.0-windows target as it has an impact on generation of .deps.json files. For some reason the runtime doesn't select the runtime and operating system dependent assemblies in the runtimes subfolder. I'm not deep enough in understanding why that happens and why my next try setting PrivateAssets to compile doesn't make things better.

As an example, I have a project FWK referencing System.Net.Http.WinHttpHandler, a project A referencing FWK and a test project A.Tests referencing A.

With both, all and compile as PrivateAssets, my tests don't work throwing

System.PlatformNotSupportedException : WinHttpHandler is only supported on .NET Framework and .NET runtimes on Windows. It is not supported for Windows Store Applications (UWP) or Unix platforms.
   at System.Net.Http.WinHttpHandler..ctor()

because obviously the correct assembly in runtimes\win\lib\net8.0 is not loaded as required to get the application working on Windows.

When I leave out the PrivateAssets attribute, the correct assembly is loaded at runtime. Everything works fine except that I don't have control over whether any code in assembly A uses the package directly, be it intentionally or accidentally.

What is the proposed way of achieving the desired encapsulation?

BTW: the projects all target net8.0-windows so the best way for us would be to directly have the runtime assembly copied to the output folder, but that's another story

@PetSerAl
Copy link

Does adding this target to project file helps?

<Target Name="RemoveTransitivePackageReferences" BeforeTargets="ResolveAssemblyReferences" DependsOnTargets="ResolveLockFileReferences">
	<ItemGroup>
		<TransitivePackageReferenceToRemove Include="%(Reference.NuGetPackageId)" Exclude="@(PackageReference)" KeepDuplicates="False" Condition="%(Reference.NuGetSourceType) == 'Package' And %(Reference.NuGetPackageId) != ''">
			<NuGetPackageId>%(Reference.NuGetPackageId)</NuGetPackageId>
		</TransitivePackageReferenceToRemove>
		<Reference Remove="@(TransitivePackageReferenceToRemove)" MatchOnMetadata="NuGetPackageId" Condition="%(Reference.NuGetSourceType) == 'Package'" />
		<TransitivePackageReferenceToRemove Remove="@(TransitivePackageReferenceToRemove)" />
	</ItemGroup>
</Target>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests