When using .NET Standard facades on .NET Framework, automatic binding redirects should be enabled

[terrajobst]

We now automatically inject facades into the .NET Framework compilation when consuming a library that targets .NET Standard.

We need to make sure that when these facades are injected, that we forcefully enable automatic generation of binding redirects. That's necessary because we deploy files that might have higher versions then what the .NET Standard binary is referencing. These files aren't necessarily part of the runtime unification table or are explicitly opted-out by a minor version bump.

In non-SDK-style projects, the property <AutoGenerateBindingRedirects> is defaulted to false and relies on project templates to be set. The latter is only the case when the project got created targeting .NET Framework 4.5.1 or later. When an existing project is upgraded to 4.5.1 or later, the property is still false. I suggest we set it to true when we detect .NET Standard facades are needed.

My understanding is that in SDK-style projects the <AutoGenerateBindingRedirects> is already true if the project is targeting .NET Framework and producing an application.

[terrajobst]

@nguerrera @dsplaisted @ericstj @weshaggard

[ericstj]

I believe the property is AutoGenerateBindingRedirects.

Currently it is defaulted for SDK projects:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

Lines 53 to 55 in cd25d6b

	<PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' and '$(HasRuntimeOutput)' == 'true'">
	<AutoGenerateBindingRedirects Condition="'$(AutoGenerateBindingRedirects)' == ''">true</AutoGenerateBindingRedirects>
	</PropertyGroup>

I believe for non-SDK projects it is set to "true" in the template for File>New Project. I agree that we could just make it true by default (assuming project doesn't set it to false). Should it be done here or MSBuild? @rainersigwald

[ericstj]

Ping @livarcocc @dsplaisted please triage.

[rainersigwald]

AutoGenerateBindingRedirects is turned on in the template for .exe projects, but not DLLs (for non-SDK projects). No default value is provided in common targets, and since it's only compared to true, that is equivalent to defaulting to false.

It seems like it would be a breaking change to set a default for all projects now.

[livarcocc]

We are not taking any new bugs for 2.0.0, unless we have strong evidence that it really breaks a scenario and we really must take it.

[terrajobst]

I'd say this is a very severe bug we need to fix.

Scenario

• The customer has an existing application targeting .NET Framework 4.5 or lower
• The customer retargets the app to .NET Framework 4.6.1
• The customer references a .NET Standard 1.6 class library (P2P or NuGet)
• Customers runs the app

Expected Behavior

The app works.

Actual Behavior

The application crashes with FileNotFoundException:

Unhandled Exception: System.IO.FileLoadException:
Could not load file or assembly 'System.Runtime, Version=4.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies.
The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)
   at ConsoleApp3.Program.Main(String[] args)

Manually setting AutoGenerateBindingRedirect to True solves the problem, but there are no UI affordances for it and the error message isn't actionable.

[nguerrera]

The symptom is severe, but:

• The workaround is trivial
• Is it even a regression? Didn't you always need binding redirects to reference NETStandard.Library from NETFramework?

[terrajobst]

Customers couldn't use .NET Standard 1.5+ assets in .NET Framework 4.6.1. IOW, the standard was created to match what was shipped in the platform. With .NET Standard 2.0 we've retroactively made .NET Framework 4.6.1 implement .NET Standard 2.0. This makes all our facades OOB on that version on the framework, thus requiring binding redirects.

The workaround is trivial

Only if you're knowledgeable in MSBuild™️. Seriously though, there no easy way to add this property and it's virtually undiscoverable.

[nguerrera]

For discoverability, doesn't the error list have https://github.com/Microsoft/msbuild/blob/4d2d2d2d51e18ad817db1c55dfa0d19473e7d1fc/src/Tasks/Resources/Strings.resx#L1527-L1530 ? Or is it something about the facade magic that circumvents that?

[terrajobst]

Weird, I thought I had verified that I get zero warnings. Turns out you're correct, I get this:

In that case I think I'm OK with this being fixed in 2.1.

[andriysavin]

@terrajobst I'm experiencing an issue very similar to what you described: #Azure/azure-functions-vs-build-sdk#115

[terrajobst]

@andriysavin

Is it with Azure Functions or with your own application?

[andriysavin]

@terrajobst That particular issue happens with Azure Functions msbuild task (part of AF SDK and of the Functions VS project template). My understanding is that it happens when some code is loaded dynamically. For example, functions build task loads all types in an assembly with functions, and if any of the types expose dependencies which in turn have dependency on 'System.Runtime, Version=4.1.0.0' (like Microsoft.Azure.DocumentDB.Core), it crashes or fails to load the type, depending on how its done. This compile-time error can be worked around by making such classes internal. However, when function runtime calls the function, the error happens at run time, which is even worse.
So it looks like the problem is that binding redirects are not generated for dynamically loaded assemblies, and this creates really big problems even for very trivial scenarios. And its not specific to Azure Functions.

[terrajobst]

Yep, in dyanamic scenarios binding redirects don't work at all, unfortunately. The good news is that at least for .NET Standard 2.0, the problem will eventually disappear (mostly in 4.7.1) as the need for binding redirects is no longer there. However, we're also thinking about how we can solve these issues without requiring binding redirects. We'll see.

[scottcha]

Dropping a note here on the negative impact of this bug in the Azure functions scenario. Like this issues mentions: Azure/azure-functions-vs-build-sdk#139 Even a seemingly insignificant code change in your azure functions project can introduce very hard to diagnose build errors. In my case it was only a refactor of the below code to a static function which caused the failure to be introduced. It was very surprising as 0 new unique lines of code were added to the project but due to some things outside my control with how the binding is done in azure function generator the error was introduced. Two days later I was finally able to narrow it to the simple refactor.

This is a serious bug impacting current azure function developers using VS which really needs to be fixed.

Code: inline in an existing method wasn't causing the issue but refactored in to a shared method caused the issue.

public static void AuthenticateADLSFileSystemClient(out DataLakeStoreFileSystemManagementClient adlsFileSystemClient, out string adlsAccountName, TraceWriter log)
{
log.Info($"Attempting to sign in to ad for datalake upload");
adlsAccountName = CloudConfigurationManager.GetSetting("ADLSAccountName");
//auth secrets
var domain = CloudConfigurationManager.GetSetting("Domain");
var webApp_clientId = CloudConfigurationManager.GetSetting("WebAppClientId");
var clientSecret = CloudConfigurationManager.GetSetting("ClientSecret");
var clientCredential = new ClientCredential(webApp_clientId, clientSecret);
var creds = ApplicationTokenProvider.LoginSilentAsync(domain, clientCredential).Result;
// Create client objects and set the subscription ID
adlsFileSystemClient = new DataLakeStoreFileSystemManagementClient(creds);
}

[Yvand]

Hello, I have a .NET 4.6.1 "Azure Functions" project and I experience the issue Azure/azure-functions-vs-build-sdk#139 because I have a class that inherits JsonMediaTypeFormatter.
I have no idea how to fix this, I added this in my .csproj but it doesn't change anything:

<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>

Do you have a suggestion to fix this?
Thanks

[terrajobst]

We've designed and accepted the necessary feature work. @rainersigwald will be working on this.

[livarcocc]

Is there a bug tracking this on the MSBuild side, where the fix will happen? Should we move this one there?

[dasMulli]

There's dotnet/msbuild#2481 (not sure if this is the one the feature document is meant to solve, but linked to from some places already)

[livarcocc]

Closing this issue in favor of the MSBuild one. If I got this wrong, just comment and I can re-activate.