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

Test SETUPTOOLS_USE_DISTUTILS = stdlib, local on GH Actions #2865

Merged
merged 14 commits into from
Nov 12, 2021

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Nov 9, 2021

Summary of changes

This also makes the integration workflow added in pypa/distutils#67 actually work.

Closes #2861

Pull Request Checklist

.github/workflows/main.yml Outdated Show resolved Hide resolved
Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Just a couple of questions.

tox.ini Outdated Show resolved Hide resolved
@jaraco
Copy link
Member

jaraco commented Nov 10, 2021

Well that's interesting. Tests are now failing on Python 3.10 for stdlib because Setuptools itself imports distutils.dist, which imports distutils.command.build_scripts which imports distutils.sysconfig, which emits a deprecation warning. It seems that deprecation warning had been masked by the pre-test invocation of it in pytest-virtualenv. Interestingly, the failure isn't triggered on macOS and Python 3.10. I don't know why. I believe the best thing to do here is acknowledge and ignore that deprecation warning.

@jaraco
Copy link
Member

jaraco commented Nov 10, 2021

It does look like PyPy is also, separately, importing distutils early, breaking the local tests.

In this case, it appears to be pytest-virtualenv again:

setuptools github_workflows_SETUPTOOLS_USE_DISTUTILS $ .tox/pypy/bin/python
Python 3.7.12 (44db26267d0a38e51a7e8490983ed7e7bcb84b74, Oct 28 2021, 14:16:15)
[PyPy 7.3.7 with GCC Apple LLVM 13.0.0 (clang-1300.0.29.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>> import sys
>>>> import importlib_metadata as md
>>>> for ep in md.entry_points(group='pytest11'):
....   ep.load()
....   print({'distutils'} & set(sys.modules))
....   
<module 'pytest_flake8' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_flake8.py'>
set()
<module 'pytest_enabler' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_enabler/__init__.py'>
set()
<module 'pytest_checkdocs' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_checkdocs/__init__.py'>
set()
<module 'pytest_shutil.workspace' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_shutil/workspace.py'>
set()
<module 'xdist.plugin' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/xdist/plugin.py'>
set()
<module 'xdist.looponfail' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/xdist/looponfail.py'>
set()
<module 'pytest_forked' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_forked/__init__.py'>
set()
<module 'pytest_virtualenv' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_virtualenv.py'>
{'distutils'}
<module 'pytest_cov.plugin' from '/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_cov/plugin.py'>
{'distutils'}

It seems that on PyPy, from pkg_resources import working_set is causing distutils to get imported and from the stdlib, even though SETUPTOOLS_USE_DISTUTILS='local'.

Oh, right. Now I remember. PyPy imports distutils in sysconfig:

setuptools github_workflows_SETUPTOOLS_USE_DISTUTILS $ touch distutils/__init__.py
setuptools github_workflows_SETUPTOOLS_USE_DISTUTILS $ $PYTHONPATH='.' ./.tox/pypy/bin/python
Python 3.7.12 (44db26267d0a38e51a7e8490983ed7e7bcb84b74, Oct 28 2021, 14:16:15)
[PyPy 7.3.7 with GCC Apple LLVM 13.0.0 (clang-1300.0.29.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>> import pkg_resources
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jaraco/code/public/pypa/setuptools/pkg_resources/__init__.py", line 950, in <module>
    class Environment:
  File "/Users/jaraco/code/public/pypa/setuptools/pkg_resources/__init__.py", line 954, in Environment
    self, search_path=None, platform=get_supported_platform(),
  File "/Users/jaraco/code/public/pypa/setuptools/pkg_resources/__init__.py", line 180, in get_supported_platform
    plat = get_build_platform()
  File "/Users/jaraco/code/public/pypa/setuptools/pkg_resources/__init__.py", line 385, in get_build_platform
    plat = get_platform()
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 728, in get_platform
    get_config_vars(),
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 603, in get_config_vars
    _init_posix(_CONFIG_VARS)
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 467, in _init_posix
    _temp = __import__(name, globals(), locals(), ['build_time_vars'], 0)
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib_pypy/_sysconfigdata.py", line 5, in <module>
    from distutils.spawn import find_executable
ModuleNotFoundError: No module named 'distutils.spawn'

So it's not possible on PyPy to import pkg_resources without importing distutils.

Maybe pytest-virtualenv can be made to work without that import as well.

In man-group/pytest-plugins@c08c4a8, I removed the import of pkg_resources and verified that with that change, one can load all of the pytest plugins without importing distutils. Unfortunately, the error still occurs, indicating that something else is probably importing pkg_resources before the test suite is loaded.

Confirmed the issue is fixed with pytest-virtualenv, and now the issue exists with pytest-cov/coverage:

setuptools github_workflows_SETUPTOOLS_USE_DISTUTILS $ cat distutils/__init__.py
raise ValueError()
setuptools github_workflows_SETUPTOOLS_USE_DISTUTILS $ $PYTHONPATH='.' $SETUPTOOLS_USE_DISTUTILS='local' ./.tox/pypy/bin/pytest
Traceback (most recent call last):
  File "./.tox/pypy/bin/pytest", line 8, in <module>
    sys.exit(console_main())
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/config/__init__.py", line 185, in console_main
    code = main()
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/config/__init__.py", line 143, in main
    config = _prepareconfig(args, plugins)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/config/__init__.py", line 319, in _prepareconfig
    pluginmanager=pluginmanager, args=args
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_hooks.py", line 265, in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_manager.py", line 80, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_callers.py", line 55, in _multicall
    gen.send(outcome)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/helpconfig.py", line 100, in pytest_cmdline_parse
    config: Config = outcome.get_result()
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_result.py", line 60, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_callers.py", line 39, in _multicall
    res = hook_impl.function(*args)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/config/__init__.py", line 1003, in pytest_cmdline_parse
    self.parse(args)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/config/__init__.py", line 1283, in parse
    self._preparse(args, addopts=addopts)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/_pytest/config/__init__.py", line 1192, in _preparse
    early_config=self, args=args, parser=self._parser
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_hooks.py", line 265, in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_manager.py", line 80, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_callers.py", line 60, in _multicall
    return outcome.get_result()
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_result.py", line 60, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pluggy/_callers.py", line 39, in _multicall
    res = hook_impl.function(*args)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_enabler/__init__.py", line 40, in pytest_load_initial_conftests
    _pytest_cov_check(enabled, early_config, parser, args)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_enabler/__init__.py", line 74, in _pytest_cov_check
    pytest_cov.plugin.pytest_load_initial_conftests(early_config, parser, args)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_cov/plugin.py", line 149, in pytest_load_initial_conftests
    plugin = CovPlugin(options, early_config.pluginmanager)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_cov/plugin.py", line 198, in __init__
    self.start(engine.DistMaster)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_cov/plugin.py", line 222, in start
    self.cov_controller.start()
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_cov/engine.py", line 44, in ensure_topdir_wrapper
    return meth(self, *args, **kwargs)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/pytest_cov/engine.py", line 267, in start
    self.cov.start()
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/coverage/control.py", line 551, in start
    self._init_for_start()
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/coverage/control.py", line 513, in _init_for_start
    self._inorout.configure(self.config)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/coverage/inorout.py", line 239, in configure
    add_third_party_paths(self.third_paths)
  File "/Users/jaraco/code/public/pypa/setuptools/.tox/pypy/site-packages/coverage/inorout.py", line 167, in add_third_party_paths
    config_paths = sysconfig.get_paths(scheme)
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 552, in get_paths
    return _expand_vars(scheme, vars)
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 198, in _expand_vars
    _extend_dict(vars, get_config_vars())
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 603, in get_config_vars
    _init_posix(_CONFIG_VARS)
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib-python/3/sysconfig.py", line 467, in _init_posix
    _temp = __import__(name, globals(), locals(), ['build_time_vars'], 0)
  File "/usr/local/Cellar/pypy3/7.3.7/libexec/lib_pypy/_sysconfigdata.py", line 5, in <module>
    from distutils.spawn import find_executable
  File "/Users/jaraco/code/public/pypa/setuptools/distutils/__init__.py", line 1, in <module>
    raise ValueError()
ValueError

It looks like coverage invokes sysconfig.get_paths, which imports distutils.

At this point, I'm not confident I can prevent the import of distutils on PyPy, so I'll propose to disable that check on that platform.

@jaraco
Copy link
Member

jaraco commented Nov 10, 2021

It looks like those last two commits aren't having any effect, probably because they take effect before the warnings are applied in the pytest.ini. I need to find a hook that applies them after the warnings filters are applied in pytest.ini.

@jaraco
Copy link
Member

jaraco commented Nov 10, 2021

I see now that pytest configures the warnings for each item, loading from the ini file settings each time. So it seems it may not be possible to add contextual warnings (only for Python 3.10 or PyPy) (https://github.com/pytest-dev/pytest/blob/5d87a27434dada56ba69ef7b5fa014c321dc46f4/src/_pytest/warnings.py#L52).

@jaraco jaraco force-pushed the github_workflows_SETUPTOOLS_USE_DISTUTILS branch from 955e4b1 to c08b7ee Compare November 10, 2021 22:25
@jaraco jaraco force-pushed the github_workflows_SETUPTOOLS_USE_DISTUTILS branch from c08b7ee to 2ceec37 Compare November 10, 2021 22:27
@jaraco
Copy link
Member

jaraco commented Nov 10, 2021

There's still a lingering error on DISTUTILS=local for PyPy. Pip is getting a BackendInvalid exception from pep517. Unfortunately, it masks all the details, so it's difficult to determine anything about why it's invalid.

It looks like the only place that's raised is

https://github.com/pypa/pep517/blob/19d515af0d5fd3b871c6be2369098819732c6bf1/pep517/in_process/_in_process.py#L98

It appears as if pip/pep517 thinks that the build backend is not being loaded from the backend path.

I'm not able to replicate the issue on my macOS environment with PyPy 3.7.

I'm also unable to replicate the issue on Ubuntu with PyPy 3.6.9, but I am able to replicate it on the same OS with PyPy 3.6.12 (same version as failing in GHA). Specifically:

FROM jaraco/multipy-tox
RUN wget https://downloads.python.org/pypy/pypy3.6-v7.3.3-linux64.tar.bz2 -O - | tar xjC /usr/local --strip-components 1
ENV PIP_USE_PEP517=
ENV SETUPTOOLS_USE_DISTUTILS=local
RUN git clone https://github.com/mkoeppe/setuptools
WORKDIR setuptools
RUN git checkout github_workflows_SETUPTOOLS_USE_DISTUTILS
CMD tox -e pypy -- -k upgrade_from_source -p no:cov -p no:xdist -x

Running pip directly without tox:

FROM jaraco/multipy-tox
RUN wget https://downloads.python.org/pypy/pypy3.6-v7.3.3-linux64.tar.bz2 -O - | tar xjC /usr/local --strip-components 1
ENV PIP_USE_PEP517=
ENV SETUPTOOLS_USE_DISTUTILS=local
RUN git clone https://github.com/mkoeppe/setuptools
WORKDIR setuptools
RUN git checkout github_workflows_SETUPTOOLS_USE_DISTUTILS
RUN tox -e pypy --notest
RUN .tox/pypy/bin/python setup.py sdist
RUN .tox/pypy/bin/virtualenv env
CMD env/bin/pip install --force --upgrade dist/*.zip

I patched pep517 as found in pip in order to get some more detail about the error:

diff --git a/src/pip/_vendor/pep517/in_process/_in_process.py b/src/pip/_vendor/pep517/in_process/_in_process.py
index 954a4ab05..6f9328f8c 100644
--- a/src/pip/_vendor/pep517/in_process/_in_process.py
+++ b/src/pip/_vendor/pep517/in_process/_in_process.py
@@ -95,7 +95,7 @@ def _build_backend():
             contained_in(obj.__file__, path)
             for path in extra_pathitems
         ):
-            raise BackendInvalid("Backend was not loaded from backend-path")
+            raise BackendInvalid(f"Backend was not loaded from backend-path {obj.__file__}, {extra_pathitems}")
 
     if obj_path:
         for path_part in obj_path.split('.'):
diff --git a/src/pip/_vendor/pep517/wrappers.py b/src/pip/_vendor/pep517/wrappers.py
index e031ed708..eaa18586c 100644
--- a/src/pip/_vendor/pep517/wrappers.py
+++ b/src/pip/_vendor/pep517/wrappers.py
@@ -43,6 +43,9 @@ class BackendInvalid(Exception):
         self.backend_path = backend_path
         self.message = message
 
+    def __str__(self):
+        return self.message
+
 
 class HookMissing(Exception):
     """Will be raised on missing hooks."""

And when I run with that pip, I get some useful information out of the error message:

pip._vendor.pep517.wrappers.BackendInvalid: Backend was not loaded from backend-path /setuptools/env/site-packages/setuptools/build_meta.py, ['/tmp/pip-req-build-52zttrv_']

So somehow, even though pep517 is supposed to be doing an isolated build, the setuptools from the virtualenv is getting imported for setuptools.build_meta.

After drilling down deep into the bowels of pip/pep517, I think I've figured out the issue. In env/, where some version of setuptools is installed, there exists a _distutils_precedence.pth, which causes _distutils_hack to load the shim to give setuptools._distutils precedence when SETUPTOOLS_USE_DISTUTILS=local is present. And since that var is set and because some versions of PyPy import distutils implicitly on startup, PyPy will implicitly import setuptools._distutils and by the time pep517 gets around to import setuptools.build_meta, setuptools has already been imported from site-packages and thus doesn't get loaded from the temporarily installed backend as expected/required.

I'm still unsure why these errors hadn't emerged before. Oh, it seems maybe I hadn't authorized the job to run. I've just authorized the job to run. So maybe these environments have never passed.

@jaraco
Copy link
Member

jaraco commented Nov 11, 2021

I've found another use-case that fails on PyPy. If you set TOX_WORK_DIR to /tox (or anywhere outside the source tree), simply running tox will fail if SETUPTOOLS_USE_DISTUTILS=local, because a BackendInvalid will be raised when tox attempts to replace setuptools in the virtualenv, but PyPy has already imported it from the old install. It apparently doesn't happen when TOX_WORK_DIR isn't set, presumably because the virtualenv exists somewhere in the source tree (./.tox).

@jaraco jaraco force-pushed the github_workflows_SETUPTOOLS_USE_DISTUTILS branch from 6e3dd3a to 15a2fa4 Compare November 11, 2021 14:04
@jaraco
Copy link
Member

jaraco commented Nov 11, 2021

I realize now, the main reason why I didn't implement the tests this way in the first place is because it multiplies the test suite by 2x when most of the tests are irrelevant to the difference. That is, the intention was for all tests to run under SETUPTOOLS_USE_DISTUTILS=local by default and for SETUPTOOLS_USE_DISTUTILS=stdlib to be stable but deprecated. And then there is the distutils_adoption tests that check the main assumptions about the adoption differences. Unfortunately, it turned out that due to monkeypatching in Fedora and Debian and others, it's not been possible to make SETUPTOOLS_USE_DISTUTILS=local the default, so it's worthwhile having these tests around duplicated for now.

…ion tests. Works around issue with upgrading on PyPy.
@jaraco jaraco force-pushed the github_workflows_SETUPTOOLS_USE_DISTUTILS branch from 15a2fa4 to 3542082 Compare November 11, 2021 14:12
@jaraco
Copy link
Member

jaraco commented Nov 11, 2021

It's so close to passing. It's failing now on PyPy on macOS, one test failure that I've not seen before.

@jaraco
Copy link
Member

jaraco commented Nov 12, 2021

The remaining failure is in test_distutils_local, which previously was xfailed on PyPy. I saw it passing on Unix, so thought it could be allowed again, but alas, it seems not. My guess is that distutils gets imported before .pth files are processed, so the precedence doesn't take place.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 12, 2021

Looking great, thanks for all your work!

@jaraco jaraco merged commit 8ab7799 into pypa:main Nov 12, 2021
@webknjaz
Copy link
Member

@jaraco did this break the CI?

@jaraco
Copy link
Member

jaraco commented Nov 12, 2021

I don't think it did. I'm pretty sure it was pypa/pip#10641, presumably fixed in f2de347 / #2874.

@abravalheri
Copy link
Contributor

I think there might be a problem with using:

[options.extras_require]
testing =
        ...
-	pytest-virtualenv>=1.2.7
+	pytest-virtualenv @ git+https://github.com/jaraco/pytest-plugins@distutils-deprecated#subdirectory=pytest-virtualenv

in setup.cfg.

I am almost sure PyPI forbid uploading packages with dependencies outside of PyPI.
pypa/build may also have problems when parsing the dependencies for PEP517 style builds...

@jaraco
Copy link
Member

jaraco commented Nov 14, 2021

No description provided.

@webknjaz
Copy link
Member

PyPI does not validate the deps. They may be satisfied from third-party sources.

@abravalheri
Copy link
Contributor

PyPI does not validate the deps. They may be satisfied from third-party sources.

@webknjaz I had a problem like this around a month ago with another project. I don't know when PyPI started forbidding distributions that list dependencies via URLs, but they do it now for sure.

You can also check the CI logs:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SETUPTOOLS_USE_DISTUTILS=local is not tested by CI infrastructure
4 participants