Repository navigation
Canonicalise the usd_process temp root on macOS - #105
Merged
Blogbotana merged 2 commits intoOct 20, 2026
Merged
Blogbotana merged 2 commits into
Blogbotana merged 2 commits into
Conversation
The blob store rejects links among a root's ancestors, and on macOS the temp directory sits behind the /var -> /private/var symlink, so all nine usd_process tests failed with UnsafeEntry on a pristine macOS checkout before any A04 change. Start from the canonicalized temp directory so the fixture root has no link ancestor on any platform. (cherry picked from commit 4bbef96)
Since 4bbef96 the fixture canonicalised std::env::temp_dir() unconditionally; on Windows that yields a verbatim \\?\C:\... root, which BlobStore::new rejects, so all nine usd_process tests died at fixture setup on Windows CI (master run 39305089702). Keep the plain temp path elsewhere, mirroring the ancestor-link policy in crates/host/src/storage.rs, where only macOS resolves system links. Fixes #40 Co-Authored-By: Claude Code <[email protected]> (cherry picked from commit 65997fa)
axisrow
commented
Oct 20, 2026
axisrow
left a comment
axisrow
left a comment
Collaborator
Author
There was a problem hiding this comment.
Verdict: approved
Test-only fix, net diff is one fixture change: the usd_process store root is built from temp_dir().canonicalize() on macOS and from the plain temp path everywhere else, mirroring the ancestor-link policy that crates/host/src/storage.rs implements per platform (the same cfg!(target_os = "macos") constant idiom).
Points checked:
- The macOS-only gate is right: on Windows
canonicalizeyields a\\?\verbatim path, which the store refuses as a root, so unconditional canonicalisation (the first commit) was correctly fixed up by the second. - Both branches compile on every target (
cfg!is a runtime constant), theunwrapsits in test code where the workspace clippy rules permit it, andtemp_dir()always exists, so it cannot panic in practice. - The canonicalised root still ends in the same unique fixture directory name, so parallel-fixture isolation and the process-id/uniqueness naming are unchanged.
- Consistent with #104: ancestors may be links on macOS and the store resolves them, and aligning the fixture root with the real path keeps any path comparisons inside the test exact rather than link-dependent.
3 of 4 tasks
Collaborator
Author
|
Implements the work tracked in #133 and #135 (mirrors of axisrow#23 and axisrow#40: the macOS /var symlink in temp_dir ancestors, and the Windows regression fixed by limiting the canonicalization to macOS). |
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 this changes
Mirror of the fork's #42 (
axisrow/BeamCraft, fix for the fork's issue #40), rebased ontoupstream
master73c3597. Two commits, verbatim, test-side only:4bbef96)65997fa)The
usd_processfixtures build their store root fromstd::env::temp_dir(); on macOS thatpath sits behind the
/var→/private/varsymlink and the store root check refuses it.The fixture now canonicalises the temp root first. Follows the storage-side fix of the
previous PR.
How you verified it
cargo test -p beamcraft-cli --test usd_processon macOS (passes with the canonicalTMPDIRused by this machine's sandbox; fails identically without the fix, which ishow the bug reproduces)
Provenance (clean-room)
🤖 Generated with Claude Code
Issue tracking
Fixes #348