Guard ILCompilerTargetsPath import

[ViktorHofer]

Apparently, in some cases, msbuild fails when the Project attribute in an Import statement is empty.

from a binlog created by Jiri Cincura (during publish):
D:\repos\performance\tools\dotnet\x64\sdk\8.0.100-alpha.1.23073.15\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.targets(1189,3): error MSB4020: The value "" of the "Project" attribute in element <Import> is invalid. [D:\repos\performance\src\scenarios\emptyconsolenativeaot\app\emptyconsolenativeaot.csproj]

vs. when I tried something similar on my machine (during restore):
NoImport: $(ILCompilerTargetsPath) at (1184;3) empty expression

I think it's best to guard the Import via a Condition as the ILCompilerTargetsPath property isn't availble during Restore (which doesn't import nuget props and targets) and possibly during design time builds. Still, we should follow-up and understand why MSBuild sometimes accepts an empty expression vs when it errors out.

cc @rainersigwald

[rainersigwald]

The MSBuild evaluator has a permissive mode, used in restore--the idea is that maybe we can't import the thing because it hasn't been restored yet, but if we just ignore it hopefully we'll collect enough information about the packages to restore it and then import it correctly on the "real" evaluation for build. That can sometimes cause surprising behavior around incorrectly-specified imports, as here.

[ViktorHofer]

Based on Rainer's above response, a repro for this symptom would be a build that either doesn't invoke restore first or that explicitly sets NuGet's property to disable props and targets imports: /p:ExcludeRestorePackageImports=true.

[LakshanF]

Based on Rainer's above response, a repro for this symptom would be a build that either doesn't invoke restore first or that explicitly sets NuGet's property to disable props and targets imports: /p:ExcludeRestorePackageImports=true.

The guard seems reasonable and adding some context.

NativeAot assumes that the property, ILCompilerTargetsPath, is going to be always set either directly by explicit NativeAOT package reference or via the SDK. This is to ensure that the 2 (build and runtime) NativeAOT packages are version compatible. We recently started using implicit package reference to download the NativeAOT built package and I will open a tracking issue to double check all scenarios are covered in setting the ILCompilerTargetsPath property.

[LakshanF]

Opened issue #30094 to track this on the NativeAot side.

[ViktorHofer]

NativeAot assumes that the property, ILCompilerTargetsPath, is going to be always set either directly by explicit NativeAOT package reference or via the SDK.

AFAIK with the recent changes that switched the consumption to an implicit package reference, the SDK itself doesn't set the ILCompilerTargetsPath anymore. The "SDK" link above points to the package's Sdk.props file which isn't part of the SDK but of the package.

Because of that, the import in the Microsoft.Net.Sdk.targets fails whenever restore is broken or package imports aren't happening (because of ExcludeRestorePackageImports). A side question, probably unrelated to this discussion: Why does the Sdk import the targets file instead of just the nuget package via the usual naming convention given that we use an implicit package reference now?

[LakshanF]

AFAIK with the recent changes that switched the consumption to an implicit package reference, the SDK itself doesn't set the ILCompilerTargetsPath anymore. The "SDK" link above points to the package's Sdk.props file which isn't part of the SDK but of the package.

Because of that, the import in the Microsoft.Net.Sdk.targets fails whenever restore is broken or package imports aren't happening (because of ExcludeRestorePackageImports). A side question, probably unrelated to this discussion: Why does the Sdk import the targets file instead of just the nuget package via the usual naming convention given that we use an implicit package reference now?

Yes, we should do some cleanup after moving to implicit package reference including perhaps not needing 2 packages for NativeAOT now. I'll add the need to review post implicit work to the above issue.