Skip to content

Prevent SFX restores from overwriting assets for another RID - #68254

Merged
wtgodbe merged 5 commits into
mainfrom
pureween-validate-architecture-assets
Aug 21, 2026
Merged

Prevent SFX restores from overwriting assets for another RID#68254
wtgodbe merged 5 commits into
mainfrom
pureween-validate-architecture-assets

Conversation

@PureWeen

@PureWeen PureWeen commented Aug 6, 2026

Copy link
Copy Markdown
Member

Prevent SFX restores from overwriting assets for another RID

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable. (Not applicable to this MSBuild restore-layout change; see the validation below.)
  • You've included inline docs for your change, where applicable. (No public or user-facing documentation is required.)
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Keep each SFX runtime identifier's NuGet restore state in its own directory.

Description

During local Components bring-up on an Apple Silicon Mac, restoring the framework SFX projects for osx-x64 replaced the restore state needed by osx-arm64. A later ./eng/build.sh --arch arm64 --no-restore then failed with NETSDK1047 because project.assets.json no longer contained the requested arm64 target.

This surfaced locally rather than as a CI failure. CI legs restore clean state and do not exercise switching runtime identifiers in the same working tree.

Root cause

The Runtime, Ref, and Runtime Composite SFX projects each used one shared NuGet project-extensions directory for every runtime identifier. Restoring any RID could therefore overwrite the assets for a previously restored RID. The same problem applies to architecture and OS changes, not only osx-x64 and osx-arm64.

The exact command that created the originally observed osx-x64 assets is not recoverable. However, a supported producer path is verified: bare dotnet and IDE/design-time evaluation leave TargetArchitecture unset, so eng/Common.props defaults to x64. In contrast, eng/build.sh detects and passes the arm64 host architecture.

Fix

For .sfxproj projects in the Runtime and Ref directories, this change scopes MSBuildProjectExtensionsPath by the full TargetRuntimeIdentifier. Each RID now keeps its own project.assets.json, generated NuGet props and targets, dependency graph, and cache files.

The condition deliberately applies only to .sfxproj files. It covers Runtime, Ref, and Runtime Composite while leaving the RID-independent traversal and base projects unchanged. Configuration is intentionally not included because Debug and Release already share restore state per RID by design.

This fixes the producer layout instead of adding a diagnostic after the assets have already been overwritten.

Validation

  • Restored Runtime, Ref, and Runtime Composite for both osx-x64 and osx-arm64; all six RID-scoped assets files coexisted with the correct targets.
  • Refreshed the x64 restore, then resolved arm64 assets without restoring again; the preserved arm64 assets succeeded instead of failing with NETSDK1047.
  • Verified build-time and design-time resolution, Composite static-graph restore, VMR/source-build restore, property ordering, cross-OS isolation, and that traversal/base projects remain unscoped.
  • Verified the existing Debug-to-Release ReadyToRun behavior is unchanged, which is why Configuration is not part of the path.
  • Full CI is green, including Windows, Linux, macOS x64/arm64, source-build, Components E2E, and code checks.

Full arm64 no-restore builds progressed past restore-assets resolution and reached the existing missing Web.JS build-output prerequisite. The same prerequisite is present without this change, so it is unrelated to RID scoping.

Tradeoffs

  • Each restored RID uses additional disk space for its own NuGet intermediate files.
  • Existing unscoped intermediate files remain until normal artifact cleanup, but new restores ignore them.
  • External automation that hard-codes these internal paths would need to use the evaluated MSBuild property or the RID-scoped path. No repository consumer does so.

Detect incompatible target framework and runtime identifier assets before ResolvePackageAssets and report the exact forced restore command.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 782d99e1-0c46-4561-8dd4-1a780f28794d
@PureWeen
PureWeen marked this pull request as ready for review August 6, 2026 18:32
@PureWeen
PureWeen requested review from a team and wtgodbe as code owners August 6, 2026 18:32
Copilot AI review requested due to automatic review settings August 6, 2026 18:32
@PureWeen
PureWeen marked this pull request as draft August 6, 2026 18:58
Keep the validation target aligned with the RepoTasks design-time import guard.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 782d99e1-0c46-4561-8dd4-1a780f28794d

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds an early, more actionable diagnostic for stale SFX restore assets by validating that project.assets.json contains the requested TargetFramework/RuntimeIdentifier target before NuGet asset resolution.

Changes:

  • Registers a new RepoTasks MSBuild task to inspect project.assets.json for a specific runtime target.
  • Introduces an MSBuild target in Sfx.Common.targets that fails fast with an OS/arch-specific forced-restore command.
  • Adds the C# implementation of the JSON-aware assets target check.
Show a summary per file
File Description
eng/tools/RepoTasks/RepoTasks.tasks Registers the new MSBuild task so it can be used from targets.
eng/tools/RepoTasks/CheckProjectAssetsRuntimeTarget.cs Implements JSON parsing to detect missing TFM/RID targets in assets files.
eng/targets/Sfx.Common.targets Runs the check before ResolvePackageAssets and reports a repo-specific restore command when stale assets are detected.

Copilot's findings

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

Comment thread eng/tools/RepoTasks/CheckProjectAssetsRuntimeTarget.cs Outdated
Comment thread eng/tools/RepoTasks/CheckProjectAssetsRuntimeTarget.cs Outdated
@kotlarmilos

Copy link
Copy Markdown
Member

SFX restores write RID-specific targets to the shared artifacts/obj/aspnetcore-ref/project.assets.json and artifacts/obj/aspnetcore-sfx/project.assets.json paths. A later architecture-specific --no-restore build can request a different runtime identifier and currently reaches the SDK's generic NETSDK1047 diagnostic.

Do we have use-case where we use different arch in subsequent builds? I don't understand scenario that we are trying to fix.

Copilot AI added 2 commits August 16, 2026 15:07
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 782d99e1-0c46-4561-8dd4-1a780f28794d
Include the concrete exception type while preserving the assets file path and existing remediation context.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 782d99e1-0c46-4561-8dd4-1a780f28794d
@PureWeen

Copy link
Copy Markdown
Member Author

Yes. There are two concrete paths here:

  1. Explicit architecture switches are supported. The Windows CI job currently builds x64, x86, and arm64 sequentially in the same checkout, although it correctly restores for each architecture. The failure case is a later build using --no-restore after another architecture populated the shared SFX assets path.
  2. On Apple Silicon, a bare dotnet or IDE evaluation leaves TargetArchitecture unset, so eng/Common.props defaults it to x64 and the SFX projects derive osx-x64. The repository script path is different: eng/build.sh reads uname -m through init-os-and-arch.sh and passes arm64. That gives us a supported path where a bare restore leaves x64 assets and ./eng/build.sh --arch arm64 --no-restore later requests arm64.

I reproduced the mismatch deterministically with an explicit x64 SFX restore followed by the arm64 no-restore build. Before this change it reaches NETSDK1047. With this change it stops earlier and tells you to run ./restore.sh --os-name osx --arch arm64 -p:RestoreForce=true.

I still don't know the exact command that created the original x64 assets, so I'm not claiming the IDE was definitely the source. This PR also isn't trying to make a cross-architecture no-restore build valid. It makes that stale restore state explain itself and gives the exact recovery command.

@PureWeen
PureWeen marked this pull request as draft August 17, 2026 03:03
Keep RID-specific NuGet assets side by side so architecture switches can build with --no-restore without replacing another RID's restore state.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 782d99e1-0c46-4561-8dd4-1a780f28794d
@PureWeen PureWeen changed the title Improve stale SFX restore diagnostics Scope SFX restore assets by runtime identifier Aug 17, 2026
@PureWeen
PureWeen marked this pull request as ready for review August 17, 2026 23:06
@PureWeen PureWeen changed the title Scope SFX restore assets by runtime identifier Prevent SFX restores from overwriting assets for another RID Aug 21, 2026
@wtgodbe
wtgodbe merged commit e072299 into main Aug 21, 2026
27 checks passed
@wtgodbe
wtgodbe deleted the pureween-validate-architecture-assets branch August 21, 2026 19:36
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants