gh-106045: Fix ``venv`` creation from a python executable symlink by mayeut · Pull Request #115237 · python/cpython

mayeut · 2024-02-10T10:58:25Z

Issue: venv base path does not resolve symlinks using realpath() #106045

ghost

The PR looks good 👍

ghost · 2025-01-08T20:58:13Z

+        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)


Can you move this to a helper function? This way the ensure_directories and symlink resolving logic is clearly separated,

ghost · 2025-01-08T21:01:36Z

+            subprocess.check_call(cmd)
+        data = self.get_text_file_contents('pyvenv.cfg')
+        self.assertIn('home = %s' % tree_symlink, data)
+        self.assertIn('executable = %s' %


nitpick: Since this is so path-operation heavy, it would be nice to use pathlib here instead.

ghost

I concur with @FFY00's comments. Also, as another nitpick, why use the name executable_ as a local variable? The trailing underscore is not a common thing to see, in my experience,

ghost

My view is that this is doing the right thing, but in the wrong place. Instead I think we should not resolve the executable path at all in venv, and instead we should resolve the prefix based on the executable symlink at Python startup (in getpath.py).

Based on what I understand, the issue that this PR is fixing was just fixed by #127974.

ghost · 2025-01-09T09:14:46Z

+        # 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_)


Technically we need to watch out for circular links. I think having this in a separate function (as suggested) will make that less painful when implementing.

FWIW, I don't know why there isn't something in the standard library for recursively resolving a symlink (without recursively resolving all child paths as realpath does). Perhaps one for Python ideas?

ghost · 2025-01-11T09:58:27Z

Based on what I understand, the issue that this PR is fixing was just fixed by #127974.

It appears that the linked PR does not resolve the issue.
Here are the steps I used on macOS on the main branch:

./configure --with-pydebug && make -j
mkdir ../temp-test
ln -s $(pwd)/python.exe ../temp-test/python
../temp-test/python -m venv ../temp-test/temp-test-venv
Error: Command '['.../temp-test/temp-test-venv/bin/python', '-m', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.

ghost · 2025-01-11T10:01:47Z

That PR builds on top of GH-127972, so it only works with --enable-shared.

ghost · 2025-01-12T15:01:55Z

+        - we stop if a candidate does not resolve to the same file
+          (this can happen with normpath)
+        """
+        result = os.path.abspath(path)


os.path.abspath is done first as was used before. It does normalize the path which may end-up with an invalid path in case of symlinks (e.g. "symlink_dir/../segment").

ghost · 2025-04-18T09:39:00Z

All commit authors signed the Contributor License Agreement.

ghost · 2026-04-15T06:36:14Z

This PR is stale because it has been open for 30 days with no activity.

ghost · 2026-06-05T14:12:58Z

This is approved but now shows as conflicting — it needs a rebase onto main to merge. For cross-tracking, virtualenv has the same unresolved-symlink behavior in pyvenv.cfg (pypa/virtualenv#3157).

ghost · 2026-09-07T12:09:25Z

This PR is stale because it has been open for 90 days with no activity.

ghost · 2026-10-20T22:49:17Z

This PR is superseded by #156483, a newer open pull request for the same issue (gh-106045), and it no longer merges cleanly. Closing to cull the number of open pull requests. Do not reopen this pull request without getting a go-ahead from a triager or core developer.

mayeut requested a review from vsajip as a code owner February 10, 2024 10:58

bedevere-app Bot added the awaiting review label Feb 10, 2024

bedevere-app Bot mentioned this pull request Feb 10, 2024

venv base path does not resolve symlinks using realpath() #106045

Closed

mayeut force-pushed the venv-home-follow-symlink branch from 91e8d28 to ad36335 Compare May 11, 2024 17:04

mayeut marked this pull request as draft September 28, 2024 08:53

bedevere-app Bot removed the awaiting review label Sep 28, 2024

mayeut force-pushed the venv-home-follow-symlink branch 2 times, most recently from ca424f1 to 340d9c2 Compare September 28, 2024 10:45

mayeut marked this pull request as ready for review September 28, 2024 11:15

bedevere-app Bot added the awaiting review label Sep 28, 2024

ofek mentioned this pull request Oct 22, 2024

Preserve symlinks when creating virtual environments astral-sh/uv#8433

Closed

FFY00 approved these changes Jan 8, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Jan 8, 2025

vsajip reviewed Jan 9, 2025

View reviewed changes

pelson reviewed Jan 9, 2025

View reviewed changes

pythongh-106045: Fix venv creation from a python executable symlink

a4f000b

address review comments

f9ef949

mayeut force-pushed the venv-home-follow-symlink branch from 340d9c2 to f9ef949 Compare January 11, 2025 11:41

rework _abspath_resolve_leaf to _getpath_realpath

199c65e

mayeut commented Jan 12, 2025

View reviewed changes

pelson mentioned this pull request Jan 16, 2025

gh-128670: Use the pyvenv.cfg home location in getpath when the default executable points to the same realpath as the running executable #128913

Open

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 15, 2026

gaborbernat mentioned this pull request Jun 5, 2026

Base interpreter symlink not resolved in pyvenv.cfg (home / base-executable record the symlink) pypa/virtualenv#3157

Closed

github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jun 6, 2026

coderabbitai Bot mentioned this pull request Jul 15, 2026

feat(mlxctl): complete local inference management nisavid/systools#41

Merged

github-actions Bot added the stale Stale PR or inactive for long period of time. label Sep 7, 2026

ambv closed this Oct 20, 2026

Conversation

mayeut commented Feb 10, 2024 • edited by Eclips4 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost Jan 8, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jan 8, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost Jan 9, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

ghost commented Jan 11, 2025

Uh oh!

ghost commented Jan 11, 2025

Uh oh!

ghost Jan 12, 2025

Choose a reason for hiding this comment

Uh oh!

ghost commented Apr 18, 2025 • edited by python-cla-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Apr 15, 2026

Uh oh!

ghost commented Jun 5, 2026

Uh oh!

ghost commented Sep 7, 2026

Uh oh!

ghost commented Oct 20, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mayeut commented Feb 10, 2024 •

edited by Eclips4

Loading

ghost commented Apr 18, 2025 •

edited by python-cla-bot Bot

Loading