Repository navigation
fix: preserve Windows drive paths in bundle catalog config - #5205
Conversation
yxshee
left a comment
There was a problem hiding this comment.
I checked this against main (7370e59). On main, add_source(project, r"C:\catalog.json", ...) on Linux stores /<project>/C:\catalog.json with id c--catalog, as #5204 describes. With the change, the URL is stored verbatim and the id becomes catalog, which is the id the same input gets on Windows, so a config is now consistent across hosts.
Two things I noticed, neither blocking:
- On a POSIX host the preserved
C:\catalog.jsonentry still can't be read.adapters.pyopens it asPath(url)relative to the working directory and raisesCatalog file not found: C:\catalog.json. That's a clear error, so it's fine. If you'd rather catch it atbundle catalog addtime, the issue's other option (reject a foreign drive path on POSIX) would do that. C:\catalog.jsonandC:/catalog.jsonare the same file on Windows, where both canonicalize toC:\catalog.jsonand the second add returnsunchanged. On POSIX they're now stored verbatim, so the second add fails with "already exists" because the idcatalogcollides.remove_source(project, "C:/catalog.json")also won't remove an entry stored asC:\catalog.jsonon POSIX. Normalizing withstr(PureWindowsPath(url))instead of returningurlunchanged would make both spellings behave the same way on every host.
Posted on behalf of @yxshee. AI disclosure: Claude Code (model: Claude Opus 5.5, autonomous) reproduced the issue on main, checked the change's behavior by applying the same two-line logic to main, and drafted this review. I did not run the PR branch itself.
Generated by Claude Code
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the change preserving Windows drive paths in bundle catalog configuration.
No open issues 🎉
What changed in this PR
Fixes #5204 by preserving Windows drive paths verbatim when bundle catalog configuration is updated on POSIX hosts, while retaining native-path canonicalization.
Changes:
- Bypass host-native resolution for foreign Windows drive paths.
- Add regression coverage for both slash styles, verifying persistence, repeated-add idempotency, and removal.
The PR reports two regression failures before the fix and two passes afterward; these results were not independently rerun.
| File | Description |
|---|---|
tests/specify_cli/bundles/test_catalog_config.py |
Adds POSIX regression tests for Windows drive-path preservation and catalog lifecycle operations. |
src/specify_cli/bundles/catalog_config.py |
Preserves foreign Windows drive paths and documents the behavior. |
Review effort: Balanced (auto)
|
Thank you! |
Description
Fixes #5204. Preserve Windows drive paths verbatim when a POSIX host adds a bundle catalog, preventing the host’s
Path.resolve()from rewriting them into project-relative paths. Native paths retain their existing behavior.Testing
specify --help, andgit diff --checkpassed.AI Disclosure
AI disclosure: OpenAI Codex (GPT-6, reasoning effort setting not exposed) worked autonomously to investigate the issue, generate the code and test, and run validation. This does not claim human review or testing.