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

NoTargets sdk import discrepancy #443

Open
dragosv opened this issue May 7, 2023 · 3 comments
Open

NoTargets sdk import discrepancy #443

dragosv opened this issue May 7, 2023 · 3 comments

Comments

@dragosv
Copy link

dragosv commented May 7, 2023

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?

@jeffkl
Copy link
Contributor

jeffkl commented May 9, 2023

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.

@dragosv
Copy link
Author

dragosv commented May 9, 2023

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.

@jeffkl
Copy link
Contributor

jeffkl commented May 11, 2023

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?

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

No branches or pull requests

2 participants