close
Skip to content

Restore trimming test projects serially#65668

Merged
wtgodbe merged 4 commits into
mainfrom
wtgodbe/FixAlreadyExists
Mar 6, 2026
Merged

Restore trimming test projects serially#65668
wtgodbe merged 4 commits into
mainfrom
wtgodbe/FixAlreadyExists

Conversation

@wtgodbe
Copy link
Copy Markdown
Member

@wtgodbe wtgodbe commented Mar 5, 2026

Should fix the last flavor of #61178. Trimming tests are built by BuildAfterTargetingPack.csproj, which first calls Restore serially, then calls Test in parallel. But the trimming test projects were overwriting Restore with a no-op target, so the actual restoring was happening in parallel, in PublishTrimmedProjects, which gets called in the target chain for Test. If we have Restore actually restore, it'll happen serially like we want.

@wtgodbe wtgodbe requested a review from a team as a code owner March 5, 2026 22:52
Copilot AI review requested due to automatic review settings March 5, 2026 22:52
@wtgodbe wtgodbe added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 5, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a conditional restore retry in linker trimming tests to work around intermittent NuGet restore races (notably on Windows) when multiple trimming test projects restore shared TestConsoleApps concurrently.

Changes:

  • Allows the initial Restore MSBuild invocation to continue after failure.
  • Adds a second Restore invocation that runs only if the first restore failed.
  • Documents the rationale and links the tracked issue.

Comment thread eng/testing/linker/trimmingTests.targets
Comment thread eng/testing/linker/trimmingTests.targets Outdated
@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 5, 2026

/backport to release/10.0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 5, 2026

Started backporting to release/10.0 (link to workflow run)

@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 5, 2026

That clearly didn't work. Trying something else

@wtgodbe wtgodbe changed the title Retry restore in linker tests if first one fails Restore trimming test projects serially Mar 6, 2026
@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 6, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 6, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 6, 2026

/backport to release/10.0

@wtgodbe wtgodbe merged commit 0b12e6f into main Mar 6, 2026
25 checks passed
@wtgodbe wtgodbe deleted the wtgodbe/FixAlreadyExists branch March 6, 2026 05:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

Started backporting to release/10.0 (link to workflow run)

@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 16, 2026

/backport to release/10.0

@github-actions
Copy link
Copy Markdown
Contributor

Started backporting to release/10.0 (link to workflow run)

@OvesN
Copy link
Copy Markdown

OvesN commented Mar 25, 2026

@wtgodbe I received a reported pipeline failure with highly parallelized MSBuild that looks similar to what this PR is trying to fix. From my understanding, parallel Build phases each trigger a fallback Restore that recursively
walks into the same shared dependencies, causing concurrent writes of project.assets.json to the same obj/
directories, which crashes on MoveFile because the file already exists from the other process.

In the binlog I did not see the property SkipTrimmingProjectsRestore being set at all. It looks like the
BuildDelayedProjects target in BuildAfterTargetingPack.csproj restores the projects serially, but then builds them
in parallel without passing SkipTrimmingProjectsRestore=true, so each project's PublishTrimmedProjects triggers a
redundant restore that can race.

Should SkipTrimmingProjectsRestore=true also be passed in the Build call to complete this fix?

@wtgodbe
Copy link
Copy Markdown
Member Author

wtgodbe commented Mar 25, 2026

@OvesN ah, yes, nice catch! Do you want to submit a PR for that?

@OvesN
Copy link
Copy Markdown

OvesN commented Mar 27, 2026

@wtgodbe
I'll leave this to aspnetcore team since you know better which build paths are always guaranteed to have Restore completed before reaching PublishTrimmedProjects. The Build call is clearly safe since Restore runs right above it in the same
target, but for Test and VSTest I'm not sure if Restore is always guaranteed to have run beforehand.

Do you want me to submit an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants