-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
modulegraph: Use loaders to get module content. #5157
Conversation
This comment has been minimized.
This comment has been minimized.
a655150
to
a59e45b
Compare
Tried running this against a simple script containing just one line: Command and output
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@agronholm Please give the new code a try, which should now fix the issue. |
61344fb
to
1adbe4f
Compare
Seems to work. I don't get any errors anymore. Was the hook a necessary part of this fix somehow? |
@agronholm The hook is not part of the fix, but the hook is required for freezing pydantic. |
1adbe4f
to
498aa35
Compare
While working on the following commits, I stepped over add_to_suffix__extension() returning wrong results. Obviously unit-tests have been missing.
These tests check for "recursion too deep" caused by the `__init__` module being an extension module. See pyinstaller#5131, pyinstaller#4346.
498aa35
to
b38c5d9
Compare
modulegraph's heritage includes code still based on Python 2.x capabilities on how to find a module and get its source or code. It also contained a anomaly regarding packages and their ``__init__``-file: If a package was detected, it's ``__init__`` file was loaded as a module. This, while being ugly, worked in most cases, but failed if the ``__init__`` module is an extension module (see pyinstaller#5131, pyinstaller#4346), ending in an infinite loop. This was caused by modulegraph distinguishing between the package and its ``__init__`` module. The solution is to switch to "modern" loaders, both being a loader for a specific type of modules (source, extension, etc.) and having a package characteristic (property ``is_package()``) This commit does the following - In ``_find_module_path()`` no longer return "metadata" but a loader. This also removed huge part of this function, making it much easier to understand. As a side effect, this asymmetric closing of a file in a completely other part of the code (``_safe_import_module``) could be removed. - Change ``_load_module`` to use the loaders. - Merge ``_load_package`` into `__load_module``, getting rid of the anomaly described above. - Adjust the test-cases to the new behavior (esp. loader instead of metadata-tuple and filehandle) Please note: Since we plan to change to modulegraph2 soon anyway, I did not spend too much time on creating a clean solution. See pyinstaller#4406, closes pyinstaller#5131, pyinstaller#4346.
While the latest changes fixed modulegraph to not go into an endless recursion if a package's `__init__` is an extension module, there still was an issue: The returned node was of type ``Package``, leading to ``PYZ()`` trying to load and compile the source. Since the file is an extension module, compiling fails, of course. This change adds a test-case for this.
Introduce a new node type ExtensionPackage, which signals that a package's `__init__` is an extension module. While the latest changes fixed modulegraph to not go into an endless recursion if a package's `__init__` is an extension module, there still was an issue: The returned node was of type ``Package``, leading to ``PYZ()`` trying to load and compile the source (to get "missing" the code-object). Since the file is an extension module, compiling failed, of course.
This maps the modulegraph nodes of type ExtensionPackage to simple extensions for the sake of the TOC.
Add modulegraph's new node type ExtensionPackage.
Converting modulegraph Nodes into TOC entries was done twice. The code was the same, except that one implementation did not filter node types and had less comments.
When converting the nodes to TOC entries, also change the filename for ExtensionPackage: Actually convert the packages name (``mypkg``) into the module name (``mypkg.__init__``).
b38c5d9
to
49002f3
Compare
modulegraph's heritage includes code still based on Python 2.x capabilities on
how to find a module and get its source or code. It also contained a anomaly
regarding packages and their
__init__
-file: If a package was detected,it's
__init__
file was loaded as a module. This, while being ugly, workedin most cases, but failed if the
__init__
module is an extensionmodule (see #5131, #4346), ending in an infinite loop. This was caused by
modulegraph distinguishing between the package and its
__init__
module.The solution is to switch to "modern" loaders, both being a loader for a
specific type of modules (source, extension, etc.) and having a package
characteristic (property
is_package()
)This commit does the following
In
_find_module_path()
no longer return "metadata" but a loader. Thisalso removed huge part of this function, making it much easier to understand.
As a side effect, this asymmetric closing of a file in a completely other
part of the code (
_safe_import_module
) could be removed.Change
_load_module
to use the loaders.Merge
_load_package
into `__load_module``, getting rid of the anomalydescribed above.
Adjust the test-cases to the new behavior (esp. loader instead of
metadata-tuple and filehandle)
Please note: Since we plan to change to modulegraph2 soon anyway, I did not
spend too much time on creating a clean solution.
See #4406, closes #5131, #4346.