bpo-45582: Port getpath[p].c to Python #29041

zooba · 2021-10-18T22:15:35Z

https://bugs.python.org/issue45582

zooba · 2021-10-18T22:19:37Z

This is very very WIP, but I wanted to share it now for discussion.

My plan is to set it up so that getpath isn't actually a module, but is frozen bytecode that we execute during startup. The top of the file has the expected globals, and I'll use C code to create the dict for it. Then we exec the code, and pull the results out of it.

The next step is to write tests, which can execute it directly without having to modify C code and prove correctness/etc. Because it's all possible to stub out, we can isolate it entirely from any real filesystem and simulate whatever layout/build settings we want.

It may be logical to also merge in some of site.py's functionality, and potentially sysconfig (looking at the install schemes), but those can be deferred to a later PR.

zooba · 2021-10-18T22:22:27Z

Ping @vstinner

vstinner · 2021-10-19T22:36:34Z

If I understood correctly, this code doesn't remove getpath.c yet. It's a WIP.

vstinner · 2021-10-19T22:37:58Z

For me, the most important part is to write unit tests to test that the same always produce the same . The code should be stateless, but can use the current working directory, environment variables, etc.

I wrote down all I know about the "Python Path Configuration" at: https://docs.python.org/dev/c-api/init_config.html#python-path-configuration

zooba · 2021-10-20T00:01:13Z

If I understood correctly, this code doesn't remove getpath.c yet.

Correct. I just started removing it, and obviously everything is broken :) But I'll come back to it tomorrow.

I wrote down all I know ...

Thanks. I think I've re-discovered most of that by parsing the logic from getpath.c, but what you've got there will help me understand how you designed the config mechanism. I want to remove as much as possible, but hopefully without breaking anyone trying to use it directly.

zooba · 2021-10-21T22:23:51Z

Python/initconfig.c

-        return 0;
-    }
-    /* If dirname() is the same for both then it is a dev build. */
-    if (len != _Py_find_basename(stdlib)) {


@ericsnowcurrently FYI, if my change doesn't make it in, you'll want to fix this check. On Windows, executable and stdlib have the same basename in a release build (e.g. C:\Python39\{Lib//python.exe}), but different in a dev build (C:\cpython\Lib // C:\cpython\PCbuild\amd64\python.exe)

zooba · 2021-10-21T22:26:13Z

Modules/getpath.c

@@ -1,14 +1,19 @@
 /* Return the initial module search path. */


This file is a complete rewrite - don't try and analyse the diff. Just go to "View File" under the triple-dot menu for it instead.

zooba · 2021-10-22T23:43:06Z

This is definitely not ready to merge yet, even if by some chance all the tests pass, but I'm happy to have reviews and comments/discussion.

Any significant discussion should go to the bpo issue, please. Also, if I need to be pinged, please do it on bpo.

Python/fileutils.c

ericsnowcurrently

Here's a partial review. I haven't dug into the bigger pieces of this PR.

Lib/site.py

Lib/sysconfig.py

Lib/test/test_embed.py

Misc/NEWS.d/next/Core and Builtins/2021-10-23-00-39-31.bpo-45582.YONPuo.rst

Programs/_freeze_module.c

Modules/getpath.py

ericsnowcurrently · 2021-11-04T15:56:05Z

Python/initconfig.c

-            if (isdev >= 0) {
-                config->use_frozen_modules = !isdev;
-            }
+            config->use_frozen_modules = !config->_development_env;


I purposefully left it -1 if we couldn't figure it out. So this change implies that we are always able to figure it out. Is that the case?

Given the rules we've got for figuring out if we're in a dev build or not, yes. Those rules may be insufficient, but it's all a heuristic anyway - if you put those files into a regular install, we'll assume it's a dev build.

we'll assume it's a dev build.

That's what I did at first too. However, I found that it was helpful to know if we were not able to figure it out, which is what the -1 indicates in this case.

I mean, we either find the build landmarks or we don't... what other checks do you have? If the build landmarks aren't there, it's not a dev build, and if they are but the stdlib isn't, it's all going to crash down anyway.

Lib/test/test_embed.py

Modules/getpath.py

zooba · 2021-11-12T01:26:27Z

I'm expecting another dumb error (on my part) or two, but I'm very close to having this working.

Reviews would be appreciated! Bear in mind that I'm trying to match the current (quirky) behaviour, rather than streamline anything by changing it (yet). We can do those once we know we've got something working.

zooba · 2021-11-15T23:30:38Z

There was nothing helpful in that history anyway ;-)

Tests all pass now, reviews appreciated!

Include/cpython/initconfig.h

Include/internal/pycore_fileutils.h

Python/initconfig.c

bedevere-bot · 2021-12-02T15:26:50Z

🤖 New build scheduled with the buildbot fleet by @zooba for commit 5962947 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

zooba · 2021-12-02T23:33:39Z

Found the leak! So it's ready to merge once these tests pass.

Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041

bedevere-bot added the awaiting core review label Oct 18, 2021

the-knights-who-say-ni added the CLA signed label Oct 18, 2021

zooba added the DO-NOT-MERGE label Oct 18, 2021

zooba commented Oct 21, 2021

View reviewed changes

zooba changed the title ~~bpo-42260: Port getpath[p].c to Python~~ bpo-45582: Port getpath[p].c to Python Oct 22, 2021

zooba marked this pull request as ready for review October 22, 2021 23:42

zooba requested a review from a team as a code owner October 22, 2021 23:42

eryksun reviewed Oct 23, 2021

View reviewed changes

Python/fileutils.c Outdated Show resolved Hide resolved

eryksun reviewed Oct 23, 2021

View reviewed changes

Python/fileutils.c Outdated Show resolved Hide resolved

ericsnowcurrently mentioned this pull request Oct 25, 2021

Make getpath.c easier to follow. #28909

Closed

gvanrossum mentioned this pull request Oct 26, 2021

bpo-43976: add vendor config #25718

Closed

zooba requested a review from vsajip as a code owner October 28, 2021 19:56

zooba force-pushed the bpo-42260 branch from 0c8ef95 to f5504cf Compare November 3, 2021 01:54

ericsnowcurrently reviewed Nov 4, 2021

View reviewed changes

Lib/test/test_embed.py Show resolved Hide resolved

zooba mentioned this pull request Nov 5, 2021

bpo-45720: Drop references to shlwapi #29417

Merged

zooba commented Nov 6, 2021

View reviewed changes

Modules/getpath.py Outdated Show resolved Hide resolved

bpo-45582: Port getpath[p].c to getpath.py

a63f5d8

zooba force-pushed the bpo-42260 branch from 65f77e3 to a63f5d8 Compare November 15, 2021 23:22

zooba removed the DO-NOT-MERGE label Nov 15, 2021

vstinner reviewed Nov 17, 2021

View reviewed changes

Include/cpython/initconfig.h Outdated Show resolved Hide resolved

Include/internal/pycore_fileutils.h Outdated Show resolved Hide resolved

Python/initconfig.c Outdated Show resolved Hide resolved

Python/initconfig.c Outdated Show resolved Hide resolved

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 1, 2021

zooba added 3 commits December 2, 2021 00:41

Fix memory leak and simplify some logic

b568efe

Merge remote-tracking branch 'cpython/main' into bpo-42260

7537fd5

Revert test changes

5962947

zooba added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021

Add missing PyMem_Free

c10d17c

zooba merged commit 99fcf15 into python:main Dec 3, 2021

bedevere-bot removed the awaiting core review label Dec 3, 2021

zooba deleted the bpo-42260 branch December 3, 2021 00:08

ericsnowcurrently mentioned this pull request Jan 6, 2022

bpo-46263: Do not ever expect "use_frozen_modules" to be -1. #30438

Merged

naveen521kk mentioned this pull request Mar 19, 2022

New venv launchers are broken msys2-contrib/cpython-mingw#90

Closed

naveen521kk mentioned this pull request Jun 19, 2022

Python 3.11 Update msys2-contrib/cpython-mingw#101

Closed

4 tasks

chrisburr mentioned this pull request Oct 25, 2022

Add patch for use with Python 3.11 conda-forge/crossenv-feedstock#34

Merged

filmor mentioned this pull request Nov 2, 2022

Potential use-after-free in Py_SetPythonHome and its siblings #98978

Closed

This was referenced Nov 4, 2022

Poetry tries using Python2 (usr/local/bin/python) instead of the provided distribution during install python-poetry/poetry#6940

Open

3.11 support pypa/virtualenv#2440

Closed

sys._base_executable may not exist on posix venvs made with --copies #99204

Closed

MagerValp mentioned this pull request Feb 24, 2023

Using link to execute relocated 3.11 bin/python3 fails to find libraries gregneagle/relocatable-python#31

Open

maxbachmann mentioned this pull request Feb 24, 2023

remove outdated files from documentation #102216

Merged

mwichmann mentioned this pull request Jul 18, 2023

unittest import loop with pre-release Python SCons/scons#4376

Closed

zooba mentioned this pull request Apr 10, 2022

Rewrite getpath.c in Python #89745

Closed

barneygale mentioned this pull request Jan 4, 2024

Windows os.path.isabs UNC path bug #66498

Closed

ales-erjavec added a commit to biolab/orange3-installers that referenced this pull request Apr 2, 2024

Link python 'shortcut' to Python.app/Contents/MacOS/Python

d23bf6b

Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041

ales-erjavec added a commit to biolab/orange3-installers that referenced this pull request Apr 2, 2024

Link python 'shortcut' to Python.app/Contents/MacOS/Python

e69855e

Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041

markotoplak pushed a commit to Quasars/quasar that referenced this pull request Jun 27, 2024

Link python 'shortcut' to Python.app/Contents/MacOS/Python

83b76de

Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041

markotoplak pushed a commit to Quasars/quasar that referenced this pull request Jun 27, 2024

Link python 'shortcut' to Python.app/Contents/MacOS/Python

054e316

Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041

markotoplak pushed a commit to Quasars/quasar that referenced this pull request Jun 27, 2024

Link python 'shortcut' to Python.app/Contents/MacOS/Python

e75e400

Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-45582: Port getpath[p].c to Python #29041

bpo-45582: Port getpath[p].c to Python #29041

zooba commented Oct 18, 2021 •

edited by bedevere-bot

Loading

zooba commented Oct 18, 2021

zooba commented Oct 18, 2021

vstinner commented Oct 19, 2021

vstinner commented Oct 19, 2021

zooba commented Oct 20, 2021

zooba Oct 21, 2021

zooba Oct 21, 2021

zooba commented Oct 22, 2021

ericsnowcurrently left a comment

ericsnowcurrently Nov 4, 2021

zooba Nov 4, 2021

ericsnowcurrently Nov 4, 2021

zooba Nov 4, 2021

zooba commented Nov 12, 2021

zooba commented Nov 15, 2021

bedevere-bot commented Dec 2, 2021

zooba commented Dec 2, 2021

		@@ -1,14 +1,19 @@
		/* Return the initial module search path. */

bpo-45582: Port getpath[p].c to Python #29041

bpo-45582: Port getpath[p].c to Python #29041

Conversation

zooba commented Oct 18, 2021 • edited by bedevere-bot Loading

zooba commented Oct 18, 2021

zooba commented Oct 18, 2021

vstinner commented Oct 19, 2021

vstinner commented Oct 19, 2021

zooba commented Oct 20, 2021

zooba Oct 21, 2021

Choose a reason for hiding this comment

zooba Oct 21, 2021

Choose a reason for hiding this comment

zooba commented Oct 22, 2021

ericsnowcurrently left a comment

Choose a reason for hiding this comment

ericsnowcurrently Nov 4, 2021

Choose a reason for hiding this comment

zooba Nov 4, 2021

Choose a reason for hiding this comment

ericsnowcurrently Nov 4, 2021

Choose a reason for hiding this comment

zooba Nov 4, 2021

Choose a reason for hiding this comment

zooba commented Nov 12, 2021

zooba commented Nov 15, 2021

bedevere-bot commented Dec 2, 2021

zooba commented Dec 2, 2021

zooba commented Oct 18, 2021 •

edited by bedevere-bot

Loading