MSBuildSdks icon indicating copy to clipboard operation
MSBuildSdks copied to clipboard

NoTargets sdk import discrepancy

Open dragosv opened this issue 2 years ago • 3 comments

Currently the Microsoft.NET.Sdk targets are imported if '$(CommonTargetsPath)' == '' while the props are when '$(MicrosoftCommonPropsHasBeenImported)' != 'true'. This is even more confusing given that CommonTargetsPath is an internal Microsoft.Common.targets property that gets set when Microsoft.Common.targets gets executed. Shouldn't also use the NoTargets Sdk.targets also use MicrosoftCommonPropsHasBeenImported?

dragosv avatar May 07 '23 00:05 dragosv

There are two ways users can declare NoTargets:

<Project Sdk="Microsoft.Build.NoTargets/1.0.0">
 ...
</Project>
<Project>
  <!-- Users can declare any property or import before Sdk.props -->

  <Import Project="Sdk.props" Sdk="Microsoft.Build.NoTargets" Version="1.0.0" />

  <!-- Users can declare any property or import after Sdk.props -->
 ...
  <!-- Users can declare any property or import before Sdk.targets -->

  <Import Project="Sdk.targets" Sdk="Microsoft.Build.NoTargets" Version="1.0.0" />

  <!-- Users can declare any property or import after Sdk.targets -->
</Project>

Given the second example, NoTargets is trying to detect if the user already imported Sdk.props from Microsoft.NET.Sdk or Microsoft.Common.props. The best way to detect that is the MicrosoftCommonPropsHasBeenImported property at the moment. We could possibly change it to '$(UsingMicrosoftNETSdk)' != 'true'. I'm open to changing it if it doesn't break any existing behavior.

For the Sdk.targets import, its essentially the same thing, a detection if the user already imported the common targets from the .NET SDK.

jeffkl avatar May 09 '23 17:05 jeffkl

Could we then have the same condition for both imports? The condition for importing the NetSdk targets is '$(CommonTargetsPath)' == '', while the props is based on MicrosoftCommonPropsHasBeenImported.

dragosv avatar May 09 '23 19:05 dragosv

Unfortunately, no. CommonTargetsPath is set by import Microsoft.Common.props and there's no single property that says you need to import the props at the top and the targets at the bottom. In the case where the user may have imported one or both, we need two different properties to know which to import for them. Is this causing problems for you in a scenario you can share?

jeffkl avatar May 11 '23 15:05 jeffkl