Guard NuGetRestoreTargets import in Microsoft.Common.CrossTargeting.targets

[dfederm]

Microsoft.Common.CurrentVersion.targets guards its NuGet restore targets import with Exists(''$(NuGetRestoreTargets)'') (and the cooperative ''$(IsRestoreTargetsFileLoaded)'' != ''true'' check added in #4969), but the parallel import in Microsoft.Common.CrossTargeting.targets was unguarded:

File	Line	Import
Microsoft.Common.CurrentVersion.targets	7046	<Import Condition="''$(IsRestoreTargetsFileLoaded)'' != ''true'' and Exists(''$(NuGetRestoreTargets)'')" Project="$(NuGetRestoreTargets)" />
Microsoft.Common.CrossTargeting.targets	204 (before)	<Import Project="$(NuGetRestoreTargets)" />

Customer impact

Cross-targeting (multi-TFM) projects loaded in non-NuGet contexts -- focused unit tests, custom build systems, stripped-down project types -- failed with Imported project not found unless the caller manufactured a stub NuGet.targets file. Single-targeting projects in the same situation worked silently.

Discovery

While writing tests for #13427 (ProjectReferenceTargetsProtocol_Tests.cs), the synthetic cross-targeting test had to inject an empty NuGet.targets stub via TransientTestFile and pass it as a global NuGetRestoreTargets property, purely to satisfy this unguarded import. The author flagged the asymmetry as a separate follow-up:

Microsoft.Common.CrossTargeting.targets:230 has an unconditional <Import Project="$(NuGetRestoreTargets)" /> -- no Condition="...Exists(...)" guard, unlike the parallel import in Microsoft.Common.CurrentVersion.targets:7102. [...] This is arguably a bug in CrossTargeting.targets worth a separate follow-up issue/PR -- adding the Exists() condition would make CrossTargeting.targets safer to import in non-NuGet test contexts.

This PR is that follow-up.

Change

One line in Microsoft.Common.CrossTargeting.targets, copying the canonical condition byte-for-byte from Microsoft.Common.CurrentVersion.targets:7046 (same attribute order, lowercase and, single-quotes, identical spacing):

- <Import Project="$(NuGetRestoreTargets)" />
+ <Import Condition="''$(IsRestoreTargetsFileLoaded)'' != ''true'' and Exists(''$(NuGetRestoreTargets)'')" Project="$(NuGetRestoreTargets)" />

Tests

New src/Tasks.UnitTests/CrossTargetingTargetsImportTests.cs with two [Fact]s:

1. ImportSucceedsWhenNuGetRestoreTargetsDoesNotExist -- synthetic cross-targeting project with NuGetRestoreTargets pointed at a non-existent file evaluates successfully. Fails on the pre-fix targets file with InvalidProjectFileException: The imported project "..." was not found; passes post-fix. Empirically verified by reverting the targets change, rebuilding, and re-running -- output captured to scratch.
2. ImportLoadsNuGetTargetsWhenItExists -- forward-looking regression guard: a real stub NuGet.targets with a marker target is still imported under the new guard. Passes both pre- and post-fix.

Both run on net10.0 and net472.

Backwards compatibility

Behavior change: when NuGetRestoreTargets resolves to a missing file, cross-targeting projects now evaluate successfully (the NuGet import is a no-op) instead of throwing InvalidProjectFileException. Projects that already set NuGetRestoreTargets to a real file are unaffected -- the import still happens.

No ChangeWave -- the canonical fix in CurrentVersion.targets was added in #4969 (2020) without one, and this change just closes the asymmetry rather than introducing a new policy. The new behavior is strictly looser; no consumer can reasonably depend on the prior error path.

Notes

• Move core ProjectReferenceTargets from Managed-only to Common targets #13427 (concurrent PR) injects a NuGet.targets stub purely to satisfy the unguarded import this PR fixes. Post-merge of both, that stub becomes dead code; cleanup is a separate follow-up PR to keep this one focused.
• Other unguarded $(NuGetRestoreTargets) imports across the repo: searched -- only the two known parallel spots in Microsoft.Common.{CurrentVersion,CrossTargeting}.targets. Nothing else to clean up.

cc @rainersigwald (approved #13427; knows the area).

[Copilot]

Pull request overview

This PR fixes an asymmetry between Microsoft.Common.CurrentVersion.targets and Microsoft.Common.CrossTargeting.targets by guarding the cross-targeting import of $(NuGetRestoreTargets) with the same Exists(...) + IsRestoreTargetsFileLoaded condition used in the single-targeting targets. This makes multi-targeting project evaluation succeed in non-NuGet contexts when NuGetRestoreTargets points to a missing file, aligning behavior across the two targets entry points.

Changes:

• Guard $(NuGetRestoreTargets) import in Microsoft.Common.CrossTargeting.targets with Exists('$(NuGetRestoreTargets)') and $(IsRestoreTargetsFileLoaded) != 'true'.
• Add unit tests covering both “missing NuGetRestoreTargets file should not fail evaluation” and “existing NuGetRestoreTargets file is still imported”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/Tasks/Microsoft.Common.CrossTargeting.targets	Adds the same conditional guard used in CurrentVersion.targets to prevent failures when $(NuGetRestoreTargets) is absent.
src/Tasks.UnitTests/CrossTargetingTargetsImportTests.cs	Adds regression tests to ensure evaluation succeeds when the restore targets file is missing and still imports when present.

[rainersigwald]

I am tempted to say "if there's any situation where a project tries to multitarget and doesn't have NuGet available it's broken" and not fix this just for the unit test. Does that ring false to you @dfederm?

[dfederm]

I am tempted to say "if there's any situation where a project tries to multitarget and doesn't have NuGet available it's broken" and not fix this just for the unit test. Does that ring false to you @dfederm?

I can't argue with that. This was mostly just about an observed inconsistency b/t this and Microsoft.Common.CurrentVersion.targets and to a lesser extent to make UTs easier to write (less to stub). But admittedly those aren't super strong arguments.

I defer to your judgement so feel free to close the PR if you think it should just be left as-is.