Directory.Build.props should be imported before SDK props do anything

[dsplaisted]

Using a Directory.Build.props file is one way we've told people to set properties before the common props run (see dotnet/msbuild#1603). However, Directory.Build.props is currently evaluated after Microsoft.NET.Sdk.props, which means setting some properties such as the Configuration or Platform in Directory.Build.props leads to inconsistent results, as there are other default property values which are based on them in Microsoft.NET.Sdk.props.

A fix to this would be to duplicate the logic from MSBuild that imports Directory.Build.props in the SDK targets, and then set ImportDirectoryBuildProps to false so that the common props don't try to import it again.

This would be a breaking change for projects that rely (likely accidentally) on the current behavior, so we'd need to assess the compat risk of this change.

[eerhardt]

Isn't this an MSBuild issue? It is the one who imports the Sdk.props and then the Directory.Build.props. IMO, it should be consistent between all SDKs and Directory.Build.props.

/cc @rainersigwald @jeffkl @AndyGerlicher @cdmihai

[jeffkl]

Directory.Build.props is imported from within Microsoft.Common.props which is imported after the SDK.props

https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.props#L18

So in this case, its and SDK issue not an MSBuild issue. That is unless we want to change the SDK to import Directory.Build.props and have Microsoft.Common.props know if its already been imported. We do something similar with MicrosoftCommonPropsHasBeenImported

https://github.com/Microsoft/msbuild/blob/xplat/src/Tasks/Microsoft.Common.props#L133

https://github.com/Microsoft/msbuild/blob/fdbf2d358238e8236383e81acb353937161dae29/src/Tasks/Microsoft.Common.CurrentVersion.targets#L19

[eerhardt]

My point is: if we import Directory.Build.props in the Microsoft.NET.Sdk, it is going to be inconsistent with every other SDK, leading to confusion.

If we think Directory.Build.props should come before Microsoft.NET.Sdk's props, why shouldn't it come before ALL other Sdk's .props files?

[rainersigwald]

That's up to the Sdk. MSBuild's evaluation order constraints make this complicated--usually you want to set some stuff early and some stuff after common.props. We can't disconnect the directory.build.props import from common.props (it has to be somewhere). If it's not in the right place for Microsoft.NET.Sdk, there are options:

• Change the relative import order in this SDK (as Jeff pointed out):

  sdk/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.props

  Lines 18 to 19 in b76e22d

  	<Import Project="$(MSBuildThisFileDirectory)..\build\Microsoft.NET.Sdk.props" />
  	<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />

  . Probably infeasible due to other property settings.
• Use the escape hatch common.props provides to do the d.b.props import at a time of your own choosing.
• Define a different pattern and import that instead (please don't do this).

This is a limitation of the ordered-evaluation + import-whole-files + extensibility nature of MSBuild. We could change the order of imports but that only postpones the problem. We could shard into more files, but that pushes complexity into other layers (it'd be unpleasant if SDK.props had to import a dozen files from common in the right relative order, just to allow the possibility of avoiding this problem.

[rainersigwald]

I think it generally makes sense for an SDK to pull in d.b.props first and disable the common.props import. The idea of d.b.props is that it's imported early. In the (old) default template, the common.props import was the first line, and the d.b.props import is very early in common.props--so d.b.props in the old world is basically first.

If we wind up making the core SDK a package (dotnet/msbuild#1686), we could separate out the d.b.props import into its own file so it could be more easily called by an SDK. Then the pattern for sdks could be

  <Import Project="Directory.Build.props" Sdk="Core"  />
  <Import Project="$(MSBuildThisFileDirectory)..\build\Microsoft.NET.Sdk.props"  />
  <Import Project="Microsoft.Common.props" Sdk="Core" />

It's unfortunate that this would be a "standard pattern" rather than "done for you" but I don't see a way around it that keeps explicit, ordered imports.