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

Sync raises IncompatibleRequirements even when environment markers indicate the incompatible requirements to be irrelevant for the current platform #925

Closed
AndydeCleyre opened this issue Oct 7, 2019 · 6 comments · Fixed by #927
Labels
enhancement Improvements to functionality needs discussion Need some more discussion

Comments

@AndydeCleyre
Copy link
Contributor

IMO this is simply more of #206, and I've already demonstrated the problem there. But it's been a month and no one has re-opened, so I'm opening this more specific issue to address the problem.

Here's an example dev-requirements.txt from the plumbum project:

pytest
pytest-cov
pytest-mock
idna<2.8 ; python_version < '2.7'
pycparser<2.18 ; python_version < '2.7'
paramiko<2.4 ; python_version < '2.7'
paramiko ; python_version >= '2.7'
setuptools
wheel ; python_version >= '2.7'
psutil
pip-sync dev-requirements.txt

Identical output whether in a Python 3.7.4 or 2.7.16 env:

Incompatible requirements found: paramiko (from -r dev-requirements.txt (line 7)) and paramiko<2.4 (from -r dev-requirements.txt (line 6))

No packages end up installed aside from pip-tools and its deps.

Environment Versions

  1. Arch Linux
  2. Python version: 3.7.4
  3. pip version: 19.2.3
  4. pip-tools version: 4.1.0

Steps to replicate

echo "paramiko==2.4.0 ; python_version < '2.7'" > mark.txt
echo "paramiko==2.6.0 ; python_version >= '2.7'" >> mark.txt
pip-sync mark.txt

Note that this works:

pip install --no-deps -r mark.txt

Expected result

pip-sync should ignore non-matching requirements when environment markers are present.

Actual result

pip-sync checks for conflicts as if it wants to install requirements for all platforms.

Further notes

mv mark.txt mark.in
pip-compile --no-header mark.in
asn1crypto==1.0.1         # via cryptography
bcrypt==3.1.7             # via paramiko
cffi==1.12.3              # via bcrypt, cryptography, pynacl
cryptography==2.7         # via paramiko
paramiko==2.6.0 ; python_version >= "2.7"
pycparser==2.19           # via cffi
pynacl==1.3.0             # via paramiko
six==1.12.0               # via bcrypt, cryptography, pynacl

Currently, compiling such an in-file will only include the compile-time platform's matching reqs. This hides the issue under discussion, and arguably means it's not a bug. But I believe it is generally desired for pip-sync to honor environment markers, as evidenced by the contents of #206 (closed, but not solved), #600 (merged), #459 (replaced), #460 (merged), #518 (open 2yrs), #563 (open 2yrs), #585 (open 2yrs), #896 (open), etc.

This is probably even more relevant for working with a single python version across different platforms.

@atugushev
Copy link
Member

atugushev commented Oct 8, 2019

Hello @AndydeCleyre,

Thanks for filing this issue. I would be against to enhance the pip-sync outside of pip-compile. I think pip-compile should be improved first to handle multiple environments (python versions, operation systems, etc.), and then pip-sync should honor this improvement. OTOH I can't imagine how to make a cross-environment compilation without a locking file (the way how poetry does).

Currently, compiling such an in-file will only include the compile-time platform's matching reqs. This hides the issue under discussion, and arguably means it's not a bug.

You are right. It's not a bug in the current implementation.

But I believe it is generally desired for pip-sync to honor environment markers, ...

Not pip-sync, but pip-compile is generally desired to honor env markers.

@AndydeCleyre
Copy link
Contributor Author

Thanks for being so active on here @atugushev, and for all the great feedback.

pip-compile should be enhanced first to handle multiple environments (python versions, operation systems, etc)

A few thoughts on this:

  1. Maybe that will happen a long time from now, or maybe never. I don't think there's a short term solution on the horizon for that kind of pip-compile enhancement.
  2. If/when pip-compile does receive that enhancement, wouldn't it ideally be generating a master txt, full of environment markers? "handl[ing] multiple environments (python versions, operation systems, etc)" is precisely what environment markers do. If that's so, then pip-sync needs to pay better attention to them.
  3. The runtime environment is conceptually much closer to pip-sync than pip-compile, as you can compile a txt without actually affecting the installed package state, but changing the environment state is absolutely happening when pip-sync runs. Currently pip-compile can't do env-agnostic compilations, but that is a desire expressed many times over here in the issue tracker. Users don't really want pip-compile to be so coupled with its own runtime environment, that's a limitation for now because it's easier to achieve, not because it's the best behavior for users.
  4. There is some level of "supported" handling of environment markers in the txt files. Over at Environment markers do not carry out to transitive dependencies #563 (which has remained open for two years, indicating that better environment marker support is still desirable to the community and not rejected by the developers), jazzband member @vphilippon clarifies:

#460 was really only about keeping explicit environment markers from the requirements.in, not being environment-agnostic (you can see the discussion of PR, suutari-ai covers it well). It's only a feature to deal with simple well known cases with no subdependencies where it is safe to simply keep the environment markers.

As an example, you could add enum34; python_version < '3.4' to your requirements.in, as you know it will be one of your dependencies in your example, and it's risk-free as enum34 has no dependencies, so the resulting requirements.txt should not vary. This gives you an environment-agnostic requirements.txt for this simple case, which could not be done before that PR.

In that same discussion, contributor @barrywhart proposes working around the limited environment marker support:

I think that [#460] may still help with this issue if you can identify and add the "problem" requirements to your requirements.in file along with an appropriate environment marker.

In essence, you would be working around the lack of a complete solution by manually doing what the tool does not do -- marking the problematic transitive dependencies.

All that is to say: we all know the environment markers handling isn't ideal or easy, but we are interested in making it a little better, and in having pip-sync act as smart as pip install when encountering them, and encountering them in a txt is a legitimate case that was intentionally improved upon already.

  1. I'm very surprised by

Not pip-sync, but pip-compile is generally desired to honor env markers.

With all the existing issues and discussions that I've linked, do you deny that there is a desire for pip-sync to honor env markers? Is it really better for users to have pip-sync fail on this input, rather than install the matching package to the environment?

paramiko==2.4.0 ; python_version < '2.7'
paramiko==2.6.0 ; python_version >= '2.7'
  1. Finally, pip-sync yields different results depending on pre-installed package set #896 / Include already-installed matching requirements in the to_install set, so that pip-sync reliably installs dependencies #907 is still open for now, and solving this issue (merging Exclude requirements with non-matching markers from pip-sync's merge stage #927) would further support the sentiment therein. I do hope that gets some support. In my opinion these changes only make pip-tools more practical, widely applicable, and predictable. I use pip-tools a lot, and love it. If the community here takes a firm rejection stance on these, I will reluctantly stop badgering here, and probably maintain a personal fork. These changes are real improvements for me; I guess I'm surprised to realize my vision may not be altogether aligned with the rest of the project.

If you've gotten this far, thanks for reading!

Is there anyone else you'd like to loop into these discussions? Looks like you've got over 100 commits this year more than the next-place contributor, who's in the single digits, so I really don't know who ought to be included here.

Thanks again for keeping this beautiful project alive.

@atugushev
Copy link
Member

@AndydeCleyre

With all the existing issues and discussions that I've linked, do you deny that there is a desire for pip-sync to honor env markers? Is it really better for users to have pip-sync fail on this input, rather than install the matching package to the environment?

Apologies for confusing. I meant pip-tools as a comprehensive solution, not pip-compile directly.

If the community here takes a firm rejection stance on these, I will reluctantly stop badgering here, and probably maintain a personal fork. These changes are real improvements for me; I guess I'm surprised to realize my vision may not be altogether aligned with the rest of the project.

I don't think the community rejects this. Just wait for the feedback of members who interested in this feature. As for me, I don't use pip-tools in multiple environments, so I'm useless here and can't give comprehensive feedback, sorry for that.

Is there anyone else you'd like to loop into these discussions? Looks like you've got over 100 commits this year more than the next-place contributor, who's in the single digits, so I really don't know who ought to be included here.

You should probably ask project leaders @vphilippon @davidovich.

Thanks again for keeping this beautiful project alive.

Thanks for the kind words! I appreciate it.

@atugushev
Copy link
Member

atugushev commented Oct 17, 2019

FTR, "official stance of pip-tools regarding cross-environment usage" described in #585 (comment).

AndydeCleyre added a commit to AndydeCleyre/pip-tools that referenced this issue Apr 12, 2020
@AndydeCleyre
Copy link
Contributor Author

Requesting more discussion, please.

@atugushev @vphilippon @davidovich @barrywhart @hazmat345 @merwok

I still think that as much as possible, pip-sync should work on any of the files that pip install -r works on. Despite lack of official support (at the moment), it can be useful to do so, and I don't see an advantage to holding back the minimal change that entails (#927).

I think pip-compile should be improved first to handle multiple environments (python versions, operation systems, etc.), and then pip-sync should honor this improvement.

I don't see what's wrong with reversing the order: first get pip-sync to gracefully handle the txt more like pip install -r, then potentially broaden pip-compile output possibilities in ways that are compatible with both. Instead of having pip-compile (even temporarily) produce output that pip-sync flops on, we can first improve pip-sync, breaking nothing, then improve pip-compile to take advantage of new possibilities.

@atugushev
Copy link
Member

@AndydeCleyre

You convinced me and I've approved the PR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality needs discussion Need some more discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants