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

Follow PEP 425 suggestions on distribution preference. #640

Merged
merged 1 commit into from
Jan 7, 2019
Merged

Follow PEP 425 suggestions on distribution preference. #640

merged 1 commit into from
Jan 7, 2019

Conversation

zmanji
Copy link
Collaborator

@zmanji zmanji commented Dec 29, 2018

PEP 425 describes how a single interpreter on a platform can support many
tags and a single package might have many distributions to satisfy those
supported tags. It describes a tie breaking mechanism:

Sometimes there will be more than one supported built distribution for a
particular version of a package. For example, a packager could release a
package tagged cp33-abi3-linux_x86_64 that contains an optional C extension
and the same distribution tagged py3-none-any that does not. The index of the
tag in the supported tags list breaks the tie, and the package with the C
extension is installed in preference to the package without because that tag
appears first in the list.

pex lacks this tie breaking logic currently and it is the root cause of
issue #639.

To address this issue, I have enhanced the resolver's sorter to deprioritize
packages that support the 'any' platform.

@stuhood stuhood requested a review from jsirois January 4, 2019 18:56
@stuhood
Copy link

stuhood commented Jan 4, 2019

@zmanji : Looks like there is a style issue on the first shard... would you mind fixing that? Thanks!

PEP 425 describes how a single interpreter on a platform can support many
tags and a single package might have many distributions to satisfy those
supported tags [0]. It describes a tie breaking mechanism:

> Sometimes there will be more than one supported built distribution for a
> particular version of a package. For example, a packager could release a
> package tagged cp33-abi3-linux_x86_64 that contains an optional C extension
> and the same distribution tagged py3-none-any that does not. The index of the
> tag in the supported tags list breaks the tie, and the package with the C
> extension is installed in preference to the package without because that tag
> appears first in the list.

pex lacks this tie breaking logic currently and it is the root cause of
issue #639.

To address this issue, I have enhanced the resolver's sorter to deprioritize
packages that support the 'any' platform.

[0]: https://www.python.org/dev/peps/pep-0425/#id1
@zmanji
Copy link
Collaborator Author

zmanji commented Jan 4, 2019

@stuhood I believe it should pass now.

For reference the style.sh script appears to be broken on OSX and the script doesn't fail:

┌─ ~/code/pex (sort-platform-tag) (venv)                                                                                                                                                           12:39:08
└─ ◆ ./scripts/style.sh
find: ,: unknown primary or operator
┌─ ~/code/pex (sort-platform-tag) (venv)                                                                                                                                                           12:39:11
└─ ◆ echo $?
0

Copy link
Member

@jsirois jsirois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the issue, the spec and fixing @zmanji!

@classmethod
def package_platform_tag_precedence(cls, package):
# Give all non 'any' tags higher precedence
supported_tags = package.supported_tags or []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - this all looks great. The existing supported_tags property for BinaryPackage is unsuitable for complete sorting, but this is a step forward - at least platform specific packages now win over non. I've filed #642 to track fixing the rest.

@jsirois jsirois merged commit 812a34f into pex-tool:master Jan 7, 2019
@jsirois jsirois mentioned this pull request Jan 7, 2019
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.

3 participants