Skip to content

Migrate ComputeReferenceStaticWebAssetItems - #54792

Merged
AlesProkop merged 5 commits into
dotnet:mainfrom
AlesProkop:migrate-compute-reference-static-web-asset-items
Jun 22, 2026
Merged

Migrate ComputeReferenceStaticWebAssetItems#54792
AlesProkop merged 5 commits into
dotnet:mainfrom
AlesProkop:migrate-compute-reference-static-web-asset-items

Conversation

@AlesProkop

Copy link
Copy Markdown
Member

Fixes dotnet/msbuild#14055

Context

Migrates ComputeReferenceStaticWebAssetItems to support MSBuild multithreaded task execution by avoiding process-CWD-based path resolution.

Changes Made

  • Added [MSBuildMultiThreadableTask] to ComputeReferenceStaticWebAssetItems.
  • Implemented IMultiThreadableTask and added a TaskEnvironment property with TaskEnvironment.Fallback default.
  • Updated ComputeReferenceStaticWebAssetItems to pass TaskEnvironment when loading static web assets.
  • Added a TaskEnvironment overload for StaticWebAsset.AssetsByTargetPath(...), preserving the existing overload via TaskEnvironment.Fallback.
  • Updated MakeReferencedAssetOriginalItemSpecAbsolute path resolution to use TaskEnvironment.GetAbsolutePath(...) before Path.GetFullPath(...), preserving canonicalization while resolving relative paths against the project directory.
  • Added a focused regression test verifying OriginalItemSpec is absolutized against the injected project directory rather than the process current directory.

Testing

  • Built src\StaticWebAssetsSdk\Tasks\Microsoft.NET.Sdk.StaticWebAssets.Tasks.csproj successfully.
  • Built test\Microsoft.NET.Sdk.StaticWebAssets.Tests\Microsoft.NET.Sdk.StaticWebAssets.Tests.csproj for net11.0 successfully.
  • Ran Microsoft.NET.Sdk.StaticWebAssets.Tests.ComputeReferenceStaticWebAssetItemsTest successfully.

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

This PR migrates ComputeReferenceStaticWebAssetItems to be safe for MSBuild multi-threaded task execution by eliminating reliance on process current-directory (CWD) for path resolution, aligning with the TaskEnvironment-based execution model.

Changes:

  • Marked ComputeReferenceStaticWebAssetItems as [MSBuildMultiThreadableTask], implemented IMultiThreadableTask, and threaded TaskEnvironment into asset loading and path absolutization.
  • Added a TaskEnvironment overload to StaticWebAsset.AssetsByTargetPath(...) and updated normalization to canonicalize via TaskEnvironment-based absolute paths.
  • Added a regression test ensuring OriginalItemSpec is resolved against the injected project directory (not the process CWD).

Reviewed changes

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

File Description
test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/ComputeReferenceStaticWebAssetItemsTest.cs Adds regression coverage for OriginalItemSpec absolutization using an injected TaskEnvironment project directory.
src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs Introduces AssetsByTargetPath(..., TaskEnvironment) overload and ensures item hydration/normalization can be rooted in TaskEnvironment.
src/StaticWebAssetsSdk/Tasks/ComputeReferenceStaticWebAssetItems.cs Makes the task multi-threadable and uses TaskEnvironment for MT-safe asset parsing and OriginalItemSpec absolutization.

@AlesProkop

Copy link
Copy Markdown
Member Author

Expert MSBuild multithreaded task migration review: no high-confidence actionable issues found.

Checked:

  • ComputeReferenceStaticWebAssetItems is marked [MSBuildMultiThreadableTask], implements IMultiThreadableTask, and defaults TaskEnvironment to TaskEnvironment.Fallback.
  • The task threads TaskEnvironment into static web asset hydration, so relative path-bearing metadata resolves against the project directory rather than process CWD.
  • The migrated OriginalItemSpec, ContentRoot, and RelatedAsset paths keep the previous canonicalized full-path behavior via FileInfo/DirectoryInfo.
  • No unintended AbsolutePath leakage into logs or output values was found, and no unsafe process CWD/environment/process-start APIs were introduced.

@jankratochvilcz jankratochvilcz 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.

Thanks, one bigger question here :-)

Comment thread src/StaticWebAssetsSdk/Tasks/ComputeReferenceStaticWebAssetItems.cs Outdated
Comment thread src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs Outdated

@OvesN OvesN 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.

LGTM

@AlesProkop
AlesProkop merged commit 650cc37 into dotnet:main Jun 22, 2026
25 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone Jun 24, 2026
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.

[Multithreaded] Migrate ComputeReferenceStaticWebAssetItems in SDK

4 participants