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

Respect env markers when checking resolves. #861

Merged
merged 2 commits into from
Jan 25, 2020

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jan 23, 2020

Previously we failed to do this and would erroneously fail resolves as
a result.

Fixes #851

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.
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a 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(
Copy link
Contributor

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."

Copy link
Member Author

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.

Copy link
Contributor

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.

Copy link
Member Author

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.

  1. 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.
  1. 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

Copy link
Contributor

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.

Copy link
Member Author

@jsirois jsirois Jan 25, 2020

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

Copy link
Contributor

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.

@Eric-Arellano
Copy link
Contributor

when / if we eventually drop python2 support.

Hopefully once Pip drops Python 2 support.

@jsirois
Copy link
Member Author

jsirois commented Jan 25, 2020

Hopefully once Pip drops Python 2 support.

Since we vendor pip, the decision can be independent of their timeline to some extent.

@jsirois jsirois merged commit 70d8100 into pex-tool:master Jan 25, 2020
@jsirois jsirois deleted the issues/851 branch January 25, 2020 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve error checking does not account for environment markers.
2 participants