Skip to content

Commit

Permalink
hooks: fix pkgutil.iter_modules when paths involve symbolic links
Browse files Browse the repository at this point in the history
In our pkgutil.iter_module replacement, we need to fully resolve
both the sys._MEIPASS and the given search paths, in case either
contains a symbolic link. Failing to do so leads to path mis-match
when symbolic links are involved and the given search paths are
already fully resolved. This may happen on macOS with onefile
builds if the caller of pkgutil.iter_module fully resolves the
search path(s) before calling the function (issue #6537).

Add a test that reproduces scenario from #6537.
  • Loading branch information
rokm committed Jan 25, 2022
1 parent e2945a6 commit 51df140
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 5 deletions.
9 changes: 7 additions & 2 deletions PyInstaller/hooks/rthooks/pyi_rth_pkgutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,16 @@ def _pyi_pkgutil_iter_modules(path=None, prefix=''):
yield pkgutil.ModuleInfo(importer, prefix + entry, is_pkg)
else:
# Declare SYS_PREFIX locally, to avoid clash with eponymous global symbol from pyi_rth_pkgutil hook.
SYS_PREFIX = sys._MEIPASS + os.path.sep
#
# Use os.path.realpath() to fully resolve any symbolic links in sys._MEIPASS, in order to avoid path mis-matches
# when the given search paths also contain symbolic links and are already fully resolved. See #6537 for an
# example of such a problem with onefile build on macOS, where the temporary directory is placed under /var,
# which is actually a symbolic link to /private/var.
SYS_PREFIX = os.path.realpath(sys._MEIPASS) + os.path.sep
SYS_PREFIXLEN = len(SYS_PREFIX)

for pkg_path in path:
pkg_path = os.path.normpath(pkg_path)
pkg_path = os.path.realpath(pkg_path) # Fully resolve the given path, in case it contains symbolic links.
if not pkg_path.startswith(SYS_PREFIX):
# if the path does not start with sys._MEIPASS then it cannot be a bundled package.
continue
Expand Down
2 changes: 2 additions & 0 deletions news/6537.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix handling of symbolic links in the path matching part of the
PyInstaller's ``pkgutil.iter_modules`` replacement/override.
12 changes: 11 additions & 1 deletion tests/functional/scripts/pyi_pkgutil_iter_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# SPDX-License-Identifier: (GPL-2.0-or-later WITH Bootloader-exception)
#-----------------------------------------------------------------------------

import os
import sys
import argparse
import pkgutil
Expand All @@ -27,6 +28,12 @@
default='',
help="Optional prefix to pass to iter_modules.",
)
parser.add_argument(
'--resolve-pkg-path',
action='store_true',
default=False,
help="Resolve symbolic links in package path before passing it to pkgutil.iter_modules.",
)
parser.add_argument(
'--output-file',
default=None,
Expand All @@ -43,7 +50,10 @@

# Iterate over package's module
package = importlib.import_module(args.package)
for module in pkgutil.iter_modules(package.__path__, args.prefix):
pkg_path = package.__path__
if args.resolve_pkg_path:
pkg_path = [os.path.realpath(path) for path in pkg_path]
for module in pkgutil.iter_modules(pkg_path, args.prefix):
print("%s;%d" % (module.name, module.ispkg), file=fp)

# Cleanup
Expand Down
19 changes: 17 additions & 2 deletions tests/functional/test_pkgutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def _read_results_file(filename):
]
)
@pytest.mark.parametrize('archive', ['archive', 'noarchive'])
def test_pkgutil_iter_modules(package, script_dir, tmpdir, pyi_builder, archive):
def test_pkgutil_iter_modules(package, script_dir, tmpdir, pyi_builder, archive, resolve_pkg_path=False):
# Ensure package is available
if not importable(package.split(".")[0]):
pytest.skip("Needs " + package)
Expand Down Expand Up @@ -80,10 +80,25 @@ def test_pkgutil_iter_modules(package, script_dir, tmpdir, pyi_builder, archive)
# enable/disable noarchive
*debug_args,
],
app_args=[package, '--output-file', out_frozen],
app_args=[package, '--output-file', out_frozen] + (['--resolve-pkg-path'] if resolve_pkg_path else [])
) # yapf: disable
# Read results
results_frozen = _read_results_file(out_frozen)

# Compare
assert results_unfrozen == results_frozen


# Repeat test_pkgutil_iter_modules() test with package path resolving enabled. In this mode, the test script fully
# resolves the package path before passing it to pkgutil.iter_modules(), reproducing the scenario of #6537 on macOS:
# the temporary directory used by onefile builds is placed in /var, which is symbolic link to /private/var. Therefore,
# the resolved package path (/private/var/...) in the frozen application may differ from non-resolved one (/var/...),
# and our pkgutil.iter_modules() therefore needs to explicitly resolve the given paths and the sys._MEIPASS prefix to
# ensure proper matching.
# The test is applicable only to macOS in onefile mode.
@pytest.mark.darwin
def test_pkgutil_iter_modules_resolve_pkg_path(script_dir, tmpdir, pyi_builder):
if pyi_builder._mode != 'onefile':
pytest.skip('The test is applicable only to onefile mode.')
# A single combination (altgraph package, archive mode) is enough to check for proper symlink handling.
test_pkgutil_iter_modules('altgraph', script_dir, tmpdir, pyi_builder, archive=True, resolve_pkg_path=True)

0 comments on commit 51df140

Please sign in to comment.