Skip to content

bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations #25121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 7, 2021

Conversation

zooba
Copy link
Member

@zooba zooba commented Mar 31, 2021

@brettcannon brettcannon self-requested a review April 1, 2021 16:55
@zooba
Copy link
Member Author

zooba commented Apr 2, 2021

That's all I'm getting done for now, will probably have to fix more tests. Definitely needs updated whatsnew docs, because this is a more impactful change than just having imports succeed with relative paths in sys.path, but I think it's for the best.

@eryksun
Copy link
Contributor

eryksun commented Apr 2, 2021

Do you think changes to ntpath should be split off as a separate bpo issue that's just for 3.10? I think some developers want the current behavior that classifies "/spam" as absolute, so, for future reference, it would be good to have an issue that's more focused on that change.

For resolving bpo-43105 in 3.8, nt._path_splitroot can use PathSkipRootW. It supports rooted paths, absolute drive paths, and UNC paths, including device paths such as "\\?\C:\", "\\.\DEVICE\", and "\\?\UNC\server\share\". One caveat is that PathSkipRootW returns NULL for drive-relative paths such as "C:spam". That case is simple enough to handle in C to make it consistent with PathCchSkipRoot.

@zooba
Copy link
Member Author

zooba commented Apr 6, 2021

Git is getting really messed up here, I'm going to do a reset and rebase to make sure I only have my changes in here.

@zooba zooba removed the DO-NOT-MERGE label Apr 6, 2021
@zooba
Copy link
Member Author

zooba commented Apr 6, 2021

Assuming all the tests pass, this change doesn't affect ntpath and so can be backported. Then I can easily add the ntpath changes back separately (later, because they rely on the changes in here).

@zooba
Copy link
Member Author

zooba commented Apr 6, 2021

The Azure Pipelines failure is the flaky asyncio test, so unless @eryksun spots something in the next hour or so, I'm going to hit merge.

@zooba zooba merged commit 04732ca into python:master Apr 7, 2021
@miss-islington
Copy link
Contributor

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

@zooba zooba deleted the bpo-43105 branch April 7, 2021 00:02
@miss-islington
Copy link
Contributor

Sorry, @zooba, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 04732ca993fa077a8b9640cc77fb2f152339585a 3.9

@miss-islington
Copy link
Contributor

Sorry @zooba, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 04732ca993fa077a8b9640cc77fb2f152339585a 3.8

zooba added a commit to zooba/cpython that referenced this pull request Apr 7, 2021
@bedevere-bot
Copy link

GH-25237 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Apr 7, 2021
zooba added a commit that referenced this pull request Apr 7, 2021
zooba added a commit to zooba/cpython that referenced this pull request Apr 9, 2021
zooba added a commit that referenced this pull request Apr 9, 2021
@mattip
Copy link
Contributor

mattip commented Jun 13, 2021

@zooba It is a shame there are no direct tests of os._path_splitroot, and no good documentation what the code is doing. What does "Removes everything after the root on Win32" mean? What is defined as "the root"?

Adding a new method to os in this way makes it hard for alternative interpreters like PyPy to catch up, especially since the function is required in order to bootstrap the interpreter.

kuba2k2 added a commit to kuba2k2/cpython that referenced this pull request Nov 13, 2021
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit 04732ca.
kuba2k2 added a commit to kuba2k2/cpython that referenced this pull request Nov 13, 2021
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit 0af99b4.
thexai added a commit to thexai/cpython that referenced this pull request Oct 12, 2022
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit eed7686.
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Jan 12, 2023
Summary:
This upstream PR (python/cpython#25121) makes all
module paths (e.g. in code object `co_filename`) absolute instead of relative. That
currently breaks PyPerf, so we could temporarily land this revert to accelerate
3.10 rollout without waiting for another PyPerf rollout.

Reviewed By: itamaro

Differential Revision: D42463507

fbshipit-source-id: 73dacb2
kuba2k2 added a commit to kuba2k2/cpython that referenced this pull request Dec 5, 2024
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit 04732ca.
kuba2k2 added a commit to kuba2k2/cpython that referenced this pull request Dec 5, 2024
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit 04732ca.
kuba2k2 added a commit to kuba2k2/cpython that referenced this pull request Dec 5, 2024
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit 04732ca.
kuba2k2 added a commit to kuba2k2/cpython that referenced this pull request Dec 5, 2024
…g module spec objects from file locations (pythonGH-25121)"

This reverts commit 04732ca.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants