Repository navigation
fix(windows): pass non-verbatim paths to action commands - #424
Merged
Merged
Conversation
Canonical Windows workspace roots carry the verbatim prefix (\\?\D:\...). Action argv and environment values built from the execution root marker and the workspace_root() Starlark global inherited that spelling. Git Bash cannot open verbatim paths, and Windows rejects them once a command appends /-separated components, so Cargo build scripts failed with "No such file or directory". Spell those command values without the prefix when a plain drive or UNC spelling exists and fits in MAX_PATH. Once's own filesystem calls keep the canonical path. Amp-Thread-ID: https://ampcode.com/threads/T-01a1bf6d-95f0-76f7-9a26-c062fe0363e1 Co-authored-by: Marek Fort <marek@tuist.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Action command values that Once builds from the workspace path now use the plain Windows spelling (
D:\...or\\server\share\...) instead of the verbatim spelling (\\?\D:\...). This covers two places:{{once.execution_root}}substitution in action argv and environment (resolve_execution_value, through a newonce_core::command_path).workspace_root()Starlark global.The prefix is kept when no plain spelling exists (for example
\\?\Volume{...}) or when the plain path would reachMAX_PATH(260), so long paths still resolve. Off Windows nothing changes.Why
The tuist desktop Windows build on Once 0.77.3 fails while running Cargo build scripts (job). The
proc-macro2build script action runs under Git Bash with:Root cause
std::fs::canonicalizereturns verbatim paths on Windows, and the workspace root Once analyzes and executes against is canonicalized (for example innative_invocation). Starlark rules append/-separated components toworkspace_root()and to execution-path markers. Windows does not normalize separators inside verbatim paths, and MSYS shells cannot open verbatim paths at all, so every such command path is invalid.Approach and alternatives
0.77.3 (cd31976) deliberately kept verbatim prefixes on filesystem paths and normalized them only for Cargo comparisons. This change keeps that split: Once's own Rust filesystem calls still use the canonical path. Only the string values handed to commands and Starlark get the plain spelling, which is the form shells and tools accept. Rust's
std::fstreats the two spellings the same, sohost_file_*calls built fromworkspace_root()keep working.I considered stripping the prefix where the workspace root is canonicalized. Several call sites canonicalize it independently and compare the results by prefix, so a mixed spelling there would risk containment mismatches. Converting at the command boundary is narrower. I didn't add
dunceas a direct dependency because the rule is small and a pure string function can be unit-tested on every platform.Impact
Windows actions that use
workspace_root()or execution-root paths get valid paths. Action keys on Windows change once because the argv spelling changes. Linux and macOS are unaffected.Validation
cargo test -p once-core --lib execution_path: 4 passed. The new cases cover drive and UNC stripping, a non-verbatim no-op, the volume GUID form, and long paths on both sides ofMAX_PATH.cargo clippy -p once-core -p once-frontend --all-targets -- -D warningsandcargo fmt --allare clean.cargo test -p once-frontend --no-fail-fast: everything passed except 8 tests that need toolchains missing from my Linux environment (bazel, mix/elixir, go, ruby, JVM).I couldn't run Windows locally. The
windows buildCI job and a tuist desktop Windows build on the released version are the real checks.