dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
657 stars 331 forks source link

Arcade forcing .config files to be generated for DLLs targeting net472 #9305

Open jaredpar opened 2 years ago

jaredpar commented 2 years ago

Consider this part of WorkAround.targets

  <!-- Workaround for https://github.com/Microsoft/msbuild/issues/1310 -->
  <Target Name="ForceGenerationOfBindingRedirects"
          AfterTargets="ResolveAssemblyReferences"
          BeforeTargets="GenerateBindingRedirects"
          Condition="'$(AutoGenerateBindingRedirects)' == 'true'">
    <PropertyGroup>
      <!-- Needs to be set in a target because it has to be set after the initial evaluation in the common targets -->
      <GenerateBindingRedirectsOutputType>true</GenerateBindingRedirectsOutputType>
    </PropertyGroup>
  </Target>

This effectively forces $(GenerateBindingRedirectsOutputType) to true whenever $(AutoGenerateBindingRedirects) is true. MSBuild though made a change that defaults $(AutoGenerateBindingRedirects) to true whenever a project targets net472 or a later .NET Framework TFM. Together this means that when using arcade and targeting modern .NET Framework TFM it is generating app.config files even though that is likely not the intent of the author.

If you want to get a sense of the confusion this can cause consumers as well as MSBuild team members you can look at the following internal conversation

https://teams.microsoft.com/l/message/19:3212bf033f4c4b5198643a04fa1048fa@thread.skype/1652138528354?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=4ba7372f-2799-4677-89f0-7a1aaea3706c&parentMessageId=1652138528354&teamName=.NET%20Developer%20Experience&channelName=MSBuild&createdTime=1652138528354

richlander commented 2 years ago

Might want to clarify that the link is internal-only. That said, the conversation is good and made me smile. Classic Jared.

tmat commented 2 years ago

I believe the workaround was introduced so that binding redirects are generated for unit testing projects:

https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/Tests.targets#L6

Unfortunately, https://github.com/Microsoft/msbuild/issues/1310 is still open :(

I think the best approach would be to move the logic to the test SDK.

alexperovich commented 2 years ago

@jaredpar since this was introduced to fix an issue with test projects, would it be correct to move this workaround into Tests.targets?

garath commented 1 year ago

@jaredpar bumping this, any comments on above? Or anything needed from dnceng?

jaredpar commented 1 year ago

since this was introduced to fix an issue with test projects, would it be correct to move this workaround into Tests.targets?

That seems reasonable to me.

michellemcdaniel commented 1 year ago

We are not likely going to be able to prioritize the work in the near future. Jared, if you would like to submit a PR for the workaround, we would definitely take it.

rainersigwald commented 2 weeks ago

This has caused the .NET SDK to ship a bunch of useless .dll.config files in every copy of the .NET SDK ~on Windows~.

❯ fd -a .dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Containers\tasks\net472\Microsoft.DotNet.Cli.Utils.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Containers\tasks\net472\Microsoft.NET.Build.Containers.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\DotnetTools\dotnet-format\BuildHost-netcore\Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\DotnetTools\dotnet-format\dotnet-format.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\DotnetTools\dotnet-watch\8.0.400-preview.24304.8\tools\net8.0\any\BuildHost-netcore\Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\DotnetTools\dotnet-watch\8.0.400-preview.24304.8\tools\net8.0\any\dotnet-watch.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Sdks\Microsoft.NET.Sdk\tools\net472\Microsoft.NET.Build.Tasks.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Sdks\Microsoft.NET.Sdk.BlazorWebAssembly\tools\net472\Microsoft.NET.Sdk.BlazorWebAssembly.Tasks.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Sdks\Microsoft.NET.Sdk.Publish\tools\net472\Microsoft.NET.Sdk.Publish.Tasks.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Sdks\Microsoft.NET.Sdk.Razor\tasks\net472\Microsoft.NET.Sdk.Razor.Tasks.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\Sdks\Microsoft.NET.Sdk.StaticWebAssets\tasks\net472\Microsoft.NET.Sdk.StaticWebAssets.Tasks.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\datacollector.dll.config
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24305.3\vstest.console.dll.config
.dotnet/sdk/6.0.413
❯ fd .dll.config
DotnetTools/dotnet-format/dotnet-format.dll.config
DotnetTools/dotnet-watch/6.0.413-servicing.23367.15/tools/net6.0/any/dotnet-watch.dll.config
Sdks/Microsoft.NET.Sdk/tools/net472/Microsoft.NET.Build.Tasks.dll.config
Sdks/Microsoft.NET.Sdk.BlazorWebAssembly/tools/net472/Microsoft.NET.Sdk.BlazorWebAssembly.Tasks.dll.config
Sdks/Microsoft.NET.Sdk.Publish/tools/net472/Microsoft.NET.Sdk.Publish.Tasks.dll.config
Sdks/Microsoft.NET.Sdk.Razor/tasks/net472/Microsoft.NET.Sdk.Razor.Tasks.dll.config
datacollector.dll.config
vstest.console.dll.config