diff --git a/src/rez/config.py b/src/rez/config.py index b1c5f67d6..4944aba17 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -415,6 +415,7 @@ def _parse_env_var(self, value): "suite_alias_prefix_char": Char, "cache_packages_path": OptionalStr, "package_definition_python_path": OptionalStr, + "resolve_links_on_windows": Bool, "tmpdir": OptionalStr, "context_tmpdir": OptionalStr, "default_shell": OptionalStr, diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index e3c738fd0..ef6210518 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -17,7 +17,7 @@ from rez.utils.formatting import columnise, PackageRequest, ENV_VAR_REGEX, \ header_comment, minor_header_comment from rez.utils.data_utils import deep_del -from rez.utils.filesystem import TempDirs, is_subdirectory, canonical_path +from rez.utils.filesystem import TempDirs, is_subdirectory, canonical_path, real_path from rez.utils.memcached import pool_memcached_connections from rez.utils.logging_ import print_debug, print_error, print_warning from rez.utils.which import which @@ -1896,8 +1896,8 @@ def _adjust_variant_for_bundling(cls, handle: dict, out: bool) -> None: if is_subdirectory(repo_path, bundle_path): vars_["location"] = os.path.relpath( - os.path.realpath(repo_path), - os.path.realpath(bundle_path) + real_path(repo_path), + real_path(bundle_path) ) # serializing in, make repo absolute diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 7fa820646..ed0119e71 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -125,6 +125,26 @@ # For further information, see :ref:`package-definition-sharing-code`. package_definition_python_path = None +# On Windows, whether to resolve symbolic links and junction points when +# normalising filesystem paths (primarily inside ``canonical_path``). +# +# When ``False`` (default), rez uses ``os.path.abspath``, which normalises +# separators and ``.``/``..`` components without following symlinks and without +# expanding mapped drive letters to their UNC equivalents. This preserves the +# path style supplied by the caller (drive-letter input, drive-letter output, +# UNC input, UNC output), effectively defaulting to pre-Python-3.8 behaviour of +# ``os.path.realpath`` on Windows. +# +# When ``True``, rez performs a component-by-component walk using +# ``os.path.islink`` / ``os.readlink``. This resolves actual symlinks and +# junction points without the drive-letter-to-UNC side-effect that +# ``os.path.realpath`` introduced in Python 3.8. Useful when package +# repositories are accessed through directory symlinks or junctions. +# +# This setting is a no-op on non-Windows platforms, which always resolve +# symlinks via ``os.path.realpath``. +resolve_links_on_windows = False + ############################################################################### # Extensions diff --git a/src/rez/serialise.py b/src/rez/serialise.py index 7cb61113e..d186ff63a 100644 --- a/src/rez/serialise.py +++ b/src/rez/serialise.py @@ -20,7 +20,7 @@ from rez.package_resources import package_rex_keys from rez.utils.scope import ScopeContext from rez.utils.sourcecode import SourceCode, early, late, include -from rez.utils.filesystem import TempDirs +from rez.utils.filesystem import TempDirs, real_path from rez.utils.data_utils import ModifyList from rez.exceptions import ResourceError, InvalidPackageError from rez.utils.memcached import memcached @@ -66,7 +66,7 @@ def open_file_for_write(filepath, mode=None): yield stream content = stream.getvalue() - filepath = os.path.realpath(filepath) + filepath = real_path(filepath) tmpdir = tmpdir_manager.mkdtemp() cache_filepath = os.path.join(tmpdir, os.path.basename(filepath)) @@ -123,7 +123,7 @@ def load_from_file(filepath: str, format_=FileFormat.py, update_data_callback=No Returns: dict: """ - filepath = os.path.realpath(filepath) + filepath = real_path(filepath) cache_filepath = file_cache.get(filepath) if cache_filepath: diff --git a/src/rez/suite.py b/src/rez/suite.py index 92e1e9776..b62880e18 100644 --- a/src/rez/suite.py +++ b/src/rez/suite.py @@ -5,6 +5,7 @@ from __future__ import annotations from rez.utils.execution import create_forwarding_script +from rez.utils.filesystem import real_path from rez.exceptions import SuiteError, ResolvedContextError from rez.resolved_context import ResolvedContext from rez.utils.data_utils import cached_property @@ -458,7 +459,7 @@ def save(self, path, verbose: bool = False): at `path`, then it will be overwritten. Otherwise, if `path` exists, an error is raised. """ - path = os.path.realpath(path) + path = real_path(path) if os.path.exists(path): if self.load_path and self.load_path == path: if verbose: @@ -528,7 +529,7 @@ def load(cls, path: str) -> Suite: raise SuiteError("Failed loading suite: %s" % str(e)) s = cls.from_dict(data) - s.load_path = os.path.realpath(path) + s.load_path = real_path(path) return s @classmethod diff --git a/src/rez/system.py b/src/rez/system.py index 310f4b803..7f2faf063 100644 --- a/src/rez/system.py +++ b/src/rez/system.py @@ -13,6 +13,7 @@ from rez.utils.platform_ import platform_ from rez.exceptions import RezSystemError from rez.utils.data_utils import cached_property +from rez.utils.filesystem import real_path class System(object): @@ -242,7 +243,7 @@ def rez_bin_path(self): validation_file = os.path.join(binpath, ".rez_production_install") if os.path.exists(validation_file): - return os.path.realpath(binpath) + return real_path(binpath) return None diff --git a/src/rez/tests/test_package_repository.py b/src/rez/tests/test_package_repository.py index 92d302e29..07e9277eb 100644 --- a/src/rez/tests/test_package_repository.py +++ b/src/rez/tests/test_package_repository.py @@ -1,41 +1,253 @@ -# SPDX-License-Identifier: Apache-2.0 -# Copyright Contributors to the Rez Project - - -""" -Test package repository plugin. -""" -import unittest - -from rezplugins.package_repository import filesystem -from rez.packages import create_package -from rez.tests.util import TestBase, TempdirMixin -from rez.utils.platform_ import platform_ - - -class TestFilesystemPackageRepository(TestBase, TempdirMixin): - @classmethod - def setUpClass(cls): - TempdirMixin.setUpClass() - - cls.settings = dict() - - @classmethod - def tearDownClass(cls): - TempdirMixin.tearDownClass() - - @unittest.skipIf(platform_.name != "windows", - "Skipping because this issue only affects case-insensitive platforms.") - def test_mismatching_case(self): - """Test that we get a caught PackageRepositoryError on case-insensitive platforms.""" - pool = filesystem.ResourcePool(cache_size=None) - pkg_repository = filesystem.FileSystemPackageRepository(self.root, pool) - - package = create_package("myTestPackage", data={}) - variant = next(package.iter_variants()) - case_mismatch_package = create_package("MyTestPackage", data={}) - case_mismatch_variant = next(case_mismatch_package.iter_variants()) - - pkg_repository._create_variant(variant, overrides={}) - with self.assertRaises(filesystem.PackageRepositoryError): - pkg_repository._create_variant(case_mismatch_variant, overrides={}) +# SPDX-License-Identifier: Apache-2.0 +# Copyright Contributors to the Rez Project + + +""" +Test package repository plugin. +""" +import unittest +import unittest.mock +from contextlib import contextmanager + +from rezplugins.package_repository import filesystem +from rez.exceptions import ResourceError +from rez.packages import create_package +from rez.tests.util import TestBase, TempdirMixin +from rez.utils.platform_ import platform_ +from rez.utils.resources import ResourceHandle + + +class TestFilesystemPackageRepository(TestBase, TempdirMixin): + @classmethod + def setUpClass(cls): + TempdirMixin.setUpClass() + + cls.settings = dict() + + @classmethod + def tearDownClass(cls): + TempdirMixin.tearDownClass() + + @unittest.skipIf(platform_.name != "windows", + "Skipping because this issue only affects case-insensitive platforms.") + def test_mismatching_case(self): + """Test that we get a caught PackageRepositoryError on case-insensitive platforms.""" + pool = filesystem.ResourcePool(cache_size=None) + pkg_repository = filesystem.FileSystemPackageRepository(self.root, pool) + + package = create_package("myTestPackage", data={}) + variant = next(package.iter_variants()) + case_mismatch_package = create_package("MyTestPackage", data={}) + case_mismatch_variant = next(case_mismatch_package.iter_variants()) + + pkg_repository._create_variant(variant, overrides={}) + with self.assertRaises(filesystem.PackageRepositoryError): + pkg_repository._create_variant(case_mismatch_variant, overrides={}) + + +# --------------------------------------------------------------------------- +# Helpers shared by the Windows path-form tests below. +# --------------------------------------------------------------------------- + +_MOCK_DRIVE_TO_UNC = {"n": "\\\\nas\\studio"} + + +def _unc_expanding_realpath(path: str) -> str: + """Simulate py3.8+ Windows os.path.realpath: N:\\ expansion to \\\\nas\\studio\\.""" + norm = path.replace("/", "\\") + if len(norm) >= 2 and norm[1] == ":": + drive = norm[0].lower() + rest = norm[2:] + if drive in _MOCK_DRIVE_TO_UNC: + return _MOCK_DRIVE_TO_UNC[drive] + rest + return path + + +@contextmanager +def _simulate_py38_unc_expansion(): + """Patch os.path.realpath and the filesystem plugin's platform_ reference + to reproduce the py3.8+ Windows drive-letter -> UNC expansion bug.""" + mock_plat = unittest.mock.Mock(spec=["has_case_sensitive_filesystem", "name"]) + mock_plat.has_case_sensitive_filesystem = False + mock_plat.name = "windows" + with unittest.mock.patch("os.path.realpath", side_effect=_unc_expanding_realpath): + with unittest.mock.patch.object(filesystem, "platform_", mock_plat): + yield + + +@unittest.skipIf( + platform_.name != "windows", + "Windows drive-letter / UNC path-consistency tests are Windows-only.", +) +class TestFilesystemRepoWindowsPathForms(TestBase, TempdirMixin): + """Verify that drive-letter and UNC path styles are preserved throughout + the repository lifecycle. + + Root cause of bugs #1438 / #2045: With os.path.realpath from py3.8 onward, + Windows silently converts mapped drive letters to a UNC equivalent. + canonical_path calls realpath, so FileSystemPackageRepository.__init__ + stores a UNC self.location even when the caller supplied a drive-letter + path. Subsequent make_resource_handle / get_resource_from_handle calls + that carry the original drive-letter path cause a ResourceError, due to + the apparent location mismatch. + """ + + @classmethod + def setUpClass(cls): + TempdirMixin.setUpClass() + cls.settings = {} + + @classmethod + def tearDownClass(cls): + TempdirMixin.tearDownClass() + + # ------------------------------------------------------------------ + # FileSystemPackageRepository.__init__ - self.location form + # ------------------------------------------------------------------ + + def test_repo_init_preserves_drive_letter_location(self): + """repo.location must maintain drive-letter input path to drive-letter output path. + + With the original realpath call, __init__ calls canonical_path, + converting N:\\ to \\\\nas\\studio\\. After the fix, canonical_path + on Windows must use abspath or configurably resolve symlinks, so + self.location stays as the caller supplied it. + """ + pool = filesystem.ResourcePool(cache_size=None) + with _simulate_py38_unc_expansion(): + repo = filesystem.FileSystemPackageRepository("N:\\packages", pool) + + self.assertFalse( + repo.location.startswith("\\\\"), + f"repo.location was unexpectedly expanded to UNC: {repo.location!r}", + ) + self.assertTrue( + repo.location.lower().startswith("n:\\"), + f"Expected drive-letter location starting with 'n:\\', got: {repo.location!r}", + ) + + def test_repo_init_preserves_unc_location(self): + """repo.location must maintain UNC input path to UNC output path.""" + pool = filesystem.ResourcePool(cache_size=None) + unc = "\\\\nas\\studio\\packages" + with _simulate_py38_unc_expansion(): + repo = filesystem.FileSystemPackageRepository(unc, pool) + + self.assertTrue( + repo.location.startswith("\\\\"), + f"UNC repo.location lost its UNC form: {repo.location!r}", + ) + + # ------------------------------------------------------------------ + # make_resource_handle - location comparison (base-class code path) + # ------------------------------------------------------------------ + + def test_make_resource_handle_drive_letter_no_mismatch(self): + """make_resource_handle must not raise when both the repo and the caller + use consistent drive-letter paths. + + __init__ used to UNC-expand self.location via realpath, so the base-class + make_resource_handle compared the caller's 'N:\\packages' against the + stored '\\\\nas\\studio\\packages' and raised ResourceError. + """ + pool = filesystem.ResourcePool(cache_size=None) + with _simulate_py38_unc_expansion(): + repo = filesystem.FileSystemPackageRepository("N:\\packages", pool) + try: + repo.make_resource_handle( + "filesystem.family", + location="N:\\packages", + name="mypkg", + ) + except ResourceError as exc: + self.fail( + f"make_resource_handle raised ResourceError for matching " + f"drive-letter paths: {exc}" + ) + + def test_make_resource_handle_unc_no_mismatch(self): + """make_resource_handle must not raise when both repo and caller use + consistent UNC paths.""" + pool = filesystem.ResourcePool(cache_size=None) + unc = "\\\\nas\\studio\\packages" + with _simulate_py38_unc_expansion(): + repo = filesystem.FileSystemPackageRepository(unc, pool) + try: + repo.make_resource_handle( + "filesystem.family", + location=unc, + name="mypkg", + ) + except ResourceError as exc: + self.fail( + f"make_resource_handle raised ResourceError for matching " + f"UNC paths: {exc}" + ) + + # ------------------------------------------------------------------ + # get_resource_from_handle - filesystem-plugin code path + # ------------------------------------------------------------------ + + def test_get_resource_from_handle_drive_letter_no_mismatch(self): + """get_resource_from_handle must not raise ResourceError when both + the handle and the repo use drive-letter pathing. + + The filesystem plugin overrides get_resource_from_handle and applies + canonical_path as a bridge for maintaining path-style consistency - + but that bridge cannot work if canonical_path itself expands the + drive-letter to UNC (making both sides UNC when the repo was created + with a drive-letter path, or vice-versa). + """ + pool = filesystem.ResourcePool(cache_size=None) + drive_letter_path = "N:\\packages" + + with _simulate_py38_unc_expansion(): + repo = filesystem.FileSystemPackageRepository(drive_letter_path, pool) + handle = ResourceHandle( + "filesystem.family", + { + "repository_type": "filesystem", + "location": drive_letter_path, + "name": "mypkg", + }, + ) + # Mock the pool's own get_resource_from_handle so we do not need + # actual packages on disk - we only want to exercise the location + # verification logic, not the resource loading. + with unittest.mock.patch.object( + repo.pool, "get_resource_from_handle", return_value=unittest.mock.Mock() + ): + try: + repo.get_resource_from_handle(handle, verify_repo=True) + except ResourceError as exc: + self.fail( + f"get_resource_from_handle raised ResourceError for a " + f"drive-letter handle against a drive-letter repo: {exc}" + ) + + def test_get_resource_from_handle_unc_no_mismatch(self): + """get_resource_from_handle must not raise ResourceError when both + handle and repo both use UNC paths.""" + pool = filesystem.ResourcePool(cache_size=None) + unc = "\\\\nas\\studio\\packages" + + with _simulate_py38_unc_expansion(): + repo = filesystem.FileSystemPackageRepository(unc, pool) + handle = ResourceHandle( + "filesystem.family", + { + "repository_type": "filesystem", + "location": unc, + "name": "mypkg", + }, + ) + with unittest.mock.patch.object( + repo.pool, "get_resource_from_handle", return_value=unittest.mock.Mock() + ): + try: + repo.get_resource_from_handle(handle, verify_repo=True) + except ResourceError as exc: + self.fail( + f"get_resource_from_handle raised ResourceError for a " + f"UNC handle against a UNC repo: {exc}" + ) diff --git a/src/rez/tests/test_utils_filesystem.py b/src/rez/tests/test_utils_filesystem.py index 99cbe6087..7f283a7d6 100644 --- a/src/rez/tests/test_utils_filesystem.py +++ b/src/rez/tests/test_utils_filesystem.py @@ -5,14 +5,17 @@ """ unit tests for 'rez.utils.filesystem' module """ +import os import os.path import tempfile +import unittest +import unittest.mock from rez.tests.util import TestBase from rez.tests.util import TempdirMixin from rez.utils import filesystem +from rez.utils.filesystem import canonical_path, real_path, _windows_realpath from rez.utils.platform_ import platform_ -import unittest class TestFileSystem(TestBase, TempdirMixin): @@ -86,3 +89,619 @@ def test_rename_file(self) -> None: filesystem.rename(src, dst) self.assertTrue(os.path.exists(dst)) self.assertFalse(os.path.exists(src)) + + +class TestIsSubdirectoryBasic(TestBase, TempdirMixin): + """Regression guards for is_subdirectory correctness. + + Uses real filesystem paths so the guards hold on every platform. + They protect against regressions when the internal path + normalisation inside is_subdirectory is changed - for example, + replacing os.path.realpath with real_path(). + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + TempdirMixin.setUpClass() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + TempdirMixin.tearDownClass() + + def test_direct_child_is_subdirectory(self): + parent = os.path.join(self.root, "parent") + child = os.path.join(parent, "child") + os.makedirs(child) + self.assertTrue(filesystem.is_subdirectory(child, parent)) + + def test_deep_nested_child_is_subdirectory(self): + parent = os.path.join(self.root, "deep_parent") + grandchild = os.path.join(parent, "a", "b", "c") + os.makedirs(grandchild) + self.assertTrue(filesystem.is_subdirectory(grandchild, parent)) + + def test_sibling_is_not_subdirectory(self): + base = os.path.join(self.root, "sib_base") + left = os.path.join(base, "left") + right = os.path.join(base, "right") + os.makedirs(left) + os.makedirs(right) + self.assertFalse(filesystem.is_subdirectory(left, right)) + + def test_unrelated_path_is_not_subdirectory(self): + a = os.path.join(self.root, "unrelated_a") + b = os.path.join(self.root, "unrelated_b") + os.makedirs(a) + os.makedirs(b) + self.assertFalse(filesystem.is_subdirectory(a, b)) + + +class TestCanonicalPathIdempotency(TestBase, TempdirMixin): + """canonical_path(canonical_path(x)) must equal canonical_path(x). + + This invariant guards against naive changes to canonical_path internals that + produce an asymmetry on a given platform + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + TempdirMixin.setUpClass() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + TempdirMixin.tearDownClass() + + def test_idempotent_on_existing_directory(self): + once = canonical_path(self.root, platform_) + twice = canonical_path(once, platform_) + self.assertEqual(once, twice) + + def test_idempotent_on_nested_path(self): + subdir = os.path.join(self.root, "a", "b") + os.makedirs(subdir) + once = canonical_path(subdir, platform_) + twice = canonical_path(once, platform_) + self.assertEqual(once, twice) + + +class TestRealPath(TestBase, TempdirMixin): + """Cross-platform regression guards for real_path(). + + real_path() is the form-stable absolute-path helper for file I/O and + path storage. Key contracts verified here: + + - relative paths become absolute + - case is never lowercased (unlike canonical_path on case-insensitive FSes) + - on non-Windows, symlinks are resolved (delegates to os.path.realpath) + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + TempdirMixin.setUpClass() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + TempdirMixin.tearDownClass() + + def test_result_is_absolute(self): + self.assertTrue(os.path.isabs(real_path(self.root))) + + def test_preserves_case(self): + """real_path() must not lowercase path components. + + canonical_path() lowercases on case-insensitive filesystems; + real_path() must not, so stored paths are not corrupted for + case-sensitive consumers (e.g. a Linux NFS mount from Windows). + """ + mixed = os.path.join(self.root, "FooBar") + os.makedirs(mixed) + result = real_path(mixed) + self.assertIn( + "FooBar", result, + "real_path() lowercased a path component: %r" % result, + ) + + @unittest.skipIf( + platform_.name == "windows", + "Symlink resolution via realpath is non-Windows only.", + ) + def test_resolves_symlinks_on_non_windows(self): + """On non-Windows real_path() resolves symlinks (os.path.realpath). + + Guards against real_path() being switched to abspath on all + platforms, which would silently break symlink resolution on + Linux/macOS. + """ + target = os.path.join(self.root, "real_target") + link = os.path.join(self.root, "sym_link") + os.makedirs(target) + os.symlink(target, link) + self.assertEqual(real_path(link), real_path(target)) + + +# Simulate py3.8+ Windows os.path.realpath: N:\ expands to \\nas\studio\ +_MOCK_DRIVE_TO_UNC = {"n": "\\\\nas\\studio"} + + +def _unc_expanding_realpath(path): + """Replicate the py3.8+ Windows realpath behaviour that converts a mapped + drive letter to the underlying UNC server path.""" + norm = path.replace("/", "\\") + if len(norm) >= 2 and norm[1] == ":": + drive = norm[0].lower() + rest = norm[2:] # e.g. "\\packages\\mypkg" + if drive in _MOCK_DRIVE_TO_UNC: + return _MOCK_DRIVE_TO_UNC[drive] + rest + return path + + +@unittest.skipIf( + platform_.name != "windows", + "Windows drive-letter / UNC path form tests are Windows-only.", +) +class TestCanonicalPathWindowsFormPreservation(TestBase): + """canonical_path must never change the *form* of a Windows path. + + A drive-letter path (N:\\...) must stay drive-letter. + A UNC path (\\\\server\\...) must stay UNC. + + Before the fix these tests fail because canonical_path calls + os.path.realpath, which on Python 3.8+ Windows silently converts + drive-letter paths to their underlying UNC equivalents. + """ + + def _mock_windows_platform(self): + """Return a mock Platform object that behaves like Windows.""" + m = unittest.mock.Mock(spec=["has_case_sensitive_filesystem", "name"]) + m.has_case_sensitive_filesystem = False + m.name = "windows" + return m + + # ------------------------------------------------------------------ + # Tests that must FAIL before the fix and PASS after. + # ------------------------------------------------------------------ + + def test_drive_letter_form_preserved(self): + """canonical_path on a drive-letter path must not return a UNC path. + + Fails today because os.path.realpath (py3.8+ Windows) expands + N:\\ -> \\\\nas\\studio\\, so canonical_path returns a UNC string. + """ + mock_plat = self._mock_windows_platform() + with unittest.mock.patch("os.path.realpath", side_effect=_unc_expanding_realpath): + result = canonical_path("N:\\packages\\mypkg", platform=mock_plat) + + self.assertFalse( + result.startswith("\\\\"), + f"canonical_path UNC-expanded a drive-letter path: {result!r}", + ) + self.assertTrue( + result.lower().startswith("n:\\"), + f"Expected drive-letter result starting with 'n:\\', got: {result!r}", + ) + + def test_drive_letter_form_preserved_forward_slashes(self): + """Same as above but input uses forward slashes (N:/packages).""" + mock_plat = self._mock_windows_platform() + with unittest.mock.patch("os.path.realpath", side_effect=_unc_expanding_realpath): + result = canonical_path("N:/packages/mypkg", platform=mock_plat) + + self.assertFalse( + result.startswith("\\\\"), + f"canonical_path UNC-expanded a drive-letter path: {result!r}", + ) + self.assertTrue( + result.lower().startswith("n:\\") or result.lower().startswith("n:/"), + f"Expected drive-letter result, got: {result!r}", + ) + + # ------------------------------------------------------------------ + # Tests that already PASS today and must continue to PASS after the fix. + # ------------------------------------------------------------------ + + def test_unc_form_preserved(self): + """canonical_path on a UNC path must return a UNC path.""" + mock_plat = self._mock_windows_platform() + unc = "\\\\nas\\studio\\packages\\mypkg" + # realpath on an already-UNC path returns the same UNC path unchanged. + with unittest.mock.patch("os.path.realpath", return_value=unc): + result = canonical_path(unc, platform=mock_plat) + + self.assertTrue( + result.startswith("\\\\"), + f"canonical_path changed UNC form unexpectedly: {result!r}", + ) + + def test_drive_letter_case_folded(self): + """canonical_path lowercases drive-letter paths on Windows (case-insensitive FS).""" + mock_plat = self._mock_windows_platform() + # Use a local temp path that abspath can handle so we are not + # depending on UNC expansion behaviour here. + with unittest.mock.patch("os.path.realpath", side_effect=lambda p: p): + result = canonical_path("C:\\Packages\\MyPkg", platform=mock_plat) + + self.assertEqual( + result, result.lower(), + f"canonical_path did not lowercase on case-insensitive platform: {result!r}", + ) + + +def _windows_longpaths_enabled() -> bool: + """Return True if the Windows LongPathsEnabled registry key is set. + + When False, the Win32 API cannot open paths longer than MAX_PATH (260 + chars) without an explicit ``\\?\\`` extended-length prefix. + """ + try: + import winreg + with winreg.OpenKey( + winreg.HKEY_LOCAL_MACHINE, + r"SYSTEM\CurrentControlSet\Control\FileSystem", + ) as key: + value, _ = winreg.QueryValueEx(key, "LongPathsEnabled") + return bool(value) + except OSError: + return False + + +@unittest.skipIf( + platform_.name != "windows", + "Windows symlink/junction resolution tests are Windows-only.", +) +class TestCanonicalPathWindowsSymlinkResolution(TestBase, TempdirMixin): + """canonical_path resolves symlinks/junctions on Windows when + resolve_links_on_windows=True, without expanding mapped drive letters.""" + + @classmethod + def setUpClass(cls): + TempdirMixin.setUpClass() + cls.settings = {"resolve_links_on_windows": True} + + # Probe for symlink capability. Dir symlinks require Developer + # Mode from Windows 10 Creators Update onward, or an elevated process. + probe_src = os.path.join(cls.root, "_symlink_probe_src") + probe_lnk = os.path.join(cls.root, "_symlink_probe_lnk") + os.makedirs(probe_src) + try: + os.symlink(probe_src, probe_lnk, target_is_directory=True) + os.unlink(probe_lnk) + except OSError: + raise unittest.SkipTest( + "Dir symlink creation not supported on this host " + "(enable Developer Mode or run as Admin)." + ) + finally: + if os.path.isdir(probe_src): + os.rmdir(probe_src) + + @classmethod + def tearDownClass(cls): + TempdirMixin.tearDownClass() + + def _mock_windows_platform(self): + m = unittest.mock.Mock(spec=["has_case_sensitive_filesystem", "name"]) + m.has_case_sensitive_filesystem = False + m.name = "windows" + return m + + def test_symlink_resolved(self): + """canonical_path follows a directory symlink when resolve_links_on_windows=True.""" + target = os.path.join(self.root, "real_target") + link = os.path.join(self.root, "link_to_target") + os.makedirs(target) + os.symlink(target, link, target_is_directory=True) + + result = canonical_path(link, platform=self._mock_windows_platform()) + self.assertEqual(result, target.lower()) + + def test_symlink_chain_resolved(self): + """canonical_path follows a chain of two symlinks.""" + target = os.path.join(self.root, "final_target") + mid = os.path.join(self.root, "mid_link") + link = os.path.join(self.root, "outer_link") + os.makedirs(target) + os.symlink(target, mid, target_is_directory=True) + os.symlink(mid, link, target_is_directory=True) + + result = canonical_path(link, platform=self._mock_windows_platform()) + self.assertEqual(result, target.lower()) + + def test_non_symlink_path_unchanged(self): + """canonical_path on a plain directory is stable with resolve_links_on_windows=True.""" + d = os.path.join(self.root, "plain_dir") + os.makedirs(d) + + result = canonical_path(d, platform=self._mock_windows_platform()) + self.assertEqual(result, d.lower()) + + def test_relative_symlink_resolved(self): + """canonical_path resolves a symlink whose target is a relative path.""" + target = os.path.join(self.root, "rel_target") + link_parent = os.path.join(self.root, "linkdir") + os.makedirs(target) + os.makedirs(link_parent) + link = os.path.join(link_parent, "rel_link") + os.symlink(os.path.join("..", "rel_target"), link, target_is_directory=True) + + result = canonical_path(link, platform=self._mock_windows_platform()) + self.assertEqual(result, target.lower()) + + def test_long_path_symlink_resolved(self): + """canonical_path resolves a symlink whose target exceeds MAX_PATH (260 + chars) on hosts with LongPathsEnabled set in the registry.""" + if not _windows_longpaths_enabled(): + self.skipTest( + "LongPathsEnabled not set in registry; skipping long-path test." + ) + + # self.root is typically ~60-70 chars; 220 'a' chars puts the full + # target path comfortably past the 260-char Win32 MAX_PATH limit. + target = os.path.join(self.root, "a" * 220) + link = os.path.join(self.root, "longpath_link") + os.makedirs(target) + os.symlink(target, link, target_is_directory=True) + + result = canonical_path(link, platform=self._mock_windows_platform()) + self.assertEqual(result, target.lower()) + + +@unittest.skipIf( + platform_.name != "windows", + "Windows _windows_realpath internal-branch tests are Windows-only.", +) +class TestWindowsRealpathInternals(TestBase): + """Unit tests for _windows_realpath edge-case branches. + + These use mocking so no real symlinks or network paths are required. + """ + + def test_readlink_extended_prefix_stripped(self): + """_windows_realpath strips a \\\\?\\ prefix returned by os.readlink.""" + link_path = "C:\\some\\link" + target_path = "C:\\real\\target" + + def _fake_islink(p): + return p == link_path + + with unittest.mock.patch("os.path.islink", side_effect=_fake_islink): + with unittest.mock.patch("os.readlink", return_value="\\\\?\\" + target_path): + result = _windows_realpath(link_path) + + self.assertEqual(result, os.path.normpath(target_path)) + + def test_readlink_unc_extended_prefix_stripped(self): + """_windows_realpath strips a \\\\?\\UNC\\ prefix returned by os.readlink.""" + link_path = "C:\\some\\link" + target_unc = "\\\\server\\share\\target" + + def _fake_islink(p): + return p == link_path + + with unittest.mock.patch("os.path.islink", side_effect=_fake_islink): + with unittest.mock.patch( + "os.readlink", + return_value="\\\\?\\UNC\\" + target_unc[2:], + ): + result = _windows_realpath(link_path) + + self.assertEqual(result, os.path.normpath(target_unc)) + + def test_symlink_depth_limit_terminates(self): + """_windows_realpath stops after 40 hops and returns without hanging.""" + with unittest.mock.patch("os.path.islink", return_value=True): + with unittest.mock.patch("os.readlink", return_value="C:\\loop"): + result = _windows_realpath("C:\\loop") + + self.assertIsInstance(result, str) + + +# Map two different drive letters to two different UNC roots. +# This simulates the py3.8+ realpath expansion that triggers the ValueError +# in os.path.relpath when the two paths end up on different UNC servers. +_MOCK_MULTI_DRIVE_TO_UNC = { + "n": "\\\\nas\\studio", + "m": "\\\\backup\\bundles", +} + + +def _multi_drive_unc_realpath(path): + """Expand two different drive letters to two different UNC roots.""" + norm = path.replace("/", "\\") + if len(norm) >= 2 and norm[1] == ":": + drive = norm[0].lower() + if drive in _MOCK_MULTI_DRIVE_TO_UNC: + return _MOCK_MULTI_DRIVE_TO_UNC[drive] + norm[2:] + return path + + +@unittest.skipIf( + platform_.name != "windows", + "Windows-specific real_path() call-site regression tests", +) +class TestRealPathCallSitesWindowsUNC(TestBase): + """tests that document where os.path.realpath must be replaced with + real_path() to avoid drive-letter -> UNC expansion on Windows. + + Each test demonstrates a concrete failure against direct use of + os.path.realpath. They represent call sites that need real_path + """ + + def test_bundle_relpath_does_not_raise_on_different_unc_roots(self): + """_adjust_variant_for_bundling must not raise ValueError when + os.path.realpath would have expanded drive-letter paths to UNC form. + + Exercises the real call site in resolved_context.py. is_subdirectory + is mocked True to force the relpath branch. With real_path() (abspath) + the drive-letter form is preserved and os.path.relpath succeeds. + """ + from rez.resolved_context import ResolvedContext + + bundle_path = "N:\\bundles\\mybundle" + repo_path = "N:\\bundles\\mybundle\\packages\\mypkg" + + handle = { + "variables": { + "repository_type": "filesystem", + "location": repo_path, + } + } + + with unittest.mock.patch.object( + ResolvedContext, "_get_bundle_path", return_value=bundle_path + ), unittest.mock.patch( + "rez.resolved_context.is_subdirectory", return_value=True + ): + try: + ResolvedContext._adjust_variant_for_bundling(handle, out=True) + except ValueError as e: + self.fail( + "_adjust_variant_for_bundling raised ValueError: %s\n" + "Ensure real_path() (not os.path.realpath) is used at " + "the os.path.relpath call site in resolved_context.py." % e + ) + + self.assertNotEqual( + handle["variables"]["location"], + repo_path, + "Expected location to be updated to a relative path, was unchanged.", + ) + + def test_bundle_relpath_must_not_use_canonical_path_regression(self): + """canonical_path lowercases the path, which corrupts case-sensitive + path components stored in bundle files. + + This is a regression guard: if the resolved_context.py call site is + fixed by substituting canonical_path() (which lowercases on Windows), + the stored relative path will have wrong capitalisation on Linux NFS + mounts that are case-sensitive. + + real_path() preserves the original casing, so it is the correct fix. + """ + from rez.utils.platform_ import Platform + + class _CaseInsensitivePlatform(Platform): + name = "windows" + + @property + def has_case_sensitive_filesystem(self): + return False + + mock_plat = _CaseInsensitivePlatform() + repo_path = "N:\\packages\\MyPkg\\1.0B" + bundle_path = "N:\\bundles\\MyBundle" + + relpath_via_canonical = os.path.relpath( + canonical_path(repo_path, mock_plat), + canonical_path(bundle_path, mock_plat), + ) + relpath_via_real = os.path.relpath( + real_path(repo_path), + real_path(bundle_path), + ) + + # canonical_path lowercases everything; real_path preserves case. + self.assertNotEqual( + relpath_via_canonical, + relpath_via_real, + "canonical_path and real_path produced the same relpath - " + "expected canonical_path to lowercase and real_path to preserve case.", + ) + self.assertIn( + "MyPkg", relpath_via_real, + "real_path() must preserve mixed-case package name in relpath.", + ) + self.assertIn( + "1.0B", relpath_via_real, + "real_path() must preserve mixed-case version string in relpath.", + ) + + def test_suite_save_does_not_raise_suiteerror_when_saving_over_loaded_suite(self): + """Suite.save() raises SuiteError when the save path differs from + suite.load_path due to UNC expansion. + + Suite.save() calls os.path.realpath(path) and compares it to + os.path.realpath(self.load_path). When both were originally the + same drive-letter path (e.g. N:\\suites\\mysuite), py3.8+ realpath + expands each independently to the same UNC path, so they still + compare equal and no error is raised - on the first call. + + The real failure occurs when the suite was loaded from a drive-letter + path but is saved to what looks like the same path: the UNC expansion + changes the string, so subsequent code that compares the stored + load_path against the UNC-expanded save path will see a mismatch. + + This test documents the fragility. Fix: replace os.path.realpath + with real_path() in Suite.save() and Suite.load(). + """ + from rez.suite import Suite, SuiteError + + drive_path = "N:\\suites\\mysuite" + suite = Suite() + suite.load_path = drive_path + + patch_context_names = unittest.mock.patch.object( + Suite, "context_names", + new_callable=unittest.mock.PropertyMock, + return_value=[], + ) + with unittest.mock.patch("os.path.realpath", side_effect=_unc_expanding_realpath), \ + unittest.mock.patch("os.path.exists", return_value=True), \ + unittest.mock.patch("os.makedirs") as mock_makedirs, \ + unittest.mock.patch("shutil.rmtree") as mock_rmtree, \ + unittest.mock.patch("builtins.open", unittest.mock.mock_open()), \ + patch_context_names, \ + unittest.mock.patch.object(suite, "to_dict", return_value={}), \ + unittest.mock.patch.object(suite, "get_tools", return_value={}): + try: + suite.save(drive_path) + except SuiteError as e: + self.fail("Suite.save() raised SuiteError: %s" % e) + + # Reachable only after the fix - verify the save actually ran: + # existing dir must be cleared before re-creating + mock_rmtree.assert_called_once() + # suite directory must be (re)created + mock_makedirs.assert_called() + + def test_suite_load_stores_drive_letter_load_path_not_unc(self): + """Suite.load() must store load_path in drive-letter form, not UNC. + + Currently FAILS because Suite.load() calls os.path.realpath(path) + at line 531, which on py3.8+ Windows silently expands a mapped + drive-letter path to its UNC equivalent. + + A UNC-form load_path causes Suite.save() to raise SuiteError when + the caller saves back to the same drive-letter path, because + ``os.path.realpath(drive_letter_save_path)`` returns the same UNC + string while ``self.load_path`` is also UNC - but if load_path was + set directly in drive-letter form (e.g. from user config or a + partially-fixed code path), the string comparison fails. + + Fix: replace os.path.realpath with real_path() in Suite.load(). + """ + from rez.suite import Suite + + drive_path = "N:\\suites\\mysuite" + + with unittest.mock.patch("os.path.realpath", side_effect=_unc_expanding_realpath), \ + unittest.mock.patch("os.path.exists", return_value=True), \ + unittest.mock.patch("os.path.isfile", return_value=True), \ + unittest.mock.patch("builtins.open", unittest.mock.mock_open(read_data="{}")), \ + unittest.mock.patch.object(Suite, "from_dict", return_value=Suite()): + s = Suite.load(drive_path) + + self.assertFalse( + s.load_path.startswith("\\\\"), + "Suite.load_path was UNC-expanded: %r. " + "Replace os.path.realpath with real_path() in Suite.load()." % s.load_path, + ) + self.assertTrue( + s.load_path.lower().startswith("n:\\"), + "Expected drive-letter form load_path, got: %r" % s.load_path, + ) diff --git a/src/rez/utils/filesystem.py b/src/rez/utils/filesystem.py index 6e614142a..2e7dd7e13 100644 --- a/src/rez/utils/filesystem.py +++ b/src/rez/utils/filesystem.py @@ -12,6 +12,7 @@ from contextlib import contextmanager from uuid import uuid4 import errno +import sys import weakref import atexit import posixpath @@ -328,8 +329,8 @@ def make_tmp_name(name): def is_subdirectory(path_a, path_b) -> bool: """Returns True if `path_a` is a subdirectory of `path_b`.""" - path_a = os.path.realpath(path_a) - path_b = os.path.realpath(path_b) + path_a = real_path(path_a) + path_b = real_path(path_b) try: relative = os.path.relpath(path_a, path_b) except ValueError: @@ -489,15 +490,83 @@ def to_posixpath(path: str): return posixpath.sep.join(path.split(ntpath.sep)) +_WINDOWS_MAX_PATH = 259 # Win32 MAX_PATH minus the null terminator + + +def _windows_realpath(path: str) -> str: + """Resolve symlinks and junctions on Windows without expanding mapped drives. + + ``os.path.realpath`` on Python 3.8+ Windows uses ``GetFinalPathNameByHandle`` + which expands mapped drive letters (e.g. ``N:\\``) to their underlying UNC + server paths. This function resolves only actual filesystem symlinks and + junction points, walking the path component-by-component so that the drive + root (drive-letter or UNC prefix) is never touched. + """ + path = os.path.normpath(os.path.abspath(path)) + drive, rest = os.path.splitdrive(path) + # Preserve the root separator so UNC paths keep their leading "\\" and + # drive-letter paths keep their "\". + result = drive + (os.sep if rest.startswith(os.sep) else "") + for part in rest.lstrip(os.sep).split(os.sep): + if not part: + continue + candidate = os.path.join(result, part) + depth = 0 + while os.path.islink(candidate) and depth < 40: + target = os.readlink(candidate) + # os.readlink on Windows may return an extended-length path + # (\\?\C:\... or \\?\UNC\server\share\...). Strip the prefix so + # we can work with ordinary path strings. + if target.startswith("\\\\?\\UNC\\"): + target = "\\\\" + target[8:] + elif target.startswith("\\\\?\\"): + target = target[4:] + if not os.path.isabs(target): + target = os.path.join(os.path.dirname(candidate), target) + # For paths that exceed MAX_PATH, re-add the extended-length + # prefix before calling abspath so the Win32 API (GetFullPathNameW) + # can handle the length on hosts without LongPathsEnabled in the + # registry. We strip the prefix again afterwards so the rest of + # the walk operates on ordinary path strings. + if len(target) > _WINDOWS_MAX_PATH: + if target.startswith("\\\\"): + target = "\\\\?\\UNC\\" + target[2:] + else: + target = "\\\\?\\" + target + candidate = os.path.abspath(target) + if candidate.startswith("\\\\?\\UNC\\"): + candidate = "\\\\" + candidate[8:] + elif candidate.startswith("\\\\?\\"): + candidate = candidate[4:] + candidate = os.path.normpath(candidate) + else: + candidate = os.path.normpath(os.path.abspath(target)) + depth += 1 + result = candidate + return result + + def canonical_path(path: str, platform=None): - r""" Resolves symlinks, and formats filepath. + """Return a normalised path suitable for identity comparison. + + Resolves symlinks (on non-Windows), lowercases on case-insensitive + filesystems, and normalises separators. Two paths that refer to the + same location will compare equal (``==``) after canonicalisation. - Resolves symlinks, lowercases if filesystem is case-insensitive, - formats filepath using slashes appropriate for platform. + Use this when the question is "do these two paths refer to the same + thing?" - e.g. deduplicating package repository locations, or + checking whether a loaded resource path matches a stored location. + + Do not use this when you need a path to open a file, pass to an + external tool, or store in a bundle/context that may be read on + another platform - the lowercasing it applies on case-insensitive + filesystems mutates the string and can break case-sensitive consumers + (e.g. a Linux NFS mount accessed from a Windows client). Use + :func:`real_path` for those cases instead. Args: path (str): Filepath being formatted - platform (rez.utils.platform\_.Platform): Indicates platform path is being + platform (rez.utils.platform_.Platform): Indicates platform path is being formatted for. Defaults to current platform. Returns: @@ -506,7 +575,22 @@ def canonical_path(path: str, platform=None): if platform is None: platform = platform_ - path = os.path.normpath(os.path.realpath(path)) + # On Windows, os.path.realpath from py3.8 onwards silently converts drive + # lettered paths to their UNC equivalents (N:\ -> \\server\share\). + # We check sys.platform rather than the platform argument because this is + # an OS-level behaviour of os.path.realpath, not a user-configurable choice. + if sys.platform == "win32": + # Lazy import avoids a circular dependency (config imports filesystem). + from rez.config import config # noqa: PLC0415 + if config.resolve_links_on_windows: + path = _windows_realpath(path) + else: + # Default: abspath preserves the caller's path form (drive-letter + # stays drive-letter, UNC stays UNC) and restores the pre-3.8 + # behaviour where realpath on Windows was equivalent to abspath. + path = os.path.normpath(os.path.abspath(path)) + else: + path = os.path.normpath(os.path.realpath(path)) if not platform.has_case_sensitive_filesystem: return path.lower() @@ -514,6 +598,39 @@ def canonical_path(path: str, platform=None): return path +def real_path(path: str) -> str: + """Return an absolute, form-stable path for file I/O and path operations. + + Resolves relative segments and (on non-Windows) symlinks, while + preserving the form of the input path - a drive-letter path stays + drive-letter; a UNC path stays UNC. + + Use this when you need a stable absolute path to use rather than to + compare - opening a file, building a cache key, passing to + ``os.path.relpath``, returning a path to the user, or storing a path + in a serialised format that may be read on another platform. + + Do not use this when the question is "do these two paths refer to + the same filesystem location?" - use :func:`canonical_path` instead, + which lowercases on case-insensitive filesystems so that equality + checks work regardless of capitalisation. + + Justification: ``os.path.realpath`` on Windows (Python 3.8+) + silently expands mapped drive-letter paths to their UNC + equivalents (``N:\\`` -> ``\\\\server\\share\\``). That expansion + breaks ``os.path.relpath`` across mismatched UNC roots + (``ValueError``), corrupts cache keys, and mutates capitalisation + in stored paths. ``os.path.abspath`` avoids this while still + making paths absolute and normalising separators. + + Returns: + str: Absolute path with the same drive-letter / UNC form as input. + """ + if sys.platform == "win32": + return os.path.normpath(os.path.abspath(path)) + return os.path.realpath(path) + + def encode_filesystem_name(input_str: str): """Encodes an arbitrary unicode string to a generic filesystem-compatible non-unicode filename. diff --git a/src/rez/utils/py_dist.py b/src/rez/utils/py_dist.py index 49bbcd244..0b1ab4bbf 100644 --- a/src/rez/utils/py_dist.py +++ b/src/rez/utils/py_dist.py @@ -6,6 +6,7 @@ Functions for converting python distributions to rez packages. """ from rez.exceptions import RezSystemError +from rez.utils.filesystem import real_path import importlib.metadata as importlib_metadata import shutil import sys @@ -194,7 +195,7 @@ def convert_dist(name, dest_path, make_variant: bool = True, ignore_dirs=None, files = set() for file in installed_files: path = os.path.join(eggpath, file) - path = os.path.realpath(path) + path = real_path(path) if os.path.isfile(path) and path.startswith(dist.location + os.sep): dir_ = os.path.dirname(path) @@ -209,7 +210,7 @@ def convert_dist(name, dest_path, make_variant: bool = True, ignore_dirs=None, def _dst(p): dst = os.path.relpath(p, dist.location) dst = os.path.join(root_path, dst) - return os.path.realpath(dst) + return real_path(dst) for dir_ in dirs: dst_dir = _dst(dir_) diff --git a/src/rezplugins/build_system/cmake.py b/src/rezplugins/build_system/cmake.py index 6fee115b2..e11623054 100644 --- a/src/rezplugins/build_system/cmake.py +++ b/src/rezplugins/build_system/cmake.py @@ -16,6 +16,7 @@ from rez.utils.platform_ import platform_ from rez.config import config from rez.utils.which import which +from rez.utils.filesystem import real_path from rez.vendor.schema.schema import Or from rez.shells import create_shell import argparse @@ -150,9 +151,9 @@ def _pr(s) -> None: # execute cmake within the build env _pr("Executing: %s" % ' '.join(cmd)) - if not os.path.abspath(build_path): + if not os.path.isabs(build_path): build_path = os.path.join(self.working_dir, build_path) - build_path = os.path.realpath(build_path) + build_path = real_path(build_path) actions_callback = functools.partial( self._add_build_actions, diff --git a/src/rezplugins/package_repository/filesystem.py b/src/rezplugins/package_repository/filesystem.py index 5d8e8276e..f4d098529 100644 --- a/src/rezplugins/package_repository/filesystem.py +++ b/src/rezplugins/package_repository/filesystem.py @@ -810,6 +810,20 @@ def _info(msg, *nargs) -> None: return num_removed + def make_resource_handle(self, resource_key, **variables): + # Normalize the caller-supplied location so the base-class raw string + # comparison succeeds, even when the caller uses a different case, or + # an equivalent path form (e.g. mixed-case drive letter on Windows). + # + # https://github.com/AcademySoftwareFoundation/rez/issues/2045 + # + location = variables.get("location") + if location is not None and location != self.location: + norm = canonical_path(location, platform_) + if norm == self.location: + variables["location"] = self.location + return super().make_resource_handle(resource_key, **variables) + def get_resource_from_handle(self, resource_handle, verify_repo: bool = True): if verify_repo: repository_type = resource_handle.variables.get("repository_type")