msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

[Bug]: Manged to Unmanaged ProjectReference built Platform mismatch

Open AliveDevil opened this issue 1 year ago • 2 comments

Issue Description

In a csproj referencing a vcxproj may result in mismatched platforms being built (Platform flows from csproj to vcxproj, Platform is deleted).

Steps to Reproduce

  • Unpack Repro traversal-test.zip
  • Run msbuild /Restore /bl /p:Configuration=Release dirs.proj
  • Check in artifacts\bin\Shared folder for output.

Expected Behavior

Building with Microsoft.Build.Traversal and set Platform should respect the PlatformLookupTable and not delete target Platform from ProjectReference. Output in artifacts\bin\Share should be:

  • Win32
  • x64

Without any other folder (e.g. Debug, Release, x86).

Actual Behavior

Observe that in artifacts\bin\Shared\ three folders are created - "Release", "Win32", "x64". Release was created (check binlog) by the project dependency in csproj, when built with Platform=x86 Win32 was created from dirs.proj where Platform=Win32 x64 was created from dirs.proj and not rebuilt from csproj where Platform=x64. With EnableDynamicPlatformResolution=False the output is created in artifacts\bin\Shared\x86 which is wrong as well.

Running

msbuild /Restore /bl /p:Platform=x86 traversal-test.sln

Does not create the erroneous artifacts\bin\Shared\Release-folder.

Analysis

Checking the binlog I found Target Name=_GetProjectReferencePlatformProperties Project=app.csproj adding _MSBuildProjectReferenceExistent with UndefineProperties = ;TargetFramework;TargetFramework;RuntimeIdentifier;SelfContained;Platform.

Versions & Configurations

TBA VS 17.8 .NET 8 Sdk Windows 11 23h2

AliveDevil avatar Feb 06 '24 15:02 AliveDevil

@MIchaelRShea - do you have any quick idea on reason/remedy for this?

JanKrivanek avatar Feb 20 '24 15:02 JanKrivanek

I used following workaround: In Directory.Build.props I rewrite all OutputPaths that would result in Win32-platforms to x86. I replaced the ProjectReference Platform="$(Platform)" with SetPlatform="Platform=$(Platform)". Without rewriting Win32->x86 output paths I get

  • x64
  • Win32
  • x86

AliveDevil avatar Feb 20 '24 16:02 AliveDevil

Some quick observations:

Directory.Build.props:

<OutputPath Condition="'$(Platform)' == 'AnyCPU'">$(BaseOutputPath)$(Configuration)\</OutputPath>
<OutputPath Condition="'$(Platform)' != 'AnyCPU'">$(BaseOutputPath)$(Platform)\$(Configuration)\</OutputPath>

There are two possibilites how the artifacts/bin/shared/Release folder can be created:

  • $(Platform) == 'AnyCPU'
  • $(Platform) is empty

And the second case actually hapens here:

image

It looks to me this is the reason: https://github.com/dotnet/msbuild/pull/8106 specifically: Add catch-all if statement that prevents global property Platform to be passed to referenced projects when redundant

@MIchaelRShea - can you please comment if setting the negotiated platform to empty is expected to have the side effect of Platform property being actually empty, or if it is a bug?

image

JanKrivanek avatar Mar 14 '24 20:03 JanKrivanek

@AliveDevil You are relying on Platform property in Directory.Build.props, while it is set in the .csproj file. So unless this is passed as a global property by the platform negotiation - it can result empty. So either you would need move those definitions so that they use initialized Platform prop (so either into project files or into .target file or into an explicit import file which will be imported after Platform is set) or stop relying on it (possibly e.g. via using artifacts output? https://learn.microsoft.com/en-us/dotnet/core/sdk/artifacts-output)

JanKrivanek avatar Mar 27 '24 14:03 JanKrivanek

Artifacts Output isn't an option - as the vcxproj in the attached sample project can't use them, or I'd have to recreate that within vcxproj.

So … if I were to set OutputPath relying on Platform in Directory.Build.targets, it should work, because the Platform is set in Microsoft.Cpp.props or Microsoft.Cpp.targets instead of Microsoft.Cpp.Default.props.

I'll check that later, when I have access to my Windows dev machine.

AliveDevil avatar Mar 27 '24 18:03 AliveDevil

Checked, and it required some trial and error. It looks like with Directory.Build.props defining BaseOutput and BaseIntermediate it is enough to keep the NuGet-files in the artifacts-path.

I cannot set IntermediateOutputPath, OutputPath, IntDir and OutDir in Directory.Build.targets as it is imported too late, so that NuGet imported packages store data in the wrong path (esp. CppWinRT)

Thus for vcxprojs the import must happen after Microsoft.Cpp.props but before Microsoft.Cpp.targets. At the end, I settled with a Default.Artifacts.props file that is included in the vcxproj, with content

Default.Artifacts.props
<Project>
	<PropertyGroup>
		<OutputPath Condition="'$(Platform)' == 'AnyCPU'">$(BaseOutputPath)$(Configuration)\</OutputPath>
		<OutputPath Condition="'$(Platform)' != 'AnyCPU'">$(BaseOutputPath)$(Platform)\$(Configuration)\</OutputPath>
	
		<IntermediateOutputPath Condition="'$(Platform)' == 'AnyCPU'">$(BaseIntermediateOutputPath)$(Configuration)\</IntermediateOutputPath>
		<IntermediateOutputPath Condition="'$(Platform)' != 'AnyCPU'">$(BaseIntermediateOutputPath)$(Platform)\$(Configuration)\</IntermediateOutputPath>
	
		<OutDir>$(OutputPath)</OutDir>
		<IntDir>$(IntermediateOutputPath)</IntDir>
	
		<GeneratedFilesDir>$(IntermediateOutputPath)Generated Files\</GeneratedFilesDir>
	</PropertyGroup>
</Project>

Feels a bit hacky.

AliveDevil avatar Mar 28 '24 12:03 AliveDevil

Btw. would moe projects be involved - you can still import the declarations, they just would need to be in an explicitly imported msbuild file - that way the implicit imports are processed before the explicit import

JanKrivanek avatar Apr 02 '24 10:04 JanKrivanek