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

Upgrade vendored pep517 to 0.10.0 or later #8887

Closed
paugier opened this issue Sep 17, 2020 · 16 comments · Fixed by #9813
Closed

Upgrade vendored pep517 to 0.10.0 or later #8887

paugier opened this issue Sep 17, 2020 · 16 comments · Fixed by #9813
Labels
project: vendored dependency Related to a vendored dependency type: bug A confirmed bug or unintended behavior
Milestone

Comments

@paugier
Copy link

paugier commented Sep 17, 2020

Each time I submit an issue here (pip), I'm getting not very nice feedback, but let's try again 🙂

If a build uses import colorlog (to get https://pypi.org/project/colorlog/), the module imported is pip/_vendor/pep517/colorlog.py, which is incompatible with https://pypi.org/project/colorlog/ and one gets an error like:

AttributeError: module 'colorlog' has no attribute 'StreamHandler'
@pfmoore
Copy link
Member

pfmoore commented Sep 17, 2020

Can you provide a small, self-contained example of this? It sounds like a bug if setup.py is able to see parts of pip's vendored modules, but it's difficult to be sure without a test case we can run.

@pfmoore
Copy link
Member

pfmoore commented Sep 17, 2020

By the way,

Each time I submit an issue here (pip), I'm getting not very nice feedback

I'm sorry if that's happened to you, it certainly shouldn't. I checked the issues that I could find which you've raised and couldn't see what you were referring to. Could you give an example? I would certainly hope that if you're taking the time to help us by raising issues, we'd respond positively. (That doesn't necessarily mean that we'd agree with every suggestion, but I'd hope the discussion would at least be civil...)

@paugier
Copy link
Author

paugier commented Sep 17, 2020

  • setup.py
from setuptools import setup
import colorlog as logging
handler = logging.StreamHandler()
setup()
  • pyproject.toml
[build-system]
requires = ["setuptools", "wheel", "colorlog"]
build-backend = "setuptools.build_meta"

Sorry for the first sentence of my issue. I'm tired with different things (not about pip) and that's true that the recent evolutions of pip / pyproject.toml, pep517, pep518 lead to real difficulties for me. I'm loosing a lot of time and I can't manage to find good solutions to my applications. Anyway, that's not the subject of this issue and I realize that it's for the good of most Python users.

@pfmoore
Copy link
Member

pfmoore commented Sep 17, 2020

Sorry for the first sentence of my issue.

No worries, there's a lot of change at the moment and it's hard for all of us - I could easily imagine one of us giving you a reply somewhere that translated to "aaargh, we're trying to do our best, stop coming up with more problems!!" so I wanted to make sure that wasn't what had happened 🙂

I just tried out your test case and can confirm that I see the bug. Thanks for a nice easy example to test with! I tried with both py -m pip wheel . and py -m pip wheel --use-pep517 . and both failed the same way.

I don't know much about pip's vendoring code, so I'll need to do some digging before I can work out what's going on, but I can confirm that this is not correct behaviour.

@pfmoore pfmoore added the type: bug A confirmed bug or unintended behavior label Sep 17, 2020
@duckinator
Copy link
Contributor

duckinator commented Sep 18, 2020

TL;DR: It appears to be a sys.path ordering problem. If you specify a build-system dependency where the name is the same as any file in the vendored copy of pep517, you'll have this problem.


Long version:

  1. Using setup.py-1 (see below), I found sys.path (see below)
  2. My venv did not contain colorlog, and it wasn't installed globally, so that means it's likely in /tmp.
  3. I used setup.py-2 (see below) to let me look into /tmp before pip cleaned up after itself, by running find /tmp -name colorlogs.
  4. This confirmed my suspicion that it is in /tmp — specifically (for me) /tmp/pip-build-env-*/overlay/lib/python3.7/site-packages/colorlog
  5. The path lists pip/_vendor/pep517 first.

The end result of this is that if anyone's build relies on a library with the name of any files in the vendored copy of the pep517 library, they'll encounter this same problem.


Here's the path I discovered in step 1, with some extra newlines so it's actually readable:

['/home/puppy/dev/python/duckinator/pip-8887/venv/lib/python3.7/site-packages/pip/_vendor/pep517',
'/tmp/pip-build-env-j0p_lx6e/site',
'/usr/lib64/python37.zip',
'/usr/lib64/python3.7',
'/usr/lib64/python3.7/lib-dynload',
'/tmp/pip-build-env-j0p_lx6e/overlay/lib/python3.7/site-packages',
'/tmp/pip-build-env-j0p_lx6e/overlay/lib64/python3.7/site-packages',
'/tmp/pip-build-env-j0p_lx6e/normal/lib/python3.7/site-packages',
'/tmp/pip-build-env-j0p_lx6e/normal/lib64/python3.7/site-packages']

Here's the setup.py variants mentioned above:

setup.py-1 (prints path)
from setuptools import setup
import colorlog as logging
import sys
print(sys.path)
handler = logging.StreamHandler()
setup()
setup.py-2 (sleeps 1,000 seconds)
from setuptools import setup
import colorlog as logging
import time
time.sleep(1000)
handler = logging.StreamHandler()
setup()

@duckinator
Copy link
Contributor

A single simplified setup.py which demonstrates everything I said above:

from pathlib import Path

print(list(Path('/tmp').glob('pip-build-env-*/**/colorlog'))[0])

# All of these are from pep517.
# The commented-out ones all fail because the modules get imported, but raise an exception.
import _in_process
# import build
# import check
import colorlog
import compat
import dirtools
# import envbuild
# import meta
# import wrappers

print(_in_process)
print(colorlog)
print(compat)
print(dirtools)

exit(1)

The results of running this:

  ERROR: Command errored out with exit status 1:
   command: /home/puppy/dev/python/duckinator/pip-8887/venv/bin/python3 /home/puppy/dev/python/duckinator/pip-8887/venv/lib64/python3.7/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmpobia7xyo
       cwd: /tmp/pip-req-build-0idiwuz8
  Complete output (5 lines):
  /tmp/pip-build-env-p8raq723/overlay/lib/python3.7/site-packages/colorlog
  <module '_in_process' from '/home/puppy/dev/python/duckinator/pip-8887/venv/lib/python3.7/site-packages/pip/_vendor/pep517/_in_process.py'>
  <module 'colorlog' from '/home/puppy/dev/python/duckinator/pip-8887/venv/lib/python3.7/site-packages/pip/_vendor/pep517/colorlog.py'>
  <module 'compat' from '/home/puppy/dev/python/duckinator/pip-8887/venv/lib/python3.7/site-packages/pip/_vendor/pep517/compat.py'>
  <module 'dirtools' from '/home/puppy/dev/python/duckinator/pip-8887/venv/lib/python3.7/site-packages/pip/_vendor/pep517/dirtools.py'>
  ----------------------------------------

@uranusjr
Copy link
Member

It seems to me the best solution would be for pep517 to put its in-process stuff in a subdirectory, so components in the pep517 module do not pollute sys.path. Should we move this issue to the pep517 repository?

@duckinator
Copy link
Contributor

@uranusjr yeah, that would probably make sense.

@hexagonrecursion
Copy link
Contributor

I want to work on this bug.

@uranusjr
Copy link
Member

Tou will need to send issues and pull requests to this repository instead: https://github.com/pypa/pep517

@hexagonrecursion
Copy link
Contributor

I was confused why out of the dozens of libraries pip vendors only pep517 is in the path. I was thinking maybe pep517 is not vendored correctly. bcc execsnoop cleared that up:

PCOMM            PID    PPID   RET ARGS
pip              21719  18141    0 /home/user/.conda/envs/hello/bin/pip install -e ./hello-setup/
uname            21721  21719    0 /usr/bin/uname -rs
python           21722  21719    0 /home/user/.conda/envs/hello/bin/python /home/user/pip/src/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-xmo5gc8z/overlay --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- setuptools wheel colorlog
uname            21726  21722    0 /usr/bin/uname -rs
python           21727  21719    0 /home/user/.conda/envs/hello/bin/python /home/user/pip/src/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmp1trhhp23

Note the last line: pip runs src/pip/_vendor/pep517/_in_process.py as a subprocess. This is why its parent directory is in the path.

@uranusjr
Copy link
Member

Please continue the discussion in pypa/pep517. It will raise awareness to people working on the library.

@hexagonrecursion
Copy link
Contributor

This should be closed in favor of pypa/pyproject-hooks#104

@uranusjr
Copy link
Member

uranusjr commented Feb 1, 2021

Let’s repurpose this to make sure we remember to upgrade the bundled copy of pep517 when upstream releases the fix.

@uranusjr uranusjr added the project: vendored dependency Related to a vendored dependency label Feb 1, 2021
@hexagonrecursion
Copy link
Contributor

Fix is merged pypa/pyproject-hooks#105. Awaiting release

@takluyver
Copy link
Member

Released as 0.10.0, if someone wants to update the vendoring.

@uranusjr uranusjr added this to the 21.1 milestone Mar 12, 2021
@uranusjr uranusjr changed the title Incompatibility between pep517 and colorlog (the package on PyPI) Upgrade vendored pep517 to 0.10.0 or later Mar 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
project: vendored dependency Related to a vendored dependency type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants