Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.Build.Traversal does not respect SetPlatform or AdditionalProperties on ProjectReference items #207

Closed
forrestcoward opened this issue Oct 7, 2020 · 6 comments
Labels
Feature Request New feature or request

Comments

@forrestcoward
Copy link
Contributor

The Microsoft.Build.Traversal SDK does not respect SetPlatform, SetConfiguration or AdditionalProperties on ProjectReference items, it ignores them, effectively losing the metadata. It is customary for global properties to propagate along ProjectReference.

Here is the location I am looking at that is offending:
https://github.com/microsoft/MSBuildSdks/blob/master/src/Traversal/Sdk/Sdk.targets#L123-L136

  <Target Name="Build"
          DependsOnTargets="$(BuildDependsOn)"
          Condition=" '$(IsGraphBuild)' != 'true' "
          Returns="@(CollectedBuildOutput)">
    <MSBuild Projects="@(ProjectReference)"
             Condition="'%(ProjectReference.Build)' != 'false'"
             BuildInParallel="$([MSBuild]::ValueOrDefault('%(ProjectReference.BuildInParallel)', '$(BuildInParallel)'))"
             SkipNonexistentProjects="$(SkipNonexistentProjects)"
             SkipNonexistentTargets="$(SkipNonexistentTargets)"
             StopOnFirstFailure="$(StopOnFirstFailure)"
             ContinueOnError="$(ContinueOnError)">
      <Output TaskParameter="TargetOutputs" ItemName="CollectedBuildOutput" />
    </MSBuild>
  </Target>

The solution is something like this:

  <Target Name="Build"
          DependsOnTargets="$(BuildDependsOn)"
          Condition=" '$(IsGraphBuild)' != 'true' "
          Returns="@(CollectedBuildOutput)">
    <MSBuild Projects="@(ProjectReference)"
             Condition="'%(ProjectReference.Build)' != 'false'"
             BuildInParallel="$([MSBuild]::ValueOrDefault('%(ProjectReference.BuildInParallel)', '$(BuildInParallel)'))"
             Properties="%(ProjectReference.SetConfiguration); %(ProjectReference.SetPlatform); %(ProjectReference.AdditionalProperties)"
             SkipNonexistentProjects="$(SkipNonexistentProjects)"
             SkipNonexistentTargets="$(SkipNonexistentTargets)"
             StopOnFirstFailure="$(StopOnFirstFailure)"
             ContinueOnError="$(ContinueOnError)">
      <Output TaskParameter="TargetOutputs" ItemName="CollectedBuildOutput" />
    </MSBuild>
  </Target>

Note the addition of Properties attribute.

Why is this needed? Anybody setting properties on their ProjectReference lose them when using the traversal SDK. In my case, these properties are valuable.

@forrestcoward
Copy link
Contributor Author

If this seems reasonable, I can just PR it in.

@jeffkl
Copy link
Contributor

jeffkl commented Oct 7, 2020

Sounds like a good change, there's another property SetTargetFramework

https://github.com/dotnet/msbuild/blob/master/src/Tasks/Microsoft.Common.CurrentVersion.targets#L1855

But I think AdditionalProperties does get passed through.

https://github.com/dotnet/msbuild/blob/master/src/Tasks/MSBuild.cs#L543-L553

@jeffkl jeffkl added the Feature Request New feature or request label Oct 7, 2020
@forrestcoward
Copy link
Contributor Author

You're right, it looks like AdditionalProperties is getting passed through! I just verified. So it seems we just need SetTargetFramework, SetPlatform, and SetConfiguration?

@jeffkl
Copy link
Contributor

jeffkl commented Oct 7, 2020

Yes just those 3 properties to mirror the logic used in ResolveProjectReferences. Please send a PR!

@tmds
Copy link

tmds commented Nov 26, 2020

@forrestcoward @jeffkl how do I pass variables like Configuration downstream?

I tried setting Configuration to Release like this:

<Project Sdk="Microsoft.Build.Traversal/3.0.2">
	<PropertyGroup>
		<Configuration>Release</Configuration>
	</PropertyGroup>
	<ItemGroup>
		<ProjectReference Include="console/console.csproj"/>
	</ItemGroup>
</Project>

but it doesn't get applied to the ProjectReference:

$ dotnet build
Microsoft (R) Build Engine version 16.8.0-preview-20414-02+a55ce4fbb for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
  console -> /tmp/root/console/bin/Debug/net5.0/console.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.33

@jeffkl
Copy link
Contributor

jeffkl commented Nov 30, 2020

@tmds you'll need to add the metadata to the ProjectReference to opt into this behavior:

<ItemGroup>
    <ProjectReference Include="console/console.csproj" SetConfiguration="Configuration=$(Configuration)"/>
</ItemGroup>

Or you can set it as a global property:

msbuild /Property:Configuration=Release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants