Pass ExcludeRestorePackageImports during restore to avoid redundant evaluations

[Copilot]

Context

During /restore (implicit or /t:restore), every project is evaluated twice. NuGet's restore targets re-invoke each project with the global property ExcludeRestorePackageImports=true (NuGet.targets), which MSBuild does not include in its initial restore evaluation. The differing global property set produces a distinct build configuration, forcing a redundant re-evaluation. Static graph restore already avoids this by passing the property itself.

Changes Made

• XMake.ExecuteRestore: adds ExcludeRestorePackageImports=true to the restore global properties so NuGet's inner MSBuild call matches the initial evaluation and reuses it. Gated behind change wave 18.10.
• Value casing is load-bearing: config matching compares global property values case-sensitively (ProjectPropertyInstance.Equals), and the MSBuild task inherits parent globals then overlays its own. NuGet passes the literal lowercase true, so we must too — bool.TrueString ("True") would silently fail to dedupe the evaluation.
• MSBuildConstants: new ExcludeRestorePackageImports and ExcludeRestorePackageImportsValue ("true") constants.
• ChangeWaves.md: documented under 18.10.
• Test: RestoreSetsExcludeRestorePackageImports asserts the property is set during restore.

if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_9))
{
    restoreGlobalProperties[MSBuildConstants.ExcludeRestorePackageImports] = MSBuildConstants.ExcludeRestorePackageImportsValue; // "true"
}

Testing

Evaluations counted via ProjectEvaluationStarted events in a binlog, using a repro that mimics NuGet re-invoking each project with ExcludeRestorePackageImports=true:

Scenario	Before (18.10 off)	After (18.10 on)
/t:Restore (restore phase only)	2 evals/project	1 eval/project
/restore /t:Build (full)	3 evals/project	2 evals/project

All 13 existing *Restore* XMake_Tests pass alongside the new test.

Notes

• Wall-clock on a trivial single-project repro is unchanged (~0.56s, dominated by process startup); savings scale with project count and per-project evaluation cost in real solutions.
• Setting the property during restore matches what NuGet already forces in its re-invocation (package imports are excluded during restore anyway), so behavior should be unchanged. The change wave gate provides a per-repo opt-out given this touches restore evaluation.

[ViktorHofer]

@jeffkl & @nkolev92 would you mind taking a look at this change? Thanks in advance. EDIT: Happy to react to feedback in a follow-up change.

[Copilot]

Pull request overview

This PR aims to reduce redundant project evaluations during /restore by ensuring MSBuild’s initial restore evaluation uses the same global property (ExcludeRestorePackageImports=true) that NuGet uses when it re-invokes MSBuild, allowing evaluation reuse.

Changes:

• Sets ExcludeRestorePackageImports=true in XMake.ExecuteRestore restore global properties (wave-gated).
• Introduces MSBuildConstants.ExcludeRestorePackageImports and a load-bearing lowercase "true" constant value.
• Adds a unit test asserting the property is set during restore and updates ChangeWaves documentation/rotation.

Show a summary per file

File	Description
src/MSBuild/XMake.cs	Adds ExcludeRestorePackageImports=true to restore global properties behind a change wave gate.
src/MSBuild.UnitTests/XMake_Tests.cs	Adds a test validating ExcludeRestorePackageImports is set during /t:restore.
src/Framework/MSBuildConstants.cs	Adds constants for the property name and required lowercase value.
src/Framework/ChangeWaves.cs	Adds Wave18_10 and updates AllWaves.
documentation/wiki/ChangeWaves.md	Documents the change wave rotation and the new feature entry.

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 2

[ViktorHofer]

/review

[github-actions]

✅ Expert Code Review (command) completed successfully!

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

The threat detection results could not be parsed.

Review the workflow run logs for details.

Evaluating PR #14274 for Performance & Allocation Awareness only.

[github-actions]

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

The threat detection results could not be parsed.

Review the workflow run logs for details.

Review — PR #14274: Pass ExcludeRestorePackageImports during restore

The optimization goal is sound and the implementation is mostly clean. Two actionable issues need fixing before merge.

#	Dimension	Verdict
1	Backwards Compatibility	🔴 MAJOR
2	ChangeWave Discipline	🟡 MAJOR
4	Test Coverage	🟡 MAJOR
22	Correctness & Edge Cases	🔴 MAJOR

✅ 22/24 dimensions clean.

---

Issues to address

• src/MSBuild/XMake.cs line 2074 — The unconditional dictionary assignment overwrites an explicit user-supplied /p:ExcludeRestorePackageImports=false. Guard the write with !globalProperties.ContainsKey(...) so the optimization is only applied when the user has not already provided a value. (Full recommendation in the inline comment.)

• src/MSBuild.UnitTests/XMake_Tests.cs — The ChangeWave-disabled path is not tested. A companion test RestoreDoesNotSetExcludeRestorePackageImportsWhenWaveDisabled is needed, setting MSBUILDDISABLEFEATURESFROMVERSION=18.10 and asserting the property is absent from the restore evaluation. (Full snippet in the inline comment.)

---

Everything else is clean: the "true" casing rationale is correctly documented, Wave18_10 is correctly added to AllWaves, the ChangeWaves.md entry is accurate, constants are internal, there are no concurrency issues, no cross-platform concerns, no API surface changes, and the implementation placement in ExecuteRestore is consistent with the existing MSBuildIsRestoring/MSBuildRestoreSessionId pattern.

Generated by Expert Code Review (command) for #14274 · 1.4K AIC · ⊞ 30.1K
_{Comment /review to run again}

[ViktorHofer]

@copilot address the pr feedback