Directory.build.targets not imported when cross-targeting

[tmat]

Microsoft.Common.CrossTargeting.targets doesn't import these. As a result properties and tasks defined in Directory.build.props, Directory.build.targets are not available in the outer build.

Repro:

1. Create a .NET Core Library with project:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netcoreapp1.0;net46</TargetFrameworks>
  </PropertyGroup>
</Project>

2. Add Directory.Build.targets next to the .csproj file:

<Project>  
  <Target Name="MyTarget">
    <Error Text="Boom!"/>
  </Target>
</Project>

3. Run

> msbuild /t:Restore ClassLibrary.csproj
...
> msbuild /t:MyTarget ClassLibrary.csproj
Project "ClassLibrary.csproj" on node 1 (MyTarget target(s)).
ClassLibrary.csproj : error MSB4057: The target "MyTarget" does not exist in the project.
Done Building Project "ClassLibrary.csproj" (MyTarget target(s)) -- FAILED.

If the project targets a single framework like so:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netcoreapp1.0</TargetFramework>
  </PropertyGroup>
</Project>

The target is invoked as expected:

Project "ClassLibrary.csproj" on node 1 (MyTarget target(s)).
Directory.Build.targets(7,5): error : Boom!

[tmat]

@srivatsn FYI

[jeffkl]

Nevermind I repro'd this incorrectly. You are correct, the outer build does not import Microsoft.Common.targets. @nguerrera is there a reason that Microsoft.Common.CrossTargeting.targets does not import Microsoft.Common.targets? I see it imports project extensions but since it probably should be importing directory targets like this: https://github.com/Microsoft/msbuild/blob/xplat/src/Tasks/Microsoft.Common.targets#L137

[nguerrera]

We don't import common targets by design because there are a lot of assumptions in it that don't hold for outer build. It is not really possible to give meaningful semantics to every target in common targets in the outer context.

That said, not importing Directory.* is an oversight. We may want to introduce Directory.CrossTargeting.targets so that we can support the scenario in a non breaking way. This would be consistent with nuget model where separate files are used to augment inner and outer build.

[tmat]

Do we really need a separate file? The detection whether the current build is inner or outer is simple, if anyone needs to distinguish in the file.

[jeffkl]

I agree with @tmat, we should just import Directory.Build.targets. Directory.Build.props is already being imported by the outer build so I think it would be confusing that there needs to be a separate targets file. The IsCrossTargetingBuild property gives target authors enough control over what they wanted declared.

CC @AndyGerlicher

[nguerrera]

If we use only one file, this would be a breaking change so we need to decide that we're prepared to make it or push to get it in right now.

I'm not near a dev setup right now, but I would think that Directory.Build.props is already imported. If so, this can be used to hack a workaround: Set CustomAfterMicrosoftCommonCrossTargetingTargets to Directory.Build.targets in Directory.Build.props. However, those CustomXxx points are perilous, because they suffer from the "What if two components do this?" problem.

[jeffkl]

Directory.Build.props is imported at the moment which is why I think it makes sense that Directory.Build.targets should be too. Can you explain why this would be a breaking change to import the targets file? Since the functionality is new to MSBuild 15, I assume it would be safe.

[nguerrera]

I mean it's breaking if we ship MSBuild 15 like it is now and then start importing it in MSBuild 16.

[nguerrera]

Consider a common case: AfterTargets="Build", this would run only in inner build today, but start to run in both after change.

[jeffkl]

Ah I see, I misunderstood. So you're saying if we don't fix it for RTM, then we'll need a new file for future releases....

[nguerrera]

Or maybe honor a safe opt-in from Directory.Build.props: <ImportDirectoryTargetsForCrossTargeting>true</ImportDirectoryTargetsForCrossTargeting>

Or have the courage and permission to break.

[nguerrera]

We should at least talk about it (bar check it as potentially now-or-never) in ship room. Just fixing it will simplify things for the long haul.

[tmat]

I did more experiments and it seems like Directory.build.props is imported but Directory.build.targets isn't. Odd.

[jeffkl]

We're going to bar check this with management to see what they think. The change is a copy/paste from one file to another so the risk is very low but it might be too late.

[AndyGerlicher]

FYI this will have to be in a future update. It won't be in VS2017 RTW.

[JunielKatarn]

Hi. What's the state of this issue?
Will this Directory.Build.targets be imported in some future release?

[rainersigwald]

@JunielKatarn looks like the changes from #1722 are available in our latest packages (I checked Microsoft.Build.Runtime 15.2.0-preview-000103-02, but not yet pulled into the CLI or VS builds. I believe this will go out with the 15.3 update.

[JunielKatarn]

@rainersigwald thanks!
However, I somehow made it work.
I can confirm that as of dotnet version 1.0.1, I am able to implicitly import Directory.Build.targets.

I think this use case is worth a short sample in the official docs. Is there a way to contribute to them?