-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
Respect env markers when checking resolves. #861
Conversation
Previously we failed to do this and would erroneously fail resolves as a result. Fixes pex-tool#851
Even though things were fine as-is, adding an environment marker that re-iterates the subprocess marker only applies for python_version<3 makes it both clearer at the point of use and clearer this can be dropped when / if we eventually frop python2 support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Glad to see this working. I appreciate the type hints and test!
resolved_dist = resolved_requirement_dist.distribution | ||
if resolved_dist not in requirement: | ||
unsatisfied.append( | ||
'{dist} requires {requirement} but {resolved_dist} was resolved'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know any reasons this might happen? It would help if we could add possible remedies to users. For example, "possibly because your platform does not have any pre-built wheels."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens due to conflicting requirements. Its the red text you get on a pip install surfaced as an error (pip cannot be made to error for these!). There is no easy advice to give in these cases. Sometimes the conflict is artificial - ie: the pip resolver could solve if it did enough backtracking. Sometimes the conflict is real.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens due to conflicting requirements. Its the red text you get on a pip install surfaced as an error (pip cannot be made to error for these!). There is no easy advice to give in these cases. Sometimes the conflict is artificial - ie: the pip resolver could solve if it did enough backtracking. Sometimes the conflict is real.
Expressing the first and last sentences would be very helpful, imo. Here's a possible rewording:
{dist} requires {requirement} but {resolved_dist} was resolved. This happens when Pip cannot properly resolve the requirements, usually due to conflicting versions.
While this doesn't pinpoint the exact error, it gives some insight to first audit if there are conflicting versions before looking for other things. Honestly, I didn't think to do that upon first reading the original message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm. That doesn't really help much. I think I can do better, just a sec...
In the meantime, for example, for the same requirements.
- pip exit code 0
ERROR: elasticsearch-curator 5.8.1 has requirement pyyaml==3.13, but you'll have pyyaml 5.3 which is incompatible.
ERROR: elasticsearch-curator 5.8.1 has requirement urllib3<1.25,>=1.24.2, but you'll have urllib3 1.25.8 which is incompatible.
ERROR: moto 1.3.15.dev228 has requirement python-dateutil<2.8.1,>=2.1, but you'll have python-dateutil 2.8.1 which is incompatible.
ERROR: pantsbuild-pants 1.25.0.dev1 has requirement docutils==0.14, but you'll have docutils 0.15.2 which is incompatible.
ERROR: pantsbuild-pants 1.25.0.dev1 has requirement packaging==16.8, but you'll have packaging 20.1 which is incompatible.
ERROR: pantsbuild-pants 1.25.0.dev1 has requirement pex==1.6.12, but you'll have pex 2.1.0 which is incompatible.
ERROR: pantsbuild-pants 1.25.0.dev1 has requirement PyYAML==5.1.2, but you'll have pyyaml 5.3 which is incompatible.
ERROR: pantsbuild-pants 1.25.0.dev1 has requirement wheel==0.31.1, but you'll have wheel 0.33.6 which is incompatible.
ERROR: pantsbuild-pants-contrib-mypy 1.25.0.dev1 has requirement pex==1.6.12, but you'll have pex 2.1.0 which is incompatible.
- pex exit code 1
Failed to resolve compatible distributions:
1: pantsbuild.pants.contrib.mypy==1.25.0.dev1 requires pex==1.6.12 but pex 2.1.0 was resolved
2: pantsbuild.pants==1.25.0.dev1 requires pex==1.6.12 but pex 2.1.0 was resolved
3: pantsbuild.pants==1.25.0.dev1 requires packaging==16.8 but packaging 20.1 was resolved
4: pantsbuild.pants==1.25.0.dev1 requires wheel==0.31.1 but wheel 0.33.6 was resolved
5: pantsbuild.pants==1.25.0.dev1 requires docutils==0.14 but docutils 0.15.2 was resolved
6: pantsbuild.pants==1.25.0.dev1 requires PyYAML==5.1.2 but PyYAML 5.3 was resolved
7: elasticsearch-curator==5.8.1 requires pyyaml==3.13 but PyYAML 5.3 was resolved
8: elasticsearch-curator==5.8.1 requires urllib3<1.25,>=1.24.2 but urllib3 1.25.8 was resolved
9: moto==1.3.15.dev228 requires python-dateutil<2.8.1,>=2.1 but python-dateutil 2.8.1 was resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. You know what, I take it back. This is actually pretty intuitive! I was having a hard time filling in the blanks of what the message would actually look like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this mod some formatting / coalescing in a follow-up?:
Failed to resolve compatible distributions:
1: pantsbuild.pants.contrib.mypy==1.25.0.dev1 requires pex==1.6.12 but pex 2.1.0 was resolved.
Other requirements for pex were:
pantsbuild.pants 1.25.0.dev1 requires pex==1.6.12
2: pantsbuild.pants==1.25.0.dev1 requires packaging==16.8 but packaging 20.1 was resolved.
Other requirements for packaging were:
pytest 5.3.4 requires packaging
3: pantsbuild.pants==1.25.0.dev1 requires PyYAML==5.1.2 but PyYAML 5.3 was resolved.
Other requirements for pyyaml were:
cfn-lint 0.27.2 requires pyyaml; python_version != "3.4"
moto 1.3.15.dev228 requires PyYAML>=5.1
kubernetes 10.0.1 requires pyyaml>=3.12
yq 2.8.1 requires PyYAML>=3.11
elasticsearch-curator 5.8.1 requires pyyaml==3.13
4: pantsbuild.pants==1.25.0.dev1 requires pex==1.6.12 but pex 2.1.0 was resolved.
Other requirements for pex were:
pantsbuild.pants.contrib.mypy 1.25.0.dev1 requires pex==1.6.12
5: pantsbuild.pants==1.25.0.dev1 requires wheel==0.31.1 but wheel 0.33.6 was resolved.
Other requirements for wheel were:
6: pantsbuild.pants==1.25.0.dev1 requires docutils==0.14 but docutils 0.15.2 was resolved.
Other requirements for docutils were:
botocore 1.14.9 requires docutils<0.16,>=0.10
7: moto==1.3.15.dev228 requires python-dateutil<2.8.1,>=2.1 but python-dateutil 2.8.1 was resolved.
Other requirements for python-dateutil were:
kubernetes 10.0.1 requires python-dateutil>=2.5.3
freezegun 0.3.14 requires python-dateutil!=2.0,>=1.0
elasticsearch-dsl 7.1.0 requires python-dateutil
botocore 1.14.9 requires python-dateutil<3.0.0,>=2.1
8: elasticsearch-curator==5.8.1 requires urllib3<1.25,>=1.24.2 but urllib3 1.25.8 was resolved.
Other requirements for urllib3 were:
sentry-sdk 0.14.0 requires urllib3>=1.10.0
requests 2.22.0 requires urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1
kubernetes 10.0.1 requires urllib3>=1.24.2
elasticsearch 7.5.1 requires urllib3>=1.21.1
botocore 1.14.9 requires urllib3<1.26,>=1.20
9: elasticsearch-curator==5.8.1 requires pyyaml==3.13 but PyYAML 5.3 was resolved.
Other requirements for pyyaml were:
cfn-lint 0.27.2 requires pyyaml; python_version != "3.4"
pantsbuild.pants 1.25.0.dev1 requires PyYAML==5.1.2
moto 1.3.15.dev228 requires PyYAML>=5.1
kubernetes 10.0.1 requires pyyaml>=3.12
yq 2.8.1 requires PyYAML>=3.11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is extremely helpful! I wonder if Pip would accept that as a contribution.
Hopefully once Pip drops Python 2 support. |
Since we vendor pip, the decision can be independent of their timeline to some extent. |
Previously we failed to do this and would erroneously fail resolves as
a result.
Fixes #851