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

pre-safe-import-hooks: gracefully handle cases when six is unavailable #8145

Merged
merged 2 commits into from Dec 4, 2023

Conversation

rokm
Copy link
Member

@rokm rokm commented Dec 4, 2023

In pre-safe-import hooks that deal with six (or vendored six), gracefully handle the cases when the six module is unavailable.

In general, the pre-safe-import hooks might end up being called for non-existent modules, and the hooks need to account for that (even though the behavior seems to depend on how the missing module is imported).

For example, a program with a duplicated import,

import six.moves
import six.moves

seems to end up triggerring the six.moves pre-safe-import hook on the second import. The same happens in more complex scenarios where these imports are scattered across different modules.

In pre-safe-import hooks that deal with `six` (or vendored `six`),
gracefully handle the cases when the `six` module is unavailable.

In general, the pre-safe-import hooks might end up being called
for non-existent modules, and the hooks need to account for that
(even though the behavior seems to depend on how the missing
module is imported).

For example, a program with a duplicated import,

```
import six.moves
import six.moves
```

seems to end up triggerring the `six.moves` pre-safe-import
hook on the second import. The same happens in more complex
scenarios where these imports are scattered across different
modules.
@rokm
Copy link
Member Author

rokm commented Dec 4, 2023

This is something that came up in pyinstaller/pyinstaller-hooks-contrib#666, where the introduced thorough import analysis ends up triggering the six.moves pre-safe-import hook.

The pre-safe-import hooks may in fact be called even if their corresponding module cannot be found by modulegraph - this is the mechanism that our gi.repository.* hooks (pre-safe-import and standard hooks) rely on.

The actual behavior of this particular case (missing parent package and actual hook's sub-package) seems to depend on import order (i.e., whether parent package (six) has already been determined to be missing or not). For example

import six.moves
import six.moves

triggers the hook.

import six
import six.moves

does as well.

But

import six.moves
import six

does not.

… available

If the pre-safe-import hook for `six` (or one of its vendored
versions) cannot import `six` (or the corresponding vendored
version of it), do not create the `six.move` run-time
package in the modulegraph.
@rokm
Copy link
Member Author

rokm commented Dec 4, 2023

Slight revision - I realized that if we cannot import six, we should probably not be creating the run-time package entry for six.moves in the modulegraph, either. This is mostly for the sake of consistency.

@rokm rokm enabled auto-merge (rebase) December 4, 2023 21:46
@rokm rokm merged commit 8c4d099 into pyinstaller:develop Dec 4, 2023
18 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2024
@rokm rokm deleted the harden-six-pre-safe-import-hooks branch March 7, 2024 12:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants