[wasm] Fix native build failing when temp path contains parentheses#131025
Conversation
RunShellCommand writes the compile command to a temporary batch file under
Path.GetTempPath() and runs it with 'cmd /c "<script>"'. When the temp path
contains characters cmd.exe treats specially - most commonly parentheses from a
Windows user profile name like C:\Users\John(US) - cmd's /c quote handling
strips the surrounding quotes (it sees special characters between the two
quotes) and then mis-parses the now-unquoted path at the first '(', producing:
'C:\Users\John' is not recognized as an internal or external command
and the whole native (emcc) build fails.
Invoke the script as 'cmd /S /c ""<script>""' instead: /S makes cmd strip
only the outermost pair of quotes and treat the rest verbatim, so the inner
quotes around the path are preserved.
Adds a Windows-only Wasm.Build.Tests regression that runs a native build with
%TMP%/%TEMP% pointed at a directory containing parentheses.
Fixes #120327
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62dcd5b3-7227-426b-a072-8812e9c4382d
|
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.
Pull request overview
Fixes a Windows-specific failure in the WASM native build pipeline when the temp directory path contains parentheses by adjusting how the temporary .cmd script is invoked via cmd.exe. Adds a targeted regression test in the WASM build test suite.
Changes:
- Update
Utils.RunShellCommand(Windows path) to invoke temp scripts viacmd /S /c ""<script>""to preserve quoting when special characters (e.g., parentheses) appear in the temp path. - Add a Windows-only regression test that forces
%TMP%/%TEMP%to a parenthesized directory and performs an end-to-end native publish/build.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/tasks/Common/Utils.cs | Adjusts cmd.exe invocation to robustly execute temp .cmd scripts when the temp path includes special characters like parentheses. |
| src/mono/wasm/Wasm.Build.Tests/NativeBuildTests.cs | Adds a Windows-only regression test covering native publish/build with %TMP%/%TEMP% pointing to a parenthesized temp directory. |
…path Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62dcd5b3-7227-426b-a072-8812e9c4382d
…eses Restore the non-ASCII characters (dropped in the previous commit) so the temp path exercises both the cmd.exe quoting fix (parentheses) and the UTF-8 (chcp 65001) handling in RunShellCommand, while GetRandomId keeps it unique. Rename the test accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62dcd5b3-7227-426b-a072-8812e9c4382d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/mono/wasm/Wasm.Build.Tests/NativeBuildTests.cs:72
- The test creates a dedicated TMP/TEMP directory and points the native publish to it, but never deletes it. Since the publish will generate a fair amount of temporary files, this leaves behind extra directories under BuildEnvironment.TmpPath across the test run and can increase disk usage/noise. Wrap the publish+run in a try/finally and delete the directory at the end.
string tempWithParens = Path.Combine(BuildEnvironment.TmpPath, $"tmp ({GetRandomId()}) {s_unicodeChars}");
Directory.CreateDirectory(tempWithParens);
var envVars = new Dictionary<string, string>
{
["TMP"] = tempWithParens,
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "de9fe31e37703ee278703efc52e5c889a7cb2337",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "7b783ab54922b9faef6a625a0f4326ca803051f0",
"last_reviewed_commit": "de9fe31e37703ee278703efc52e5c889a7cb2337",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "7b783ab54922b9faef6a625a0f4326ca803051f0",
"last_recorded_worker_run_id": "29689454975",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "de9fe31e37703ee278703efc52e5c889a7cb2337",
"review_id": 4730869463
}
]
} |
There was a problem hiding this comment.
Holistic Review
Motivation: Fixes #120327. On Windows, a native (emcc) WebAssembly build fails when the temp path (typically under the user profile) contains cmd special characters such as parentheses, e.g. C:\Users\John(US). Utils.RunShellCommand writes the compiler command to a temporary batch file under Path.GetTempPath() and runs it as cmd /c "<path>"; because cmd's /c quote-handling strips the surrounding quotes when special characters appear between them, the path is mis-parsed at the first ( and the build fails with 'C:\Users\John' is not recognized.... The motivation is real and well-diagnosed.
Approach: Switch the Windows invocation to cmd /S /c ""<path>"". With /S, cmd strips only the outermost pair of quotes and treats the remainder verbatim, preserving the inner quotes around the path regardless of parentheses or spaces. This is the canonical, documented workaround for this cmd.exe quoting behavior and is the minimal correct change. The Unix /bin/sh branch is untouched, which is appropriate since that path passes the quoted script as a single argv without re-parsing. A regression test (NativeBuildWithSpecialCharsInTempPath) exercises a real native publish with %TMP%/%TEMP% redirected to a directory containing parentheses (plus the existing s_unicodeChars to also cover the UTF-8/chcp path), gated to Windows via [SkipOnPlatform(TestPlatforms.AnyUnix, ...)] since the behavior is Windows-specific.
Summary: This is a small, correct, well-targeted fix with a matching regression test that reuses established test helpers (CreateWasmTemplateProject, GetRandomId, s_unicodeChars, PublishProject) and conventions. The /S + double-quote form is the standard remedy and does not regress the common no-special-character case. The single production caller (EmccCompile) benefits directly, and no other behavior changes. I see no correctness, security, or performance concerns. LGTM.
Minor, non-blocking observations (no change required): the test method is named ...SpecialCharsInTempPath while the surrounding comments emphasize parentheses specifically; the name is arguably more accurate given it also includes Unicode, so this is fine. The aot parameter is fixed to false via [BuildAndRun(aot: false)], consistent with the sibling SimpleNativeBuild pattern.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 41.4 AIC · ⌖ 10.4 AIC · ⊞ 10K
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
|
/ba-g failures are not related |
|
/backport to release/10.0 |
|
Started backporting to |
Summary
Fixes #120327. A
dotnet publishof a WebAssembly app (native/emcc build) fails on Windows when the user profile name contains parentheses, e.g.C:\Users\John(US):The project path itself is fine — the parentheses are in the home/temp path.
Root cause
Utils.RunShellCommand(used by theEmccCompiletask) writes the compiler command to a temporary batch file underPath.GetTempPath()and runs it as:Path.GetTempPath()is under the user profile (C:\Users\John(US)\AppData\Local\Temp\...), so that quoted path contains(and).cmd's/cquote-handling rule only preserves the quotes when there are no special characters between them; parentheses are special, socmdstrips the surrounding quotes and then parses the now-unquoted path up to the first(— treatingC:\Users\Johnas the command and reporting "is not recognized". The native build then fails. (Everything earlier in the build succeeds, which is why the failure only surfaces at the emcc compile step.)Fix
Invoke the script with
/Splus an extra pair of quotes:/Smakescmdstrip only the outermost pair of quotes and treat the remainder verbatim, so the inner quotes around the path are preserved and the path is passed intact regardless of parentheses/spaces. The Unix/bin/shpath is unaffected (the quoted path is passed as a single argv and never re-parsed) and is left unchanged.Test
Adds
NativeBuildTests.NativeBuildWithParenthesesInTempPath: a native (WasmBuildNative=true) build with%TMP%/%TEMP%pointed at a directory containing parentheses. It's gated to Windows ([SkipOnPlatform(TestPlatforms.AnyUnix, …)]) because thecmd.exequote-stripping behavior is Windows-specific.Verification
cmd.exe; the Unix/bin/shcode path is not affected, so it cannot be reproduced on Linux/macOS.WasmAppBuildertask and theWasm.Build.Testsproject both compile cleanly with the change.Note
This pull request was authored with GitHub Copilot.