Migrate GenerateStaticWebAssetsManifest to IMultiThreadableTask by jankratochvilcz · Pull Request #54704 · dotnet/sdk

jankratochvilcz · Jun 11, 2026

Migrates GenerateStaticWebAssetsManifest (src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs) to IMultiThreadableTask so MSBuild's multithreaded mode bypasses the per-task TaskHost wrapper.

Fixes dotnet/msbuild#14032

Perfstar regression (14d, Windows, paired MT vs non-MT, p50 sum across iterations)

Target	non-MT (ms)	MT (ms)	Δ (ms)	Δ (%)
`GenerateStaticWebAssetsManifest`	2,555	8,321	+5,766	+226 %

Changes

[MSBuildMultiThreadableTask] + implements IMultiThreadableTask with default TaskEnvironment.Fallback.
In PersistManifest, ManifestPath and (optional) ManifestCacheFilePath are absolutized once at the top via TaskEnvironment.GetAbsolutePath. All subsequent File.Exists / File.ReadAllBytes / File.WriteAllBytes / File.WriteAllText calls use the absolutized values. Log messages continue to use the original user-provided property values.

Compatibility sins audit (per migration skill)

Sin 1 (output contamination): no [Output] property on this task.
Sin 2 (error-message inflation): unchanged — log messages still use ManifestPath / ManifestCacheFilePath strings.
Sin 3 (?? swallowing exceptions): no new null-coalescing.
Sin 4 (try/catch scope): PersistManifest has no try/catch.
Sin 5 (canonicalization): no dictionary/set keyed by these paths; only equality of file contents (hashes) matters.
Sin 6 (exception type): GetAbsolutePath("") throws ArgumentException. ManifestPath is [Required] so MSBuild rejects empty values upstream. The optional ManifestCacheFilePath is guarded by string.IsNullOrEmpty before calling GetAbsolutePath.

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

jankratochvilcz · Jun 11, 2026

Follow-up: deeper call-stack audit (call paths into StaticWebAsset)

Tracing all helpers invoked from Execute(), one transitive call-stack hazard is shared with all SDK tasks that hydrate StaticWebAsset items:

StaticWebAsset.FromTaskItem / FromV1TaskItem → StaticWebAsset.Normalize() (instance method, src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs:1067) calls:

NormalizeContentRootPath(ContentRoot) → Path.GetFullPath(path) (:1080)
Path.GetFullPath(RelatedAsset) (:1072)

Both Path.GetFullPath calls go through BCL and resolve against Environment.CurrentDirectory, not MSBuild's MT-safe AsyncLocal FileUtilities.CurrentThreadWorkingDirectory.

Why it doesn't bite in practice today: in production the SDK targets that produce these items populate ContentRoot and RelatedAsset with already-absolute paths (e.g. via MSBuild's %(FullPath) well-known modifier, which is MT-safe). Path.GetFullPath(already_absolute) becomes a pure canonicalization that doesn't read Environment.CurrentDirectory. So this PR does not introduce a regression — the latent hazard predates the migration and is exercised by every existing SWA task that consumes StaticWebAsset items.

Why not fix it here: StaticWebAsset is a data type without a TaskEnvironment reference. Plumbing one in would change a widely-used public signature and is well outside the scope of a per-task attribute / IMultiThreadableTask migration. Should be tracked as a separate follow-up.

Copilot

Pull request overview

This PR migrates GenerateStaticWebAssetsManifest to MSBuild’s IMultiThreadableTask pattern so multithreaded builds can run the task in-proc (avoiding TaskHost overhead) while ensuring relative path I/O is rooted at TaskEnvironment.ProjectDirectory rather than the process CWD.

Changes:

Marked GenerateStaticWebAssetsManifest as [MSBuildMultiThreadableTask], implemented IMultiThreadableTask, and added a TaskEnvironment property defaulting to TaskEnvironment.Fallback.
Updated PersistManifest to absolutize ManifestPath / ManifestCacheFilePath once via TaskEnvironment.GetAbsolutePath(...) and use those absolute paths for all file I/O.
Added a new test that mutates the process current directory to validate the task writes under TaskEnvironment.ProjectDirectory.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs	Converts the task to `IMultiThreadableTask` and routes manifest/cache file I/O through `TaskEnvironment`-anchored absolute paths.
test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/GenerateStaticWebAssetsManifestMultiThreadingTest.cs	Adds coverage ensuring relative manifest paths resolve against `TaskEnvironment.ProjectDirectory`, not the process CWD.

- Add [MSBuildMultiThreadableTask] attribute and IMultiThreadableTask - Absolutize ManifestPath and ManifestCacheFilePath via TaskEnvironment - Pass TaskEnvironment to StaticWebAsset.FromTaskItemGroup - Add decoy-CWD test (Pattern A) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Match the defensive pattern used elsewhere in StaticWebAsset (e.g., NormalizeContentRootPath, ResolveFile) — skip absolutization when the input is null or empty instead of letting GetAbsolutePath throw. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address self-review comments: - manifestPath -> absolutizedManifestPath - hasCacheFile -> isManifestCacheFileConfigured - manifestCachePath -> absolutizedManifestCacheFilePath - fileExists -> manifestFileExists Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The conditional expression is typed as string and cannot implicitly convert to AbsolutePath. Declare the locals as string, matching other migrated tasks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

AlesProkop

LGTM now.

This test file was added on main by dotnet#54704 after this branch migrated Microsoft.NET.Sdk.StaticWebAssets.Tests to MSTest.Sdk, so the merge reintroduced xUnit [Fact] usage that no longer compiles (CS0246 FactAttribute/Fact). Convert it to [TestClass]/[TestMethod] with [DoNotParallelize], matching the sibling MultiThreading tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jankratochvilcz mentioned this pull request Jun 11, 2026

[Multithreaded] Migrate GenerateStaticWebAssetsManifest in SDK dotnet/msbuild#14032

Closed

This was referenced Jun 11, 2026

Could Not Resolve SDK - repo testfx issue #51530

Closed

NuGet failing with Response status code does not indicate success: 503 (Service Unavailable) dotnet/arcade#11723

Open

jankratochvilcz marked this pull request as ready for review June 16, 2026 17:14

Copilot AI review requested due to automatic review settings June 16, 2026 17:14

Copilot started reviewing on behalf of jankratochvilcz June 16, 2026 17:15 View session

jankratochvilcz force-pushed the mt/migrate-generate-static-web-assets-manifest branch from afeb601 to 07808d9 Compare June 16, 2026 17:18

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs

jankratochvilcz and others added 2 commits June 17, 2026 10:51

jankratochvilcz force-pushed the mt/migrate-generate-static-web-assets-manifest branch from 4aecac5 to 4099d77 Compare June 17, 2026 08:51

jankratochvilcz commented Jun 18, 2026

View reviewed changes

jankratochvilcz and others added 2 commits June 18, 2026 15:38

Merge branch 'main' into mt/migrate-generate-static-web-assets-manifest

8f278bd

OvesN requested changes Jun 18, 2026

View reviewed changes

Comment thread src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs Outdated

Update src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs

f1e503c

jankratochvilcz commented Jun 18, 2026

View reviewed changes

Comment thread src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs Outdated

Apply suggestion from @jankratochvilcz

df4a1f3

jankratochvilcz commented Jun 18, 2026

View reviewed changes

Comment thread src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs Outdated

Apply suggestion from @jankratochvilcz

2b65e2e

OvesN approved these changes Jun 18, 2026

View reviewed changes

AlesProkop requested changes Jun 22, 2026

View reviewed changes

Comment thread src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs Outdated

Fix CS0029 by declaring absolutized manifest paths as string

45c81d2

The conditional expression is typed as string and cannot implicitly convert to AbsolutePath. Declare the locals as string, matching other migrated tasks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

AlesProkop approved these changes Jun 22, 2026

View reviewed changes

jankratochvilcz enabled auto-merge (squash) June 22, 2026 08:59

build-analysis Bot mentioned this pull request Jun 22, 2026

Run-file tests failing with "The project file could not be loaded." #54819

Closed

Merge branch 'main' into mt/migrate-generate-static-web-assets-manifest

09b4ae0

jankratochvilcz merged commit 3d73390 into dotnet:main Jun 22, 2026
25 checks passed

dotnet-maestro Bot mentioned this pull request Jun 23, 2026

[main] Source code updates from dotnet/sdk dotnet/dotnet#7335

Merged

dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone Jun 24, 2026

MichaelSimons mentioned this pull request Jun 24, 2026

Disable in-process parallel test execution #54967

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate GenerateStaticWebAssetsManifest to IMultiThreadableTask - #54704

jankratochvilcz commented Jun 11, 2026 •

edited by OvesN

Loading

Uh oh!

jankratochvilcz commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlesProkop left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

jankratochvilcz commented Jun 11, 2026 • edited by OvesN Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perfstar regression (14d, Windows, paired MT vs non-MT, p50 sum across iterations)

Changes

Compatibility sins audit (per migration skill)

Uh oh!

jankratochvilcz commented Jun 11, 2026

Follow-up: deeper call-stack audit (call paths into StaticWebAsset)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlesProkop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jankratochvilcz commented Jun 11, 2026 •

edited by OvesN

Loading