GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-66409: check if exec_prefix is the same as prefix before searching executable_dir by FFY00 · Pull Request #127974 · python/cpython

Conversation

@FFY00

@FFY00 FFY00 commented Dec 15, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Make pyvenv style virtual environments easier to configure when embedding Python #66409

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@FFY00
pythonGH-66409: check if exec_prefix is the same as prefix before sea…
07d2b13
…rching executable_dir

Signed-off-by: Filipe Laíns <[email protected]
@bedevere-app bedevere-app Bot mentioned this pull request Dec 15, 2024
Make pyvenv style virtual environments easier to configure when embedding Python #66409
Open
@bedevere-app bedevere-app Bot added the awaiting core review label Dec 15, 2024
@FFY00
Add news
d456359
Signed-off-by: Filipe Laíns <[email protected]
@FFY00 FFY00 mentioned this pull request Dec 15, 2024
GH-127970: find the runtime library when dladdr is available #127972
Merged
@FFY00
FFY00 merged commit 34e840f into python:main Jan 8, 2025
@bedevere-app bedevere-app Bot removed the awaiting core review label Jan 8, 2025
@FFY00 FFY00 mentioned this pull request Jan 8, 2025
venv base path does not resolve symlinks using realpath() #106045
Closed
@pelson

pelson commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

Looks like this has actually resolved the issue in #106045.

I'm going to write a test for it in any case - watch this space.

FFY00 reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pelson

pelson commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

Actually, this might have also introduced a bug in that it now over-resolves the executable.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FFY00

FFY00 commented Jan 9, 2025

Copy link
Copy Markdown
Member Author

Please open an issue with reproduction instructions and I'll have a look 👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pelson

pelson commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

The following test fails before your change:

diff --git a/Lib/test/test_getpath.py b/Lib/test/test_getpath.py
index f86df9d0d03..1c2e2a0b3fc 100644
--- a/Lib/test/test_getpath.py
+++ b/Lib/test/test_getpath.py
@@ -864,6 +864,55 @@ 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",
+        )
+        # 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/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(

With:

AssertionError: {'exe[132 chars]': '/some/_internal/prefix', 'base_executable'[206 chars]ad']} != {'exe[132 chars]': '/usr', 'base_executable': '/some/_internal[188 chars]ad']}
- {'base_exec_prefix': '/some/_internal/prefix',
+ {'base_exec_prefix': '/usr',
-  'base_executable': '/usr/bin/python3',
?                       -

+  'base_executable': '/some/_internal/prefix/bin/python3',
?                        +++++++++ ++++++++++

   'base_prefix': '/some/_internal/prefix',
   'exec_prefix': '/venv',
   'executable': '/venv/bin/python3',
   'module_search_paths': ['/some/_internal/prefix/lib/python98.zip',
                           '/some/_internal/prefix/lib/python9.8',
-                          '/some/_internal/prefix/lib/python9.8/lib-dynload'],
?                             --------- ----------

+                          '/usr/lib/python9.8/lib-dynload'],
?                            +

   'module_search_paths_set': 1,
   'prefix': '/venv'}

After your change, it fails with:

  {'base_exec_prefix': '/some/_internal/prefix',
-  'base_executable': '/usr/bin/python3',
?                       -

+  'base_executable': '/some/_internal/prefix/bin/python3',
?                        +++++++++ ++++++++++

   'base_prefix': '/some/_internal/prefix',
   'exec_prefix': '/venv',
   'executable': '/venv/bin/python3',
   'module_search_paths': ['/some/_internal/prefix/lib/python98.zip',
                           '/some/_internal/prefix/lib/python9.8',
                           '/some/_internal/prefix/lib/python9.8/lib-dynload'],
   'module_search_paths_set': 1,
   'prefix': '/venv'}

In conclusion: I think this change has improved the situation. The issue that remains can be considered a new one, and is the one I mention in #106045 (comment), namely:

One thing to note: I think the calculation of sys._base_executable is over-resolving symlinks today - this is a separate bug which I guess means that homebrew users can't take a venv from a venv and have it survive beyond the next patch update.

I'll report it now.

FFY00 reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pelson

pelson commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

Also, to get it off my chest: I worry about the tests in test_getpath since they are only mocking the real behaviour. To give an example of a problem that might get missed if we use test_getpath as the only place for testing the startup behaviour:

Imagine that we add a test in test_getpath which validates that a prefix /some/public/prefix is a symlink to another real one /some/_internal/prefix. If we write a test that ensures that the prefix is not resolved to the internal one, it should pass today. If we then update getpath.c to do recursive path readlink, instead of file only readlink, then the test will continue to pass even though the underlying implementation changed. This is because there is a mocked implementation of realpath which meets our assumptions today, but doesn't guarantee to be the same as getpath_realpath.

My conclusion is that test_getpath is only good for quick developer validation, and that we should duplicate many of the tests using the real implementation, as is done in test_venv. Are there equivalent tests which use subprocess for the non venv case?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FFY00

FFY00 commented Jan 9, 2025

Copy link
Copy Markdown
Member Author

Yes, although the coverage isn't perfect, we test the real implementation in test_embed.

pelson reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jan 9, 2025
gh-106045: Fix venv creation from a python executable symlink #115237
Closed
sys._base_executable resolves symlinks too eagerly, resulting in undesirable venv from venv behaviour #128670
Open
@pelson

pelson commented Jan 10, 2025

Copy link
Copy Markdown
Contributor

@FFY00 - since this change also fixes a bug which affects venv and virtualenv, is this something that we can backport to 3.12 & 3.13, or do you consider this change to actually be a new feature for 3.14 onwards?

Similarly, for #128670 a fix there would ideally be backported as far as we can.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FFY00

FFY00 commented Jan 11, 2025

Copy link
Copy Markdown
Member Author

Not really, the reason this works is because of GH-127972, which cannot be backported. This PR just syncs exec_prefix up with prefix.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Jan 21, 2025

Copy link
Copy Markdown
Member

test_embed.test_init_pyvenv_cfg() started to fail in some cases since this change: see #128690 issue.

I wrote #129137 to update test_embed for this change. Please have a look.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jan 28, 2026
macOS built Python3.11b5 can't find lib-dynload in relocatable framework if framework is relocated and python run from external symlink #95400
Open
Make python slightly more relocatable #62509
Closed
@FFY00 FFY00 mentioned this pull request Mar 3, 2026
exec_prefix not found when prefix is found at runtime when install tree is moved #107658
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@FFY00 @pelson @vstinner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.