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 — gh-123085: _compile_importlib: Avoid copying sources before compilation by encukou · Pull Request #124131 · python/cpython

Conversation

@encukou

@encukou encukou commented Sep 16, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Make the test helper compile directly from the “installed” sources.
This should avoid doing some unnecessary work, as well as possible issues in copying too much/too little metadata to a different filesystem. Hopefully, it'll fix the buildbot failure.

  • Issue: importlib.resources.files() doesn't work correctly when importlib library is compiled #123085

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@encukou
_compile_importlib: Avoid copying sources before compilation
cade8d2
@bedevere-app bedevere-app Bot mentioned this pull request Sep 16, 2024
importlib.resources.files() doesn't work correctly when importlib library is compiled #123085
Open
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Sep 16, 2024
@encukou encukou added the skip news label Sep 16, 2024
@encukou

encukou commented Sep 16, 2024

Copy link
Copy Markdown
Member Author

This now doesn't copy resources of importlib.resources, which we currently don't have.
Assuming the overall approach is fine, would it be OK to mention resources in a comment, or should this also copy non-.py files? Either way, there's no good way to test what would happen if some resources were there.
jaraco reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

encukou commented Sep 16, 2024

Copy link
Copy Markdown
Member Author

!buildbot fedora
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Sep 16, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit cade8d2 🤖

The command will test the builders whose names match following regular expression: fedora

The builders matched are:

  • s390x Fedora Clang PR
  • PPC64LE Fedora Stable Clang Installed PR
  • s390x Fedora Rawhide Clang Installed PR
  • s390x Fedora LTO + PGO PR
  • AMD64 Fedora Stable Clang PR
  • aarch64 Fedora Rawhide PR
  • PPC64 Fedora PR
  • aarch64 Fedora Rawhide LTO + PGO PR
  • PPC64LE Fedora Rawhide LTO + PGO PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • aarch64 Fedora Rawhide Refleaks PR
  • s390x Fedora Rawhide Clang PR
  • AMD64 Fedora Stable LTO PR
  • PPC64LE Fedora Rawhide PR
  • aarch64 Fedora Stable Refleaks PR
  • aarch64 Fedora Stable PR
  • s390x Fedora PR
  • aarch64 Fedora Stable Clang PR
  • s390x Fedora Clang Installed PR
  • AMD64 Fedora Rawhide Refleaks PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • PPC64LE Fedora Stable Clang PR
  • PPC64LE Fedora Stable LTO PR
  • aarch64 Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide Refleaks PR
  • AMD64 Fedora Rawhide NoGIL PR
  • AMD64 Fedora Stable PR
  • aarch64 Fedora Stable LTO + PGO PR
  • AMD64 Fedora Rawhide PR
  • PPC64LE Fedora Rawhide Clang PR
  • s390x Fedora Refleaks PR
  • aarch64 Fedora Rawhide Clang PR
  • s390x Fedora LTO PR
  • PPC64LE Fedora Rawhide Clang Installed PR
  • PPC64LE Fedora Rawhide LTO PR
  • s390x Fedora Rawhide LTO + PGO PR
  • PPC64LE Fedora Stable Refleaks PR
  • PPC64LE Fedora Stable PR
  • aarch64 Fedora Stable LTO PR
  • AMD64 Fedora Rawhide LTO PR
  • AMD64 Fedora Stable Clang Installed PR
  • AMD64 Fedora Rawhide Clang PR
  • aarch64 Fedora Rawhide NoGIL refleaks PR
  • PPC64LE Fedora Stable LTO + PGO PR
  • aarch64 Fedora Rawhide LTO PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • aarch64 Fedora Rawhide Clang Installed PR
  • s390x Fedora Rawhide PR
  • s390x Fedora Rawhide LTO PR
  • aarch64 Fedora Stable Clang Installed PR
  • AMD64 Fedora Stable LTO + PGO PR
  • AMD64 Fedora Rawhide LTO + PGO PR
  • AMD64 Fedora Rawhide Clang Installed PR
  • AMD64 Fedora Stable Refleaks PR
  • s390x Fedora Rawhide Refleaks PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
encukou marked this pull request as ready for review September 17, 2024 09:01
@encukou
encukou requested review from FFY00, jaraco and warsaw as code owners September 17, 2024 09:01
@bedevere-app bedevere-app Bot added the awaiting core review label Sep 17, 2024
vstinner
vstinner reviewed Sep 17, 2024
View reviewed changes
Comment thread Lib/test/test_importlib/resources/test_files.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
encukou and others added 3 commits September 17, 2024 14:25
@encukou
Remove debug print
5455d06
@encukou
Add comment
211b436
@jaraco
Rely on pathlib.Path.glob for simplicity.
8cd8fe9
@jaraco

jaraco commented Sep 17, 2024

Copy link
Copy Markdown
Member

!buildbot fedora
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Sep 17, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @jaraco for commit 8cd8fe9 🤖

The command will test the builders whose names match following regular expression: fedora

The builders matched are:

  • s390x Fedora Clang PR
  • PPC64LE Fedora Stable Clang Installed PR
  • s390x Fedora Rawhide Clang Installed PR
  • s390x Fedora LTO + PGO PR
  • AMD64 Fedora Stable Clang PR
  • aarch64 Fedora Rawhide PR
  • PPC64 Fedora PR
  • aarch64 Fedora Rawhide LTO + PGO PR
  • PPC64LE Fedora Rawhide LTO + PGO PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • aarch64 Fedora Rawhide Refleaks PR
  • s390x Fedora Rawhide Clang PR
  • AMD64 Fedora Stable LTO PR
  • PPC64LE Fedora Rawhide PR
  • aarch64 Fedora Stable Refleaks PR
  • aarch64 Fedora Stable PR
  • s390x Fedora PR
  • aarch64 Fedora Stable Clang PR
  • s390x Fedora Clang Installed PR
  • AMD64 Fedora Rawhide Refleaks PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • PPC64LE Fedora Stable Clang PR
  • PPC64LE Fedora Stable LTO PR
  • aarch64 Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide Refleaks PR
  • AMD64 Fedora Rawhide NoGIL PR
  • AMD64 Fedora Stable PR
  • aarch64 Fedora Stable LTO + PGO PR
  • AMD64 Fedora Rawhide PR
  • PPC64LE Fedora Rawhide Clang PR
  • s390x Fedora Refleaks PR
  • aarch64 Fedora Rawhide Clang PR
  • s390x Fedora LTO PR
  • PPC64LE Fedora Rawhide Clang Installed PR
  • PPC64LE Fedora Rawhide LTO PR
  • s390x Fedora Rawhide LTO + PGO PR
  • PPC64LE Fedora Stable Refleaks PR
  • PPC64LE Fedora Stable PR
  • aarch64 Fedora Stable LTO PR
  • AMD64 Fedora Rawhide LTO PR
  • AMD64 Fedora Stable Clang Installed PR
  • AMD64 Fedora Rawhide Clang PR
  • aarch64 Fedora Rawhide NoGIL refleaks PR
  • PPC64LE Fedora Stable LTO + PGO PR
  • aarch64 Fedora Rawhide LTO PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • aarch64 Fedora Rawhide Clang Installed PR
  • s390x Fedora Rawhide PR
  • s390x Fedora Rawhide LTO PR
  • aarch64 Fedora Stable Clang Installed PR
  • AMD64 Fedora Stable LTO + PGO PR
  • AMD64 Fedora Rawhide LTO + PGO PR
  • AMD64 Fedora Rawhide Clang Installed PR
  • AMD64 Fedora Stable Refleaks PR
  • s390x Fedora Rawhide Refleaks PR
All reactions

Sorry, something went wrong.

Uh oh!

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

jaraco commented Sep 17, 2024

Copy link
Copy Markdown
Member

I pushed a new commit to trim down the logic substantially. Let me know what you think.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jaraco
jaraco approved these changes Sep 17, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Sep 17, 2024
@encukou

encukou commented Sep 18, 2024

Copy link
Copy Markdown
Member Author

Looks good, thank you!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
encukou merged commit 42c8b05 into python:main Sep 18, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 18, 2024
@encukou
encukou deleted the compile-importlib branch September 18, 2024 12:14
@zware zware mentioned this pull request Sep 18, 2024
Fix make htmllive target #124219
Merged
@jaraco

jaraco commented Sep 19, 2024

Copy link
Copy Markdown
Member

I cherry-picked the change to python/importlib_resources@cc5d9cbb6e.
encukou reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
savannahostrowski pushed a commit to savannahostrowski/cpython that referenced this pull request Sep 22, 2024
@encukou @jaraco @savannahostrowski
pythongh-123085: _compile_importlib: Avoid copying sources before com…
ef597bc 
…pilation (pythonGH-124131)

Co-authored-by: Jason R. Coombs <[email protected]>
@encukou encukou mentioned this pull request Jan 7, 2025
[3.12] gh-123085: Fix issue in inferred caller when resources package has no source (GH-123102) #124021
Merged
@encukou encukou added the needs backport to 3.12 only security fixes label Jan 7, 2025
@miss-islington-app

miss-islington-app Bot commented Jan 7, 2025

Copy link
Copy Markdown

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 7, 2025
@encukou @jaraco @miss-islington
pythongh-123085: _compile_importlib: Avoid copying sources before com…
1871e2e 
…pilation (pythonGH-124131)

(cherry picked from commit 42c8b05)

Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Jason R. Coombs <[email protected]>
@bedevere-app

bedevere-app Bot commented Jan 7, 2025

Copy link
Copy Markdown

GH-128581 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 7, 2025
encukou added a commit that referenced this pull request Jan 7, 2025
@miss-islington @encukou @jaraco
[3.12] gh-123085: _compile_importlib: Avoid copying sources before co…
e758eb3 
…mpilation (GH-124131) (GH-128581)

gh-123085: _compile_importlib: Avoid copying sources before compilation (GH-124131)
(cherry picked from commit 42c8b05)

Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Jason R. Coombs <[email protected]>
@encukou encukou added the needs backport to 3.13 only security fixes label Aug 18, 2025
@miss-islington-app

miss-islington-app Bot commented Aug 18, 2025

Copy link
Copy Markdown

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 18, 2025
@encukou @jaraco @miss-islington
pythongh-123085: _compile_importlib: Avoid copying sources before com…
fdbaa06 
…pilation (pythonGH-124131)

(cherry picked from commit 42c8b05)

Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Jason R. Coombs <[email protected]>
@bedevere-app

bedevere-app Bot commented Aug 18, 2025

Copy link
Copy Markdown

GH-137914 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Aug 18, 2025
pablogsal pushed a commit that referenced this pull request Aug 18, 2025
@miss-islington @encukou @jaraco
[3.13] gh-123085: _compile_importlib: Avoid copying sources before co…
fc2e84f 
…mpilation (GH-124131) (#137914)

gh-123085: _compile_importlib: Avoid copying sources before compilation (GH-124131)
(cherry picked from commit 42c8b05)

Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Jason R. Coombs <[email protected]>
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
@jaraco jaraco jaraco approved these changes
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner
@FFY00 FFY00 Awaiting requested review from FFY00 FFY00 is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

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.

4 participants

@encukou @bedevere-bot @jaraco @vstinner
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.