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

MSBuild missing automatic binding redirects when using NuGet transitive pinning #40284

Open
AArnott opened this issue Jan 27, 2023 · 17 comments
Open
Labels
Area-NetSDK untriaged Request triage from a team member

Comments

@AArnott
Copy link
Contributor

AArnott commented Jan 27, 2023

Issue Description

MSBuild doesn't produce binding redirects automatically for indirect nuget dependencies, leading to runtime failure of my code.

Steps to Reproduce

git clone https://github.com/microsoft/vs-servicehub.git
cd vs-servicehub
git checkout 7659b150cd580d35f3751b33341bcb50f63c32dd
Microsoft.ServiceBroker.sln

Within VS, run the Microsoft.ServiceHub.Analyzers.Tests (net472) tests.

Expected Behavior

I expect them to pass (as they do at the command line).

Actual Behavior

Most of the tests fail with this error:

System.TypeInitializationException : The type initializer for 'Microsoft.CodeAnalysis.Testing.ReferenceAssemblies' threw an exception.
---- System.IO.FileLoadException : Could not load file or assembly 'NuGet.Packaging, Version=5.6.0.5, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)

Analysis

The Directory.Packages.props file forces the NuGet.Protocol transitive dependency to 6.4.0, where it would naturally be in the 5.x range. Because this is not a direct dependency, and the indirect dependency holds PrivateAssets="compile" on it such that it isn't passed to my compiler (or RAR?), msbuild doesn't produce the necessary binding redirect.

When I add my own top-level dependency on NuGet.Protocol, such that it goes to RAR, the binding redirect is created and the tests pass.

I believe MSBuild should be enhanced to be aware of transitive dependencies so that it can produce the necessary binding redirects.

Versions & Configurations

.NET SDK 7.0.101
Dev17.5

@AArnott
Copy link
Contributor Author

AArnott commented Jan 27, 2023

This might be related: NuGet/Home#10505

@rainersigwald
Copy link
Member

@jeffkl is this what you would expect from transitive-pinned dependency? It is very surprising to me.

@KalleOlaviNiemitalo
Copy link

#2547 is another case of missing binding redirects.

@jeffkl
Copy link
Contributor

jeffkl commented Jan 30, 2023

When I add my own top-level dependency on NuGet.Protocol, such that it goes to RAR, the binding redirect is created and the tests pass.

@AArnott what if you add a top-level dependency with PrivateAssets=Compile? Wouldn't the assembly also not get passed to MSBuild for binding redirect information?

@AArnott
Copy link
Contributor Author

AArnott commented Jan 31, 2023

@jeffkl No. This top-level reference works just as well at producing the required binding redirect:

<PackageReference Include="NuGet.Protocol" PrivateAssets="compile" />

I couldn't tell you why, as I don't know the RAR system that deeply. But I know it's more complicated than just directly referenced assemblies.

@sharwell
Copy link
Member

I just fixed another one this morning:
dotnet/roslyn-analyzers#6469

@jeffkl
Copy link
Contributor

jeffkl commented Feb 1, 2023

Just an update, I am actively investigating a fix but am on call this week. I'll let you know when I've made progress on this.

@jeffkl
Copy link
Contributor

jeffkl commented Feb 3, 2023

I think I have fix ready for all of the transitive pinning dependency flow problems: NuGet/NuGet.Client#4953

@AArnott
Copy link
Contributor Author

AArnott commented Feb 3, 2023

That's great, @jeffkl. Do you think it will resolve my missing binding redirect issue?

@AR-May
Copy link
Member

AR-May commented Feb 21, 2023

Team triage: closing as a duplicate

@AR-May AR-May closed this as completed Feb 21, 2023
@sharwell sharwell closed this as not planned Won't fix, can't repro, duplicate, stale Feb 21, 2023
@sharwell
Copy link
Member

sharwell commented Feb 21, 2023

@AR-May It's difficult to understand if the linked item will truly resolve this. In order for binding redirects to be generated, the change would need to impact the inputs to ResolveAssemblyReference such that its SuggestedRedirects output includes consideration for assemblies that are transitive dependencies of the current project but are not included as references of the current project.

I updated the resolution state to reflect that this was marked as a duplicate of external issue NuGet/NuGet.Client#4953.

@AArnott
Copy link
Contributor Author

AArnott commented Feb 21, 2023

Several bugs have been linked from this one. Which one is the alleged duplicate, @AR-May? I share @sharwell's concerns.

@rainersigwald
Copy link
Member

NuGet/Home#12270 is about transitive assets flowing incorrectly through transitive-pinned packages and looks like the canonical issue to me.

@AArnott
Copy link
Contributor Author

AArnott commented Feb 22, 2023

@rainersigwald Thanks for looking. But 12270 appears to be a distinct issue. That one talks about how PrivateAssets should be aggregated when it appears along multiple paths. This issue is about how even a single path with PrivateAssets="compile" creates a problem because RAR doesn't ever see the reference assembly, even when CPVM changes the assembly version, such that no binding redirect is created for it, although CPVM creates a need for it.

@AndreasClassen
Copy link

Hi @AArnott. Did you find a solution to your problem? We are experiencing the same issue.

We use exclude="compile" inside our own NuGet packages to limit the publicly visible API surface to that of the packaged library itself. This is also what Microsoft recommends in the documentation.

This leads to PrivateAssets="compile" being added to the PackageReference inside the project file.

That in turn leads to the problematic behavior of msbuild that you describe in your initial post: msbuild does not add binding redirects for transitive dependencies.

In our case, we have a project that references two NuGet Packages: foo and bar. The package foo has a dependency on a NuGet Package CoreWcf.Primitives with exclude="compile" set. The package bar also has a dependency on CoreWcf.Primitives, but with another version (also with exclude="compile" set). In the project that references both foo and bar, no binding redirects for CoreWcf.Primitives will be generated. This leads to a runtime exception at some point.

A workaround, quite ugly, is to reference CoreWcf.Primitives directly from the project. Another workaround is to remove the exclude="compile" from the foo and bar packages.

@AArnott
Copy link
Contributor Author

AArnott commented Apr 18, 2024

No, I haven't spent any more time on this issue.

@rainersigwald
Copy link
Member

Reactivating (I appear to have missed the ping last year). Moving to SDK since I think this is about what happens to the asset between restore (create project.assets.json) and RAR, which is handled in SDK tasks (for .NET SDK projects).

@rainersigwald rainersigwald reopened this Apr 18, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels Apr 18, 2024
@rainersigwald rainersigwald transferred this issue from dotnet/msbuild Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

7 participants