dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.66k stars 1.06k forks source link

No DisableTransitiveProjectReferences analog for package references? #11803

Open mdrexel opened 4 years ago

mdrexel commented 4 years ago

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?

Hixon10 commented 7 months ago

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 commented 7 months ago

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

mdrexel commented 7 months ago

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:

KirillOsenkov commented 7 months ago

I just wish PackageDownload supported GeneratePathProperty

kygagner commented 7 months ago

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 commented 3 months ago

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

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 commented 1 month ago

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>