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

imports of distutils pick up wrong parent module #12490

Open
jaraco opened this issue Jun 19, 2024 · 7 comments
Open

imports of distutils pick up wrong parent module #12490

jaraco opened this issue Jun 19, 2024 · 7 comments
Labels
topic: collection related to the collection phase

Comments

@jaraco
Copy link
Contributor

jaraco commented Jun 19, 2024

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 #12044 or other open issues.

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

jaraco added a commit to pypa/distutils that referenced this issue Jun 19, 2024
@jaraco
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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

@jaraco
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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:

spec = importlib.util.find_spec(module_name)

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
Copy link
Contributor Author

jaraco commented Jun 19, 2024

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.

@Zac-HD Zac-HD added the topic: collection related to the collection phase label Jun 24, 2024
jaraco added a commit to pypa/distutils that referenced this issue Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: collection related to the collection phase
Projects
None yet
Development

No branches or pull requests

2 participants