sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Add BeforeMicrosoftNETSdkTargets

Open js6pak opened this issue 1 year ago • 4 comments

Add a hook similar to AfterMicrosoftNETSdkTargets, BeforeTargetFrameworkInferenceTargets, CustomBeforeMicrosoftCommonCrossTargetingTargets, etc.

This provides a unified entry point for custom msbuild SDKs and msbuild NuGet packages to execute just after the project file regardless of whether the build is cross targeting or not.

My main use case for this is a custom msbuild SDK that sets the TargetFrameworks based on a property set in the project file.

js6pak avatar Apr 21 '24 19:04 js6pak

My main use case for this is a custom msbuild SDK that sets the TargetFrameworks based on a property set in the project file.

We use BeforeTargetFrameworkInferenceTargets in dotnet/runtime for that. Why can't you use that instead?

ViktorHofer avatar Apr 22 '24 10:04 ViktorHofer

We use BeforeTargetFrameworkInferenceTargets in dotnet/runtime for that. Why can't you use that instead?

Because at that point the decision of whether the build is cross targeting or not is already made (actually it's made at the very beginning of Microsoft.NET.Sdk's Sdk.targets) so you can only set TargetFramework not TargetFrameworks.

js6pak avatar Apr 22 '24 15:04 js6pak

@baronfel we had a discussion about adding additional hook points but I'm not sure where that discussion was. Thoughts?

marcpopMSFT avatar Apr 23 '24 20:04 marcpopMSFT

@baronfel

js6pak avatar Jun 30 '24 12:06 js6pak

This looks generally ok, do we doc the AfterX target anywhere that we'd need to update now that this hook is available?

baronfel avatar Jul 18 '24 00:07 baronfel

Looking at the existing AfterSdk... targets, there are only ~150 repos on GitHub that use this mechanism, and the vast majority of those are clones of MS-authored repos like the SDK, xamarin, etc - this means that the scope of adoption is probably quite low and we won't need to worry about this going viral.

baronfel avatar Jul 18 '24 00:07 baronfel