Reopen of #3903: extras_require isn't honored if a package is already "discovered" without them
asottile opened this issue · 19 comments
- Pip version: 10.0.0.dev0 (b9f70f2) -- also reproduces with 8.1.2, 9.0.1, and other versions I've tried
- Python version: python2.7 -- also reproduces with other versions I've tried (3.5, 3.6)
- Operating system: linux_x86_64 (In my case ubuntu xenial) -- also reproduces on other linuxes and assumedly osx / windows as well
Description:
Installing pre-commit==0.13.0 and bravado-core should also install the dependencies of jsconschema[format] (from bravado-core) (for example webcolors)
What I've run:
# removing cache is unnecessary, but for sanity's sake
rm -rf venv ~/.cache/pip
virtualenv venv
. venv/bin/activate
echo -e 'pre-commit==0.13.0\nbravado-core' > requirements.txt
pip install -r requirements.txt
# I expect this to contain webcolors
pip freeze -l
Output
$ # removing cache is unnecessary, but for sanity's sake
$ rm -rf venv ~/.cache/pip
$ virtualenv venv
New python executable in /tmp/foo/venv/bin/python
Installing setuptools, pip, wheel...done.
$ . venv/bin/activate
(venv) $ echo -e 'pre-commit==0.13.0\nbravado-core' > requirements.txt
(venv) $ pip install -r requirements.txt
Collecting pre-commit==0.13.0 (from -r requirements.txt (line 1))
Downloading pre_commit-0.13.0-py2.py3-none-any.whl (135kB)
100% |████████████████████████████████| 143kB 1.4MB/s
Collecting bravado-core (from -r requirements.txt (line 2))
Downloading bravado-core-4.7.2.tar.gz
Collecting aspy.yaml (from pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading aspy.yaml-0.3.0-py2.py3-none-any.whl
Collecting jsonschema (from pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading jsonschema-2.6.0-py2.py3-none-any.whl
Collecting cached-property (from pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading cached_property-1.3.0-py2.py3-none-any.whl
Collecting pyyaml (from pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading PyYAML-3.12.tar.gz (253kB)
100% |████████████████████████████████| 256kB 879kB/s
Collecting virtualenv (from pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading virtualenv-15.1.0-py2.py3-none-any.whl (1.8MB)
100% |████████████████████████████████| 1.8MB 387kB/s
Collecting nodeenv>=0.11.1 (from pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading nodeenv-1.1.2.tar.gz
Collecting python-dateutil (from bravado-core->-r requirements.txt (line 2))
Downloading python_dateutil-2.6.0-py2.py3-none-any.whl (194kB)
100% |████████████████████████████████| 194kB 875kB/s
Collecting simplejson (from bravado-core->-r requirements.txt (line 2))
Downloading simplejson-3.10.0.tar.gz (77kB)
100% |████████████████████████████████| 81kB 1.3MB/s
Requirement already satisfied: six in ./venv/lib/python2.7/site-packages (from bravado-core->-r requirements.txt (line 2))
Collecting swagger-spec-validator>=2.0.1 (from bravado-core->-r requirements.txt (line 2))
Downloading swagger-spec-validator-2.1.0.tar.gz
Collecting pytz (from bravado-core->-r requirements.txt (line 2))
Downloading pytz-2017.2-py2.py3-none-any.whl (484kB)
100% |████████████████████████████████| 491kB 709kB/s
Collecting functools32; python_version == "2.7" (from jsonschema->pre-commit==0.13.0->-r requirements.txt (line 1))
Downloading functools32-3.2.3-2.zip
Building wheels for collected packages: bravado-core, pyyaml, nodeenv, simplejson, swagger-spec-validator, functools32
Running setup.py bdist_wheel for bravado-core ... done
Stored in directory: /home/asottile/.cache/pip/wheels/f7/e7/1f/1925a22efa17f032ac89a5b9879f391440e74a0bac1831ce66
Running setup.py bdist_wheel for pyyaml ... done
Stored in directory: /home/asottile/.cache/pip/wheels/2c/f7/79/13f3a12cd723892437c0cfbde1230ab4d82947ff7b3839a4fc
Running setup.py bdist_wheel for nodeenv ... done
Stored in directory: /home/asottile/.cache/pip/wheels/b8/d5/01/5e8f6e846212747b154528bfc49796590883bedb4137793b08
Running setup.py bdist_wheel for simplejson ... done
Stored in directory: /home/asottile/.cache/pip/wheels/43/c5/ef/edcebbb19becffd2ba75bf219afdbb4ca85198b2d909f1b31b
Running setup.py bdist_wheel for swagger-spec-validator ... done
Stored in directory: /home/asottile/.cache/pip/wheels/2c/ca/49/214a7376ac5ab178f350dc30e9afc4aefd1d59bd0e1da2786b
Running setup.py bdist_wheel for functools32 ... done
Stored in directory: /home/asottile/.cache/pip/wheels/3c/d0/09/cd78d0ff4d6cfecfbd730782a7815a4571cd2cd4d2ed6e69d9
Successfully built bravado-core pyyaml nodeenv simplejson swagger-spec-validator functools32
Installing collected packages: pyyaml, aspy.yaml, functools32, jsonschema, cached-property, virtualenv, nodeenv, pre-commit, python-dateutil, simplejson, swagger-spec-validator, pytz, bravado-core
Successfully installed aspy.yaml-0.3.0 bravado-core-4.7.2 cached-property-1.3.0 functools32-3.2.3.post2 jsonschema-2.6.0 nodeenv-1.1.2 pre-commit-0.13.0 python-dateutil-2.6.0 pytz-2017.2 pyyaml-3.12 simplejson-3.10.0 swagger-spec-validator-2.1.0 virtualenv-15.1.0
(venv) $ # I expect this to contain webcolors
(venv) $ pip freeze -l
appdirs==1.4.3
aspy.yaml==0.3.0
bravado-core==4.7.2
cached-property==1.3.0
functools32==3.2.3.post2
jsonschema==2.6.0
nodeenv==1.1.2
packaging==16.8
pre-commit==0.13.0
pyparsing==2.2.0
python-dateutil==2.6.0
pytz==2017.2
PyYAML==3.12
simplejson==3.10.0
six==1.10.0
swagger-spec-validator==2.1.0
virtualenv==15.1.0
Ah ha, I see what you're doing here and I understand what's going on. The problem is essentially that pip doesn't have a real dependency resolver, so instead of combining dependencies correctly it just uses the first instance of it sees. Fixing that is tracked in #988, however I thought that we had implemented combining extras, so I'm going to leave this open. Thanks!
For what it's worth, we're working around this in our "upgrade-requirements" tool: https://github.com/Yelp/requirements-tools/blob/04edbf5b6233779e851ab599ede856ca0ff31427/requirements_tools/upgrade_requirements.py#L150-L166
The approximate algorithm:
- Set the requirements to be installed to
-r requirements.txt -r requirements-dev.txt - Run
pip install ...on the requirements - Trace the requirements, if any are unsatisfied, add them to a list
- Set that list to the "to be installed" and GOTO 1
There's also some (clever?) reexec code to prevent infinite recursion
It looks like you should be using pip-tools. Take a look at this (fixed) issue for instance – jazzband/pip-tools#408
We have slightly different goals (in requirements-tools) and conventions than pip-tools (this is also a different problem than the one described in pip-tools#408) -- that said, pip-tools also solves this problem in a similar way
A quick scan suggests it probably is yes.
@dstufft - Is this really blocked by #988 though? A fairly naive tweak would be hugely useful here, since "x depends on y, x[extra] depends on y[extra]" seems almost like a pattern that should be encouraged. (I have definitely thought about doing something like this with Twisted, and I didn't realize it would be broken).
I only say this because #988 seems like one of those "white whale" tickets that might be open for another decade, and it would be a shame to let this egregiously wrong behavior just sit there in the meanwhile.
I know there's a GSOC student working on it, but their progress report blog doesn't seem to reference #988 directly, and there are no updates on the issue itself. So I just don't know how they're doing, and based on the herculean comment history, it seems likely they might be struggling 🙃. If they're doing awesome and it'll be fixed by the end of the summer then please ignore me!
I just don't know how they're doing [snip] it seems likely they might be struggling
As the student myself, I think I'm going fine. :)
There was preparatory work that was almost needed for making it easier to work on pip and the then comes implementation of the dependency resolver in it. Basically half of the entire project period was that. :)
it'll be fixed by the end of the summer
Just worth mentioning that even if it isn't, I'll continue to work on it, even after GSoC is over, to finish it.
@glyph We already have some logic like what you describe to try and combine extras, but the problems essentially does stem from #988 to the best I can tell without investing a bunch of time to figure out if that is indeed the case. Extras aren't special here, if one project depends on foo[a] and another depends on foo it needs to resolve those the same as it would if one project depended on foo >= 1 and another depending on foo < 3. Much like that, we even have hackish logic to try and cope with that now, but there are a laundry list of edge cases with the hacks that really boil down to "fix #988".
Could we maybe improve it independently of that? Yea there's probably more hacks to layer on, but I don't think the pip project is going to be doing it, particularly before @pradyunsg completes his work. If you write a PR I'll be glad to review and merge it though.
WRT to @pradyunsg's work, I think they're progressing fine and there's nothing at this point to me to indicate that it is at risk of not getting completed, either by the end of the summer or after.
I just stumble across this bug. Here is another example:
$ docker run -it python:3 pip install httpbin
[SNIP]
Successfully installed Flask-0.12.2 Flask-Common-0.2.0 Gunicorn-19.7.1 Jinja2-2.9.6 MarkupSafe-1.0 Werkzeug-0.12.2 WhiteNoise-3.3.1 blinker-1.4 brotlipy-0.7.0 cffi-1.11.2 click-6.7 colorama-0.3.9 crayons-0.1.2 dateparser-0.6.0 decorator-4.1.2 flask-cache-0.13.1 flask-limiter-0.9.5.1 greenlet-0.4.12 httpbin-0.6.1 humanize-0.5.1 itsdangerous-0.24 limits-1.2.1 maya-0.3.3 meinheld-0.6.1 pendulum-1.3.0 pycparser-2.18 python-dateutil-2.6.1 pytz-2017.2 pytzdata-2017.2.2 raven-6.2.1 regex-2017.9.23 ruamel.yaml-0.15.34 six-1.11.0 tzlocal-1.4
The package above depends on raven[flask] which installs blinker. The command below does not install blinker.
$ docker run -it python:3 pip install raven httpbin
[SNIP]
Successfully installed Flask-0.12.2 Flask-Common-0.2.0 Gunicorn-19.7.1 Jinja2-2.9.6 MarkupSafe-1.0 Werkzeug-0.12.2 WhiteNoise-3.3.1 brotlipy-0.7.0 cffi-1.11.2 click-6.7 colorama-0.3.9 crayons-0.1.2 dateparser-0.6.0 decorator-4.1.2 flask-cache-0.13.1 flask-limiter-0.9.5.1 greenlet-0.4.12 httpbin-0.6.1 humanize-0.5.1 itsdangerous-0.24 limits-1.2.1 maya-0.3.3 meinheld-0.6.1 pendulum-1.3.0 pycparser-2.18 python-dateutil-2.6.1 pytz-2017.2 pytzdata-2017.2.2 raven-6.2.1 regex-2017.9.23 ruamel.yaml-0.15.34 six-1.11.0 tzlocal-1.4
As this ticket is blocked by the development of the dependency resolver (#988), I thought I would mention here that the team is looking for help from the community to move forward on that subject.
We need to better understand the circumstances under which the new resolver fails, so are asking for pip users with complex dependencies to:
- Try the new resolver (use version 20.1, run
--unstable-feature=resolver) - Break it :P
- File an issue
You can find more information and more detailed instructions here
@pfmoore @uranusjr @ilanschnell Here's another thing to try out with the new resolver, to make sure we actually fixed this. :)
I believe we already have a test case similar to this in test_new_resolver.py.
Yup, and I just tried it out and I see webcolors in the reproduction filed above. :)
Basically, the new resolver fixes this.
Sidenote: The title of this issue confused me for a while. I thought it’s describing a problem where an already installed package would make a subsequence pip install miss an extra (it wouldn’t, even using the legacy resolver).
The actual issue here is however that pip misses extras if the same package is requested multiple times in the same install command, but the one without extras is listed before the one with extras.
It seems that this bug is still present: https://travis-ci.org/github/ericpre/hyperspy/builds/699059458
https://github.com/hyperspy/hyperspy/blob/2e02941d35d7594a8ac8506b129d75db5171a5a1/setup.py#L81-L82
In this case:
pytest-cov is listed in the extras_require "coverage" and depends on pytest>=4.6 but pytest is also listed in the "tests" extra_require with pytest>=3.6. The pytest-cov dependency requirement (pytest>=4.6) is not honoured in this case.
We have now closed the blocker bug #988! pip 20.3 is now out and has the new dependency resolver on by default (documentation), in case anyone reading this issue wants to take fresh action.
I expect the new resolver to not have this issue. IIRC there is a test for specifically this issue 🙂
yep does indeed