Merge pull request #6373 from ByteB4rb1e/bugfix/5978

Refactor virtualenv `bin/` / `Scripts/` path resolution using `sysconfig` mechanism

Author: matteius

news/6737.bugfix.rst

@@ -0,0 +1,22 @@
+# Improved virtualenv scripts path resolution
+
+## Summary
+
+This PR refactors the logic for determining virtual environment script paths
+by leveraging ``sysconfig``'s built-in mechanisms. By removing
+platform-dependent logic, ``pipenv`` now offers enhanced compatibility with
+POSIX-like environments, including Cygwin and MinGW. The fix also mitigates
+execution inconsistencies in non-native Windows environments, improving
+portability across platforms.
+
+## Motivation
+
+The original logic for determining the scripts path was unable to handle the
+deviations of MSYS2 MinGW CPython identifying as ``nt`` platform, yet using a
+POSIX ``{base}/bin`` path, instead of ``{base}/Scripts``.
+
+## Changes
+
+Removed custom logic for determining virtualenv scripts path in favor of
+retrieving the basename of the path string returned by
+``sysconfig.get_path('scripts')```.

pipenv/environment.py

@@ -26,6 +26,7 @@
 from pipenv.utils.indexes import prepare_pip_source_args
 from pipenv.utils.processes import subprocess_run
 from pipenv.utils.shell import temp_environ
+from pipenv.utils.virtualenv import virtualenv_scripts_dir
 from pipenv.vendor.importlib_metadata.compat.py39 import normalized_name
 from pipenv.vendor.pythonfinder.utils import is_in_path
 
@@ -246,16 +247,12 @@ def script_basedir(self) -> str:
     @property
     def python(self) -> str:
         """Path to the environment python"""
-        if self._python is not None:
-            return self._python
-        if os.name == "nt" and not self.is_venv:
-            py = Path(self.prefix).joinpath("python").absolute().as_posix()
-        else:
-            py = Path(self.script_basedir).joinpath("python").absolute().as_posix()
-        if not py:
-            py = Path(sys.executable).as_posix()
-        self._python = py
-        return py
+        if self._python is None:
+            self._python = (
+                (virtualenv_scripts_dir(self.prefix) / "python").absolute().as_posix()
+            )
+
+        return self._python
 
     @cached_property
     def sys_path(self) -> list[str]:

pipenv/project.py

@@ -69,6 +69,7 @@
     system_which,
 )
 from pipenv.utils.toml import cleanup_toml, convert_toml_outline_tables
+from pipenv.utils.virtualenv import virtualenv_scripts_dir
 from pipenv.vendor import plette, tomlkit
 
 try:
@@ -411,11 +412,19 @@ def is_venv_in_project(self) -> bool:
     @property
     def virtualenv_exists(self) -> bool:
         venv_path = Path(self.virtualenv_location)
+
+        scripts_dir = self.virtualenv_scripts_location
+
         if venv_path.exists():
-            if os.name == "nt":
-                activate_path = venv_path / "Scripts" / "activate.bat"
+
+            # existence of active.bat is dependent on the platform path prefix
+            # scheme, not platform itself. This handles special cases such as
+            # Cygwin/MinGW identifying as 'nt' platform, yet preferring a
+            # 'posix' path prefix scheme.
+            if scripts_dir.name == "Scripts":
+                activate_path = scripts_dir / "activate.bat"
             else:
-                activate_path = venv_path / "bin" / "activate"
+                activate_path = scripts_dir / "activate"
             return activate_path.is_file()
 
         return False
@@ -612,6 +621,10 @@ def virtualenv_src_location(self) -> Path:
         loc.mkdir(parents=True, exist_ok=True)
         return loc
 
+    @property
+    def virtualenv_scripts_location(self) -> Path:
+        return virtualenv_scripts_dir(self.virtualenv_location)
+
     @property
     def download_location(self) -> Path:
         if self._download_location is None:
@@ -1422,10 +1435,10 @@ def proper_case_section(self, section):
     def finders(self):
         from .vendor.pythonfinder import Finder
 
-        scripts_dirname = "Scripts" if os.name == "nt" else "bin"
-        scripts_dir = Path(self.virtualenv_location) / scripts_dirname
         finders = [
-            Finder(path=str(scripts_dir), global_search=gs, system=False)
+            Finder(
+                path=str(self.virtualenv_scripts_location), global_search=gs, system=False
+            )
             for gs in (False, True)
         ]
         return finders
@@ -1463,12 +1476,14 @@ def _which(self, command, location=None, allow_global=False):
         is_python = command in ("python", Path(sys.executable).name, version_str)
 
         if not allow_global:
+            scripts_location = virtualenv_scripts_dir(location_path)
+
             if os.name == "nt":
-                p = find_windows_executable(str(location_path / "Scripts"), command)
+                p = find_windows_executable(str(scripts_location), command)
                 # Convert to Path object if it's a string
                 p = Path(p) if isinstance(p, str) else p
             else:
-                p = location_path / "bin" / command
+                p = scripts_location / command
         elif is_python:
             p = Path(sys.executable)
         else:

pipenv/routines/shell.py

@@ -5,6 +5,7 @@
 
 from pipenv.utils.project import ensure_project
 from pipenv.utils.shell import cmd_list_to_shell, system_which
+from pipenv.utils.virtualenv import virtualenv_scripts_dir
 from pipenv.vendor import click
 
 
@@ -60,7 +61,6 @@ def do_run(project, command, args, python=False, pypi_mirror=None):
 
     Args are appended to the command in [scripts] section of project if found.
     """
-    from pathlib import Path
 
     from pipenv.cmdparse import ScriptEmptyError
 
@@ -79,12 +79,7 @@ def do_run(project, command, args, python=False, pypi_mirror=None):
         # Get the exact string representation of virtualenv_location
         virtualenv_location = str(project.virtualenv_location)
 
-        # Use pathlib for path construction but convert back to string
-        from pathlib import Path
-
-        virtualenv_path = Path(virtualenv_location)
-        bin_dir = "Scripts" if os.name == "nt" else "bin"
-        new_path = str(virtualenv_path / bin_dir)
+        new_path = str(virtualenv_scripts_dir(virtualenv_location))
 
         # Update PATH
         paths = path.split(os.pathsep)

pipenv/utils/virtualenv.py

@@ -3,6 +3,7 @@
 import re
 import shutil
 import sys
+import sysconfig
 from pathlib import Path
 
 from pipenv import environments, exceptions
@@ -13,6 +14,20 @@
 from pipenv.utils.shell import find_python, shorten_path
 
 
+def virtualenv_scripts_dir(b):
+    """returns a system-dependent scripts path
+
+    POSIX environments (including Cygwin/MinGW64) will result in
+    `{base}/bin/`, native Windows environments will result in
+    `{base}/Scripts/`.
+
+    :param b: base path
+    :type b: str
+    :returns: pathlib.Path
+    """
+    return Path(f"{b}/{Path(sysconfig.get_path('scripts')).name}")
+
+
 def warn_in_virtualenv(project):
     # Only warn if pipenv isn't already active.
     if environments.is_in_virtualenv() and not project.s.is_quiet():

tests/unit/test_utils.py

@@ -1,10 +1,11 @@
 import os
+import sys
 from unittest import mock
 
 import pytest
 
 from pipenv.exceptions import PipenvUsageError
-from pipenv.utils import dependencies, indexes, internet, shell, toml
+from pipenv.utils import dependencies, indexes, internet, shell, toml, virtualenv
 
 # Pipfile format <-> requirements.txt format.
 DEP_PIP_PAIRS = [
@@ -547,3 +548,17 @@ def test_is_env_truthy_does_not_exisxt(self, monkeypatch):
         name = "ZZZ"
         monkeypatch.delenv(name, raising=False)
         assert shell.is_env_truthy(name) is False
+
+    @pytest.mark.utils
+    # substring search in version handles special-case of MSYS2 MinGW CPython
+    # https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-python/0017-sysconfig-treat-MINGW-builds-as-POSIX-builds.patch#L24
+    @pytest.mark.skipif(os.name != "nt" or "GCC" in sys.version, reason="Windows test only")
+    def test_virtualenv_scripts_dir_nt(self):
+        """
+        """
+        assert str(virtualenv.virtualenv_scripts_dir('foobar')) == 'foobar\\Scripts'
+
+    @pytest.mark.utils
+    @pytest.mark.skipif(os.name == "nt" and "GCC" not in sys.version, reason="POSIX test only")
+    def test_virtualenv_scripts_dir_posix(self):
+        assert str(virtualenv.virtualenv_scripts_dir('foobar')) == 'foobar/bin'