Skip to content

gh-136264: Fix --relative-paths for PEP 739's build-details.json - #138510

Merged
AA-Turner merged 5 commits into
python:mainfrom
AA-Turner:pep739-relative-paths
Sep 8, 2025
Merged

gh-136264: Fix --relative-paths for PEP 739's build-details.json#138510
AA-Turner merged 5 commits into
python:mainfrom
AA-Turner:pep739-relative-paths

Conversation

@AA-Turner

@AA-TurnerAA-Turner commented Sep 4, 2025•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member
  • KeyError is not raised for defaultdict, use an explicit test
  • Fix relative paths on different drives on Windows
  • Add a round-trip test

@AA-Turner
Fix `--relative-paths for PEP 739's build-details.json
eae8c9d
* KeyError is not raised for defaultdict
* Fix relative paths on different drives on Windows
* Add a round-trip test
@AA-TurnerAA-Turner added the needs backport to 3.14 bugs and security fixes label Sep 4, 2025
@AA-TurnerAA-Turner changed the title gh-136264: Fix `--relative-paths for PEP 739's build-details.jsongh-136264: Fix --relative-paths for PEP 739's build-details.jsonSep 4, 2025
@bedevere-appbedevere-appBot mentioned this pull request Sep 4, 2025
Closed
AA-Turner added a commit to AA-Turner/cpython that referenced this pull request Sep 4, 2025
@AA-Turner
Revert relative-path changes.
6a2cc71
Split to python#138510
@AA-TurnerAA-Turner mentioned this pull request Sep 4, 2025
Closed
itamaro
itamaro requested changes Sep 7, 2025
View reviewed changes

@itamaroitamaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like CI is unhappy

Comment threadTools/build/generate-build-details.py
Comment threadTools/build/generate-build-details.py Outdated
@AA-Turner@itamaro
Update generate-build-details.py
69684c2
Co-authored-by: Itamar Oren <itamarost@gmail.com>
AA-Turner
AA-Turner commented Sep 8, 2025
View reviewed changes
Comment threadLib/test/test_build_details.py Outdated
AA-Turner
AA-Turner commented Sep 8, 2025
View reviewed changes
Comment threadLib/test/test_build_details.py Outdated
@AA-Turner
AA-Turner merged commit 057ee17 into python:mainSep 8, 2025
51 checks passed
@miss-islington-app

miss-islington-appBot commented Sep 8, 2025

Copy link
Copy Markdown

Thanks @AA-Turner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 8, 2025
@AA-Turner@itamaro@miss-islington
pythongh-136264: Fix--relative-pathsfor PEP 739's build-details…
0537581
….json (pythonGH-138510)
* KeyError is not raised for defaultdict
* Fix relative paths on different drives on Windows
* Add a round-trip test
(cherry picked from commit 057ee17)
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
@bedevere-app

bedevere-appBot commented Sep 8, 2025

Copy link
Copy Markdown

GH-138638 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Sep 8, 2025
@AA-Turner
AA-Turner deleted the pep739-relative-paths branch September 8, 2025 09:15
@AA-TurnerAA-Turner mentioned this pull request Sep 8, 2025
Closed
hugovk added a commit that referenced this pull request Sep 9, 2025
@miss-islington@AA-Turner
@itamaro@hugovk
[3.14]gh-136264: Fix--relative-pathsfor PEP 739's build-detail…
3894503
…s.json (GH-138510) (#138638)
* gh-136264: Fix ``--relative-paths`` for PEP 739's build-details.json (GH-138510)
* KeyError is not raised for defaultdict
* Fix relative paths on different drives on Windows
* Add a round-trip test
(cherry picked from commit 057ee17)
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
* Update test_build_details.py
* Update Lib/test/test_build_details.py
---------
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@AA-Turner@itamaro@lkollar
pythongh-136264: Fix--relative-pathsfor PEP 739's build-details…
85a7626
….json (python#138510)
* KeyError is not raised for defaultdict
* Fix relative paths on different drives on Windows
* Add a round-trip test
Co-authored-by: Itamar Oren <itamarost@gmail.com>
@DinoV

DinoV commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Hey @AA-Turner we may be a special snowflake here but the import importlib and then importlib.util.... usage has broken running this test internally for Cinder. It seems like this is usually imported by pyrun (and/or the new REPL) but there may be other ways for things to run w/o importing it. Do you think this should be a import importlib.util?

@AA-Turner

AA-Turner commented Sep 11, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Hmm, does this pattern reproduce generally, or is it just specific to this test?

If the former, I'm inclined to think it might be a bug in Cinder.

Will return to consider this tonight (UK time).

A

facebook-github-bot pushed a commit to facebookincubator/cinderx that referenced this pull request Sep 11, 2025
@DinoV@facebook-github-bot
Always import importlib.util
0fa0547
Summary:
An upstream change uses importlib.util after only importing importlib: python/cpython#138510
In normal CPython this gets mported via runpy so it pretty much always available. In our dev environment we don't import it, so this just makes sure it gets imported for test purposes.
Reviewed By: yoney
Differential Revision: D82228954
fbshipit-source-id: 86e1cd776d2a3819ffff9c6069613d141ab88dd1
@DinoV

DinoV commented Sep 12, 2025

Copy link
Copy Markdown
Contributor

I'm not sure how to trigger it on stock CPython which is why I just started the discussion rather than opening an issue. But generally "import foo" followed by using a subpackage of foo without importing it is a bit of an anti-pattern - it's relying on someone else having imported the subpackage earlier.

In my (limited) experiments importlib.util was either being imported via pyrepl (normal startup path) or runpy (doing python -c "import importlib; print(dir(importlib))". I didn't dig past that and just made sure we have it imported.

There may be some other embedding scenarios but maybe not too many running the test suite!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@itamaroitamaroitamaro approved these changes
@FFY00FFY00Awaiting requested review from FFY00FFY00 is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AA-Turner@DinoV@itamaro