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

pip isolated build breaks when a pth file imports setuptools and setup_requires is defined #7778

Open
gaborbernat opened this issue Feb 24, 2020 · 14 comments
Labels

Comments

@gaborbernat
Copy link

@gaborbernat gaborbernat commented Feb 24, 2020

The following script reproduces the problem:

rm -rf venv

python3.8 -m venv venv

venv/bin/pip install pip setuptools wheel -U
venv/bin/pip list

echo 'import setuptools' 1>venv/lib/python3.8/site-packages/magic.pth

venv/bin/pip install --no-deps https://files.pythonhosted.org/packages/60/5f/f17dd17ba9ee47fb0629789bbd320e833622634444877b2e7d90922aa0e5/virtualenv-20.0.5.tar.gz
Looking in indexes: http://localhost:3141/bernat/bb
Collecting pip
  Downloading http://localhost:3141/bernat/bb/%2Bf/4ae/14a42d8adba32/pip-20.0.2-py2.py3-none-any.whl (1.4MB)
     |████████████████████████████████| 1.4MB 48.8MB/s
Collecting setuptools
  Downloading http://localhost:3141/bernat/bb/%2Bf/316/484eebff54cc1/setuptools-45.2.0-py3-none-any.whl (584kB)
     |████████████████████████████████| 593kB 51.1MB/s
Collecting wheel
  Downloading http://localhost:3141/bernat/bb/%2Bf/df2/77cb51e61359a/wheel-0.34.2-py2.py3-none-any.whl
Installing collected packages: pip, setuptools, wheel
  Found existing installation: pip 19.2.3
    Uninstalling pip-19.2.3:
      Successfully uninstalled pip-19.2.3
  Found existing installation: setuptools 41.2.0
    Uninstalling setuptools-41.2.0:
      Successfully uninstalled setuptools-41.2.0
Successfully installed pip-20.0.2 setuptools-45.2.0 wheel-0.34.2
Package    Version
---------- -------
pip        20.0.2
setuptools 45.2.0
wheel      0.34.2
Looking in indexes: http://localhost:3141/bernat/bb
Collecting https://files.pythonhosted.org/packages/60/5f/f17dd17ba9ee47fb0629789bbd320e833622634444877b2e7d90922aa0e5/virtualenv-20.0.5.tar.gz
  Using cached virtualenv-20.0.5.tar.gz (8.0 MB)
  Installing build dependencies ... -
done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... error
    ERROR: Command errored out with exit status 1:
     command: /Users/bgabor8/git/github/pip-bug/venv/bin/python3.8 /Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py prepare_metadata_for_build_wheel /var/folders/kt/btg285ds5kx4l1k398lb7df80000gr/T/tmpphcu52qx
         cwd: /private/var/folders/kt/btg285ds5kx4l1k398lb7df80000gr/T/pip-req-build-ykiwdkxl
    Complete output (40 lines):
    /Users/bgabor8/git/github/pip-bug/venv/bin/python3.8: No module named pip
    Traceback (most recent call last):
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/installer.py", line 128, in fetch_build_egg
        subprocess.check_call(cmd)
      File "/Users/bgabor8/.pyenv/versions/3.8.1/lib/python3.8/subprocess.py", line 364, in check_call
        raise CalledProcessError(retcode, cmd)
    subprocess.CalledProcessError: Command '['/Users/bgabor8/git/github/pip-bug/venv/bin/python3.8', '-m', 'pip', '--disable-pip-version-check', 'wheel', '--no-deps', '-w', '/var/folders/kt/btg285ds5kx4l1k398lb7df80000gr/T/tmppps3v_ps', '--quiet', 'setuptools_scm>=2']' returned non-zero exit status 1.

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 257, in <module>
        main()
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 240, in main
        json_out['return_val'] = hook(**hook_input['kwargs'])
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 110, in prepare_metadata_for_build_wheel
        return hook(metadata_directory, config_settings)
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/build_meta.py", line 158, in prepare_metadata_for_build_wheel
        self.run_setup()
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/build_meta.py", line 143, in run_setup
        exec(compile(code, __file__, 'exec'), locals())
      File "setup.py", line 6, in <module>
        setup(
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/__init__.py", line 143, in setup
        _install_setup_requires(attrs)
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/__init__.py", line 138, in _install_setup_requires
        dist.fetch_build_eggs(dist.setup_requires)
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/dist.py", line 718, in fetch_build_eggs
        resolved_dists = pkg_resources.working_set.resolve(
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 781, in resolve
        dist = best[req.key] = env.best_match(
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 1066, in best_match
        return self.obtain(req, installer)
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 1078, in obtain
        return installer(requirement)
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/dist.py", line 777, in fetch_build_egg
        return fetch_build_egg(self, req)
      File "/Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/setuptools/installer.py", line 130, in fetch_build_egg
        raise DistutilsError(str(e))
    distutils.errors.DistutilsError: Command '['/Users/bgabor8/git/github/pip-bug/venv/bin/python3.8', '-m', 'pip', '--disable-pip-version-check', 'wheel', '--no-deps', '-w', '/var/folders/kt/btg285ds5kx4l1k398lb7df80000gr/T/tmppps3v_ps', '--quiet', 'setuptools_scm>=2']' returned non-zero exit status 1.
    ----------------------------------------
ERROR: Command errored out with exit status 1: /Users/bgabor8/git/github/pip-bug/venv/bin/python3.8 /Users/bgabor8/git/github/pip-bug/venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py prepare_metadata_for_build_wheel /var/folders/kt/btg285ds5kx4l1k398lb7df80000gr/T/tmpphcu52qx Check the logs for full command output.

Ran into this while trying to fix pypa/virtualenv#1657

@triage-new-issues triage-new-issues bot added the triage label Feb 24, 2020
@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 24, 2020

Note, even putting the import into a try-catch has no effect, e.g switch the above pth generation with:

echo 'import magic' 1>venv/lib/python3.8/site-packages/magic.pth
printf 'try:\n    import setuptools\nexcept:\n    pass' 1>venv/lib/python3.8/site-packages/magic.py

Note an empty magic.py makes things work... so the import of setuptools breaks things somehow. cc @pfmoore you implemented the PEP-517 support, so tagging you in case you know what is happening here.

@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 24, 2020

So the issue is here, https://github.com/pypa/pip/blob/master/src/pip/_internal/build_env.py#L87-L108. Seems pip part of building the environment injects some additional pth files via the sitecustomize.py... This is how build dependencies are injected. The problem is setuptools setup_requires work of a working set that's initiated at setuptools import... And it's not updated later. So by the time pip injects the additional pth files/paths the working set has already been constructed and is not refreshed. Adding

from pkg_resources import _initialize_master_working_set
 _initialize_master_working_set()

to that generated script fixes the issue. Not sure what's a good solution here though, as obviously pip might not be able to cater all build envs that could build-cache before the sitecustomize.py... though maybe for setuptools we should for legacy reasons. Alternatively, pip could also use a pth files... and put it under a site path at the front of sys.path to ensure it's executed first... before other pth files can be triggered. @pfmoore @pradyunsg ideas? Not even sure how should be this handled backwards and forward compatible way 🤷‍♂

@gaborbernat gaborbernat changed the title pip isolated build breaks when a pth file tries to import setuptools pip isolated build breaks when a pth file imports setuptools and setup_requires is defined Feb 24, 2020
gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Feb 24, 2020
Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
gaborbernat referenced this issue Feb 24, 2020
- check build requirements for conflicts
- better isolation (ignore system site packages)
- support 2 prefixes: a "normal" one, and an "overlay" one
  (with higher priority over "normal")
gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Feb 24, 2020
Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Feb 24, 2020
Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Feb 24, 2020
Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
gaborbernat added a commit to pypa/virtualenv that referenced this issue Feb 24, 2020
…nt (#1657)

* Ensure distutils configuration values do not escape virtual environment

Distutils has some configuration files where the user may alter paths to
point outside of the virtual environment. Defend against this by
installing a pth file that resets this to their expected path.

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>

* fix CI failure due to #pypa/pip/issues/7778

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
@pfmoore

This comment has been minimized.

Copy link
Member

@pfmoore pfmoore commented Feb 24, 2020

I have to say that my immediate reaction is "don't do that". I'm not a huge fan of .pth files that execute code like this.

What is the actual use case for this?

@pfmoore

This comment has been minimized.

Copy link
Member

@pfmoore pfmoore commented Feb 24, 2020

I should also say that not reacting to changes in the working set sounds like a bug (or deliberate limitation?) in setuptools.

@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 24, 2020

Needed for pypa/virtualenv#1635

That being said pth files are officially documentend interface so I don't think we can say don't do that.

@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 24, 2020

Well depends where you stand. Should virtual environments support setuptools, or setuptools support virtual environments? Given the issue arrives only in virtual environments, I'm tempted to put here the responsibility on the virtual environment.

@pfmoore

This comment has been minimized.

Copy link
Member

@pfmoore pfmoore commented Feb 24, 2020

Well depends where you stand. Should virtual environments support setuptools, or setuptools support virtual environments?

My point was that if, as you say, this is legitimate usage, then why is it OK for setuptools not to support it? But never mind, debating who's at fault isn't productive here.

What is the actual use case for this?

So if I'm understanding correctly, the use case is a project that wants to use setup_requires as part of its build process, and also has a pyproject.toml so that build isolation is triggered, and also the environment from which pip is being run includes a .pth file installed that imports setuptools? And unfortunately virtualenv ticks the first two boxes, so fails to install for users that have an environment with the third condition?

How necessary is setup_requires here? I thought that was generally deprecated these days? Is there a reason pyproject.toml isn't a suitable replacement for this use case?

As I've said elsewhere, I'm not against a PR that fixes this. I'm just saying that I don't think there's any particular reason to consider that the assumptions pip's current code is making are particularly unreasonable.

But we're going to get nowhere debating who or what is doing something "wrong" here. I'm happy to drop this discussion so that the conversation can focus on developing a fix (wherever that fix ends up going).

@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 25, 2020

So if I'm understanding correctly, the use case is a project that wants to use setup_requires as part of its build process, and also has a pyproject.toml so that build isolation is triggered, and also the environment from which pip is being run includes a .pth file installed that imports setuptools? And unfortunately virtualenv ticks the first two boxes, so fails to install for users that have an environment with the third condition?

Yes. All these three need to come into play for the issue to arise. There's an outstanding issue for virtualenv that distutils configuration files that set prefix and script_path ends up pip installing in wrong locations, so my proposed fix for this is to add a pth file to the created virtual environment that defends against this by erasing those configuration values within the virtualenv. I don't think this is a deliberate limitation, most likely has to do with the fact that distutils was created before the virtual environment was a thing. Note here the fact that I've chosen to use a pth file, it's just an implementation detail. The alternative would be to ship our own site.py to fix this, as did the old virtualenv... this would end up causing the exact same issue though, and as we've had a few issues with using custom site.py I prefer the pth as being more robust.

My point was that if, as you say, this is legitimate usage, then why is it OK for setuptools not to support it?

I am not sure what setuptools should/can do here to assist with the issue. At best maybe provide an API to disable those configurations values, so we don't have to monkeypatch. But that would not bring us any more ahead in the pip build side.

How necessary is setup_requires here? I thought that was generally deprecated these days? Is there a reason pyproject.toml isn't a suitable replacement for this use case?

For older target machines (think Ubuntu LTS, Debian LTS) that don't have the newer pip versions, it helps to fallback to setup_requires. So, in essence, it's necessary as a fallback mechanism.

I'm just saying that I don't think there's any particular reason to consider that the assumptions pip's current code is making are particularly unreasonable.

Unreasonable, definitely not. A bit brittle when pth files come into play? I think yes. And given using a pth file instead of the sitecustomize.py would alleviate the issue I think it's worthwhile to make the change. So I'm proposing this if we agree it's a good change can draw up a PR.

@uranusjr

This comment has been minimized.

Copy link
Member

@uranusjr uranusjr commented Feb 25, 2020

According to pypa/setuptools#1742 setup_requires is deprecated. This would be one more case against its un-deprecation. Would it be a possibility to tell those users to stick to virtualenv 19.x?

(Edit: For clarification, I don’t object to fixing the issue if it’s reasonable.)

OT: Old Debian and Ubuntu releases seem to pop up often recently (requires-python, setup.cfg, and this). I hope we can stop caring about them soon :(

@pfmoore

This comment has been minimized.

Copy link
Member

@pfmoore pfmoore commented Feb 25, 2020

I am not sure what setuptools should/can do here to assist with the issue.

My impression was that you'd said setuptools queries the working set at import time and caches that value. I was assuming that "calculate the working set as needed" would fix the issue. Apologies if I misunderstood.

So I'm proposing this if we agree it's a good change can draw up a PR.

That sounds reasonable to me - thanks.

@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 26, 2020

FYI, I've released virtualenv 20.0.6, that generates a pth file for every virtual environment (to patch distutils), so this issue might get more important; if nothing else we'll see how common the use case is. (and based on that we can have a good guess what to do next) - would it be possible to do a pip patch release in the next week, would this PR manifest?

@D0han

This comment has been minimized.

Copy link

@D0han D0han commented Feb 26, 2020

This is a big problem for us.

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Feb 26, 2020
Until pypa/pip#7778 is fixed and released.

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Feb 26, 2020
Until pypa/pip#7778 is fixed and released.

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>
@ofek

This comment has been minimized.

Copy link
Contributor

@ofek ofek commented Feb 26, 2020

This is also affecting us greatly.

@gaborbernat

This comment has been minimized.

Copy link
Author

@gaborbernat gaborbernat commented Feb 26, 2020

For now side-step the issue in virtualenv, by reverting our fix - https://virtualenv.pypa.io/en/latest/changelog.html#v20-0-7-2020-02-26

omad added a commit to opendatacube/datacube-core that referenced this issue Feb 27, 2020
Write version number to static file on install,
to avoid overhead of looking it up every time datacube
is imported.

Using pyproject.toml avoids issue pypa/pip#7778
omad added a commit to opendatacube/datacube-core that referenced this issue Feb 27, 2020
Write version number to static file on install,
to avoid overhead of looking it up every time datacube
is imported.

Using pyproject.toml avoids issue pypa/pip#7778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.