Skip to content

fix: windows drive letters remap to unc paths - #2104

Open
maxnbk wants to merge 25 commits into
AcademySoftwareFoundation:mainfrom
maxnbk:fix/windows-drive-letters-remap-to-unc-paths
Open

fix: windows drive letters remap to unc paths#2104
maxnbk wants to merge 25 commits into
AcademySoftwareFoundation:mainfrom
maxnbk:fix/windows-drive-letters-remap-to-unc-paths

Conversation

@maxnbk

@maxnbk maxnbk commented May 14, 2026

Copy link
Copy Markdown
Contributor

Closes #2045 and #1438 .
Partial but not complete fix for #1909 (Intentional, I feel that the work that was occurring in the gitbash PR would ultimately resolve this behavior?) .

Explanations:

os.path.realpath on Python 3.8+ Windows silently expands mapped drive letters to their underlying UNC paths. Because canonical_path calls realpath, FileSystemPackageRepository was storing a UNC self.location even when the caller supplied a drive-letter path. This caused make_resource_handle, which does a raw string comparison, to raise ResourceError on every drive-letter handle against a UNC-stored repository.

The fix:

On Windows, canonical_path now uses os.path.abspath instead of os.path.realpath. This preserves path style (drive-letter stays drive-letter, UNC stays UNC) and restores the pre-3.8 normalization behavior without a network-resolution side-effect. A make_resource_handle override is also added to the filesystem plugin to handle residual case or separator mismatches via canonical_path.

Extension of work:

A resolve_links_on_windows config flag (default False) has been added for sites that need some form of symlink/junction resolution on Windows. When enabled, a purpose-built _windows_realpath walks components of the path using os.readlink, resolving real symlinks without actually touching the drive root, in order to avoid the UNC-expansion side-effect entirely. Long-path support (\\?\ prefix handling) is included, with the relevant test skipped on hosts that do not have LongPathsEnabled=1 in the registry. Note: I'm aware that we need to move off of the winreg module, but since we have it for now, might as well use it.

Testing:

This was tested against an actual Drive-letter-map-with-matching-UNC-network-share .

==== EDIT ====

This PR has been extended with a broader real_path migration.

real_path() helper - A new utility function in filesystem.py that replaces os.path.realpath at all call sites where path-style stability is required.
On Windows, it is delegated to os.path.abspath to preserve the drive-letter style.
On all other platforms, it is delegated to os.path.realpath to preserve symlink resolution.
Importantly, no config-flag is required - the correct behavior is derived from the OS, not user preference. canonical_path from the original PR slice now calls real_path internally.

Call-site migration - All non-test os.path.realpath code that touched stored, serialized, or compared paths have been updated:

  • filesystem.py - canonical_path, is_subdirectory
  • resolved_context.py - _adjust_variant_for_bundling
  • serialise.py - open_file_for_write, load_from_file
  • suite.py - Suite.save, Suite.load
  • system.py - rez_bin_path
  • rezplugins/build_system/cmake.py - also corrects a pre-existing dead-code bug (os.path.abspath used as a truthiness check where os.path.isabs was intended)
  • utils/py_dist.py - egg-to-rez file path comparisons and copy destinations

Relationship to prior work - Several earlier attempts addressed this problem class but were either incomplete/partial, lacked tests, or relied on opt-in config flags:

==== END EDIT ====

==== EDIT 2 (2026-07-17) ====

This PR has been further extended with two enhancements to _windows_realpath (the opt-in resolve_links_on_windows code path):

Intermediate-directory symlink re-walk - The original component walk resolved symlinks only on the final path component. If an intermediate directory was itself a symlink (e.g., /a/b/target where /a/x), the intermediate symlink was not resolved. The walk has been converted from a linear for loop to a component-stack while loop: when a symlink resolves to an absolute target, the target's components are pushed back onto the front of the stack so that any symlinks in the target's prefix are re-walked transitively. A total_depth budget of 40 hops guards against symlink loops (the same limit os.path.realpath uses) and is enforced at both the inner resolution loop and the re-walk point.

Junction detection on Python 3.8–3.11 - os.path.islink() does not detect Windows junctions (mount points) on any Python version; it checks IO_REPARSE_TAG_SYMLINK only. os.path.isjunction() was added in Python 3.12 but is unavailable on 3.8–3.11, which rez still supports. A _is_link_or_junction helper now provides three-tier detection: os.path.islinkos.path.isjunction (3.12+) -> st_reparse_tag == 0xA0000003 fallback (the IO_REPARSE_TAG_MOUNT_POINT value from the Windows SDK, exposed via os.lstat().st_reparse_tag on Python 3.8+). This allows _windows_realpath to resolve junctions via os.readlink on all supported Python versions. Note: os.readlink on a junction returns the substitute name with a \??\ device-namespace prefix, which CPython internally converts to \\?\ - the existing extended-length prefix stripping handles this correctly.

==== END EDIT 2 ====

==== EDIT 3 2026-07-29 ====

Three rounds of adversarial self-review were performed against the branch, with findings verified against the codebase. Many valid findings were addressed, and some deliberate non-fixes are documented below with rationale.

Addressed:

  • serialize.py file_cache key mismatch. open_file_for_write used os-path.realpath(real_path(filepath)) for the cache key, which on Windows expands driver letters to UNC. load_from_fileusedreal_path(filepath), preserving the drive letter. Write stored under\server\share\foo.py, while read looked it up as N:\foo.py, creating a cache miss. Fixed by usingreal_pathfor the cache key (matching the read side) andresolve_path` separately for the physical write target.
  • Suite.save() re-introduced UNC expansion. os.path.realpath(path) was used before safe_rmtree, expanding N:\suites\mysuiteto\nas\studio\stuites\mysuite. Replaced with a new resolve_path()` function (see below).
  • test_save_through_symlink_uses_samefile would fail on Windows CI. On Windows default config, resolve_path returns abspath (no symlink resolution), so rmtree_arg would be the unresolved alias path. Fixed by mocking rez.suite.resolve_path to return the symlink target, so the test verifies rmtree-call-wiring identically on all platforms.
  • _add_extended_length_prefix didn't guard against already-prefixed paths. Calling it on \\?\C:\foo would produce \\?UNC\?\C:\foo. Added a case-sensitive already-prefixed guard.
  • samefile fallback did filesystem IO on every mismatch. A 50-package resolve with legacy UNC handle would do 150 network stats. Added class-level _location_aliases memoization on FileSystemPackageRepository. The first mismatch still hits, but subsequent lookups for the same path-pair are fast.
  • To the above, stale False results in the memoization cache. Only True results are cached. A False (genuine mismatch) is re-checked every time, so a repo moved away and back in the same process is detected correctly.
  • Useless test assertion: os.path.islink(alias) was always True (the test created the symlink), so the or short-circuited. Replaced with an assertNotIn("alias_suite", rmtree_arg) / assertIn("actual_suite", rmtree_arg).
  • resolve_path imported locally inside open_file_for_write. Moved to module-level alongside real_path.
  • Misleading Suite.save() comment, claiming that symlink resolution happens, but on Windows default config, it doesn't. Updated to accurately describe the Windows default-config behavior.

New function: resolve_path()

  • This was added to utils.filesystem.py to fill the gap between real_path (no symlink resolution) and os.path.realpath (UNC expansion on Windows). This resolves symlinks in a form-preserving way. It uses _windows_realpath when resolve_links_on_windows is configured true, otherwise abspath. Used by serialize.py (write target) and suite.py (rmtree target).

Identified but not addressed:

  • is_subdirectory / relpath case inconsistency: is_subdirectory now canonicalizes (lowercases on Windows), but the subsequent os.path.relpath in _adjust_variant_for_bundling uses real_path (preserves case). If paths differ only in case, is_subdirectory returns True but relpath could produce a different result. Rationale: Same-case is the norm in practice. The inconsistency only manifests with mixed-case paths that are otherwise identical. Not worth the complexity of threading canonical paths through relpath at this time. Recommend addressing only if a real-world case mismatch is reported.
  • Windows default config behavior with resolve_path does not resolve symlinks. When resolve_links_on_windows=False (default), resolve_path returns abspath (no symlink resolution). This means Suite.save() through a symlink alias will sever the symlink, and _windows_realpath long-path probe prefixing only helps when the flag is enabled. Rationale: This is the inherent tradeoff of the opt-in flag design. The alternative, always resolving symlinks on Windows, is exactly the behavior os.path.realpath provides, which this PR exists to avoid. The comment in Suite.save() now accurately documents this. Recommend leaving as-is. Users who need symlink resolution on Windows should enable the resolve_links_on_windows flag, and after this behavior is proven out, the flag could be removed and behavior defaulted-on. Documented in the config-setting comment.
  • probe vs candidate asymmetry for long paths. probe is prefixed for filesystem IO. candidate, used for os.path.join and os.path.dirname, is not. Rationale: os.path.join and dirname are pure string operations with no filesystem access, so the unprefixed candidate works fine. No current code does filesystem IO on candidate directly. Recommend leaving as-is. If future code adds filesystem IO on candidate, prefix it at that point.
  • make_resource_handle can't bridge drive-letter <-> UNC paths. The override canonicalizes via canonical_path, which preserves form. A drive-letter N:\Packages and UNC \\nas\studio\Packages canonicalize to different strings. Rationale: get_resource_from_handle (the resolution side) has the same samefile fallback that covers this. Handle creation is less critical. If the handle is created in the wrong form, the fallback catches it on resolution. Recommend leaving as acceptable for now. If handle-creation mismatches become a problem, add a samefile check to make_resource_handle as well.

New behaviors have reasonably comprehensive test coverage, and some test fixes are genuine test fixes for previous behavior that was effectively self-asserting.

==== END EDIT 3 ====

Disclosure:

This PR was AI-assisted with Claude Code, Sonnet 4.6, primarily for diagnostics, logic-tracing, documentation and edge-case verification.

This PR was AI-assisted with GLM-5.2, primarily for python-version-specific research and edge-case detection.

@maxnbk
maxnbk requested a review from a team as a code owner May 14, 2026 19:50
@codecov

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.46%. Comparing base (3a50d1b) to head (30f9a41).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/rez/utils/filesystem.py 90.42% 5 Missing and 4 partials ⚠️
src/rezplugins/package_repository/filesystem.py 57.14% 4 Missing and 5 partials ⚠️
src/rez/utils/py_dist.py 0.00% 4 Missing ⚠️
src/rezplugins/build_system/cmake.py 33.33% 1 Missing and 1 partial ⚠️
src/rez/rezconfig.py 0.00% 1 Missing ⚠️
src/rez/suite.py 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2104      +/-   ##
==========================================
+ Coverage   61.29%   61.46%   +0.16%     
==========================================
  Files         164      164              
  Lines       20568    20692     +124     
  Branches     3575     3605      +30     
==========================================
+ Hits        12607    12718     +111     
- Misses       7089     7092       +3     
- Partials      872      882      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maxnbk
maxnbk force-pushed the fix/windows-drive-letters-remap-to-unc-paths branch from ae4aeb2 to 92b2b07 Compare May 14, 2026 20:01
@maxnbk maxnbk added the devdays26 ASWF Dev Days 2026 label May 14, 2026
@maxnbk
maxnbk force-pushed the fix/windows-drive-letters-remap-to-unc-paths branch from 92b2b07 to 1681466 Compare May 14, 2026 20:36
@JeanChristopheMorinPerso

Copy link
Copy Markdown
Member

@maxnbk What's the relationship to #1856?

During the TSC, @instinct-vfx said that the expected default behavior is that the style should stay consistent based on how the repo was configured?

@JeanChristopheMorinPerso JeanChristopheMorinPerso added this to the Next milestone May 21, 2026
@cfxegbert

Copy link
Copy Markdown
Contributor

Can you please look at #1856. There are a few other places that realpath needs to be replaced.

@maxnbk

maxnbk commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

I'll take a look and see what else needs resolving. I may have to add some more tests to find all the edge-cases. Goal is to incorporate all solutions from all mismatched PRs, as I think there were approximately 3 all trying to solve the same problem with partial/incomplete solutions, and nobody got all corners.

Evidently that extends to me, so I'll re-evaluate and see what I need to do to extend.
Might be this weekend though. :)

@maxnbk
maxnbk force-pushed the fix/windows-drive-letters-remap-to-unc-paths branch 3 times, most recently from a561892 to 29a15fe Compare May 26, 2026 17:30
@JeanChristopheMorinPerso JeanChristopheMorinPerso modified the milestones: 3.4.0, Next May 30, 2026
@cfxegbert

Copy link
Copy Markdown
Contributor

@JeanChristopheMorinPerso Looks like this didn't get merged into the 3.4.0. What can we do to get this out there?

@JeanChristopheMorinPerso

Copy link
Copy Markdown
Member

Hey @cfxegbert, this was intentional. We had a few more questions to answer and decided to push this to a follow up release. We are planning to do a 3.4.1 release soon-ish to get this PR out.

When I reviewed the PR, I noticed that os.path.islink doesn't do what we thought it would on windows. At least I think.

@herronelou

Copy link
Copy Markdown
Contributor

adding comment here too for visibility, but we've been running rez 3.3 with a patch of #1856 in production since January and that has solved the issues we'd been having since changing python versions.

@maxnbk
maxnbk force-pushed the fix/windows-drive-letters-remap-to-unc-paths branch 2 times, most recently from 6598792 to 13ffe39 Compare July 17, 2026 21:07
@maxnbk
maxnbk force-pushed the fix/windows-drive-letters-remap-to-unc-paths branch 2 times, most recently from 740afb5 to 5067e5a Compare July 27, 2026 20:06
maxnbk added 9 commits July 27, 2026 21:06
…is remapped by rez to a UNC path

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…repository filesystem to ensure its consumption

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…-compatible symlink/junction-point resolution behavior

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…ion-behavior intended to produce realpath-like behavior

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…support in optional symlink-resolution for windows

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…ath-aware.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…d network windows path styles

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…ation

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
maxnbk added 16 commits July 27, 2026 21:06
…migration points

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…s as needed by py3.8+ realpath changes

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…lization, not for opening/storing/passing

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…roper use of canonical_path vs real_path in resolved_context

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…orm-specific symlink resolution

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…via resolved_context

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…stead.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…ndows_realpath

Tests that _windows_realpath resolves symlinks on intermediate directory
components, not just the final component. Covers the case where /a is a
symlink to /real_a and /a/b/link must resolve through /real_a/b/link.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
…ink to an absolute path

When a symlink resolves to an absolute target, the target's prefix
components may themselves contain symlinks that haven't been walked yet.
Convert the linear for-loop into a component-stack walk so that resolved
absolute paths are re-walked from their root, matching POSIX
os.path.realpath semantics for transitive symlink resolution.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
Add mock-based tests verifying that _windows_realpath resolves junctions
even when os.path.islink returns False (Python 3.8-3.11 behavior), and
that plain directories are not passed to os.readlink. Add a real-filesystem
test that creates a junction via mklink /J and verifies canonical_path
resolves it when resolve_links_on_windows=True.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
… 3.8-3.11

os.path.islink does not detect Windows junctions (mount points) on any
Python version.  Add _is_link_or_junction that checks islink first, then
falls back to os.path.isjunction (3.12+) or st_reparse_tag inspection
(3.8-3.11) to detect junctions.  os.readlink already handles both
symlinks and junctions, so no change is needed in the resolution logic
beyond the while-loop condition.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
Tests for:
- _strip_extended_length_prefix (DOS/UNC/mixed-case stripping, volume
  mount-point preservation)
- Suite.save() samefile() overwrite guard and symlink-aware rmtree
- get_resource_from_handle samefile() fallback for mixed path forms
- is_subdirectory symlink resolution (escape detection)
- _windows_realpath root-relative target drive inheritance (py3.8-3.12)
- _windows_realpath long-path candidate probe prefixing

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
Add _strip_extended_length_prefix (case-insensitive, preserves volume
mount-point paths) and _add_extended_length_prefix (guards against
already-prefixed paths). Replace inline prefix stripping in
_windows_realpath with these helpers.

_windows_realpath fixes:
- Prefix candidate with \?\ before filesystem probes when path exceeds
  MAX_PATH, so islink/isjunction/lstat/readlink succeed on hosts without
  LongPathsEnabled.
- Root-relative symlink targets (\targets\pkg) now inherit the link's
  drive instead of the process CWD drive (Python 3.8-3.12 where
  ntpath.isabs returns True for drive-less paths).

Add resolve_path(): resolves symlinks in a form-preserving way, filling
the gap between real_path (no symlink resolution) and os.path.realpath
(UNC expansion on Windows). Uses _windows_realpath when
resolve_links_on_windows is enabled, otherwise abspath.

Switch is_subdirectory from real_path to canonical_path so containment
checks resolve symlinks consistently with canonical_path.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
serialise.py: Use real_path for the file_cache key (matching
load_from_file's key) and resolve_path for the physical write target,
so atomic_write replaces the symlink's target, not the symlink itself.
Previously os.path.realpath expanded drive letters to UNC, causing a
cache key mismatch between write and read sides.

suite.py: Use os.path.samefile() for the overwrite guard (handles
symlinks, junctions, case-only differences). Use resolve_path before
safe_rmtree to avoid severing symlinks.

py_dist.py: Replace path.startswith(dist.location + os.sep) with
is_subdirectory() so symlink escapes are detected.

App.py: Replace os.path.realpath with real_path to avoid UNC expansion
in the GUI's stored path list.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
get_resource_from_handle now falls back to os.path.samefile() when
canonical_path comparison fails, bridging drive-letter vs UNC path forms
that refer to the same physical location. This fixes legacy contexts
serialized with UNC paths when the repo is configured with a drive letter
(or vice versa).

Positive samefile results are memoized in a class-level _location_aliases
dict to avoid repeated network I/O on every resource lookup. Negative
results are not cached, so a repo moved away and back in the same process
is re-checked correctly.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
Pre-existing in origin/main (PR AcademySoftwareFoundation#2108); surfaces with ruff 0.16.0
which flags unparenthesized implicit string concatenation in
collections (ISC004). Can be dropped once fixed upstream.

Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
@maxnbk
maxnbk force-pushed the fix/windows-drive-letters-remap-to-unc-paths branch from 5067e5a to 30f9a41 Compare July 27, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devdays26 ASWF Dev Days 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: Repository mismatch with mounted network drives

4 participants