The Wayback Machine - https://web.archive.org/web/20211018231517/https://github.com/python/cpython/pull/28776
Skip to content
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

Jump to bottom

bpo-45396: Always import custom frozen modules. #28776

Closed
wants to merge 8 commits into from
Closed

bpo-45396: Always import custom frozen modules. #28776

wants to merge 8 commits into from

Conversation

@ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Oct 6, 2021 •
edited by bedevere-bot

If a Python build has a custom array set to PyImport_FrozenModules (see Python/frozen.c) then that array is used instead of the default one. In that case the whole point is that the custom frozen modules should be used. So in this change we ignore the PyConfig.use_frozen_modules setting if there are custom frozen modules. We also change the "-X frozen_modules" CLI option to "-X frozen_stdlib" to reflect the more specific context.

https://bugs.python.org/issue45396
@ericsnowcurrently
PyConfig.use_frozen_modules -> use_frozen_stdlib.
6438977
@ericsnowcurrently
use_frozen() -> use_frozen_stdlib().
1e5eee3
@ericsnowcurrently
Add _PyImport_FrozenStdlib.
8019f88
@ericsnowcurrently
Do not disable frozen modules if custom.
698aa33
@ericsnowcurrently
"-X frozen_modules" -> "-X frozen_stdlib".
952c9ba
@ericsnowcurrently
_imp._override_frozen_modules_for_tests -> _override_frozen_stdlib_fo…
05483f5 
…r_tests.
@ericsnowcurrently
Add a NEWS entry.
f494943
@ericsnowcurrently
Copy link
Member Author

@ericsnowcurrently ericsnowcurrently commented Oct 6, 2021

@brettcannon, this is the result of #28633 (comment).

@ericsnowcurrently
Fix test_embed.
7d2fad6
@ericsnowcurrently ericsnowcurrently mentioned this pull request Oct 7, 2021
Open
44 tasks
@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Oct 13, 2021

Can we have a bit more discussion about this before this is merged? Do you have an example of a real-word app that sets PyImport_FrozenModules? Depending on why that app is doing that, it might or might not make sense to allow disabling loading frozen modules. For example, if they use it to shadow .py modules that they have elsewhere in their package, it might still make sense to disable the frozen modules. Just because Brett initially misunderstood the meaning of -X frozen_modules doesn't mean that his interpretation is the most useful.

@gvanrossum gvanrossum mentioned this pull request Oct 13, 2021
Open
@ericsnowcurrently
Copy link
Member Author

@ericsnowcurrently ericsnowcurrently commented Oct 13, 2021

Setting PyImport_FrozenModules to define custom frozen modules is part of the API. Setting the data of an entry to NULL isn't so obviously a part of the API. However, the code in import.c definitely already supports this mechanism. So my intention was to preserve the existing behavior, whether or not folks are using it.

Regardless, the point is probably moot. If gh-28778 is merged (which I expect) then this PR is unnecessary. The only reason I hadn't closed this one is on the chance that the other PR ends up closed. In the meantime I'm closing this one.

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

@warsaw warsaw

Assignees
No one assigned
Labels
awaiting core review CLA signed
Projects
None yet
Milestone
No milestone
4 participants
@ericsnowcurrently @gvanrossum @the-knights-who-say-ni @bedevere-bot