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

Add props file to buildMultiTargeting in Microsoft.NET.Test.Sdk nuspec #580

Merged
merged 4 commits into from
Mar 15, 2017
Merged

Add props file to buildMultiTargeting in Microsoft.NET.Test.Sdk nuspec #580

merged 4 commits into from
Mar 15, 2017

Conversation

natemcmaster
Copy link
Contributor

By adding this property file to buildMultiTargeting, NuGet will import this as a top-level file in test projects with multiple target frameworks. Currently, the properties in from Microsoft.NET.Test.Sdk are only available during the inner build.

Example of the effect:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netcoreapp1.1;net46</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
  </ItemGroup>
  <Target Name="Peek">
    <Message Text="IsTestProject = $(IsTestProject)" Importance="high" />
  </Target>
</Project>

Before
Executing dotnet msbuild /t:Peek shows:

Microsoft (R) Build Engine version 15.1.548.43366
Copyright (C) Microsoft Corporation. All rights reserved.

  IsTestProject = 

After

Microsoft (R) Build Engine version 15.1.548.43366
Copyright (C) Microsoft Corporation. All rights reserved.

  IsTestProject = true

@msftclas
Copy link

msftclas commented Mar 9, 2017

@natemcmaster,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@natemcmaster
Copy link
Contributor Author

👍 I don't have push rights, so someone will have to merge this for me.

@@ -17,9 +17,10 @@
</dependencies>
</metadata>
<files>
<file src="Microsoft.NET.Test.Sdk.props" target="buildMultiTargeting\" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include the targets file as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to. NuGet will still import and execute the targets file on the "inner" build. It looks like the current targets file only generates a "Program.cs" file, and which depends on TargetFrameworkIdentifier being set. It probably will work incorrectly when running in the "outer" build.

Btw, if you're not familiar with inner vs outer, here is conceptually what MSBuild is doing in multi-targeting projects:

<!-- this executes when `msbuild.exe /t:Build` runs on the project -->
<Target Name="Build" Condition="'$(TargetFrameworks)' != '' AND '$(TargetFramework)' == ''">
    <!-- import buildMultiTargeting/Microsoft.NET.Test.Sdk.props  -->
    <ItemGroup>
      <_TargetFramework Include="$(TargetFrameworks)" />
    </ItemGroup>
    <!-- starts a new "build" of the project, once per target framework -->
    <MSBuild Projects="$(MSBuildProjectFile)"
             Targets="InnerBuild"
             Properties="TargetFramework=%(_TargetFramework.Identity)" />
</Target>

<Target Name="InnerBuild" Condition="'$(TargetFramework)' != ''">
   <!-- import build/$(TargetFramework)/Microsoft.NET.Test.Sdk.{props, targets} -->
   <!-- gather *.cs files, run compiler, etc. -->
</Target>

@harshjain2 harshjain2 merged commit 5ec39c8 into microsoft:master Mar 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants