Skip to content

Add support for pkgutil.iter_modules#5959

Merged
rokm merged 2 commits intopyinstaller:developfrom
rokm:pkgutil-itermodules
Jul 11, 2021
Merged

Add support for pkgutil.iter_modules#5959
rokm merged 2 commits intopyinstaller:developfrom
rokm:pkgutil-itermodules

Conversation

@rokm
Copy link
Copy Markdown
Member

@rokm rokm commented Jun 30, 2021 •
edited
Loading

Implement support for pkgutil.iter_modules.

This is done by an rthook that monkey-patches the function with our implementation, which extends the contents listed by original implementation (on-filesystem resources) with PYZ-embedded resources.

Closes #1905.
Fixes #5167 (the iter_modules not returning anything unless noarchive is used; however, the modules still need to be collected by collect_submodules in the first place).
Fixes #5799.

@rokm rokm force-pushed the pkgutil-itermodules branch 3 times, most recently from d87b987 to 6f8cc30 Compare July 1, 2021 06:17
@rokm rokm mentioned this pull request Jul 1, 2021
Merged
@rokm rokm force-pushed the pkgutil-itermodules branch from 6f8cc30 to 843e527 Compare July 1, 2021 09:46
@rokm
Copy link
Copy Markdown
Member Author

rokm commented Jul 1, 2021

The initial plan for this was to implement support for iter_modules in our FrozenImporter. However, it turns out that pkgutil does not even consider it when resolving importers, because pkgutil.get_importer looks only for importers that are in sys.path_hooks (i.e., path entry finders), whereas FrozenImporter is registered on sys.meta_path (as meta path finder).

While it is possible to create a factory/finder for sys.path_hooks, this would mean that FrozenImporter becomes responsible for both PYZ-embedded resources and the on-filesystem resources (binary extensions, or .pyc modules in noarchive build). This seems rather excessive, and introduces additional complications (such as requiring os.path and/or pathlib).

All in all, it seems easier and cleaner to monkey-patch pkgutil.iter_modules to extend the results of original implementation (which lists on-filesystem resources discovered by python's FileFinder) with PYZ-embedded entries from FrozenImporter's TOC.

@rokm rokm mentioned this pull request Jul 1, 2021
Closed
@rokm rokm marked this pull request as ready for review July 1, 2021 12:13
Legorooj
Legorooj approved these changes Jul 8, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@Legorooj Legorooj left a comment

Choose a reason for hiding this comment

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

Looks good.

bwoodsend
bwoodsend requested changes Jul 8, 2021
View reviewed changes
rokm added 2 commits July 8, 2021 11:49
@rokm
tests: add tests for pkgutil.iter_modules
6168be4 
Test that pkgutil.iter_modules() lists same contents of a package in
unfrozen and frozen version. For tests, we use altgraph (pure python)
and psutil (contains binary extensions), as well as psutil.tests
(as an example of a sub-package).

We also test both archive and noarchive mode, because in the latter,
.pyc modules end up directly on filesystem, and are subject of python's
built-in FileFinder.

There, noarchive variants of tests pass without additional changes,
while archive variants fail as frozen versions do not list PYZ-embedded
modules.
@rokm
hooks: add rthook for pkgutil to override iter_modules
4447920 
Provide custom implementation of iter_modules() that works with
our FrozenImporter and allows listing of sub-modules that are
embedded in the PYZ archive.
@rokm rokm force-pushed the pkgutil-itermodules branch from 843e527 to 4447920 Compare July 8, 2021 09:49
@bwoodsend bwoodsend added the merge-on-ci-pass This PR is ready to merge providing CI passes label Jul 8, 2021
@rokm rokm merged commit 9d57730 into pyinstaller:develop Jul 11, 2021
@rokm rokm deleted the pkgutil-itermodules branch July 11, 2021 08:37
This was referenced Aug 8, 2021
Closed
Closed
copybara-service bot pushed a commit to GoogleCloudPlatform/gcpdiag that referenced this pull request Oct 7, 2021
@schweikert @copybara-github
disable pyinstaller hack for iter_modules
1185afa 
This is not needed with pyinstaller 4.4 and in fact causes
every module to be listed twice.

See also: pyinstaller/pyinstaller#5959

Change-Id: Ic188da1bea27ed8fbb2526d7171df8ece75b88ed
GitOrigin-RevId: 77601a8
@gentlegiantJGC gentlegiantJGC mentioned this pull request Jan 22, 2022
Closed
@tplessas tplessas mentioned this pull request Jun 1, 2022
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bwoodsend bwoodsend bwoodsend requested changes

@Legorooj Legorooj Legorooj approved these changes

Assignees

No one assigned

Labels

merge-on-ci-pass This PR is ready to merge providing CI passes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A Module Error Bug Submodules within a namespace package do not appear Support pkgutil.iter_modules

3 participants

@rokm @bwoodsend @Legorooj