GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||
GeezerVison is showing github.com — bpo-45582: Port getpath[p].c to Python by zooba · Pull Request #29041 · python/cpython Conversation
|
|
This is very very WIP, but I wanted to share it now for discussion. My plan is to set it up so that 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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ping @vstinner |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
If I understood correctly, this code doesn't remove getpath.c yet. It's a WIP. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Correct. I just started removing it, and obviously everything is broken :) But I'll come back to it tomorrow.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
There was nothing helpful in that history anyway ;-) Tests all pass now, reviews appreciated! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Found the leak! So it's ready to merge once these tests pass. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041
Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041
Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041
Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041
Problem on MacOS exec_prefix is not resolved when symlink to bin/python3.11 Probably due to python/cpython#29041
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.