pypa / pip

The Python package installer
https://pip.pypa.io/
MIT License
9.48k stars 3.01k forks source link

Pip 19.1 pip install --no-use-pep517 not working with nested requirements files #6433

Closed tom-dalton-fanduel closed 5 years ago

tom-dalton-fanduel commented 5 years ago

Relevent issues:

https://github.com/pypa/pip/issues/6375 https://github.com/pypa/pip/issues/6314 https://github.com/pypa/pip/pull/6370

Environment

Description

pip install fails with a requirements file that references a setup.py file, in a project that has a pyproject.toml file that has no build tooling section.

My project setup:

setup.py  # Contains the actual project/package requirements
pyproject.toml  # Contains only 2 lines of black configuration
requirements/base.txt  # Contains only `-e .`
requirements/testing.txt  # Contains `-r base.txt` and then other stuff like pytest etc.
src/mypackage/...  # Actual project/package code

This setup allows us to:

The suggested workaround in https://github.com/pypa/pip/pull/6370#issuecomment-486153504 doesn't seem to work for me:

pip install --no-use-pep517 -q --upgrade --requirement /..../src/requirements/development.txt
ERROR: Error installing 'file:///.... (from -r /..../src/requirements/_base.txt (line 1))': editable mode is not supported for pyproject.toml-style projects. pip is processing this project as pyproject.toml-style because it has a pyproject.toml file. Since the project has a setup.py and the pyproject.toml has no "build-backend" key for the "build_system" value, you may pass --no-use-pep517 to opt out of pyproject.toml-style processing. See PEP 517 for details on pyproject.toml-style projects.

pip install -r requirements/testing.txt fails

Expected behavior

Historically (in <= 19.0.3) pip would install the project and testing/dev requirements (into the current virtualenv).

How to Reproduce

Set up a project as per above, try to install the dev/testing requirements with pip install -r foo.txt.

Output

As per description.

gaborbernat commented 5 years ago

So I get now pip is PEP compliant, but this now breaks peoples workflow without any sane workaround, I consider it as a PEP oversight. PEP-517/8 on purpose did not want to cover editable installs, considering something to be addressed down the line. As such I would argue PEP-517/PEP-518 no longer applies when editable install mode is enabled. It's the build frontend dependent on how this case is handled for now. In such a case, pip should fallback to the old system, until we define editable installs inside a PEP. @pfmoore want to weigh in? If we want people to explicitly require the old behaviour that's okay, but then --no-use-pep517 should work independently if the pyproject.toml is there or what it contains.

cjerdonek commented 5 years ago

ERROR: Error installing 'file:///....

What does the "..." say? What is it trying to install at that point?

pitrou commented 5 years ago

Asking people to type the obscure --no-use-pep517 (should people even have to know what PEP 517 is?) when the well-known pip install -e used to work is not a sane workaround. I'm astonished that the pip maintainers estimate it's fine to break well-known practices just out of theoretical purity.

Of course, there's still python setup.py develop...

con-f-use commented 5 years ago

Is the underscore in _base.txt intendend. What happens when you specify --no-use-pep517 in the requirements file:

# .../requirements/_base.txt
--no-use-pep517 -e mypackage/
...
cjerdonek commented 5 years ago

pip install --no-use-pep517 -q --upgrade --requirement /..../src/requirements/development.txt

Also, your command invocation doesn't seem to match up with the project setup you listed because development.txt doesn't appear there. If you're nesting requirements files, it would be good to be clearer on the situation.

tom-dalton-fanduel commented 5 years ago

ERROR: Error installing 'file:///....

What does the "..." say? What is it trying to install at that point?

Sorry, I removed the paths for brevity. Here's the full output:

pip install --no-use-pep517 -q --upgrade --requirement /var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/src/requirements/development.txt
ERROR: Error installing 'file:///var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion (from -r /var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/src/requirements/_base.txt (line 1))': editable mode is not supported for pyproject.toml-style projects. pip is processing this project as pyproject.toml-style because it has a pyproject.toml file. Since the project has a setup.py and the pyproject.toml has no "build-backend" key for the "build_system" value, you may pass --no-use-pep517 to opt out of pyproject.toml-style processing. See PEP 517 for details on pyproject.toml-style projects.
make: *** [/var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/_build/requirements.txt] Error 1
tom-dalton-fanduel commented 5 years ago

Sorry, we do nest requirements. I simplified this but the full hierarchy is:

requirements/development.txt : has -r testing.txt at the top, then dev-only requirements like ipython requirements/testing.txt : has -r _base.txt at the top, then testing-only requirements like pytest requirements/_base.txt : has one line -e .

cjerdonek commented 5 years ago

@tom-dalton-fanduel Have you tried adding the option inside the appropriate requirements file, instead of only at the root command?

@pitrou That's a fair point re: the name. I'm guessing the expectation was that the option would only have to be used infrequently. The only "user-friendly" name that PEP 517 provides for the new behavior is pyproject.toml-style (in contrast to setup.py-style). So maybe a friendlier alias would be something using those words (e.g. --setup-py-style or something along those lines).

tom-dalton-fanduel commented 5 years ago

@tom-dalton-fanduel Have you tried adding the option inside the appropriate requirements file, instead of only at the root command?

I had great hopes but it failed spectacularly!

Running virtualenv with interpreter /usr/bin/python3.6
pip 19.1 from /var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/venv/lib/python3.6/site-packages/pip (python 3.6)
pip install -q --upgrade --requirement /var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/src/requirements/development.txt
Usage: pip [options]

ERROR: Invalid requirement: --no-use-pep517 -e .
pip: error: no such option: --no-use-pep517

make: *** [/var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/_build/requirements.txt] Error 1
pitrou commented 5 years ago

Right, I wouldn't mind having to add a new option (though I'm not sure why -e couldn't imply it automatically), but --no-use-pep517 reminds me of --single-version-externally-managed, only worse :-)

con-f-use commented 5 years ago

@tom-dalton-fanduel : I seem to recall something about only certain pip options being allowed inside requirement files. Some where disallowed for security-related reasons.

In any case --no-use-pep517 should definitely be allowed there, and it's a separate issue if it isn't.

Evpok commented 5 years ago

I agree with @pitrou, given that PEP 517 does not define editable installs, asking for one should imply not using PEP 517.

tom-dalton-fanduel commented 5 years ago

Apologies for the confusion my failed attempts to simplify the problem. We do have a short term workaround in that we can just force-downgrade pip to 19.0.3. If we can't find a good workaround (or the intended workaround does not work in my case), I'd be keen to understand how we can effectively replicate our current setup.py-for-package and requirements-files-for-testing system using the new pyproject.toml file. It sounds like others are in the same boat so clear guidelines on best (or even good) practice for this would be helpful. At the moment I haven't been able to find a doc/blog/anything that explains what the migration path should/could be.

gaborbernat commented 5 years ago

created #6434 to differentiate between no backends section (this issue) and develop mode with a project that has requires+build-backend.

con-f-use commented 5 years ago

I agree with @pitrou, given that PEP 517 does not define editable installs, asking for one should imply not using PEP 517.

Which will cause problems, when a later PEP specifies how editable installs work with pep517/8. But maybe the later "editable install" should then be called something different, like "source-install", "develope-install" or "repo-install" and not use the -e, --editable flag but rather its own.

cjerdonek commented 5 years ago

(though I'm not sure why -e couldn't imply it automatically)

@pitrou The issue (as @con-f-use also stated above) isn't purity but rather that pyproject.toml was meant to be the switch to turn on standards-defined behavior (which is what would permit other tools to be used in place of e.g. pip). It provided a rare opportunity in the packaging world for a way forward that isn't saddled by legacy behavior. If -e implied the legacy behavior, then we'd be stuck in the same situation as before. We wouldn't be able to define and implement a standards-defined editable mode that deviates from how pip currently behaves because at the point of rolling it out, we'd then be breaking everyone already using -e with pyproject.toml. (And yes, there is of course some breakage now, but it's a lot smaller than it would be later since PEP 517 is still relatively new.)

con-f-use commented 5 years ago

Unless, as I said, you define -e as legacy-only, then define a new, distinct flag for pep517-style editable installs (and maybe eventually deprecate -e).

con-f-use commented 5 years ago

Regarding https://github.com/pypa/pip/issues/6433#issuecomment-486173101, I filed #6438 which seems like a legitimate issue.

con-f-use commented 5 years ago

By the way, here is a (somewhat) minimal example:

$ tree .
.
├── sublevel-1.txt
├── pyproject.toml
├── setup.py
├── src
│   └── req517
│       ├── __init__.py
└── top-level.txt

$  find . -type f -exec echo -e "\n{}\n================" \; -exec cat {} \;

./sublevel-1.txt
================
-e .

./top-level.txt
================
-r sublevel-1.txt

./pyproject.toml
================
[build-system]
requires = ["setuptools"]

./setup.py
================
#!/usr/bin/python
from setuptools import setup, find_packages

setup(
    name='req517',
    version='0.0',
    package_dir={'': 'src'},
    packages=find_packages('src'),
)

./src/req517/__init__.py
================
from __future__ import print_function
def oh_hell():
    print('Hello from ' + str(__name__))

oh_hell()

Output:

$ pip install --no-use-pep517 -r top-level.txt 
Obtaining file:///tmp/tmp.DCWhnP86F9 (from -r level-1.txt (line 1))
ERROR: Error installing 'file:///tmp/tmp.DCWhnP86F9 (from -r level-1.txt (line 1))': editable mode is not supported for pyproject.toml-style projects. pip is processing this project as pyproject.toml-style because it has a pyproject.toml file. Since the project has a setup.py and the pyproject.toml has no "build-backend" key for the "build_system" value, you may pass --no-use-pep517 to opt out of pyproject.toml-style processing. See PEP 517 for details on pyproject.toml-style projects.

$ pip install --no-use-pep517 -r sublevel-1.txt 
Obtaining file:///tmp/tmp.DCWhnP86F9 (from -r level-1.txt (line 1))
  Installing build dependencies ... done
Installing collected packages: req517
  Running setup.py develop for req517
Successfully installed req517
jriddy commented 5 years ago

Unless, as I said, you define -e as legacy-only, then define a new, distinct flag for pep517-style editable installs (and maybe eventually deprecate -e).

I think this is a reasonable thing to do. I like PEP 517 and (especially) PEP 518, but I have always figured the editable use case, which is completely undefined in either, was outside their scopes. The assertion of @cjerdonek "that pyproject.toml was meant to be the switch to turn on standards-defined behavior" sounds like you guys are interpreting the mere presence of this file as a purity switch. I think people want the features that the standards provides without sacrificing useful legacy behaviro.

Continuing to support -e at all implies that PEP 517 support is not wanted because PEP 517 has nothing to say about editable installs. I know that "editable installs" are in many ways a hodge-podge of implementation-defined, non-standardized behaviors, but a lot of people rely on them and find them useful. Is there a non-ridiculous way way to get this behavior? Can we get a [tool.pip] table in pyproject.toml where I can put a use-pep517 = false? Can we just expect that once there's a good PEP for editable installs, it will be expected to propose an alternate switch like -d for --develop or -s for --source-install. Migrating to a new API and deprecating the old is a much friendlier and more useful way for a package manager to ditch cruft and move on.

jriddy commented 5 years ago

I'm complaining but the more I read on this, the more I'm understanding why you want to get away from this.

For the record, a work around for this issue seems to be using the PIP_USE_PEPE517 environment variable or a use-pep517 = false in the [install] section of a pip.conf

cjerdonek commented 5 years ago

@tom-dalton-fanduel A couple hours ago I posted https://github.com/pypa/pip/pull/6447 to permit using --use-pep517 and --no-use-pep517 inside requirements files (fixing #6438). In your case, this would let you add --no-use-pep517 before -e . inside your requirements/base.txt. Will this address your issue?

I think putting --no-use-pep517 at your top-level invocation isn't what we'd want to get working because that would apply to all requirements in all requirements files, when really you just want it to apply to the one line -e ..

tom-dalton-fanduel commented 5 years ago

@tom-dalton-fanduel A couple hours ago I posted #6447 to permit using --use-pep517 and --no-use-pep517 inside requirements files (fixing #6438). In your case, this would let you add --no-use-pep517 before -e . inside your requirements/base.txt. Will this address your issue?

I think putting --no-use-pep517 at your top-level invocation isn't what we'd want to get working because that would apply to all requirements in all requirements files, when really you just want it to apply to the one line -e ..

Yes this sounds like it would work for our use case. Is there a way I can test this that wouldn't require building that dev version of pip locally? E.g. pip install -U pip==19.1.0-pr6447 or similar?

Longer term it seems like we may want to actually start using PEP 517 functionality, is there any documentation on what the [future] best practice for our sort of setup will/should look like? At the moment we have package requirements specified in setup.py, and then testing and dev requirements 'added on' in the requirements/* files. It's not clear to me if this is actually considered bad practice or if there is in fact no clear "standard" way to do this. The pytest docs at least suggest installing the package-under-test in editable mode, so I guess (hope?) what we're doing isn't completely bonkers :-)

xavfernandez commented 5 years ago

@tom-dalton-fanduel You can test the #6447 with pip install https://github.com/cjerdonek/pip/archive/requirements-file-no-use-pep517.zip

cjerdonek commented 5 years ago

I believe this should also work:

$ pip install -U git+https://github.com/pypa/pip.git@refs/pull/6447/head#egg=pip
sbidoul commented 5 years ago

I personally like the idea of saying -e means legacy setup.py develop, and later introducing a new option (--inplace?) when it becomes standardized.

tom-dalton-fanduel commented 5 years ago

@tom-dalton-fanduel You can test the #6447 with pip install https://github.com/cjerdonek/pip/archive/requirements-file-no-use-pep517.zip

I tried the new version and it still failed... but then I realised I forgot to add the new option to my _base requirements. I added it, and:

~/repos/sudobot pip-fix-test (sudobot) › pip install -r src/requirements/development.txt 
...
Successfully installed freezegun-0.3.11 sudobot
~/repos/sudobot pip-fix-test (sudobot) › 

:tada:

Great success!

tom-dalton-fanduel commented 5 years ago

This is no longer an issue with pip 19.1.1 due to https://github.com/pypa/pip/issues/6434 - editible instlals with a pyproject.toml file are working again. Thanks to everyone involved :)