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 — bpo-45582: Port getpath[p].c to Python by zooba · Pull Request #29041 · python/cpython

Conversation

@zooba

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

Uh oh!

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

Copy link
Copy Markdown
Member

https://bugs.python.org/issue45582

https://bugs.python.org/issue45582

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-bot bedevere-bot added the awaiting core review label Oct 18, 2021
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 18, 2021
@zooba zooba added the DO-NOT-MERGE label Oct 18, 2021
@zooba

zooba commented Oct 18, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

@zooba

zooba commented Oct 18, 2021

Copy link
Copy Markdown
Member Author

Ping @vstinner

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Oct 19, 2021

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Oct 19, 2021

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@zooba

zooba commented Oct 20, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

zooba
zooba commented Oct 21, 2021
View reviewed changes
Comment thread Python/initconfig.c Outdated

@zooba zooba Oct 21, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
zooba
zooba commented Oct 21, 2021
View reviewed changes
Comment thread Modules/getpath.c Outdated

@zooba zooba Oct 21, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

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

All reactions
@zooba zooba changed the title bpo-42260: Port getpath[p].c to Python bpo-45582: Port getpath[p].c to Python Oct 22, 2021
@zooba
zooba marked this pull request as ready for review October 22, 2021 23:42
@zooba
zooba requested a review from a team as a code owner October 22, 2021 23:42
@zooba

zooba commented Oct 22, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

eryksun
eryksun reviewed Oct 23, 2021
View reviewed changes
Comment thread Python/fileutils.c Resolved

Uh oh!

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

eryksun
eryksun reviewed Oct 23, 2021
View reviewed changes
Comment thread Python/fileutils.c Resolved

Uh oh!

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

@ericsnowcurrently ericsnowcurrently mentioned this pull request Oct 25, 2021
Make getpath.c easier to follow. #28909
Closed
@gvanrossum gvanrossum mentioned this pull request Oct 26, 2021
bpo-43976: add vendor config #25718
Closed
@zooba
zooba requested a review from vsajip as a code owner October 28, 2021 19:56
@zooba
zooba force-pushed the bpo-42260 branch from 0c8ef95 to f5504cf Compare November 3, 2021 01:54
ericsnowcurrently
ericsnowcurrently reviewed Nov 4, 2021
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/site.py Resolved

Uh oh!

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

Comment thread Lib/site.py Resolved

Uh oh!

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

Comment thread Lib/sysconfig.py Resolved

Uh oh!

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

Comment thread Lib/test/test_embed.py Resolved

Uh oh!

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

Comment thread Lib/test/test_embed.py Resolved

Uh oh!

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

Comment thread Lib/test/test_embed.py Resolved

Uh oh!

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

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

Uh oh!

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

Comment thread Programs/_freeze_module.c Resolved

Uh oh!

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

Comment thread Modules/getpath.py Resolved

Uh oh!

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

Comment thread Python/initconfig.c Outdated

@ericsnowcurrently ericsnowcurrently Nov 4, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sorry, something went wrong.

Uh oh!

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

All reactions

@zooba zooba Nov 4, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

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

All reactions

@ericsnowcurrently ericsnowcurrently Nov 4, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

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

All reactions

@zooba zooba Nov 4, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

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

All reactions
ericsnowcurrently
ericsnowcurrently reviewed Nov 4, 2021
View reviewed changes
Comment thread Lib/test/test_embed.py Resolved

Uh oh!

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

@zooba zooba mentioned this pull request Nov 5, 2021
bpo-45720: Drop references to shlwapi #29417
Merged
zooba
zooba commented Nov 6, 2021
View reviewed changes
Comment thread Modules/getpath.py Resolved

Uh oh!

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

@zooba

zooba commented Nov 12, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

@zooba
bpo-45582: Port getpath[p].c to getpath.py
a63f5d8
@zooba
zooba force-pushed the bpo-42260 branch from 65f77e3 to a63f5d8 Compare November 15, 2021 23:22
@zooba

zooba commented Nov 15, 2021

Copy link
Copy Markdown
Member Author

There was nothing helpful in that history anyway ;-)

Tests all pass now, reviews appreciated!

All reactions

Sorry, something went wrong.

Uh oh!

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

@zooba zooba removed the DO-NOT-MERGE label Nov 15, 2021
vstinner
vstinner reviewed Nov 17, 2021
View reviewed changes
Comment thread Include/cpython/initconfig.h Resolved

Uh oh!

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

Comment thread Include/internal/pycore_fileutils.h Resolved

Uh oh!

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

Comment thread Python/initconfig.c Resolved

Uh oh!

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

Comment thread Python/initconfig.c Resolved

Uh oh!

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021
@zooba
Add missing PyMem_Free
c10d17c
@zooba

zooba commented Dec 2, 2021

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@zooba
zooba merged commit 99fcf15 into python:main Dec 3, 2021
@bedevere-bot bedevere-bot removed the awaiting core review label Dec 3, 2021
@zooba
zooba deleted the bpo-42260 branch December 3, 2021 00:08
@ericsnowcurrently ericsnowcurrently mentioned this pull request Jan 6, 2022
bpo-46263: Do not ever expect "use_frozen_modules" to be -1. #30438
Merged
@naveen521kk naveen521kk mentioned this pull request Mar 19, 2022
New venv launchers are broken msys2-contrib/cpython-mingw#90
Closed
@naveen521kk naveen521kk mentioned this pull request Jun 19, 2022
Python 3.11 Update msys2-contrib/cpython-mingw#101
Closed
3 of 4 tasks
@chrisburr chrisburr mentioned this pull request Oct 25, 2022
Add patch for use with Python 3.11 conda-forge/crossenv-feedstock#34
Merged
@filmor 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 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 maxbachmann mentioned this pull request Feb 24, 2023
remove outdated files from documentation #102216
Merged
@mwichmann mwichmann mentioned this pull request Jul 18, 2023
unittest import loop with pre-release Python SCons/scons#4376
Closed
@zooba zooba mentioned this pull request Apr 10, 2022
Rewrite getpath.c in Python #89745
Closed
@barneygale 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
@ales-erjavec
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
@ales-erjavec
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
@ales-erjavec @markotoplak
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
@ales-erjavec @markotoplak
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
@ales-erjavec @markotoplak
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
@FFY00 FFY00 mentioned this pull request Dec 15, 2024
Inconsistent platform support for taking the loaded libpython path into account in getpath #127970
Closed
@Lekensteyn Lekensteyn mentioned this pull request Jul 19, 2026
sys.executable can be a relative path contrary to documented behavior [3.11 regression] #154160
Open
@fedonman fedonman mentioned this pull request Jul 19, 2026
gh-154160: Make sys.executable absolute when resolved from a relative PATH entry #154163
Open
This was referenced Jul 19, 2026
Prefix search is filesystem-centric #41525
Closed
Recommend closing resolved or non-actionable issues (oldest-first batch, 2004–2007) #150566
Closed
@mattheww mattheww mentioned this pull request Sep 6, 2026
The search for pyvenv.cfg doesn't match PEP 405 #77483
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments
@eryksun eryksun eryksun left review comments
@ericsnowcurrently ericsnowcurrently ericsnowcurrently left review comments
@arhadthedev arhadthedev arhadthedev requested changes
@vsajip vsajip Awaiting requested review from vsajip vsajip is a code owner

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.

7 participants

@zooba @vstinner @bedevere-bot @eryksun @ericsnowcurrently @arhadthedev @the-knights-who-say-ni
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 01402503 — 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.