Skip to content

gh-121735: Fix module-adjacent references in zip files#123037

Merged
jaraco merged 8 commits intopython:mainfrom
jaraco:gh-121735/zip-module-resources
Sep 12, 2024
Merged

gh-121735: Fix module-adjacent references in zip files#123037
jaraco merged 8 commits intopython:mainfrom
jaraco:gh-121735/zip-module-resources

Conversation

@jaraco
Copy link
Copy Markdown
Member

@jaraco jaraco commented Aug 15, 2024 •
edited
Loading

Merge after #123028.

@jaraco jaraco added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Aug 15, 2024
@jaraco jaraco changed the title Fix module-adjacent references in zip files gh-121735: Fix module-adjacent references in zip files Aug 15, 2024
@bedevere-app bedevere-app bot mentioned this pull request Aug 15, 2024
Closed
@jaraco
Copy link
Copy Markdown
Member Author

jaraco commented Aug 15, 2024

Note that a lot of refactoring happened in the test suite to enable testing this behavior. The history of the refactoring can be found in python/importlib_resources@f77da58...23afc90. These changes were released with importlib_resources 6.4.1.

@jaraco
Copy link
Copy Markdown
Member Author

jaraco commented Aug 15, 2024

I'm a little apprehensive about the prospect of backporting this to Python 3.12, given the amount of refactoring that had to happen in the tests to make them viable. I may opt to backport the fix but limit the amount of changes backported to the tests, depending on how complicated it turns out to be.

@jaraco jaraco marked this pull request as ready for review August 15, 2024 19:55
@jaraco jaraco mentioned this pull request Aug 16, 2024
Merged
FFY00
FFY00 approved these changes Aug 16, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

This looks okay to me.

I don't really have any strong opinion regarding backporting the changes to 3.12. I agree it would be nicer to backport without the large test suite refactoring, but that isn't unreasonable to me if the changes don't end up applying cleanly. Not backporting the test suite changes also increases the possibility for test suite bugs to sneak into the 3.12 branch, as that won't be maintained as closely as the main branch. Additionally, it can also cause conflicts when backporting other fixes, so, eh, I guess I am +0.5 on backporting this PR as a whole.

@jaraco
Copy link
Copy Markdown
Member Author

jaraco commented Aug 16, 2024

@Gatsik - the implementation provided herein has already been reviewed and released as importlib_resources 6.4.1. It proves the fix and is available.

The one thing that it misses is that it doesn't fix the issue when the importlib library itself is compiled -- in that case inferring caller doesn't work correctly (my attempt to fix it: Gatsik@50c1596)

It's not obvious to me from this fix what the failure mode is. I think I see it now - if the _common module is available as bytecode but has no source, then __file__ may not be valid. I think that issue would affect both zip files and direct disk files. Would you be willing to write up a separate issue, either in this repo or in python/importlib_resources that describes the missed expectation and why it's an important use-case to support? It's certainly something worthy of consideration, but it's also something that can be considered independent of this PR.

@Gatsik Gatsik mentioned this pull request Aug 17, 2024
Open
@Gatsik
Copy link
Copy Markdown

Gatsik commented Aug 17, 2024

@jaraco sure! i created an issue #123085

@jaraco jaraco mentioned this pull request Aug 17, 2024
Merged
1 task
@jaraco jaraco merged commit ba687d9 into python:main Sep 12, 2024
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Sep 12, 2024

Thanks @jaraco for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 12, 2024
@jaraco @miss-islington
pythongh-121735: Fix module-adjacent references in zip files (pythonG…
6ec873e 
…H-123037)

* pythongh-116608: Apply style and compatibility changes from importlib_metadata.

* pythongh-121735: Ensure module-adjacent resources are loadable from a zipfile.

* pythongh-121735: Allow all modules to be processed by the ZipReader.

* Add blurb

* Remove update-zips script, unneeded.

* Remove unnecessary references to removed static fixtures.

* Remove zipdata fixtures, unused.
(cherry picked from commit ba687d9)

Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Sep 12, 2024

Sorry, @jaraco, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker ba687d9481c04fd160795ff8d8568f5c9f877128 3.12

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Sep 12, 2024

GH-123986 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 12, 2024
@bedevere-bot
Copy link