Fix cross-drive commonpath on Windows. (#2709)

More work towards #2658.

Author: jsirois

pex/cache/dirs.py

@@ -7,7 +7,7 @@
 import os
 
 from pex.common import safe_rmtree
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.enum import Enum
 from pex.exceptions import production_assert
 from pex.orderedset import OrderedSet
@@ -897,7 +897,7 @@ def venv_dir(self):
         if not self.interpreter.is_venv:
             return None
         cached_venv_root = CacheDir.VENVS.path()
-        if cached_venv_root != commonpath((cached_venv_root, self.interpreter.prefix)):
+        if cached_venv_root != safe_commonpath((cached_venv_root, self.interpreter.prefix)):
             return None
         head, contents_hash = os.path.split(self.interpreter.prefix)
         pex_hash = os.path.basename(head)

pex/cache/root.py

@@ -5,7 +5,7 @@
 
 import os.path
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
@@ -23,6 +23,6 @@
 def path(expand_user=True):
     # type: (bool) -> str
 
-    if expand_user or _USER_DIR != commonpath((_USER_DIR, _CACHE_DIR)):
+    if expand_user or _USER_DIR != safe_commonpath((_USER_DIR, _CACHE_DIR)):
         return _CACHE_DIR
     return os.path.join("~", os.path.relpath(_CACHE_DIR, _USER_DIR))

pex/cli/commands/lock.py

@@ -18,7 +18,7 @@
 from pex.cli.command import BuildTimeCommand
 from pex.commands.command import JsonMixin, OutputMixin
 from pex.common import pluralize, safe_delete, safe_mkdtemp, safe_open
-from pex.compatibility import commonpath, shlex_quote
+from pex.compatibility import safe_commonpath, shlex_quote
 from pex.dependency_configuration import DependencyConfiguration
 from pex.dist_metadata import (
     Constraint,
@@ -307,7 +307,11 @@ def sync(
             if to_unlink:
                 to_unlink_by_pin[
                     (distribution.metadata.project_name, distribution.metadata.version)
-                ] = [file for file in to_unlink if abs_venv_dir == commonpath((abs_venv_dir, file))]
+                ] = [
+                    file
+                    for file in to_unlink
+                    if abs_venv_dir == safe_commonpath((abs_venv_dir, file))
+                ]
         if confirm and to_unlink_by_pin:
             for (project_name, version), files in to_unlink_by_pin.items():
                 print(project_name, version, ":", file=sys.stderr)
@@ -325,7 +329,7 @@ def sync(
                 safe_delete(file)
                 parent_dirs.add(os.path.dirname(file))
             for parent_dir in sorted(parent_dirs, reverse=True):
-                if not os.listdir(parent_dir) and abs_venv_dir == commonpath(
+                if not os.listdir(parent_dir) and abs_venv_dir == safe_commonpath(
                     (abs_venv_dir, parent_dir)
                 ):
                     os.rmdir(parent_dir)

pex/compatibility.py

@@ -13,6 +13,7 @@
 from abc import ABCMeta
 from sys import version_info as sys_version_info
 
+from pex.os import WINDOWS
 from pex.typing import TYPE_CHECKING, cast
 
 if TYPE_CHECKING:
@@ -243,6 +244,12 @@ def commonpath(paths):
                     paths="\n".join(paths)
                 )
             )
+        if WINDOWS and len({os.path.splitdrive(path)[0] for path in paths}) > 1:
+            raise ValueError(
+                "Can't mix paths from different drives, given:\n{paths}".format(
+                    paths="\n".join(paths)
+                )
+            )
 
         def components(path):
             # type: (Text) -> Iterable[Text]
@@ -274,6 +281,18 @@ def append(piece):
         return os.path.join(*prefix)
 
 
+def safe_commonpath(paths):
+    # type: (Sequence[Text]) -> Optional[Text]
+
+    # Handle the ValueError cases outlined in os.path.safe_commonpath and return None instead:
+    # > Raise ValueError if paths contain both absolute and relative pathnames, if paths are on
+    # > different drives, or if paths is empty.
+    try:
+        return commonpath(paths)
+    except ValueError:
+        return None
+
+
 if PY3:
     from shlex import quote as _shlex_quote
 else:

pex/pep_427.py

@@ -15,7 +15,7 @@
 
 from pex import pex_warnings, windows
 from pex.common import is_pyc_file, iter_copytree, open_zip, safe_open, touch
-from pex.compatibility import commonpath, get_stdout_bytes_buffer
+from pex.compatibility import commonpath, get_stdout_bytes_buffer, safe_commonpath
 from pex.dist_metadata import CallableEntryPoint, Distribution, ProjectNameAndVersion
 from pex.enum import Enum
 from pex.executables import chmod_plus_x
@@ -215,7 +215,8 @@ def record_files(
             names=[
                 name
                 for name in zf.namelist()
-                if not name.endswith("/") and data_rel_path != commonpath((data_rel_path, name))
+                if not name.endswith("/")
+                and data_rel_path != safe_commonpath((data_rel_path, name))
             ],
         )
         if os.path.isdir(data_path):

pex/pex_builder.py

@@ -26,7 +26,7 @@
     safe_mkdtemp,
     safe_open,
 )
-from pex.compatibility import commonpath, to_bytes
+from pex.compatibility import safe_commonpath, to_bytes
 from pex.compiler import Compiler
 from pex.dist_metadata import Distribution, DistributionType, MetadataError
 from pex.enum import Enum
@@ -456,7 +456,7 @@ def _precompile_source(self):
             if path.endswith(".py")
             # N.B.: Some of our vendored code does not work with all versions of Python we support;
             # so we just skip compiling it.
-            and vendored_dir != commonpath((vendored_dir, path))
+            and vendored_dir != safe_commonpath((vendored_dir, path))
         ]
 
         compiler = Compiler(self.interpreter)

pex/repl/pex_repl.py

@@ -12,7 +12,7 @@
 
 from pex.cli_util import prog_path
 from pex.common import pluralize
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.dist_metadata import Distribution
 from pex.layout import Layout
 from pex.os import is_exe
@@ -155,7 +155,7 @@ def _create_repl_data(
     layout = Layout.identify_original(env.PEX or pex)
     pex_root = os.path.abspath(env.PEX_ROOT)
     venv = venv or pex_info.venv
-    venv_pex = venv and pex_root == commonpath((os.path.abspath(pex), pex_root))
+    venv_pex = venv and pex_root == safe_commonpath((os.path.abspath(pex), pex_root))
     pex_prog_path = prog_path(env.PEX if venv_pex else pex)
     pex_cli_run_in_use = _pex_cli_run_in_use()
     ephemeral = "ephemeral " if pex_cli_run_in_use else ""

pex/venv/virtualenv.py

@@ -16,7 +16,7 @@
 
 from pex.atomic_directory import AtomicDirectory, atomic_directory
 from pex.common import safe_mkdir, safe_open
-from pex.compatibility import commonpath, get_stdout_bytes_buffer
+from pex.compatibility import commonpath, get_stdout_bytes_buffer, safe_commonpath
 from pex.dist_metadata import Distribution, find_distributions
 from pex.enum import Enum
 from pex.executor import Executor
@@ -373,7 +373,7 @@ def create_atomic(
             link_target = os.readlink(abs_path)
             if not os.path.isabs(link_target):
                 continue
-            if virtualenv.bin_dir == commonpath((virtualenv.bin_dir, link_target)):
+            if virtualenv.bin_dir == safe_commonpath((virtualenv.bin_dir, link_target)):
                 rel_dst = os.path.relpath(link_target, virtualenv.bin_dir)
                 TRACER.log(
                     "Replacing absolute symlink {src} -> {dst} with relative symlink".format(

tests/integration/cli/commands/test_issue_2059.py

@@ -3,7 +3,7 @@
 import os.path
 import sys
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.typing import TYPE_CHECKING
 from testing import PY310, ensure_python_interpreter, run_pex_command
 from testing.cli import run_pex3
@@ -51,4 +51,4 @@ def test_pypy_impl_tag_handling(tmpdir):
         python=python,
     )
     result.assert_success()
-    assert pex_root == commonpath([pex_root, os.path.realpath(result.output.strip())])
+    assert pex_root == safe_commonpath([pex_root, os.path.realpath(result.output.strip())])

tests/integration/cli/commands/test_issue_2098.py

@@ -6,7 +6,7 @@
 from textwrap import dedent
 
 from pex.common import safe_open, touch
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.interpreter import PythonInterpreter
 from testing import run_pex_command
 from testing.cli import run_pex3
@@ -101,4 +101,4 @@ def test_missing_download_lock_analysis_handling(
 
     data = json.loads(result.output)
     assert "1.3.24" == data["version"]
-    assert pex_root == commonpath([pex_root, data["file"]])
+    assert pex_root == safe_commonpath([pex_root, data["file"]])

tests/integration/cli/commands/test_issue_2268.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.resolve.resolver_configuration import ResolverVersion
 from pex.typing import TYPE_CHECKING
 from testing import run_pex_command
@@ -71,4 +71,4 @@ def test_abi_none_locking(
         ]
     )
     result.assert_success()
-    assert pex_root == commonpath([pex_root, result.output.strip()])
+    assert pex_root == safe_commonpath([pex_root, result.output.strip()])

tests/integration/cli/commands/test_venv_create.py

@@ -15,7 +15,7 @@
 from pex import dist_metadata
 from pex.cli.commands.venv import InstallLayout
 from pex.common import open_zip, safe_open
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.dist_metadata import Distribution
 from pex.interpreter import PythonInterpreter
 from pex.pep_440 import Version
@@ -124,7 +124,7 @@ def assert_venv(
     _, stdout, _ = venv.interpreter.execute(
         args=["-c", "import cowsay, os; print(os.path.realpath(cowsay.__file__))"]
     )
-    assert venv.site_packages_dir == commonpath([venv.site_packages_dir, stdout.strip()])
+    assert venv.site_packages_dir == safe_commonpath([venv.site_packages_dir, stdout.strip()])
 
     _, stdout, _ = venv.interpreter.execute(args=["-m", "cowsay", "--version"])
     assert "5.0" == stdout.strip()
@@ -188,7 +188,7 @@ def assert_flat(
     sys_path_entry = dest if layout is InstallLayout.FLAT else "{dest}.zip".format(dest=dest)
     env = make_env(PYTHONPATH=sys_path_entry)
 
-    assert sys_path_entry == commonpath(
+    assert sys_path_entry == safe_commonpath(
         [
             sys_path_entry,
             subprocess.check_output(
@@ -330,7 +330,7 @@ def assert_deps_only(
 ):
     # type: (...) -> None
 
-    assert expected_prefix == commonpath(
+    assert expected_prefix == safe_commonpath(
         [
             expected_prefix,
             subprocess.check_output(

tests/integration/resolve/test_issue_1918.py

@@ -6,7 +6,7 @@
 
 import pytest
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.dist_metadata import Requirement
 from pex.pip.version import PipVersion, PipVersionValue
 from pex.requirements import VCS
@@ -149,4 +149,4 @@ def test_redacted_requirement_handling(
         ]
     )
     result.assert_success()
-    assert pex_root == commonpath([pex_root, result.output.strip()])
+    assert pex_root == safe_commonpath([pex_root, result.output.strip()])

tests/integration/test_excludes.py

@@ -15,7 +15,7 @@
 import pytest
 
 from pex.common import safe_open
-from pex.compatibility import PY2, commonpath
+from pex.compatibility import PY2, safe_commonpath
 from pex.dist_metadata import Requirement
 from pex.executor import Executor
 from pex.pep_503 import ProjectName
@@ -489,7 +489,7 @@ def run_pex(*args):
     )
     assert 42 == data.pop("foo")
     bar_module_path = data.pop("bar")
-    assert venv.site_packages_dir == commonpath(
+    assert venv.site_packages_dir == safe_commonpath(
         (venv.site_packages_dir, bar_module_path)
     ), bar_module_path
     assert not data

tests/integration/test_integration.py

@@ -19,7 +19,7 @@
 from pex import targets
 from pex.cache.dirs import CacheDir, InterpreterDir
 from pex.common import environment_as, safe_mkdir, safe_open, safe_rmtree, temporary_dir, touch
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.dist_metadata import Distribution, Requirement, is_wheel
 from pex.fetcher import URLFetcher
 from pex.fs import safe_symlink
@@ -1675,7 +1675,7 @@ def run_isort_pex(pex_python=None):
                 pex_hash=pex_hash,
                 has_interpreter_constraints=False,
             )
-        assert expected_venv_home == commonpath([pex_interpreter, expected_venv_home])
+        assert expected_venv_home == safe_commonpath([pex_interpreter, expected_venv_home])
         return pex_interpreter
 
     isort_pex_interpreter1 = run_isort_pex()

tests/integration/test_interpreter.py

@@ -3,7 +3,7 @@
 import os.path
 import sys
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.interpreter import PythonInterpreter
 from pex.typing import TYPE_CHECKING
 from pex.variables import ENV
@@ -23,7 +23,7 @@ def assert_no_isolated_leak(python):
         with ENV.patch(PEX_ROOT=pex_root), PythonInterpreter._cleared_memory_cache():
             interpreter = PythonInterpreter.from_binary(python)
             assert not any(
-                pex_root == commonpath((pex_root, entry)) for entry in interpreter.sys_path
+                pex_root == safe_commonpath((pex_root, entry)) for entry in interpreter.sys_path
             ), (
                 "The cached interpreter info for {python} contains leaked entries:\n"
                 "{entries}".format(python=python, entries="\n".join(interpreter.sys_path))

tests/integration/test_issue_1336.py

@@ -3,7 +3,7 @@
 
 import os
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.typing import TYPE_CHECKING
 from testing import PY310, ensure_python_interpreter, run_pex_command, subprocess
 
@@ -34,6 +34,6 @@ def test_pip_leak(tmpdir):
         python=python,
     )
     result.assert_success()
-    assert os.path.realpath(pex_root) == commonpath(
+    assert os.path.realpath(pex_root) == safe_commonpath(
         [os.path.realpath(pex_root), result.output.strip()]
     )

tests/integration/test_issue_1949.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.typing import TYPE_CHECKING
 from testing import IntegResults, built_wheel, run_pex_command
 from testing.cli import run_pex3
@@ -59,11 +59,11 @@ def create_pex_from_lock(*additional_args):
 
     result = create_pex_from_lock()
     result.assert_success()
-    assert pex_root == commonpath((pex_root, result.output.strip()))
+    assert pex_root == safe_commonpath((pex_root, result.output.strip()))
 
     result = create_pex_from_lock("not_boto===2.49.0a1")
     result.assert_success()
-    assert pex_root == commonpath((pex_root, result.output.strip()))
+    assert pex_root == safe_commonpath((pex_root, result.output.strip()))
 
     result = create_pex_from_lock("not_boto===2.49a1")
     result.assert_failure()
@@ -101,7 +101,7 @@ def create_pex_from_pex_repository(requirement):
 
     result = create_pex_from_pex_repository("not_boto===2.49.0a1")
     result.assert_success()
-    assert pex_root == commonpath((pex_root, result.output.strip()))
+    assert pex_root == safe_commonpath((pex_root, result.output.strip()))
 
     result = create_pex_from_pex_repository("not_boto===2.49a1")
     result.assert_failure()

tests/integration/test_issue_2087.py

@@ -5,7 +5,7 @@
 import os.path
 from textwrap import dedent
 
-from pex.compatibility import commonpath
+from pex.compatibility import safe_commonpath
 from pex.typing import TYPE_CHECKING
 from testing import run_pex_command
 
@@ -47,4 +47,4 @@ def test_long_wheel_names(tmpdir):
 
     data = json.loads(result.output)
     assert "3.16.0" == data["version"]
-    assert pex_root == commonpath((pex_root, data["path"]))
+    assert pex_root == safe_commonpath((pex_root, data["path"]))