Msbuild: Include .config file when copying output files of a dependent project

[tmat]

Consider scenario where project A references project B and project B has an App.config file. The build copies .dll/.exe and .pdb files of B to the output directory of A, but doesn't currently copy .dll.config/.exe.config files that may contain binding redirects and other configuration.

I propose the build copies the configuration file as well, so that binding redirects are not lost. Since the latest CoreFX binaries heavily rely on presence of binding redirects having binding redirects flow seamlessly thru build system is becoming more and more important.

[tmat]

/cc @ericstj

[ericstj]

I think you can override this behavior by setting AllowedReferenceRelatedFileExtensions.

It sounds like @tmat is suggesting to add .config to this list.

[jeffkl]

Be aware that binding redirects are only loaded from the initial program's app.config. For example, if you have foo.exe and foo.exe.config which loads bar.dll, the .NET Framework will not look for redirects within a bar.dll.config. If foo.exe launches a bar.exe in a separate process, the binding redirects will be loaded. I'm not sure from your example if you have EXEs referencing DLLs or other EXEs.

Also, if bar.dll has logic to read the config file using the standard API calls, it will be given the foo.exe.config.

``` C#
var mySetting = ConfigurationManager.AppSettings["mysetting"];

If `bar.dll` has the logic to manually load it's own app.config, it can get to all of it's settings:

``` C#
var appConfig = ConfigurationManager.OpenExeConfiguration(Assembly.GetExecutingAssembly().Location);
var mySetting = appConfig.AppSettings["mysetting"];

But again, binding redirects are only loaded from the initial .exe.config and as far as I know, there is no way for a class library to provide assembly redirects to its callers. I think this is why the app.config was left out of the list of dependencies to copy. But as @ericstj has said, you can easily add to the list in your project A.

<AllowedReferenceRelatedFileExtensions>
    .pdb;
    .xml;
    .pri;
    .config
</AllowedReferenceRelatedFileExtensions>

[tmat]

Be aware that binding redirects are only loaded from the initial program's app.config. For example, if you have foo.exe and foo.exe.config which loads bar.dll, the .NET Framework will not look for redirects within a bar.dll.config.

It is true that .NET assembly binder only uses .exe.config, however other frameworks (like unit testing frameworks including xunit) do load binding redirects from .dll.config files next to unit test .dlls.

Also in some scenario a project adds a reference to another project in order to get the .exe copied to the output directory and then call create process on it.

[tmat]

I propose we add .dll.config and .exe.config to those extensions by default.

[ericstj]

binding redirects are only loaded from the initial .exe.config and as far as I know,

BindingRedirects will be loaded from whichever config was configured for the AppDomain. Most well behaved plugin hosts will provide a convention like .dll.config for plugins to specify their own config file. Azure, MSTest, Xunit all do this. MSBuild does not 😉

[jeffkl]

Can you point us to where these other systems do binding redirects for plugins? I'd love to know more.

[ericstj]

What are you looking for? The APIs they call? I believe it's AppDomainSetup.ConfigurationFile but it's been a while since I looked at the code. You can probably reach out to MSTest folks, Azure SDK, or go poke around the Xunit repo (keeping license in mind of course).

[rainersigwald]

It's important to note that MSBuild does not by default create a new AppDomain for task assemblies.

[ericstj]

I'll start by saying that this is a bit of a tangent for this issue on related files. Let me know if you have a better issue to move this discussion.

It's important to note that MSBuild does not by default create a new AppDomain for task assemblies.

Noted: MSBuild has an architecture that prevents people from using the package ecosystem in .NET. I understand this was done for performance, but sometimes you need an option that favors correctness at the expense of performance.

@tmat and I were discussing the other day that it probably makes more sense for MSBuild to hook AssemblyResolve. That way tasks don't have to worry about binding redirects. You'd still hit issues with ordering where you encounter a task early that loads an older version of a library than one needed by a later task, but at least it would handle a common case where folks have conflicts within a task. Right now tasks themselves have to deal with this. Other ideas: If you have a complete view of all task directories you could unify up front for all possible assemblies. If you don't have a complete view you could do a look when you're about to instantiate a task to determine if it would have a conflict and open a new app domain if it would.

[rainersigwald]

MSBuild has an architecture that prevents people from using the package ecosystem in .NET. I understand this was done for performance, but sometimes you need an option that favors correctness at the expense of performance.

I don't think this is a fair characterization. The task-loading mechanisms were designed without consideration for the package ecosystem in .NET because they were designed over a decade ago and that ecosystem didn't exist then.

It's entirely reasonable to ask for improvements now that it _does_ exist. Let's move that discussion to #1312.

[rainersigwald]

WRT the subject of this issue, including .config sounds reasonable. My only concern is that it might be nice to be able to restrict it to .dll.config and .exe.config to avoid accidentally vacuuming up unrelated files.

@tmat

Since the latest CoreFX binaries heavily rely on presence of binding redirects

This is news to me. Is there documentation on what's changing here available somewhere? Or a couple of key PRs for me to educate myself?

[tmat]

@rainersigwald +1 for .dll.config and .exe.config. That's what I ended up doing in a global .targets of Roslyn project file: https://github.com/dotnet/roslyn/pull/14879/commits/1977e1daa7bb541b73e218d1cfe01e983d4666df

@ericstj Might be able to point out any documentation there is. I don't think there is much. It just simply happened as a side effect of some versioning decisions. It is not uncommon for packages to expose version X.Y.Z.0 in ref\netstandard1.3 directory while the implementation assembly in lib\net46 or the assembly shipping in .NET FX 4.6.* is actually X.Y.Z'.0 where Z' < Z.

[rainersigwald]

Ah, conveniently the list of extensions would be happy to have the double-extension part: https://github.com/Microsoft/msbuild/blob/master/src/XMakeTasks/AssemblyDependency/ReferenceTable.cs#L868-L878

(I was a bit worried that we were calling Path.GetExtension somewhere which would have returned just .config.)

[nguerrera]

@krwq @piotrpMSFT @dsplaisted

[jaredpar]

@tmat and I were discussing the other day that it probably makes more sense for MSBuild to hook AssemblyResolve. That way tasks don't have to worry about binding redirects.

Why would we prefer this route vs. the route taken by Visual Studio? Visual Studio has a similar style extension environment and approaches the problem by essentially dictating the version of .Net Standard it will run in it's process. It then sets up the binding redirects appropriately.

This means that in order for an extension to run correctly in Visual Studio it simply needs to use a version of Net Standard which is compatible with that of Visual Studio. The global binding redirects take care of all the nasty resolve issues. Extensions really don't have to worry about the redirects.

[jeffkl]

The team discussed this and weren't quite completely for or against it. We decided to error on the side of user request so I've sent a PR to include .config files.

[tmat]

@jaredpar VS also has binding redirects in devenv.config file. Packages can define additional binding redirects that get merged into devenv.config.

[jaredpar]

@tmat i'm not sure the merging would have any value for MSBuild.

The value of merging in Visual Studio is so that different extensions can have a coherent view of their dependencies. It allows extensions to deploy / override shared components in their VSIX and ensure that others extensions see the new version as well. That's valuable in Visual Studio because extensions often communicate using shared assets.

That's not the case in MSBuild. Extensions are pretty much independent and don't really communicate directly with each other. Hence I don't see a lot of value in merging the config files.

[tmat]

@jaredpar Good point.

[tmat]

@jaredpar Although, suppose there is an extension that depends on a library A that depends on a library B and the extension also depends on library B but newer version. Then the extension itself needs binding redirect for B.

[jaredpar]

@tmat they shouldn't if each extension deploys the library separately + it has a strong name. In that case the "Load From" context rules should cause everything to work. In each case the CLR logic should essentially be the following:

1. Is the assembly loaded? If yes then done
2. Can the assembly be loaded by the primary load context? If yes then done
3. Can the assembly be loaded from the directory that established the Load From context.

In the end this should cause the assembly to be loaded twice into the application: one copy from each extension directory.

I actually recently had to debug this exact case in Visual Studio for loading Microsoft.ApplicationInsights :)

[tmat]

I meant the case where there is a single extension, not multiple extensions.

MyExtension -> LibA -> LibB (v1)
MyExtension -> LibB (v2)

LibB v1 needs to be redirected to v2. The directory where MyExtension is installed will have LibA and LibB (v2).

[jaredpar]

@tmat gotcha. In that case yes they need to deal with assembly resolution in some way.

[rainersigwald]

This change appears to have broken several customers:

https://developercommunity.visualstudio.com/content/problem/29031/dll-appconfig-being-built-in-vs-2017.html
https://developercommunity.visualstudio.com/content/problem/19696/vs-always-copies-all-appconfig-files-to-output-fol.html
https://developercommunity.visualstudio.com/content/problem/197297/class-library-config-files-included-with-client-ap.html

How to opt out

Define a property in your project

<!-- Redefine this list without .dll.config; see https://github.com/Microsoft/msbuild/issues/1307 -->
<AllowedReferenceRelatedFileExtensions>.pdb;.xml;.pri</AllowedReferenceRelatedFileExtensions>

That will prevent *.dll.config and *.exe.config from being copied.

Note that doing this overrides a property defined in common targets, so it may cause problems in the future if the default list changes (to something that you do want).