dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.68k stars 1.06k forks source link

dotnet build with output folder causes assemblies to stomp on each other for Multiple TFMs #8964

Open hvanbakel opened 6 years ago

hvanbakel commented 6 years ago

Steps to reproduce

Create new net standard library. Set project file to be:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>net46;netstandard2.0</TargetFrameworks>
    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
  </PropertyGroup>

</Project>

open command line and run

dotnet build -o .\out

Extract the nupkg generated in the out folder In the extracted folder, open the lib\net46 folder and decompile the assembly that is in there.

Expected behavior

This assembly should have

[assembly: TargetFramework(".NETFramework,Version=v4.6", FrameworkDisplayName = ".NET Framework 4.6")]

Actual behavior

instead it has:

[assembly: TargetFramework(".NETStandard,Version=v2.0", FrameworkDisplayName = "")] Which results in similar errors as reported here: dotnet/standard/issues/542

The origin is most likely that the build step creates the 46 version first, then OVERWRITES the dll with the netstandard20 version and then packs that into both folders.

Environment data

C:\Program Files (x86)\Microsoft Visual Studio 14.0>dotnet --info .NET Command Line Tools (2.1.2)

Product Information: Version: 2.1.2 Commit SHA-1 hash: 5695315371

Runtime Environment: OS Name: Windows OS Version: 10.0.10586 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\2.1.2\

Microsoft .NET Core Shared Framework Host

Version : 2.0.3 Build : a9190d4a75f4a982ae4b4fa8d1a24526566c69df

To be fair, this issue doesn't occur when there is no output folder specified.

livarcocc commented 6 years ago

@nguerrera can you comment on this? I believe that when you specify the output we honor it no matter what, even if in this case, the dlls will stomp on each other.

Should we do the appendtfm in this case? Or we explicitly decided not to?

livarcocc commented 6 years ago

@rohit21agrawal can you comment on this? This works for dotnet pack. Seems to fail only for dotnet build with the GeneratePackageOnBuild set to true.

Is this something that the SDK is missing to set?

nguerrera commented 6 years ago

Seems like -o is just broken for multi-targeting. It sets outputpath as a global property and that nullifies our appending of TFM. I think we need to change what -o passes to something like /p:OutputPathOverride=value and then we can build the real OutputPath from override + TFM.

nguerrera commented 6 years ago

I don't see it working for dotnet build.

livarcocc commented 6 years ago

Good point. Ok, I am moving this back to 2.1.300. This is pretty bad.

nguerrera commented 6 years ago

This is tricky to fix. We need to use OutputPathOverride when directly when single-targeted for maximum compat.

dasMulli commented 6 years ago

Alternatively, one if the imported targets could set TreatAsLocalProperty="OutputPath" but that would be a dirty fix I guess..

ozziepeeps commented 6 years ago

This bit me, too, and cost me an afternoon! Thanks for filing the issue and workaround @hvanbakel. Confirmed that not setting output folder worked around the issue for me.

jimmyca15 commented 4 years ago

I got hit by this today. I tried to see if I could include the TargetFramework into the output path but it didn't work.

Something like dotnet build /p:OutDir=bin\shared\$(TargetFrameWork)"

Nirmal4G commented 4 years ago

Seems like -o is just broken for multi-targeting. It sets outputpath as a global property and that nullifies our appending of TFM. I think we need to change what -o passes to something like /p:OutputPathOverride=value and then we can build the real OutputPath from override + TFM.

@nguerrera My preferred solution would be that we map BaseOutputPath to -o instead of OutputPath for both legacy and Sdk-style projects. With BaseOutputPath (dotnet/msbuild#5238) support added to Common targets, we could just do that.