MicrosoftCommonPropsHasBeenImported is set after importing Directory.Build.props

[andrew-boyarshin]

Steps to reproduce

Importing microsoft.build.centralpackageversions SDK (2.0.52) from the Directory.Build.props.

Directory.Build.props

<Project>
  <Import Project="Sdk.props" Sdk="Microsoft.Build.CentralPackageVersions" />
</Project>

Expected behavior

No warning is issued.

Actual behavior

C:\Users\Andrew\.nuget\packages\microsoft.build.centralpackageversions\2.0.52\Sdk\Sdk.props(14,3): warning MSB4011: "C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\Current\Microsoft.Common.props" cannot be imported again. It was already imported at "C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.props (40,3)". This is most likely a build authoring error. This subsequent import will be ignored. [D:\dev\SharpGenTools\SdkTests\Functions\Functions.csproj]

Reason

microsoft.build.centralpackageversions correctly checks for MicrosoftCommonPropsHasBeenImported, but it's not set yet. Microsoft.Common.props sets it after importing Directory.Build.props file.

Environment data

dotnet --info

.NET SDK (reflecting any global.json):
 Version:   5.0.100-preview.5.20253.6
 Commit:    7e7a355253

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19613
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\

Host (useful for support):
  Version: 5.0.0-preview.5.20253.6
  Commit:  6d40a8ff51

.NET SDKs installed:
  2.1.800 [C:\Program Files\dotnet\sdk]
  2.1.805 [C:\Program Files\dotnet\sdk]
  3.1.201 [C:\Program Files\dotnet\sdk]
  3.1.300-preview-015135 [C:\Program Files\dotnet\sdk]
  3.1.300-preview-015139 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.3.20170.4 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.5.20253.6 [C:\Program Files\dotnet\sdk]

MSBuild Binary Log

Process = "C:\Program Files\dotnet\dotnet.exe"
MSBuild executable path = "C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\MSBuild.dll"
Command line arguments = ""C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\MSBuild.dll" -maxcpucount -verbosity:m /t:GenerateSharpGenBindings /bl "-distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,C:\Program Files\dotnet\sdk\5.0.100-preview.5.20253.6\dotnet.dll""
Current directory = "D:\dev\SharpGenTools\SdkTests\Functions"
MSBuild version = "16.7.0-preview-20229-03+2cee6d020"

[andrew-boyarshin]

Cross-referencing my SharpGen branch.

[jeffkl]

I think the correct solution here is to only use it in Directory.Build.targets via the <Sdk /> element.

<Sdk Name="Microsoft.Build.CentralPackageVersions" Version="1.0.0" />

I can update the docs to make this more clear. I'm not sure how to detect that Sdk.props is being imported in Directory.Build.props so it might not be possible to tell the user something is wrong.

[andrew-boyarshin]

@jeffkl wouldn't it be better to set MicrosoftCommonPropsHasBeenImported before importing Directory.Build.props? This way, it enables detection of whether... Microsoft.Common.props has been imported. Fixing the docs doesn't solve the problem of MicrosoftCommonPropsHasBeenImported not being a reliable indicator that Microsoft.Common.props has been imported.

[andrew-boyarshin]

Also, because of the reason above, I believe the move from MSBuild repo is erroneous, since it's not specific to the specific SDK, but instead, it applies to any scenario trying to check from Directory.Build.props (or any descendant import) if Microsoft.Common.props has already been imported.

[jeffkl]

Yes the root of the problem is that there's no property set that tells Microsoft.Build.CentralPackageVersions that its been imported in Directory.Build.props. The MicrosoftCommonPropsHasBeenImported should probably be set earlier but that might be a breaking change. Perhaps we need to add a new property that is set at the very top of Microsoft.Common.props to prevent that?

[CZEMacLeod]

@jeffkl Ironically - I am looking for just such a property to indicate that Microsoft.Build.CentralPackageVersions has been imported in my SDK! CZEMacLeod/MSBuild.SDK.SystemWeb#26

[jeffkl]

I'll add a property, tracking in #320

[jeffkl]

Microsoft.Build.CentralPackageVersions must be imported in Directory.Build.targets so I'm going to make a change to log an error if its imported in Directory.Build.props.