Need a way to run targets before / after sdk.targets

[jaredpar]

Roslyn is in the process of attempting to move to the new project file syntax + use directory build props / targets files. This has been a very laborious process because rather than be a syntactic shift in how build files are defined, it's actually a rather big semantic shift as well. This is due to rather large changes in when items get executed.

In the old world our project files were defined as such:

<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="..\..\..\..\build\Targets\Settings.props" />
  .... 
  <Import Project="..\..\..\..\build\Targets\Imports.targets" />
</Project>

This structure is not particularly specific to Roslyn. It is used in many non-trivial solutions that need to support global configuration and targets.

The key feature of this structure is that it gives us complete control over when our global files get executed. In particular it allows us to control build elements before and after the standard props and targets are imported. Note: that is four locations, not two: before props, after props, before targets, after targets. That flexibility is useful in constructing custom build logic and allowing builds to work around bugs in the core targets.

This level of flexibility is simply not present in the new project file syntax + dir props files. The exact points which could be hooked before simply cannot be anymore. In particular it's not possible to properly hook after common targets (which is where we typically work around bugs in the build).

There are rough equivalent hooks for before / after common props. But there are no equivalent hooks for before / after targets. This leads to blocking issues in Roslyn adopting the new format. This is because we relied on after targets to work around bugs / limitation in the SDK. Lacking an equivalent hook we can't work around the bugs anymore and are blocked.

I will continue to push on the bugs. But at the same time it seems we should address the more fundamental issue here. The new system needs to have equivalent hooking points else other builds are likely to be blocked from migrating as well. This should be a simple enough change, just need to add the following to the top / bottom of Sdk.targets

<Import Project="$(CustomBeforeSdkTargets)" Condition="'$(CustomBeforeSdkTargets)' != ''" />
...
<Import Project="$(CustomAfterSdkTargets)" Condition="'$(CustomAfterSdkTargets)' != ''" />

[dasMulli]

I've been using the explicit SDK imports to work around issues and make use of properties defined by the targets import. Could this already solve your issues?

E.g. this one before mono included NuGet's pack targets or to use preview builds of them

<Project>
  <PropertyGroup>
    <NuGetBuildTasksPackTargets>junk-value-to-avoid-conflicts</NuGetBuildTasksPackTargets>
  </PropertyGroup>
  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />

  <!-- Other content -->

  <ItemGroup>
    <PackageReference Include="NuGet.Build.Tasks.Pack" Version="4.0.0" PrivateAssets="All" />
  </ItemGroup>
  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
  <!-- custom imports using $(TargetFrameworkIdentifier) and $(TargetFrameworkVersion) here -->
</Project>

[jaredpar]

@dasMulli

I've been using the explicit SDK imports to work around issues and make use of properties defined by the targets import. Could this already solve your issues?

The new syntax though makes the Sdk.targets import implicit. There is no way, at least that I'm aware of, to make it explicit.

[nguerrera]

@jaredpar, I'm not sure what you mean. The import of Sdk.targets in @dasMulli's example is explicit. It happens at exactly the point where <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" /> occurs. You have 100% control of the import when you expand the <Project Sdk... form to <Import Sdk....

[nguerrera]

By "new syntax", do you mean <Project Sdk="Microsoft.NET.Sdk"> exclusively? It is confusing because both <Project Sdk... and <Import Sdk... were introduced to msbuild at the same time. The former is sugar for the latter at top and bottom of Project, but once you desugar, you can put them where you like.

[nguerrera]

With that said, I'm not opposed to the proposal here in support of the terser synax for more scenarios.

[nguerrera]

If we do as proposed, I suggest $(CustomAfterMicrosoftNETSdkTargets) matching the full names used in Microsoft.Common.targets and friends. The problem with $(CustomAfterSdkTargets) is that there is more than one MSBuild Sdk and they all have a Sdk.targets. If we end up needing hooks on all them, then the variable will conflict.

[jaredpar]

@nguerrera

I'm not sure what you mean. The import of Sdk.targets in @dasMulli's example is explicit. It happens at exactly the point where

This sample isn't using the <Project Sdk="Microsoft.NET.Sdk"> format which is what we were trying to move to.

The former is sugar for the latter at top and bottom of Project, but once you desugar, you can put them where you like.

Using explicit <Import ... /> in every project though defeats the purpose of the Directory.Build.props / targets work.

[jaredpar]

@nguerrera

If we do as proposed, I suggest $(CustomAfterMicrosoftNETSdkTargets) matching the full names used in Microsoft.Common.targets and friends.

makes sense.

[jeffkl]

I really want $(CustomBeforeMicrosoftNETSdkProps) and $(CustomAfterMicrosoftNETSdkProps) as well. Some properties are set by the SDK but after Directory.Build.props so its hard to centralize logic in some cases. If I could get Sdk.props to import something just before the project is evaluated, that would be amazing.

Would a pull request be accepted to add these extension points?

[baronfel]

I think this is a reasonable request - @dsplaisted @marcpopMSFT do you see anything horrible about adding more extensibility points? Of course we'd need to document them on the existing .NET SDK extensibility docs.

It sounds like between @jaredpar and @jeffkl we have 4 proposed new extensibility points:

• CustomBeforeMicrosoftNETSdkProps
• CustomAfterMicrosoftNETSdkProps
• CustomBeforeMicrosoftNETSdkTargets
• CustomAfterMicrosoftNETSdkTargets

As a general rule, we may want to have a convention-based Import for specific SDK-name-based Imports for all SDKs, but that's could also be a next-step. We now have Jared, Jeff, and an internal customer that would like this set of extensibility points.

[baronfel]

@dsplaisted had the following feedback:

• in general the idea of more extension points is ok, but specifically if we go with a strict interpretation of
  • Before Sdk.props means before any props from a given SDK
  • After Sdk.props means after every props from a given SDK
  • and so on for before/after targets

So we need more investigation to make sure that these specific proposed extensibility points actually solve Jared/Jeffs needs. As a counterexample, if we have strict before/after for Sdk.props then Jeff wouldn't be able to set things in Directory.Build.props that would actually impact 'before Sdk.props'.