From 16ecabb18d4249daae1fc60e2e67573c1a2efe78 Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Wed, 15 Jan 2025 12:24:57 +0100 Subject: [PATCH 1/4] Resolve base_executable based on path if it ultimately points to the same file as the executable being run once symlinks are resolved --- Lib/test/test_getpath.py | 20 ++++++++++---------- Modules/getpath.py | 20 ++++++++++++++------ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_getpath.py b/Lib/test/test_getpath.py index f86df9d0d03485..3b34c155168189 100644 --- a/Lib/test/test_getpath.py +++ b/Lib/test/test_getpath.py @@ -400,7 +400,7 @@ def test_venv_non_installed_zip_path_posix(self): ns.add_known_file("/path/to/non-installed/lib/python9.8/os.py") ns.add_known_dir("/path/to/non-installed/lib/python9.8/lib-dynload") ns.add_known_file("/venv/pyvenv.cfg", [ - r"home = /path/to/non-installed" + r"home = /path/to/non-installed/bin" ]) expected = dict( executable="/venv/bin/python", @@ -837,28 +837,28 @@ def test_PYTHONHOME_in_venv(self): ns = MockPosixNamespace( argv0="/venv/bin/python", PREFIX="/usr", - ENV_PYTHONHOME="/pythonhome", + ENV_PYTHONHOME="/pythonhome-p:/pythonhome-e", ) # Setup venv ns.add_known_xfile("/venv/bin/python") ns.add_known_file("/venv/pyvenv.cfg", [ r"home = /usr/bin" ]) - # Seutup PYTHONHOME - ns.add_known_file("/pythonhome/lib/python9.8/os.py") - ns.add_known_dir("/pythonhome/lib/python9.8/lib-dynload") + # Setup PYTHONHOME + ns.add_known_file("/pythonhome-p/lib/python9.8/os.py") + ns.add_known_dir("/pythonhome-e/lib/python9.8/lib-dynload") expected = dict( executable="/venv/bin/python", prefix="/venv", exec_prefix="/venv", - base_prefix="/pythonhome", - base_exec_prefix="/pythonhome", + base_prefix="/pythonhome-p", + base_exec_prefix="/pythonhome-e", module_search_paths_set=1, module_search_paths=[ - "/pythonhome/lib/python98.zip", - "/pythonhome/lib/python9.8", - "/pythonhome/lib/python9.8/lib-dynload", + "/pythonhome-p/lib/python98.zip", + "/pythonhome-p/lib/python9.8", + "/pythonhome-e/lib/python9.8/lib-dynload", ], ) actual = getpath(ns, expected) diff --git a/Modules/getpath.py b/Modules/getpath.py index be2210345afbda..ed7e4a969cff4c 100644 --- a/Modules/getpath.py +++ b/Modules/getpath.py @@ -382,15 +382,23 @@ def search_up(prefix, *landmarks, test=isfile): # the base installation — isn't set (eg. when embedded), try to find # it in 'home'. if not base_executable: - # First try to resolve symlinked executables, since that may be - # more accurate than assuming the executable in 'home'. + _home_executable = joinpath(executable_dir, basename(executable)) + _realpath_executable = None try: - base_executable = realpath(executable) - if base_executable == executable: - # No change, so probably not a link. Clear it and fall back - base_executable = '' + _realpath_executable = realpath(executable) + + # If the realpath of the executable identified in home is the same as + # the realpath of the venv's executable use the (potentially not fully + # resolved via realpath) path from home. + if realpath(_home_executable) == _realpath_executable: + base_executable = _home_executable except OSError: pass + if not base_executable: + if _realpath_executable and _realpath_executable != executable: + # Try to resolve symlinked executables, since that may be + # more accurate than assuming the executable in 'home'. + base_executable = _realpath_executable if not base_executable: base_executable = joinpath(executable_dir, basename(executable)) # It's possible "python" is executed from within a posix venv but that From 55410611eed3bbccd74e1b5982d28e08e5d689aa Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Thu, 16 Jan 2025 12:13:38 +0100 Subject: [PATCH 2/4] Add tests for venv and getpath in the case where Python is symlinked --- Lib/test/test_getpath.py | 50 ++++++++++++++++++++++++++++++++++++++++ Lib/test/test_venv.py | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/Lib/test/test_getpath.py b/Lib/test/test_getpath.py index 3b34c155168189..567360e2367890 100644 --- a/Lib/test/test_getpath.py +++ b/Lib/test/test_getpath.py @@ -864,6 +864,56 @@ def test_PYTHONHOME_in_venv(self): actual = getpath(ns, expected) self.assertEqual(expected, actual) + def test_venv_w_symlinked_base_executable(self): + """ + If we symlink the base executable, and point to it via home in pyvenv.cfg, + we should have it as sys.executable (and sys.prefix should be the resolved location) + """ + ns = MockPosixNamespace( + argv0="/venv/bin/python3", + PREFIX="/some/_internal/prefix", + base_exec_prefix="/foo/bar" + ) + # Setup venv + ns.add_known_xfile("/venv/bin/python3") + ns.add_known_xfile("/usr/local/bin/python3") + ns.add_known_xfile("/some/_internal/prefix/bin/python3") + + ns.add_known_file("/venv/pyvenv.cfg", [ + # The published based executable location is /usr/local/bin - we don't want to + # expose /some/internal/directory (this location can change under our feet) + r"home = /usr/local/bin" + ]) + ns.add_known_link("/venv/bin/python3", "/usr/local/bin/python3") + ns.add_known_link("/usr/local/bin/python3", "/some/_internal/prefix/bin/python3") + + ns.add_known_file("/some/_internal/prefix/lib/python9.8/os.py") + ns.add_known_dir("/some/_internal/prefix/lib/python9.8/lib-dynload") + + # Put a file completely outside of /usr/local to validate that the issue + # in https://github.com/python/cpython/issues/106045 is resolved. + ns.add_known_dir("/usr/lib/python9.8/lib-dynload") + + expected = dict( + executable="/venv/bin/python3", + prefix="/venv", + exec_prefix="/venv", + base_prefix="/some/_internal/prefix", + base_exec_prefix="/some/_internal/prefix", + # It is important to maintain the link to the original executable, as this + # is used when creating a new virtual environment (which should also have home + # set to /usr/local/bin to avoid bleeding the internal path to the venv) + base_executable="/usr/local/bin/python3", + module_search_paths_set=1, + module_search_paths=[ + "/some/_internal/prefix/lib/python98.zip", + "/some/_internal/prefix/lib/python9.8", + "/some/_internal/prefix/lib/python9.8/lib-dynload", + ], + ) + actual = getpath(ns, expected) + self.assertEqual(expected, actual) + # ****************************************************************************** DEFAULT_NAMESPACE = dict( diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index 6e23097deaf221..d413ed2179d763 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -752,6 +752,50 @@ def test_zippath_from_non_installed_posix(self): out, err = check_output(cmd) self.assertTrue(zip_landmark.encode() in out) + @unittest.skipIf(os.name == 'nt', 'not relevant on Windows') + @requireVenvCreate + def test_venv_from_venv_with_symlink(self): + """ + Test that we can create a venv from a venv using a base Python that is + a symlink, without exposing the location of the symlink in pyvenv.cfg. + """ + rmtree(self.env_dir) + public_prefix = os.path.realpath(tempfile.mkdtemp()) + self.addCleanup(rmtree, public_prefix) + public_bin_dir = os.path.join(public_prefix, 'bin') + os.mkdir(public_bin_dir) + public_exe = os.path.join(public_bin_dir, self.exe) + os.symlink(sys.executable, public_exe) + cmd = [public_exe, + "-m", + "venv", + "--without-pip", + "--without-scm-ignore-files", + self.env_dir] + + subprocess.check_call(cmd) + + # Verify that we don't expose the internal prefix to the first venv config: + contents = (pathlib.Path(self.env_dir) / 'pyvenv.cfg').read_text() + self.assertIn(f'home = {public_bin_dir}\n', contents) + + # Now use the venv to make another, and assert that the internal env is + # also not exposed there. + second_venv = os.path.realpath(tempfile.mkdtemp()) + self.addCleanup(rmtree, second_venv) + + cmd = [os.path.join(self.env_dir, 'bin', 'python3'), + "-m", + "venv", + "--without-pip", + "--without-scm-ignore-files", + second_venv] + + subprocess.check_call(cmd) + + contents = (pathlib.Path(second_venv) / 'pyvenv.cfg').read_text() + self.assertIn(f'home = {public_bin_dir}\n', contents) + @requireVenvCreate def test_activate_shell_script_has_no_dos_newlines(self): """ From 8dca58a73b0af64e1b16467d4c2e99a25e803e71 Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Thu, 16 Jan 2025 12:24:32 +0100 Subject: [PATCH 3/4] Include a news fragment and use the default executable name to determine if it is a good idea to use the home value for base_executable determination --- Lib/test/test_getpath.py | 23 +++++++++---------- ...-01-16-12-24-13.gh-issue-128670.Mtt-w_.rst | 2 ++ Modules/getpath.py | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-12-24-13.gh-issue-128670.Mtt-w_.rst diff --git a/Lib/test/test_getpath.py b/Lib/test/test_getpath.py index 567360e2367890..35a7d02354ef69 100644 --- a/Lib/test/test_getpath.py +++ b/Lib/test/test_getpath.py @@ -870,32 +870,31 @@ def test_venv_w_symlinked_base_executable(self): we should have it as sys.executable (and sys.prefix should be the resolved location) """ ns = MockPosixNamespace( - argv0="/venv/bin/python3", + argv0="/venv/bin/python9", PREFIX="/some/_internal/prefix", - base_exec_prefix="/foo/bar" ) # Setup venv - ns.add_known_xfile("/venv/bin/python3") - ns.add_known_xfile("/usr/local/bin/python3") - ns.add_known_xfile("/some/_internal/prefix/bin/python3") + ns.add_known_xfile("/venv/bin/python9") + ns.add_known_xfile("/usr/local/bin/python9") + ns.add_known_xfile("/some/_internal/prefix/bin/python9") ns.add_known_file("/venv/pyvenv.cfg", [ # The published based executable location is /usr/local/bin - we don't want to - # expose /some/internal/directory (this location can change under our feet) + # expose /some/_internal/prefix (this location can change under our feet) r"home = /usr/local/bin" ]) - ns.add_known_link("/venv/bin/python3", "/usr/local/bin/python3") - ns.add_known_link("/usr/local/bin/python3", "/some/_internal/prefix/bin/python3") + ns.add_known_link("/venv/bin/python9", "/usr/local/bin/python9") + ns.add_known_link("/usr/local/bin/python9", "/some/_internal/prefix/bin/python9") ns.add_known_file("/some/_internal/prefix/lib/python9.8/os.py") ns.add_known_dir("/some/_internal/prefix/lib/python9.8/lib-dynload") - # Put a file completely outside of /usr/local to validate that the issue - # in https://github.com/python/cpython/issues/106045 is resolved. + # Put a file completely outside /usr/local to validate that the issue + # in gh-106045 is resolved. ns.add_known_dir("/usr/lib/python9.8/lib-dynload") expected = dict( - executable="/venv/bin/python3", + executable="/venv/bin/python9", prefix="/venv", exec_prefix="/venv", base_prefix="/some/_internal/prefix", @@ -903,7 +902,7 @@ def test_venv_w_symlinked_base_executable(self): # It is important to maintain the link to the original executable, as this # is used when creating a new virtual environment (which should also have home # set to /usr/local/bin to avoid bleeding the internal path to the venv) - base_executable="/usr/local/bin/python3", + base_executable="/usr/local/bin/python9", module_search_paths_set=1, module_search_paths=[ "/some/_internal/prefix/lib/python98.zip", diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-12-24-13.gh-issue-128670.Mtt-w_.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-12-24-13.gh-issue-128670.Mtt-w_.rst new file mode 100644 index 00000000000000..d2e9bd0fbabf87 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-12-24-13.gh-issue-128670.Mtt-w_.rst @@ -0,0 +1,2 @@ +sys._base_executable is no longer always a fully resolved symlink when +running from a venv. The home value in pyvenv.cfg is used to determine the base executable when its realpath is the same as the running executable's realpath. A venv created from another will no longer expose the realpath of the executable in this scenario. diff --git a/Modules/getpath.py b/Modules/getpath.py index ed7e4a969cff4c..ce5d746fc31b7c 100644 --- a/Modules/getpath.py +++ b/Modules/getpath.py @@ -382,7 +382,7 @@ def search_up(prefix, *landmarks, test=isfile): # the base installation — isn't set (eg. when embedded), try to find # it in 'home'. if not base_executable: - _home_executable = joinpath(executable_dir, basename(executable)) + _home_executable = joinpath(executable_dir, DEFAULT_PROGRAM_NAME) _realpath_executable = None try: _realpath_executable = realpath(executable) From 4ce731f9bd7817a97d17403a3d8d5992b033476a Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Tue, 21 Jan 2025 15:54:18 +0100 Subject: [PATCH 4/4] Refine the getpath implementation for pyvenv case, and get the tests to pass --- Lib/test/test_embed.py | 17 ++++------------- Lib/test/test_venv.py | 26 ++++++++++++++++++++++---- Modules/getpath.py | 18 +++++++++--------- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 1b55cd156d759d..9860284a1c381c 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -1618,13 +1618,7 @@ def test_init_pyvenv_cfg(self): tempfile.TemporaryDirectory() as pyvenv_home: ver = sys.version_info - if not MS_WINDOWS: - lib_dynload = os.path.join(pyvenv_home, - sys.platlibdir, - f'python{ver.major}.{ver.minor}{ABI_THREAD}', - 'lib-dynload') - os.makedirs(lib_dynload) - else: + if MS_WINDOWS: lib_folder = os.path.join(pyvenv_home, 'Lib') os.makedirs(lib_folder) # getpath.py uses Lib\os.py as the LANDMARK @@ -1639,9 +1633,7 @@ def test_init_pyvenv_cfg(self): print("include-system-site-packages = false", file=fp) paths = self.module_search_paths() - if not MS_WINDOWS: - paths[-1] = lib_dynload - else: + if MS_WINDOWS: paths = [ os.path.join(tmpdir, os.path.basename(paths[0])), pyvenv_home, @@ -1650,7 +1642,7 @@ def test_init_pyvenv_cfg(self): executable = self.test_exe base_executable = os.path.join(pyvenv_home, os.path.basename(executable)) - exec_prefix = pyvenv_home + exec_prefix = sys.base_exec_prefix config = { 'base_prefix': sysconfig.get_config_var("prefix"), 'base_exec_prefix': exec_prefix, @@ -1659,16 +1651,15 @@ def test_init_pyvenv_cfg(self): 'base_executable': base_executable, 'executable': executable, 'module_search_paths': paths, + 'use_frozen_modules': bool(not support.Py_DEBUG), } if MS_WINDOWS: config['base_prefix'] = pyvenv_home config['stdlib_dir'] = os.path.join(pyvenv_home, 'Lib') - config['use_frozen_modules'] = bool(not support.Py_DEBUG) else: # cannot reliably assume stdlib_dir here because it # depends too much on our build. But it ought to be found config['stdlib_dir'] = self.IGNORE_CONFIG - config['use_frozen_modules'] = bool(not support.Py_DEBUG) env = self.copy_paths_by_env(config) self.check_all_configs("test_init_compat_config", config, diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index d413ed2179d763..0de5ff87eb24ec 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -684,7 +684,6 @@ def test_zippath_from_non_installed_posix(self): shutil.copy2(sys.executable, bindir) libdir = os.path.join(non_installed_dir, platlibdir, self.lib[1]) os.makedirs(libdir) - landmark = os.path.join(libdir, "os.py") abi_thread = "t" if sysconfig.get_config_var("Py_GIL_DISABLED") else "" stdlib_zip = f"python{sys.version_info.major}{sys.version_info.minor}{abi_thread}" zip_landmark = os.path.join(non_installed_dir, @@ -748,9 +747,9 @@ def test_zippath_from_non_installed_posix(self): subprocess.check_call(cmd, env=child_env) # Now check the venv created from the non-installed python has # correct zip path in pythonpath. - cmd = [self.envpy(), '-S', '-c', 'import sys; print(sys.path)'] + cmd = [self.envpy(), '-S', '-c', "import sys; print('\\n'.join(sys.path))"] out, err = check_output(cmd) - self.assertTrue(zip_landmark.encode() in out) + self.assertIn(zip_landmark, out.decode()) @unittest.skipIf(os.name == 'nt', 'not relevant on Windows') @requireVenvCreate @@ -764,8 +763,9 @@ def test_venv_from_venv_with_symlink(self): self.addCleanup(rmtree, public_prefix) public_bin_dir = os.path.join(public_prefix, 'bin') os.mkdir(public_bin_dir) - public_exe = os.path.join(public_bin_dir, self.exe) + public_exe = os.path.join(public_bin_dir, 'python3') os.symlink(sys.executable, public_exe) + underlying_prefix = sys.base_prefix cmd = [public_exe, "-m", "venv", @@ -796,6 +796,24 @@ def test_venv_from_venv_with_symlink(self): contents = (pathlib.Path(second_venv) / 'pyvenv.cfg').read_text() self.assertIn(f'home = {public_bin_dir}\n', contents) + # Now validate the important sys values. + venv2_sys, _ = check_output( + [os.path.join(second_venv, 'bin', 'python3'), + '-S', '-c', + "import sys; print(f'{sys._base_executable=}\\n{sys.base_exec_prefix=}\\n{sys.base_prefix=}')"], + encoding='utf8', + ) + self.assertEqual( + '\n'.join([ + # The base executable should be the public one, while the exec_prefix can be the + # internal one (and the same as the original interpreter's base_prefix). + f"sys._base_executable='{public_exe}'", + f"sys.base_exec_prefix='{underlying_prefix}'", + f"sys.base_prefix='{underlying_prefix}'", + ]), + venv2_sys.strip(), + ) + @requireVenvCreate def test_activate_shell_script_has_no_dos_newlines(self): """ diff --git a/Modules/getpath.py b/Modules/getpath.py index ce5d746fc31b7c..5d814636018abf 100644 --- a/Modules/getpath.py +++ b/Modules/getpath.py @@ -383,22 +383,22 @@ def search_up(prefix, *landmarks, test=isfile): # it in 'home'. if not base_executable: _home_executable = joinpath(executable_dir, DEFAULT_PROGRAM_NAME) - _realpath_executable = None + _executable_realpath = None try: - _realpath_executable = realpath(executable) + _executable_realpath = realpath(executable) # If the realpath of the executable identified in home is the same as - # the realpath of the venv's executable use the (potentially not fully - # resolved via realpath) path from home. - if realpath(_home_executable) == _realpath_executable: + # the realpath of the venv's executable, then use the (potentially not + # fully resolved via realpath) path from home. + if realpath(_home_executable) == _executable_realpath: base_executable = _home_executable except OSError: pass if not base_executable: - if _realpath_executable and _realpath_executable != executable: + if _executable_realpath and _executable_realpath != executable: # Try to resolve symlinked executables, since that may be # more accurate than assuming the executable in 'home'. - base_executable = _realpath_executable + base_executable = _executable_realpath if not base_executable: base_executable = joinpath(executable_dir, basename(executable)) # It's possible "python" is executed from within a posix venv but that @@ -407,10 +407,10 @@ def search_up(prefix, *landmarks, test=isfile): # # In this case, try to fall back to known alternatives if os_name != 'nt' and not isfile(base_executable): - base_exe = basename(executable) + _base_exe = basename(executable) for candidate in (DEFAULT_PROGRAM_NAME, f'python{VERSION_MAJOR}.{VERSION_MINOR}'): candidate += EXE_SUFFIX if EXE_SUFFIX else '' - if base_exe == candidate: + if _base_exe == candidate: continue candidate = joinpath(executable_dir, candidate) # Only set base_executable if the candidate exists.