Skip to content

Revert passing ExcludeRestorePackageImports during restore (#14274) - #14349

Merged
ViktorHofer merged 2 commits into
mainfrom
viktorhofer/revert-exclude-restore-package-imports
Jul 14, 2026
Merged

Revert passing ExcludeRestorePackageImports during restore (#14274)#14349
ViktorHofer merged 2 commits into
mainfrom
viktorhofer/revert-exclude-restore-package-imports

Conversation

@ViktorHofer

Copy link
Copy Markdown
Member

Reverts the change from #14274, which injected ExcludeRestorePackageImports=true into the implicit restore global properties (behind change wave 18.10) so that NuGet's restore re-invocation would reuse MSBuild's initial evaluation instead of forcing a second one.

Why revert

The change breaks restore for projects that have a nonexistent <ProjectReference>. Once the top-level restore evaluation shares the same global property set that NuGet's inner _GenerateRestoreProjectPathWalk invocation uses, that walk runs in the same project instance and its unfiltered _RestoreProjectPathItems (which contains the raw ProjectReference paths, including missing ones) leaks into _GenerateRestoreGraph's _GenerateRestoreGraphProjectEntry MSBuild call. That call does not set SkipNonexistentProjects, so it fails with MSB3202 instead of skipping the missing project.

NuGet.targets(571,5): error MSB3202: The project file "...\TestLibrary\TestLibrary.csproj" was not found.

This regressed the dotnet/sdk test ItCanTestAMultiTFMProjectWithImplicitRestore — see dotnet/sdk#55245 for the full root-cause analysis and repro.

Details

  • Only the implicit-restore (-restore switch / ExecuteRestore) path was affected; explicit dotnet restore / -t:Restore never added the property.
  • Reproduced with dotnet build -t:Restore /p:ExcludeRestorePackageImports=true on a multi-TFM project with a dangling ProjectReference: without the property restore succeeds (missing project skipped gracefully); with it, MSB3202.

What this PR does

  • Removes the ExcludeRestorePackageImports=true injection in XMake.ExecuteRestore, replacing it with a comment documenting why the property must not be set there.
  • Removes the ExcludeRestorePackageImports / ExcludeRestorePackageImportsValue constants and the two related unit tests.
  • Removes the 18.10 ChangeWaves.md bullet for this feature.
  • Retains change wave 18.10 itself — it is still used by the -getProperty/-getItem evaluation change (Add opt-in partial (stop-after-pass) project evaluation #14290).

PR #14274 injected ExcludeRestorePackageImports=true into the implicit
restore global properties (behind change wave 18.10) so that NuGet's
restore re-invocation reused MSBuild's initial evaluation instead of
forcing a second one.

This breaks restore for projects that have a nonexistent
<ProjectReference>. Once the top-level restore evaluation shares the
same global property set that NuGet's inner _GenerateRestoreProjectPathWalk
invocation uses, the walk runs in the same project instance and its
unfiltered _RestoreProjectPathItems (raw ProjectReference paths,
including missing ones) leaks into _GenerateRestoreGraph's
_GenerateRestoreGraphProjectEntry MSBuild call, which does not set
SkipNonexistentProjects and fails with MSB3202 instead of skipping the
missing project.

This regressed the dotnet/sdk test
ItCanTestAMultiTFMProjectWithImplicitRestore (dotnet/sdk#55245).

Revert the change, leaving a comment in ExecuteRestore documenting why
the property must not be set here. Change wave 18.10 is retained because
it is still used by the -getProperty/-getItem evaluation change (#14290).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 395f543d-fb00-4a06-95b1-a7738432a141

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reverts the /restore (and restore-only /t:Restore) behavior introduced in #14274 that injected ExcludeRestorePackageImports=true into restore global properties (behind change wave 18.10), after it was found to break restore in scenarios with a nonexistent <ProjectReference>.

Changes:

  • Removed the ExcludeRestorePackageImports=true global-property injection from XMake.ExecuteRestore and replaced it with an explanatory comment documenting the failure mode.
  • Deleted the MSBuildConstants.ExcludeRestorePackageImports* constants and removed the unit tests that validated the injected-property behavior.
  • Updated ChangeWaves.md to remove the 18.10 bullet that described the reverted feature.
Show a summary per file
File Description
src/MSBuild/XMake.cs Stops injecting ExcludeRestorePackageImports during restore; adds rationale comment to prevent reintroduction.
src/MSBuild.UnitTests/XMake_Tests.cs Removes tests that asserted the (now reverted) injected restore global property behavior.
src/Framework/MSBuildConstants.cs Removes now-unused ExcludeRestorePackageImports* constants.
documentation/wiki/ChangeWaves.md Removes the 18.10 changewave bullet describing the reverted restore behavior.

Copilot's findings

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

Comment thread documentation/wiki/ChangeWaves.md Outdated
Comment thread src/MSBuild.UnitTests/XMake_Tests.cs
ViktorHofer added a commit to dotnet/sdk that referenced this pull request Jul 14, 2026
Regressed by MSBuild #14274 (ExcludeRestorePackageImports during implicit
restore), which fails with MSB3202 on multi-TFM projects that have a
nonexistent <ProjectReference>. The MSBuild change is being reverted in
dotnet/msbuild#14349. Re-enable tracked by #55263.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 395f543d-fb00-4a06-95b1-a7738432a141
- Add RestoreDoesNotInjectExcludeRestorePackageImports test asserting
  restore does not inject the property by default, guarding against
  silent reintroduction of the reverted #14274 behavior.
- Remove a stray "arget)" typo left on the 18.10 -getProperty bullet.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 395f543d-fb00-4a06-95b1-a7738432a141
@ViktorHofer

Copy link
Copy Markdown
Member Author

Thanks for the review — addressed both points in 2bf46f00b3:

  • Regression test: Added RestoreDoesNotInjectExcludeRestorePackageImports, which asserts restore leaves ExcludeRestorePackageImports empty by default, so the reverted behavior can''t silently reappear. Verified passing on net10.0 and net472.
  • ChangeWaves.md typo: Removed the stray arget) on the 18.10 -getProperty/-getItem bullet.

@ViktorHofer
ViktorHofer merged commit bfb7513 into main Jul 14, 2026
14 checks passed
@ViktorHofer
ViktorHofer deleted the viktorhofer/revert-exclude-restore-package-imports branch July 14, 2026 08:46
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

Successfully merging this pull request may close these issues.

3 participants