Fix missing dependencies of relevant constraints #1037

jeevb · 2020-01-21T21:23:42Z

Environment:
OS: Mac OS X (10.14.6) - Darwin Kernel Version 18.7.0
pip version: 20.0.2
pip-tools version: 4.4.0

A bug exists where dependencies of relevant constraints may not be added to the output file as shown below:

constraints.txt:

ipython==7.11.1
jedi==0.15.2
prompt-toolkit==3.0.2
jupyter-client==5.3.4
traitlets==4.3.3

requirements.in:

-c constraints.txt
ipykernel

requirements.txt:

#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile requirements.in
#
appnope==0.1.0            # via ipykernel
ipykernel==5.1.3
ipython==7.11.1           # via ipykernel
jupyter-client==5.3.4     # via ipykernel
tornado==6.0.3            # via ipykernel
traitlets==4.3.3          # via ipykernel

Expected requirements.txt:

#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile
#
appnope==0.1.0            # via ipykernel, ipython
backcall==0.1.0           # via ipython
decorator==4.4.1          # via ipython, traitlets
ipykernel==5.1.3
ipython-genutils==0.2.0   # via traitlets
ipython==7.11.1           # via ipykernel
jedi==0.15.2              # via ipython
jupyter-client==5.3.4     # via ipykernel
jupyter-core==4.6.1       # via jupyter-client
parso==0.5.2              # via jedi
pexpect==4.8.0            # via ipython
pickleshare==0.7.5        # via ipython
prompt-toolkit==3.0.2     # via ipython
ptyprocess==0.6.0         # via pexpect
pygments==2.5.2           # via ipython
python-dateutil==2.8.1    # via jupyter-client
pyzmq==18.1.1             # via jupyter-client
six==1.14.0               # via python-dateutil, traitlets
tornado==6.0.3            # via ipykernel, jupyter-client
traitlets==4.3.3          # via ipykernel, ipython, jupyter-client, jupyter-core
wcwidth==0.1.8            # via prompt-toolkit

# The following packages are considered to be unsafe in a requirements file:
# setuptools

Changelog-friendly one-liner: Fixes bug where dependencies of relevant constraints may be missing from output file.

Contributor checklist

Provided the tests for the changes.
Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

codecov · 2020-01-21T21:43:19Z

Codecov Report

Merging #1037 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #1037      +/-   ##
=========================================
+ Coverage    99.5%   99.5%   +<.01%     
=========================================
  Files          34      34              
  Lines        2418    2441      +23     
  Branches      306     307       +1     
=========================================
+ Hits         2406    2429      +23     
  Misses          6       6              
  Partials        6       6

Impacted Files	Coverage Δ
piptools/resolver.py	`100% <100%> (ø)`	⬆️
tests/test_cli_compile.py	`100% <100%> (ø)`	⬆️
tests/test_resolver.py	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0170297...2a31bda. Read the comment docs.

jeevb · 2020-01-21T22:00:54Z

Would appreciate a review on this PR. Let me know if there is something I can do to make this better.

atugushev

Hello @jeevb,

Thanks for your contribution. I've tried your test case with the latest pip-tools and got the expected results. Are you sure there is a bug to fix?

jeevb · 2020-01-23T05:53:12Z

Thanks for following up on this @atugushev. I'm getting the following error running the test on the current master: 30e90bef49dd1cc90c753836de271a64332c329b

E       AssertionError: assert {'aiohttp==3....'yarl==1.4.2'} == {'aiohttp==3....'yarl==1.4.2'}
E         Extra items in the right set:
E         'idna==2.8 (from yarl==1.4.2)'
E         Full diff:
E         - {'yarl==1.4.2', 'aiohttp==3.6.2'}
E         + {'yarl==1.4.2', 'idna==2.8 (from yarl==1.4.2)', 'aiohttp==3.6.2'}

Same error running off of the 4.4.0 tag: 814c8380803a1ba4117578dda0443f24a8adec8c. I have a test branch here if you would like to test for yourself: https://github.com/jeevb/pip-tools/tree/jeev/test-reqs-when-using-constraint-file. You should be able to reproduce with pytest -v tests/test_resolver.py.

jeevb · 2020-01-25T07:17:24Z

@atugushev: Were you able to reproduce the issue?

atugushev · 2020-01-26T05:31:19Z

@jeevb

Thanks for pinging. I've looked it more deeply. It seems there's a misunderstanding on how pip-compile works with constraints file (i.e., -c constraints.txt). Please read more info about how to use constraints here. You may achieve the "expected results" by using:

-r constraints.txt
ipykernel

This construction in requirements.in:

-c constraints.txt
ipykernel

means that you want to include ipykernel and all its dependencies to the requirements.txt and make sure that these dependencies respect constraints from constraints.txt. The other packages in constraints.txt which are not related to ipykernel(or any other packages and sub-dependencies in requirements.in) would be ignored.

jeevb · 2020-01-26T05:54:49Z

@atugushev

Thanks for taking the time to present a detailed explanation.

means that you want to include ipykernel and all its dependencies to the requirements.txt and make sure that these dependencies respect constraints from constraints.txt. The other packages in constraints.txt which are not related to ipykernel(or any other packages and sub-dependencies in requirements.in) would be ignored.

This is exactly what I am trying to achieve, but requirements.txt is missing dependencies of dependencies, which I originally thought should be included since they are within the dependency tree of ipykernel.

Also, with regard to the test case in this PR, can you clarify what the expected result is?["aiohttp==3.6.2", "yarl==1.4.2"] or
["aiohttp==3.6.2", "idna==2.8 (from yarl==1.4.2)", "yarl==1.4.2"]
as I have written it?

atugushev · 2020-01-26T06:56:21Z

@jeevb

requirements.txt is missing dependencies of dependencies

Could you elaborate, which one exactly?

jeevb · 2020-01-26T07:07:02Z

@jeevb

is missing dependencies of dependencies

Could you elaborate, which one exactly?

The following are all dependencies of dependencies of ipykernel that are missing from the requirements.txt in my example above. But per #1041 this is expected behavior.

backcall==0.1.0
decorator==4.4.1
jedi==0.15.2
parso==0.5.2
pexpect==4.8.0
pickleshare==0.7.5
prompt-toolkit==3.0.2
pygments==2.5.2
wcwidth==0.1.8

If the failure of the test in this PR is expected too, please free to close this.

atugushev · 2020-01-26T07:21:00Z

@jeevb

If the failure of the test in this PR is expected too, please free to close this.

Indeed, at least backcall should be included in requirements.txt, because the reverse dependency graph is backcall -> ipython -> ipykernel. This looks like a bug 🤔

atugushev · 2020-01-26T07:28:24Z

But locally on Mac OS X I've got completely different results, see:

$ cat constraints.txt
ipython==7.11.1
jedi==0.15.2
prompt-toolkit==3.0.2

$ cat requirements.in
-c constraints.txt
ipykernel

$ pip-compile
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile
#
appnope==0.1.0            # via ipykernel
decorator==4.4.1          # via traitlets
ipykernel==5.1.3
ipython-genutils==0.2.0   # via traitlets
ipython==7.11.1           # via ipykernel
jupyter-client==5.3.4     # via ipykernel
jupyter-core==4.6.1       # via jupyter-client
python-dateutil==2.8.1    # via jupyter-client
pyzmq==18.1.1             # via jupyter-client
six==1.14.0               # via python-dateutil, traitlets
tornado==6.0.3            # via ipykernel, jupyter-client
traitlets==4.3.3          # via ipykernel, jupyter-client, jupyter-core

Note backcall in the output.

atugushev · 2020-01-26T07:30:56Z

@jeevb

Could you try to compile with --rebuild flag?

$ rm requirements.txt
$ pip-compile --rebuild

jeevb · 2020-01-26T13:19:19Z

@atugushev

Yes I am able to exactly reproduce your output above after clearing my pip and pip-tools caches via rm -rf ~/Library/Caches/{pip,pip-tools} now.

However, I am still not seeing backcall and a few other dependencies in the output. It does not appear to be in the output you posted as well. It appears that dependencies of requirements in constraints.txt are not just included in requirements.txt, which is consistent with what you mentioned in #1041.

atugushev · 2020-01-26T15:43:04Z

@jeevb

I suspect that's because backcall is a sub dependency of ipython which is a constraint. I can reproduce the same pattern with flask.

Without constraints:

$ cat requirements.in
flask

$ pip-compile 
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile
#
click==7.0                # via flask
flask==1.1.1
itsdangerous==1.1.0       # via flask
jinja2==2.10.3            # via flask
markupsafe==1.1.1         # via jinja2
werkzeug==0.16.0          # via flask

With jinja2==2.10.3 constraint:

$ cat constraints.txt
jinja2==2.10.3

$ cat requirements.in
-c constraints.txt
flask

$ pip-compile -r
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile
#
click==7.0                # via flask
flask==1.1.1
itsdangerous==1.1.0       # via flask
jinja2==2.10.3            # via flask
werkzeug==0.16.0          # via flask

Note markupsafe (jinja's dependency) is not included.

Also, I'm inclined to think that we finally found the cause of weird bugs with the cache:

cat /Users/albert/Library/Caches/pip-tools/depcache-py3.8.json | python -m json.tool
{
    "__format__": 1,
    "dependencies": {
        "click": {
            "7.0": []
        },
        "flask": {
            "1.1.1": [
                "Jinja2>=2.10.1",
                "Werkzeug>=0.15",
                "click>=5.1",
                "itsdangerous>=0.24"
            ]
        },
        "itsdangerous": {
            "1.1.0": []
        },
        "jinja2": {
            "2.10.3": []
        },
        "werkzeug": {
            "0.16.0": []
        }
    }
}

atugushev · 2020-01-26T16:00:37Z

@jeevb

It appears that dependencies of requirements in constraints.txt are not just included in requirements.txt, which is consistent with what you mentioned in #1041.

This is definitely a bug :)

jeevb · 2020-01-26T16:14:56Z

I suspect that's because backcall is a sub dependency of ipython which is a constraint.

I agree completely with this.

This is definitely a bug :)

I think the problem is that dependencies of constraints are not resolved per here. We then cache this result, and when a requirement is no longer marked as a constraint in a later round of resolution, it continues to use this old (and wrong) cache result.

jeevb · 2020-01-26T16:52:33Z

With this PR, all dependencies are properly resolved:

$ cat constraints.txt
jinja2==2.10.3
$ cat requirements.in
-c constraints.txt
flask
$ pip-compile -r
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile
#
click==7.0                # via flask
flask==1.1.1
itsdangerous==1.1.0       # via flask
jinja2==2.10.3            # via flask
markupsafe==1.1.1         # via jinja2
werkzeug==0.16.0          # via flask
$ cat ~/Library/Caches/pip-tools/depcache-py3.7.json | jq .
{
  "__format__": 1,
  "dependencies": {
    "click": {
      "7.0": []
    },
    "flask": {
      "1.1.1": [
        "Jinja2>=2.10.1",
        "Werkzeug>=0.15",
        "click>=5.1",
        "itsdangerous>=0.24"
      ]
    },
    "itsdangerous": {
      "1.1.0": []
    },
    "jinja2": {
      "2.10.3": [
        "MarkupSafe>=0.23"
      ]
    },
    "markupsafe": {
      "1.1.1": []
    },
    "werkzeug": {
      "0.16.0": []
    }
  }
}

jeevb · 2020-01-26T20:52:20Z

@atugushev

After digging around a little on pip-tools and pip I think I finally have a reasonable fix for this issue. Would love to hear what you think.

atugushev

Awesome fix! 🎉 I've tried it locally — it works with my projects. I think it's worth to add a functional test to cover the main use-case or we could add it in a following up PR (both options fine by me). Also, I left one minor comment.

tests/test_resolver.py

…per PR comment)

jeevb · 2020-01-28T05:51:05Z

@atugushev

Happy to add the functional test in this PR. Is there an example or some documentation that I can follow?

…nts are properly resolved

tests/test_cli_compile.py

jeevb · 2020-01-30T04:57:46Z

This should be ready for a final review!

Looks like there are some unrelated CI errors. I wonder if these might be fixed with a build restart...

atugushev

LGTM 👍

atugushev · 2020-01-30T14:20:37Z

@jeevb thanks for the awesome work! Much appreciated! 🎉 🎉 🎉

atugushev · 2020-01-31T10:27:21Z

pip-tools v4.4.1 is released.

jeevb requested a review from vphilippon January 21, 2020 21:28

jeevb requested review from atugushev and removed request for vphilippon January 22, 2020 20:11

atugushev reviewed Jan 23, 2020

View reviewed changes

atugushev mentioned this pull request Jan 26, 2020

Dependencies are droped when using a combination of -r & -c in layered requirements #1041

Closed

atugushev added the bug fix label Jan 26, 2020

jeevb force-pushed the jeev/fix-missing-reqs-when-using-constraint-file branch from 9d36cac to 600f437 Compare January 26, 2020 16:36

jeevb force-pushed the jeev/fix-missing-reqs-when-using-constraint-file branch 7 times, most recently from ccbb0e0 to 65d8eb0 Compare January 26, 2020 18:28

skip resolving deps of constraints to avoid polluting cache

73c0610

jeevb force-pushed the jeev/fix-missing-reqs-when-using-constraint-file branch from 65d8eb0 to 73c0610 Compare January 26, 2020 20:38

test for iter dependencies

62be62c

update readme

4e2d696

atugushev mentioned this pull request Jan 28, 2020

pip-compile skips transitive dependencies on python 3.7 #928

Closed

atugushev reviewed Jan 28, 2020

View reviewed changes

tests/test_resolver.py Show resolved Hide resolved

check that dependencies of constraints are ignored in separate test (…

ea4d874

…per PR comment)

add functional test for checking if dependencies of relevant constrai…

9deec5b

…nts are properly resolved

jeevb commented Jan 28, 2020

View reviewed changes

tests/test_cli_compile.py Outdated Show resolved Hide resolved

jeevb requested a review from atugushev January 29, 2020 20:51

atugushev added setuptools Related to compiling requirements with `setuptools` build backend resolver Related to dependency resolver and removed setuptools Related to compiling requirements with `setuptools` build backend labels Jan 30, 2020

cleanup test per PR recommendation

2a31bda

atugushev approved these changes Jan 30, 2020

View reviewed changes

atugushev added this to the 4.4.1 milestone Jan 30, 2020

atugushev merged commit 90a052b into jazzband:master Jan 30, 2020

jeevb deleted the jeev/fix-missing-reqs-when-using-constraint-file branch January 30, 2020 14:20

This was referenced Feb 10, 2020

Workflow for layered requirements (e.g. prod<-test<-dev requirements)? #398

Closed

editable packages in a constraint file causes pip-compile to fail silently #979

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing dependencies of relevant constraints #1037

Fix missing dependencies of relevant constraints #1037

jeevb commented Jan 21, 2020 •

edited by atugushev

Loading

codecov bot commented Jan 21, 2020 •

edited

Loading

jeevb commented Jan 21, 2020 •

edited

Loading

atugushev left a comment

jeevb commented Jan 23, 2020 •

edited

Loading

jeevb commented Jan 25, 2020

atugushev commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

atugushev commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020

atugushev commented Jan 26, 2020

atugushev commented Jan 26, 2020 •

edited

Loading

atugushev commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

atugushev commented Jan 26, 2020

atugushev commented Jan 26, 2020

jeevb commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020

atugushev left a comment

jeevb commented Jan 28, 2020

jeevb commented Jan 30, 2020 •

edited

Loading

atugushev left a comment

atugushev commented Jan 30, 2020

atugushev commented Jan 31, 2020

Fix missing dependencies of relevant constraints #1037

Fix missing dependencies of relevant constraints #1037

Conversation

jeevb commented Jan 21, 2020 • edited by atugushev Loading

Contributor checklist

codecov bot commented Jan 21, 2020 • edited Loading

Codecov Report

jeevb commented Jan 21, 2020 • edited Loading

atugushev left a comment

Choose a reason for hiding this comment

jeevb commented Jan 23, 2020 • edited Loading

jeevb commented Jan 25, 2020

atugushev commented Jan 26, 2020 • edited Loading

jeevb commented Jan 26, 2020 • edited Loading

atugushev commented Jan 26, 2020 • edited Loading

jeevb commented Jan 26, 2020

atugushev commented Jan 26, 2020

atugushev commented Jan 26, 2020 • edited Loading

atugushev commented Jan 26, 2020 • edited Loading

jeevb commented Jan 26, 2020 • edited Loading

atugushev commented Jan 26, 2020

atugushev commented Jan 26, 2020

jeevb commented Jan 26, 2020 • edited Loading

jeevb commented Jan 26, 2020 • edited Loading

jeevb commented Jan 26, 2020

atugushev left a comment

Choose a reason for hiding this comment

jeevb commented Jan 28, 2020

jeevb commented Jan 30, 2020 • edited Loading

atugushev left a comment

Choose a reason for hiding this comment

atugushev commented Jan 30, 2020

atugushev commented Jan 31, 2020

jeevb commented Jan 21, 2020 •

edited by atugushev

Loading

codecov bot commented Jan 21, 2020 •

edited

Loading

jeevb commented Jan 21, 2020 •

edited

Loading

jeevb commented Jan 23, 2020 •

edited

Loading

atugushev commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

atugushev commented Jan 26, 2020 •

edited

Loading

atugushev commented Jan 26, 2020 •

edited

Loading

atugushev commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 26, 2020 •

edited

Loading

jeevb commented Jan 30, 2020 •

edited

Loading