pytest icon indicating copy to clipboard operation
pytest copied to clipboard

imports of distutils pick up wrong parent module

Open jaraco opened this issue 1 year ago • 7 comments

In pypa/distutils#259, I'm tracking down an emergent regression in the import logic that breaks the test suite on pytest 8.1 and 8.2.

It looks like it's successfully discovering distutils/cygwincompiler.py, but then when a relative import from there imports .version, it's getting $prefix/lib/distutils/version.py.

I haven't yet root caused it, and it may be related to ~~https://github.com/pytest-dev/pytest/issues/12044 or~~ other open issues.

I'll continue to investigate until I have a better reproducer.

jaraco avatar Jun 19 '24 17:06 jaraco

I tested to see if the proposed fix for 12044 has any effect, but it does not, so it's unlikely related.

jaraco avatar Jun 19 '24 18:06 jaraco

Disabling --doctest-modules causes the error to relocate:

_____________________________________________ ERROR collecting distutils/tests/test_archive_util.py ______________________________________________
ImportError while importing test module '/Users/jaraco/code/pypa/distutils/distutils/tests/test_archive_util.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
distutils/tests/test_archive_util.py:26: in <module>
    from .compat.py38 import check_warnings
E   ModuleNotFoundError: No module named 'distutils.tests.compat'

And further disabling --import-mode importlib leads to an "import file mismatch":

_____________________________________________ ERROR collecting distutils/tests/test_archive_util.py ______________________________________________
import file mismatch:
imported module 'distutils.tests.test_archive_util' has this __file__ attribute:
  /opt/homebrew/Cellar/[email protected]/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/distutils/tests/test_archive_util.py
which is not the same as the test file we want to collect:
  /Users/jaraco/code/pypa/distutils/distutils/tests/test_archive_util.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules

That gives some clue about what might be going on.

jaraco avatar Jun 19 '24 18:06 jaraco

I put a breakpoint on _pytest.pathlib.import_path and when it first gets hit, it's looking at ./conftest.py and distutils is not yet in sys.modules, but on the next call, when collecting distutils/tests/test_archive_util.py, distutils is already in sys.modules and pointing to the stdlib distutils. I need to figure out what's importing it.

jaraco avatar Jun 19 '24 18:06 jaraco

Here's what the call stack looks like when distutils is imported from stdlib:

  /Users/jaraco/code/pypa/distutils/.tox/py311/bin/pytest(8)<module>()
-> sys.exit(console_main())
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(207)console_main()
-> code = main()
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(160)main()
-> config = _prepareconfig(args, plugins)
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(347)_prepareconfig()
-> config = pluginmanager.hook.pytest_cmdline_parse(
  /Users/jaraco/code/pypa/distutils/.tox/py311/lib/python3.11/site-packages/pluggy/_hooks.py(513)__call__()
-> return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  /Users/jaraco/code/pypa/distutils/.tox/py311/lib/python3.11/site-packages/pluggy/_manager.py(120)_hookexec()
-> return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  /Users/jaraco/code/pypa/distutils/.tox/py311/lib/python3.11/site-packages/pluggy/_callers.py(103)_multicall()
-> res = hook_impl.function(*args)
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(1150)pytest_cmdline_parse()
-> self.parse(args)
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(1499)parse()
-> self._preparse(args, addopts=addopts)
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(1403)_preparse()
-> self.hook.pytest_load_initial_conftests(
  /Users/jaraco/code/pypa/distutils/.tox/py311/lib/python3.11/site-packages/pluggy/_hooks.py(513)__call__()
-> return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  /Users/jaraco/code/pypa/distutils/.tox/py311/lib/python3.11/site-packages/pluggy/_manager.py(120)_hookexec()
-> return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  /Users/jaraco/code/pypa/distutils/.tox/py311/lib/python3.11/site-packages/pluggy/_callers.py(103)_multicall()
-> res = hook_impl.function(*args)
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(1228)pytest_load_initial_conftests()
-> self.pluginmanager._set_initial_conftests(
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(589)_set_initial_conftests()
-> self._try_load_conftest(
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(627)_try_load_conftest()
-> self._loadconftestmodules(
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(667)_loadconftestmodules()
-> mod = self._importconftest(
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/config/__init__.py(718)_importconftest()
-> mod = import_path(
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/pathlib.py(545)import_path()
-> pkg_root, module_name = resolve_pkg_root_and_module_name(
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/pathlib.py(825)resolve_pkg_root_and_module_name()
-> if module_name and is_importable(module_name, path):
  /Users/jaraco/code/pytest-dev/pytest/src/_pytest/pathlib.py(857)is_importable()
-> spec = importlib.util.find_spec(module_name)
  <frozen importlib.util>(95)find_spec()
  <frozen importlib._bootstrap>(1176)_find_and_load()
  <frozen importlib._bootstrap>(1147)_find_and_load_unlocked()
  <frozen importlib._bootstrap>(690)_load_unlocked()
  <frozen importlib._bootstrap_external>(940)exec_module()
  <frozen importlib._bootstrap>(241)_call_with_frames_removed()
> /opt/homebrew/Cellar/[email protected]/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/distutils/__init__.py(12)<module>()
-> import sys

It seems that resolve_pkg_root_and_module_name() for ./conftest.py is somehow importing distutils, perhaps because .. is also called distutils.

jaraco avatar Jun 19 '24 18:06 jaraco

I've confirmed that module_name is distutils.conftest, so it is the upwards traversal that's leading to an attempt to import distutils.conftest, which picks up distutils from stdlib and then fails to find .conftest.

jaraco avatar Jun 19 '24 18:06 jaraco

Alright. I think I've distilled the issue.

Starting from this near-minimal repro:

 distutils @ tree
.
├── conftest.py
└── distutils
    ├── __init__.py
    ├── tests
    │   ├── __init__.py
    │   └── test_archive_util.py
    └── unique.py

3 directories, 5 files
 distutils @ cat distutils/tests/test_archive_util.py
import distutils.unique  # noqa


def test_something():
    pass

It's important that . is named "distutils".

Then run pytest 8.2.2 on Python 3.11 thus:

pytest --import-mode importlib -o consider_namespace_packages=true

Important - Don't run with python -m as that will add . to the search path and suppress the issue.

Also relevant, don't have Setuptools installed as it has a distutils hack that may interfere.

Output from that command looks something like:

============================================================== test session starts ===============================================================
platform darwin -- Python 3.11.9, pytest-8.2.2, pluggy-1.5.0
rootdir: /Users/jaraco/draft/distutils
plugins: typeguard-4.3.0
collected 0 items / 1 error                                                                                                                      

===================================================================== ERRORS =====================================================================
_____________________________________________ ERROR collecting distutils/tests/test_archive_util.py ______________________________________________
ImportError while importing test module '/Users/jaraco/draft/distutils/distutils/tests/test_archive_util.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
distutils/tests/test_archive_util.py:1: in <module>
    import distutils.unique  # noqa
E   ModuleNotFoundError: No module named 'distutils.unique'
================================================================ warnings summary ================================================================
<frozen importlib.util>:95
  <frozen importlib.util>:95: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================ short test summary info =============================================================
ERROR distutils/tests/test_archive_util.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================================== 1 warning, 1 error in 0.03s ===========================================================

Tracing the behavior, I've found that the wrong distutils gets imported as part of resolve_pkg_root_and_module_name('/Users/jaraco/draft/distutils/conftest.py', True) when is_importable('distutils.conftest', '/Users/jaraco/draft/distutils') is called and that invokes importlib.util.find_spec('distutils.conftest') here:

https://github.com/pytest-dev/pytest/blob/57bc6df510899ba77de5129d975f148c4a868725/src/_pytest/pathlib.py#L855

Although that call results in an ImportError, a side effect of the call is that distutils was imported (from sys.path) and added to sys.modules. Thereafter, distutils and its submodules are loaded from the standard library instead of from the project under test.

jaraco avatar Jun 19 '24 20:06 jaraco

Of course, setting consider_namespace_packages=false bypasses the issue, and that could work for distutils in this particular case, but that approach is inadequate in general (consider if distutils was itself a namespace package), and also means that distutils requires special configuration relative to other skeleton projects. This issue would probably affect projects like jaraco.functools if checked out to a jaraco folder (except it doesn't, I guess because namespace packages can be multi-homed; that is, even if jaraco is found elsewhere in site-packages, it's still compatible with the project under test).

On the other hand, this issue narrowly affects distutils because of its special relationship with the legacy in stdlib.

I'm slightly tempted to suggest we just sweep this issue under the rug - acknowledge it exists and otherwise have distutils patch out the undesirable behavior or just disable the namespace packages consideration.

jaraco avatar Jun 19 '24 20:06 jaraco