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-compile fails with current master branch of pip #853

Closed
pganssle opened this issue Jul 19, 2019 · 24 comments · Fixed by #916
Closed

pip-compile fails with current master branch of pip #853

pganssle opened this issue Jul 19, 2019 · 24 comments · Fixed by #916
Labels
pip Related to pip

Comments

@pganssle
Copy link

pganssle commented Jul 19, 2019

If you install any version of pip after cad71a711787ed66 and try to run pip-compile on any file, you will get an error in the constructor for PackageFinder. In the linked commit, it complains about unexpected keyword argument trusted_hosts and in later commits it complains about various other keyword arguments. The current master (4cf08e85c09ad213) complains about find_links (see full traceback below), failing on this line.

The latest behavior (failing with find_links) starts with in 7d08bb37a55ff7cf.

I will note that considering the fact that pip-tools is apparently making use of pip's internal (private) API, it would be a good idea to add a CI job to build against the master branch of pip, so you can more easily catch these issues before release.

Steps to reproduce:

  1. Create temporary directory (optional):
mkdir -p /tmp/pip_compile_repro
cd /tmp/pip_compile_repro
  1. Install the latest pip from github:
git clone https://github.com/pypa/pip.git
cd pip
# In case this behavior changes in a more recent version of pip
git checkout 4cf08e85c09ad2136ff7212719bc8d126490a3cd
cd ..
pip install ./pip
  1. Create a requirements.in file and attempt to compile it
echo "six" > requirements.in
pip-compile requirements.in

Actual result

Gives a traceback:

$ pip-compile requirements.in
Traceback (most recent call last):
  File ".../bin/pip-compile", line 10, in <module>
    sys.exit(cli())
  File ".../lib/python3.7/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File ".../lib/python3.7/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File ".../lib/python3.7/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File ".../lib/python3.7/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File ".../lib/python3.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File ".../lib/python3.7/site-packages/piptools/scripts/compile.py", line 256, in cli
    repository = PyPIRepository(pip_options, session, build_isolation)
  File ".../lib/python3.7/site-packages/piptools/repositories/pypi.py", line 95, in __init__
    self.finder = PackageFinder(**finder_kwargs)
TypeError: __init__() got an unexpected keyword argument 'find_links'

TL;DR:

  File ".../lib/python3.7/site-packages/piptools/repositories/pypi.py",
        line 95, in __init__
    self.finder = PackageFinder(**finder_kwargs)
TypeError: __init__() got an unexpected keyword argument 'find_links'

Expected Result

$ pip-compile requirements.in 
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile requirements.in
#
six==1.12.0

CC @cjerdonek in case he wants to help out.

@cjerdonek
Copy link

It would help for me to know which parts of PackageFinder pip-tools needs / uses while refactoring of PackageFinder continues.

@pradyunsg
Copy link
Contributor

pradyunsg commented Jul 20, 2019

I'll also add that I'm hoping that at some point, we'd be able to pull out the logic for PackageFinder out of pip, into a new package.

That's obviously after the ongoing refactoring, and it'll help to know how PackageFinder is used here since I expect pip-tools to be a consumer of that library eventually.

@atugushev
Copy link
Member

Hi @cjerdonek, thanks for helping!

It would help for me to know which parts of PackageFinder pip-tools needs / uses while refactoring of PackageFinder continues.

First of all, pip-tools uses PackageFinder to parse pip options (index_urls, find_links, allow_all_prereleases, trusted_hosts and format_control) from requirements.in via pip._internal.req.req_file.parse_requirements and write them to requirements.txt.

Secondly, pip-tools's package resolver uses PackageFinder's logic:

Hope it helps.

@atugushev
Copy link
Member

Hello @pganssle, thanks for raising this issue!

If you install any version of pip after cad71a711787ed66 and try to run pip-compile on any file, you will get an error in the constructor for PackageFinder. In the linked commit, it complains about unexpected keyword argument trusted_hosts and in later commits it complains about various other keyword arguments. The current master (4cf08e85c09ad213) complains about find_links (see full traceback below), failing on this line.

CI has been failing since 19.1, as soon as refactoring of PackageFinder has been started. Unfortunately we can't fix it now before 19.2, because it may be broken any time after the fix on pip-tools. So we ought to wait for a release and fix ASAP after the release.

I will note that considering the fact that pip-tools is apparently making use of pip's internal (private) API, it would be a good idea to add a CI job to build against the master branch of pip, so you can more easily catch these issues before release.

I agree with you to a certain extent, it's a good idea. But as i said before we can't fix issues caused by pip master before a release, so CI notifications won't much help, but make noise. And still we can track that something is happening in master when ci-jobs starts to fail (even silently) and be prepared to make quick fix in pip-tools.

Actually, i have no idea how to prevent breaking pip-tools by new pip releases with refactorings or major changes. Maybe @cjerdonek @pradyunsg have any thoughts on that?

@atugushev atugushev added the pip Related to pip label Jul 22, 2019
@antonagestam
Copy link

For users finding this issue, a workaround is to downgrade pip: pip install 'pip<19.2'

@cjerdonek
Copy link

But as i said before we can't fix issues caused by pip master before a release, so CI notifications won't much help, but make noise.

Couldn't you still update pip-tools so that CI against pip master passes, as changes to pip occur before any pip release? That would prevent pip-tools from falling too far behind and make it so that a pip-tools release would be ready to go (provided there weren't any last minute changes to pip that break pip-tools before a pip release).

@atugushev
Copy link
Member

atugushev commented Jul 24, 2019

@cjerdonek

Spending couple days to fix pip>=19.2 compatibility and refactoring the pip-tools afterwards, i've realised that it's important to keep CI green agains pip master. Yes, as you've said before, it prevents from falling too far behind. But it also helps to find bugs on pip side before the release (like this pypa/pip#6772). The only downside is we have to refactor pip-tools side by side with pip, even in the middle of the process.

@pradyunsg
Copy link
Contributor

I'd say you don't have to be in sync with every commit but it'd definitely help.

TBH, I even think that if y'all and Chris are okay with it, we could discuss design details on the pip PRs, if need be.

@vdboor
Copy link

vdboor commented Jul 25, 2019

The only downside only is we have to refactor pip-tools side by side with pip, even in the middle of the process.

wouldn't setting allow_failures in Travis help? Have a sense of what's would be failing without being forced to fix that for a stable release

@laszewsk
Copy link

Here is my currint solution (I use this in my makefiles)

pip install 'pip<19.2'
pip-compile setup.py
pip install pip -U

This way I have i only for the duration of the compile. It woudl however be better to have a current release that works with the newest version of pip

@laura-surcel
Copy link

laura-surcel commented Jul 25, 2019

Hey, guys!
Thank you @atugushev for the fix. I see you made a new release on Github yesterday but it's not in pypi yet. Do you have an ETA for this? (We'll also downpin it for the moment)

@davidovich
Copy link
Contributor

@laura-surcel: @vphilippon and I are the only ones allowed to make a release. I can see what is possible, but I have been very far from the python world in the last years. I will try to release tonight.

@davidovich
Copy link
Contributor

@vphilippon beat me to it thanks !

@atugushev
Copy link
Member

@vdboor

wouldn't setting allow_failures in Travis help? Have a sense of what's would be failing without being forced to fix that for a stable release

It’s already set actually, but nobody pays attention on it. But we should

@laszewsk
Copy link

I confirmed that the fix works on macOS python 3.7.4 with pip 19.2.1
I think you can close i guess

@atugushev
Copy link
Member

I'm concerned about that for each major pip release we will have to add new code branches (to provide compatibility) and new CI dimension for the pip release (to prevent decrease test coverage). The pip doesn't have public API and pip-tools uses internal API, which can be changed any time (even each major release).

What about pip vendoring?

The pip had been vendored before (because of huge refactoring on pip==10.0, see #580) and later it was removed (see #657). But now we face the same issues.

Pros:

  1. a pip release will not break pip-tools anymore
  2. less code, no more if PIP_VERSION < (X, Y), no more legacy code
  3. no dimension for pip in CI matrix, less ci jobs, because there will be only jobs for supported python versions (7 vs 44 jobs in Travis-CI, 4 vs 36 in AppVeyor)
  4. it doesn't affect pip-sync, since it installs packages via python -m pip install ...

Cons:

  1. update pip-tools for each pip release (not mandatory though)
  2. pipenv will not be happy, since it's vendoring pip-tools and also vendoring pip.
  3. waste of space and increase package weight (from 90KB to ~1.5MB)
  4. ??

/cc @vphilippon @techalchemy

@cjerdonek
Copy link

Is it really necessary to keep supporting pip N versions back? What about supporting only the most recent versions? Then old supporting code can be deleted when new code is added.

@pradyunsg
Copy link
Contributor

Using pip-shims should reduce some of the pain of maintaining support.

@atugushev
Copy link
Member

atugushev commented Aug 2, 2019

@cjerdonek

Is it really necessary to keep supporting pip N versions back? What about supporting only the most recent versions? Then old supporting code can be deleted when new code is added.

I can't say for sure how much is it necessary, but pip-tools works since 8.* and we still get reports from users with ancient pip versions (e.g., the last one is #865, which has pip==9.0.1 on Ubuntu 18.04).

Supporting the most recent versions sounds reasonable to me. The question is how many? However it should be discussed with band members.


@pradyunsg

Using pip-shims should reduce some of the pain of maintaining support.

Thank you for the reference! Yes, i've seen that package recently though. Could be useful and might replace pip_compat.py. But i'm concerned more about increasing branches like this ones.

@pradyunsg
Copy link
Contributor

pradyunsg commented Aug 3, 2019

i'm concerned more about increasing branches like this ones.

That's just the nature of the beast. :(

My best suggestion to reduce the problem here is to trim the space, like @cjerdonek suggested -- only supporting a few "recent" versions of pip (something like last 2 minor version releases), instead of everything till like, pip 9 and all.

@cjerdonek
Copy link

Supporting the most recent versions sounds reasonable to me. The question is how many?

In thinking about this, it seems like the main important thing is that for each pip release, there is at least one version of pip-tools that users can use. Then, if someone wants to use a newer version of pip-tools, they would just possibly need to use a newer version of pip. (If someone can't be bothered to upgrade pip, then how important is it that they be able to upgrade pip-tools?)

With this strategy, a given pip-tools version would only need to support one minor version at a time (e.g. only pip 19.3.x). And the pip-tools release cadence would basically be the same as pip's (maybe needing to release whenever pip comes out with a new minor version, which it would have to do anyways), but releases could be done more often, too.

@atugushev
Copy link
Member

@cjerdonek

If someone can't be bothered to upgrade pip, then how important is it that they be able to upgrade pip-tools?

That's a great question!

@atugushev
Copy link
Member

atugushev commented Sep 22, 2019

I've added a cron job in Travis CI.

image

I believe, there should be a way to exclude PIP=master from "allow_failures" to
get failure notifications on pip master for the cron job. Unfortunately, I have no idea how to achieve that (I've tried a lot), since Travis CI has no support for conditional statements in an "allow_failures" block.

@atugushev
Copy link
Member

atugushev commented Oct 6, 2019

Hey folks! I've added a cron job to test pip-tools against pip master in #916. I would appreciate any feedback and reviews. Thanks!


image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pip Related to pip
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants