[browser] Fix WebAssembly publish output path message - #132829
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
Pull request overview
This PR adjusts WebAssembly browser (wasmbrowser) build/publish logging so console output identifies the servable wwwroot directory (e.g., publish/wwwroot/) rather than only the generic publish/ path, and adds coverage to ensure the message appears exactly once per phase.
Changes:
- Add high-importance MSBuild messages for
$(TargetDir)/wwwroot(build) and$(PublishDir)/wwwroot(publish), suppressed during nested publish. - Add a Wasm template test that asserts the new
(wwwroot) -> .../wwwroot/message occurs exactly once in both build and publish output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs | Adds a test asserting the (wwwroot) output-directory messages appear once for build and publish. |
| src/mono/nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets | Emits new high-importance messages for build/publish wwwroot output directories, skipping nested publish. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 8afff773-0b32-46c7-8faa-231ce9df9ff1
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🔵 Needs a closer look
The new test and build-phase message appear to compute/output inconsistent path forms (absolute vs relative), making the assertions and console output behavior likely incorrect/brittle.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
src/mono/nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets:980
PublishDiris normalized from the relative$(OutputPath)in this targets file (so the publish message prints a relativebin/.../publish/wwwroot/), but the build message uses$(TargetDir)(typically rooted/full). That risks inconsistent output (absolute for build, relative for publish) and makes the message less likely to match what terminal logging and tests expect. Prefer basing the build message on$(OutputPath)for consistency with the publish path computation here.
src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs:69- The test builds the expected
(wwwroot)output directories usingprojectDir, producing an absolute path. The WebAssembly SDK targets setPublishDir/OutputPathas relative (bin/...) paths, and the console output message is intended to show those relative paths; this assertion is likely to fail by mismatching absolute vs relative.
This issue also appears on line 73 of the same file.
src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs:77
- Same as build: the publish assertion currently expects an absolute
.../bin/.../publish/wwwroot/path, butPublishDiris defined as a relativebin/.../publish/path in the imported targets. This makes the test brittle and likely incorrect for the actual console output.
string publishOutputDirectory = Path.Combine(
projectDir,
"bin",
config.ToString(),
DefaultTargetFramework,
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Fixes #99292
Related change in SDK dotnet/sdk#55963
wasmbrowserpublish currently reports the genericpublish/directory even though the servable application is underpublish/wwwroot/. Add shared WebAssembly build and publish messages that identify thewwwrootoutput directory, while suppressing nested-publish duplicates.The Wasm template tests assert the exact build and publish paths and require one occurrence per phase.
Expected console output:
The corresponding
dotnet/sdkchange should remove the obsolete Blazor 6.0_BlazorCopyFilesToOutputDirectoryoutput message so the shared target is the single source of truth. That change is outside this repository.Validation: Mono/libraries baselines and browser-wasm package build succeeded. The focused WBT test assembly builds, but test execution is blocked in this environment by the local workload package-version check in
eng/testing/workloads-browser.targets.Note
This pull request description was generated with GitHub Copilot.