Prevent built-in artifacts output functionality from conflicting with Microsoft.Build.Artifacts SDK

[dsplaisted]

Fixes #33052

[joeloff]

Looks good. Should we expect to generalize the logic for other SDKs in the future?

[jeffkl]

When a user references Microsoft.Build.Artifacts as a package, its .props is imported by .nuget.g.props which is imported after this. So I don't think UsingMicrosoftArtifactsSdk will be set yet since this is imported immediately after Directory.Build.props.

[dsplaisted]

This is only imported early on if UseArtifactsOutput or ArtifactsPath are set in Directory.Build.props:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.props

Lines 64 to 65 in 9d157dc

	<Import Project="$(MSBuildThisFileDirectory)..\targets\Microsoft.NET.DefaultArtifactsPath.props"
	Condition="'$(UseArtifactsOutput)' == 'true' Or '$(ArtifactsPath)' != ''"/>

If they are set later on (ie in the project file), then this file will be imported later, during the .targets evaluation: https://github.com/dotnet/sdk/blob/main/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.DefaultOutputPaths.targets#L42-L43

So if the Microsoft.Build.Artifacts NuGet package is being used and ArtifactsPath is set in Directory.Build.props, then the built-in SDK artifacts functionality will be used. If ArtifactsPath is set in the project file (as seems to be suggested in the Microsoft.Build.Artifacts documentation), then the Microsoft.Build.Artifacts logic will be used.

Does this sound OK? Do you know if anyone using Microsoft.Build.Artifacts is setting ArtifactsPath in Directory.Build.props? IE, something like:

<Project>
  <PropertyGroup>
    <BaseArtifactsPath>$(MSBuildThisFileDirectory)artifacts</BaseArtifactsPath>
    <ArtifactsPath>$(BaseArtifactsPath)\$(MSBuildProjectName)</ArtifactsPath>
  </PropertyGroup>
</Project>

[dsplaisted]

@jeffkl I went ahead and merged this because we wanted to get the fix in before the .NET 8 Preview 6 branch, Let me know what you think about this and we can follow up with another change if need be.

[jeffkl]

I've also created a pull request to disable Microsoft.Build.Artifacts if the .NET SDK's built-in functionality is enabled: microsoft/MSBuildSdks#458

[dsplaisted]

Looks good. Should we expect to generalize the logic for other SDKs in the future?

No, this is because we added a specific property that was already in use by this specific SDK.