Skip to content

Regression: VS2017 / Package References in library projects do not generate binding redirects #1595

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

Closed
MichaelKetting opened this issue Sep 20, 2017 · 14 comments

Comments

@MichaelKetting
Copy link

MichaelKetting commented Sep 20, 2017

This might be releated to #1351 but since all the existing issues only reference EXE and/or WinEXE project types, it also feels like a different issue.

In VS2013 and VS2015, when I created a Class Library project and added NuGet dependencies, I would get binding redirects generated into the app.config file.

In VS2017, this behavior still works vor Full Framework Class Library projects using packages.config. However, when I create a .NET Standard Class Library project or switch to PackageReferences in the Full Framework Class Library, this does not work. It also doesn't help to set
<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>.

It can be workarounded by setting <GenerateBindingRedirectsOutputType>true</GenerateBindingRedirectsOutputType> in the project file.

I assume the reason is in
https://github.com/Microsoft/msbuild/blob/master/src/Tasks/Microsoft.Common.CurrentVersion.targets#L2109

  <Target Name="GenerateBindingRedirectsUpdateAppConfig"
    AfterTargets="GenerateBindingRedirects"
    Condition="'$(AutoGenerateBindingRedirects)' == 'true' and '$(GenerateBindingRedirectsOutputType)' == 'true' and Exists('$(_GenerateBindingRedirectsIntermediateAppConfig)')">

and
https://github.com/Microsoft/msbuild/blob/master/src/Tasks/Microsoft.Common.CurrentVersion.targets#L326
<GenerateBindingRedirectsOutputType Condition="'$(OutputType)'=='exe' or '$(OutputType)'=='winexe'">true</GenerateBindingRedirectsOutputType>
If GenerateBindingRedirectsOutputType would also check for type 'Library', it would work. Actually, why check at all?

While there exists a workaround, for library authors this is a pretty major problem since the concept of semantically versioned NuGet packages breaks down if the tooling doesn't support binding redirects - it is common and intended to have mismatched dependency version numbers. In turn, this means it's not possible to run unit tests on your library without binding redirects.

Edit: Sorry, I think I might have created a duplicate of this one here: dotnet/msbuild#1310

Edit 2: Added documentation about both .NET Standard and Full Framework with Package References.

@dasMulli
Copy link
Contributor

Since binding redirects are used in full framework only (the whole App.config actually), it makes sense to not do it for .NET Standard libraries..
The idea is that a full framework exe (or library with GenerateBindingRedirectsOutputType set to true) referencing the .NET Standard project or NuGet package would have redirects generated.

The only scenario I can think of is when a .NET Standard library is loaded via reflection without the host application having binding redirects set up. (Note that this can also fail at the moment if a full framework host and lib reference conflicting files but none detects a conflict during build since they are built independently so no redirects are generated)

@MichaelKetting
Copy link
Author

MichaelKetting commented Sep 20, 2017

I don't think so since you also get this when using PackageReferences in Full Framework and/or running Full Framework unit tests on a .NET Standard library. I know for a fact that the problem exits because NHibernate has exactly this problem when they are running their test suite:

https://www.re-motion.org/jira/browse/RMLNQ-115?focusedCommentId=20790&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-20790

https://github.com/nhibernate/nhibernate-core/blob/master/src/NHibernate/NHibernate.csproj

Edit: Their tests fail when they update Remotion.Linq to v2.2 but leave Remotion.Linq.EagerFetching at v2.1 (with a reference to Remotion.Linq v2.1)

Edit 2: I've no made it clear in the initial post that this (primarily?) affectes Full .NET Framework projects with Package References. But the additional problem with .NET Standard Libraries likely still applies as well.

Edit 3: I'll admit, it might primarily be an issue in the msbuild github project, but I only stumbled over that after everthing was already posted but there are a lot of issues about this so getting the right project to post it is a bit...involved :)

@dasMulli
Copy link
Contributor

Yeah I agree that the need to also set GenerateBindingRedirectOutputType is a pain for old projects. New sdk-based test projects (.net core / .net framework) already do this.
I was just saying it makes little sense for .NET Standard projects. You can’t run a .NET Standard project and you can’t have a .NET Standard unit test project.

@MichaelKetting
Copy link
Author

Ah, I see. Thanks for clarifying :)

@MichaelKetting MichaelKetting changed the title Regression: VS2017 / .NET Standard Library does not generate binding redirects Regression: VS2017 / Package References in library projects do not generate binding redirects Sep 20, 2017
@ngbrown
Copy link

ngbrown commented Sep 20, 2017

I'm the one working on this in NHibernate. Some, but not all of the exception occurs while using reflection to load types from a specific assembly (not even the two ReLinq assemblies in question). Other exceptions are during normal code flow between two referenced packages that have the version out of sync (NHibernate to Remotion.Linq.EagerFetching to Remotion.Linq).

All projects are VS2017 SDK type projects, but are targeting the .NET 4.61 Framework. So the test project is a library, not an EXE (or netcoreapp).

Changing the referenced "Microsoft.NET.Test.Sdk" from 15.0.0 to 15.3.0 seems to clear the test failures (by generating the correct binding redirects in the *.dll.config). I didn't need to add any other properties to the .csproj. I can turn the problem back on by moving "Microsoft.NET.Test.Sdk" back to 15.0.0.

So my question is, how will this affect downstream projects? What do they need to do to ensure no errors?

@MichaelKetting
Copy link
Author

MichaelKetting commented Sep 20, 2017

Thanks, for the update!

The big question is now, why isn't the SDK using the latest version out of the box? A quick google search didn't yield anything useful, so if someone has an answer it would be appreciated!

Edit: I just noticed that it's "Microsoft.NET.Test.Sdk" and not "Microsoft.NET.Sdk" (without the "Test").

@dasMulli
Copy link
Contributor

There is a good summary at dotnet/standard#481 (discussion issue for an announcement).

The nasty problem needing manual csproj edits are classic ("non-SDK") test projects using .net standard 2.0 assemblies. (also discussed at dotnet/msbuild#1310)

The test SDK is a separate component (although the host that executes is integrated into the CLI / VS distribution). The test SDK NuGet package includes build logic that sets the required properties.

@MichaelKetting
Copy link
Author

Thanks. This means, if you are just using your own test runner with a classic library project that contains tests and uses package references, you're still stuck with adding the binding redirect property overrides.

Not sure yet how this will scale out.

@MichaelKetting
Copy link
Author

Okay, thanks to some [additional input[(https://www.re-motion.org/jira/browse/RMLNQ-115?focusedCommentId=20795&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-20795) by @ngbrown the biggest issue seems to now be that test projects created before the release of v15.3 are stuck with the test toolset that doesn't support binding redirects unless they do a manual upgrade of their dependencies.

That's not really a good place to be in as a library author: users of your code with an existing code will get errors but they have to figure out that they need to fix their project to get rid of them. Is there an established best practice for this?

Also, it doesn't solve the problem for non-test projects (like @dasMulli mentioned). Also thanks for the link to dotnet/standard#481. No I know what I can read while I'm on the bus...

I guess my best-practice question gets answered by

Plan 2: Ensure binding redirects are on by default. Short term, this means we need to fix our target files to make sure we turn on automatic binding redirect generation. However, binding redirects don't work well in all scenarios, when there is no application project (like unit tests or add-ins). We need to work on a plan to bring the regular “higher wins” binding policy without binding redirects. This needs a proposal and proper vetting, but it seems we've reached the point where this is necessary.

@bradwilson
Copy link
Contributor

@dasMulli wrote:

I was just saying it makes little sense for .NET Standard projects. You can’t run a .NET Standard project and you can’t have a .NET Standard unit test project.

This is incorrect. xUnit.net can run .NET Standard test DLLs with the desktop CLI and MSBuild runners, precisely the place where you'd need binding redirects.

@dasMulli
Copy link
Contributor

@bradwilson "can run" or is it a supported and encouraged way? Having a project targeting the framework intended to run on would create the right assets (either a .config file or .deps.json / .runtimeconfig.json) and even resolve TFM/+RID specific NuGet assets correctly.. (bait-and-switch packages? entity framework + SQLite in test projects comes to my mind)

I can hack a csproj to build a .NET Standard .exe file that runs on CLR and CoreCLR, question is whether or not it is a good idea..

@bradwilson
Copy link
Contributor

bradwilson commented Sep 20, 2017

We discourage its use, but don't outright prohibit it. (That is to say, we don't prohibit it on the desktop or inside UWP; we do prohibit it within dotnet test and dotnet xunit.)

In the past users have made PCLs with unit tests and then run them on various platforms (for example, desktop CLR and UWP). On desktop, the binding redirects are used; on UWP, their loader will bring the right version to the party.

I'm not sure why you're saying a .deps.json file is required, unless you have somehow assumed that all .NET Standard libraries will be used with the .NET Core assembly loader. That seems short-sighted to me, if so.

@bradwilson
Copy link
Contributor

/cc @onovotny

@nguerrera
Copy link
Contributor

Closing as duplicate of dotnet/msbuild#1310

JL03-Yue pushed a commit that referenced this issue Mar 19, 2024
…34c-9bd3-abe5921a5d3d

[release/6.x] Update dependencies from dotnet/arcade
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

5 participants