From a4f000be2c0fd169bf0a22b364c2f6d865bafce4 Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 10 Feb 2024 11:53:04 +0100 Subject: [PATCH 1/3] gh-106045: Fix ``venv`` creation from a python executable symlink --- Lib/test/test_venv.py | 51 +++++++++++++++++++ Lib/venv/__init__.py | 11 +++- Misc/ACKS | 1 + ...-02-10-11-41-42.gh-issue-106045.eVZFt2.rst | 2 + 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2024-02-10-11-41-42.gh-issue-106045.eVZFt2.rst diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index 0b09010c69d4ea..743fcbb836fee6 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -892,6 +892,57 @@ def test_venvwlauncher(self): except subprocess.CalledProcessError: self.fail("venvwlauncher.exe did not run %s" % exename) + @requires_subprocess() + @unittest.skipIf(os.name == 'nt', 'not relevant on Windows') + @unittest.skipUnless(can_symlink(), 'Needs symlinks') + def test_executable_symlink(self): + """ + Test creation using a symlink to python executable. + """ + rmtree(self.env_dir) + with tempfile.TemporaryDirectory() as symlink_dir: + executable_symlink = os.path.join( + os.path.realpath(symlink_dir), + os.path.basename(sys.executable)) + os.symlink(os.path.abspath(sys.executable), executable_symlink) + cmd = [executable_symlink, "-m", "venv", "--without-pip", + self.env_dir] + subprocess.check_call(cmd) + data = self.get_text_file_contents('pyvenv.cfg') + executable = sys._base_executable + path = os.path.dirname(executable) + self.assertIn('home = %s' % path, data) + self.assertIn('executable = %s' % + os.path.realpath(sys.executable), data) + + @requires_subprocess() + @unittest.skipIf(os.name == 'nt', 'not relevant on Windows') + @unittest.skipUnless(can_symlink(), 'Needs symlinks') + @requireVenvCreate + def test_tree_symlink(self): + """ + Test creation using a symlink to python tree. + """ + rmtree(self.env_dir) + executable_abspath = os.path.abspath(sys._base_executable) + tree_abspath = os.path.dirname(os.path.dirname(executable_abspath)) + with tempfile.TemporaryDirectory() as symlink_dir: + tree_symlink = os.path.join( + os.path.realpath(symlink_dir), + os.path.basename(tree_abspath)) + executable_symlink = os.path.join( + tree_symlink, + os.path.basename(os.path.dirname(executable_abspath)), + os.path.basename(sys._base_executable)) + os.symlink(tree_abspath, tree_symlink) + cmd = [executable_symlink, "-m", "venv", "--without-pip", + self.env_dir] + subprocess.check_call(cmd) + data = self.get_text_file_contents('pyvenv.cfg') + self.assertIn('home = %s' % tree_symlink, data) + self.assertIn('executable = %s' % + os.path.realpath(sys._base_executable), data) + @requireVenvCreate class EnsurePipTest(BaseTest): diff --git a/Lib/venv/__init__.py b/Lib/venv/__init__.py index dc4c9ef3531991..3c9a15978306fa 100644 --- a/Lib/venv/__init__.py +++ b/Lib/venv/__init__.py @@ -163,7 +163,16 @@ def create_if_needed(d): 'Python interpreter. Provide an explicit path or ' 'check that your PATH environment variable is ' 'correctly set.') - dirname, exename = os.path.split(os.path.abspath(executable)) + # only resolve executable symlinks, not the full chain, see gh-106045 + # we don't want to overwrite the executable used in context + executable_ = os.path.abspath(executable) + while os.path.islink(executable_): + link = os.readlink(executable_) + if os.path.isabs(link): + executable_ = link + else: + executable_ = os.path.join(os.path.dirname(executable_), link) + dirname, exename = os.path.split(executable_) if sys.platform == 'win32': # Always create the simplest name in the venv. It will either be a # link back to executable, or a copy of the appropriate launcher diff --git a/Misc/ACKS b/Misc/ACKS index deda334bee7417..5767d0c4452fe1 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -419,6 +419,7 @@ Eric Daniel Scott David Daniels Derzsi Dániel Lawrence D'Anna +Matthieu Darbois Ben Darnell Kushal Das Jonathan Dasteel diff --git a/Misc/NEWS.d/next/Library/2024-02-10-11-41-42.gh-issue-106045.eVZFt2.rst b/Misc/NEWS.d/next/Library/2024-02-10-11-41-42.gh-issue-106045.eVZFt2.rst new file mode 100644 index 00000000000000..1a3292976bc9d3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-10-11-41-42.gh-issue-106045.eVZFt2.rst @@ -0,0 +1,2 @@ +Fix ``venv`` creation from a python executable symlink. Patch by Matthieu +Darbois. From f9ef949a0a55c082a96911597005c9e44c8203ad Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 11 Jan 2025 12:41:47 +0100 Subject: [PATCH 2/3] address review comments --- Lib/test/test_venv.py | 43 +++++++++++++++++-------------------------- Lib/venv/__init__.py | 30 +++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index 743fcbb836fee6..81d5c347ccb3da 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -900,20 +900,17 @@ def test_executable_symlink(self): Test creation using a symlink to python executable. """ rmtree(self.env_dir) - with tempfile.TemporaryDirectory() as symlink_dir: - executable_symlink = os.path.join( - os.path.realpath(symlink_dir), - os.path.basename(sys.executable)) - os.symlink(os.path.abspath(sys.executable), executable_symlink) - cmd = [executable_symlink, "-m", "venv", "--without-pip", - self.env_dir] + exe = pathlib.Path(sys.executable).absolute() + with tempfile.TemporaryDirectory() as tmp_dir: + symlink_dir = pathlib.Path(tmp_dir).resolve(strict=True) + exe_symlink = symlink_dir / exe.name + exe_symlink.symlink_to(exe) + cmd = [exe_symlink, "-m", "venv", "--without-pip", self.env_dir] subprocess.check_call(cmd) data = self.get_text_file_contents('pyvenv.cfg') - executable = sys._base_executable - path = os.path.dirname(executable) + path = os.path.dirname(sys._base_executable) self.assertIn('home = %s' % path, data) - self.assertIn('executable = %s' % - os.path.realpath(sys.executable), data) + self.assertIn('executable = %s' % exe.resolve(), data) @requires_subprocess() @unittest.skipIf(os.name == 'nt', 'not relevant on Windows') @@ -924,24 +921,18 @@ def test_tree_symlink(self): Test creation using a symlink to python tree. """ rmtree(self.env_dir) - executable_abspath = os.path.abspath(sys._base_executable) - tree_abspath = os.path.dirname(os.path.dirname(executable_abspath)) - with tempfile.TemporaryDirectory() as symlink_dir: - tree_symlink = os.path.join( - os.path.realpath(symlink_dir), - os.path.basename(tree_abspath)) - executable_symlink = os.path.join( - tree_symlink, - os.path.basename(os.path.dirname(executable_abspath)), - os.path.basename(sys._base_executable)) - os.symlink(tree_abspath, tree_symlink) - cmd = [executable_symlink, "-m", "venv", "--without-pip", - self.env_dir] + exe = pathlib.Path(sys._base_executable).absolute() + tree = exe.parent.parent + with tempfile.TemporaryDirectory() as tmp_dir: + symlink_dir = pathlib.Path(tmp_dir).resolve(strict=True) + tree_symlink = symlink_dir / tree.name + exe_symlink = tree_symlink / exe.relative_to(tree) + tree_symlink.symlink_to(tree) + cmd = [exe_symlink, "-m", "venv", "--without-pip", self.env_dir] subprocess.check_call(cmd) data = self.get_text_file_contents('pyvenv.cfg') self.assertIn('home = %s' % tree_symlink, data) - self.assertIn('executable = %s' % - os.path.realpath(sys._base_executable), data) + self.assertIn('executable = %s' % exe.resolve(), data) @requireVenvCreate diff --git a/Lib/venv/__init__.py b/Lib/venv/__init__.py index 3c9a15978306fa..4a4d7c99533b9a 100644 --- a/Lib/venv/__init__.py +++ b/Lib/venv/__init__.py @@ -133,6 +133,26 @@ def _same_path(cls, path1, path2): else: return path1 == path2 + @classmethod + def _abspath_resolve_leaf(cls, path): + """Returns the absolute path, resolving links to the last component. + + If there's a cycle, os.path.abspath(path) is returned + """ + path = os.path.abspath(path) + result = path + while os.path.islink(result): + link = os.readlink(result) + if os.path.isabs(link): + result = link + else: + result = os.path.join(os.path.dirname(result), link) + result = os.path.abspath(result) + if result == path: + # circular links + break + return result + def ensure_directories(self, env_dir): """ Create the directories for the environment. @@ -164,15 +184,7 @@ def create_if_needed(d): 'check that your PATH environment variable is ' 'correctly set.') # only resolve executable symlinks, not the full chain, see gh-106045 - # we don't want to overwrite the executable used in context - executable_ = os.path.abspath(executable) - while os.path.islink(executable_): - link = os.readlink(executable_) - if os.path.isabs(link): - executable_ = link - else: - executable_ = os.path.join(os.path.dirname(executable_), link) - dirname, exename = os.path.split(executable_) + dirname, exename = os.path.split(self._abspath_resolve_leaf(executable)) if sys.platform == 'win32': # Always create the simplest name in the venv. It will either be a # link back to executable, or a copy of the appropriate launcher From 199c65e64e706bdeaf5194875d961ce48dbdc70d Mon Sep 17 00:00:00 2001 From: mayeut Date: Sun, 12 Jan 2025 15:43:40 +0100 Subject: [PATCH 3/3] rework _abspath_resolve_leaf to _getpath_realpath --- Lib/test/test_venv.py | 3 ++- Lib/venv/__init__.py | 48 ++++++++++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index 81d5c347ccb3da..cbd8e89beb5d14 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -895,6 +895,7 @@ def test_venvwlauncher(self): @requires_subprocess() @unittest.skipIf(os.name == 'nt', 'not relevant on Windows') @unittest.skipUnless(can_symlink(), 'Needs symlinks') + @unittest.skipUnless(sysconfig.get_config_var('HAVE_READLINK'), "Requires HAVE_READLINK support") def test_executable_symlink(self): """ Test creation using a symlink to python executable. @@ -908,7 +909,7 @@ def test_executable_symlink(self): cmd = [exe_symlink, "-m", "venv", "--without-pip", self.env_dir] subprocess.check_call(cmd) data = self.get_text_file_contents('pyvenv.cfg') - path = os.path.dirname(sys._base_executable) + path = os.path.dirname(os.path.abspath(sys._base_executable)) self.assertIn('home = %s' % path, data) self.assertIn('executable = %s' % exe.resolve(), data) diff --git a/Lib/venv/__init__.py b/Lib/venv/__init__.py index 4a4d7c99533b9a..9a153040316477 100644 --- a/Lib/venv/__init__.py +++ b/Lib/venv/__init__.py @@ -134,23 +134,37 @@ def _same_path(cls, path1, path2): return path1 == path2 @classmethod - def _abspath_resolve_leaf(cls, path): - """Returns the absolute path, resolving links to the last component. - - If there's a cycle, os.path.abspath(path) is returned + def _getpath_realpath(cls, path): + """Mimics getpath.realpath + + It only mimics it for HAVE_READLINK. + There are a few differences listed here: + - we ensure that we have a resolvable abspath first + (i.e. exists and no symlink loop) + - we stop if a candidate does not resolve to the same file + (this can happen with normpath) """ - path = os.path.abspath(path) - result = path - while os.path.islink(result): - link = os.readlink(result) - if os.path.isabs(link): - result = link - else: - result = os.path.join(os.path.dirname(result), link) - result = os.path.abspath(result) - if result == path: - # circular links - break + result = os.path.abspath(path) + try: + real_path = os.path.realpath(result, strict=True) + except OSError: + logger.warning('Unable to resolve %r real path', result) + return result + if sysconfig.get_config_var('HAVE_READLINK'): + while os.path.islink(result): + link = os.readlink(result) + if os.path.isabs(link): + candidate = link + else: + candidate = os.path.join(os.path.dirname(result), link) + candidate = os.path.normpath(candidate) + # shall exists and be the same file as the original one + valid = os.path.exists(candidate) and os.path.samefile(real_path, candidate) + if not valid: + logger.warning('Stopped resolving %r because %r is not the same file', + result, candidate) + break + result = candidate return result def ensure_directories(self, env_dir): @@ -184,7 +198,7 @@ def create_if_needed(d): 'check that your PATH environment variable is ' 'correctly set.') # only resolve executable symlinks, not the full chain, see gh-106045 - dirname, exename = os.path.split(self._abspath_resolve_leaf(executable)) + dirname, exename = os.path.split(self._getpath_realpath(executable)) if sys.platform == 'win32': # Always create the simplest name in the venv. It will either be a # link back to executable, or a copy of the appropriate launcher