Skip to content

Commit e6aa703

Browse files
clintharrisonjsirois
authored andcommitted
Avoid reordering of equivalent packages from multiple fetchers (#762)
`Iterator.iter()` iterates over the set of links returned from `Crawler.crawl()`. This means the hash seed randomization potentially changes the order, when there are two packages equivalent in both version, package type (wheel, etc), platform tags, and they're both remote. Ideally, these would not be any different, so it's not really a correctness issue, but I am using pex's resolver internals to store the actual URLs used, and they flap over time between different-but-equivalent ones.
1 parent e33ccd6 commit e6aa703

File tree

3 files changed

+28
-3
lines changed

3 files changed

+28
-3
lines changed

pex/sorter.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ def package_precedence(cls, package, precedence=DEFAULT_PACKAGE_PRECEDENCE):
3737
package.version, # highest version
3838
cls.package_type_precedence(package, precedence=precedence), # type preference
3939
cls.package_platform_tag_precedence(package), # platform preference
40-
package.local) # prefer not fetching over the wire
40+
package.local, # prefer not fetching over the wire
41+
package.url, # otherwise equivalent files should deterministically sort
42+
)
4143

4244
def __init__(self, precedence=None):
4345
self._precedence = precedence or self.DEFAULT_PACKAGE_PRECEDENCE

tests/test_resolvable.py

+24-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import pex.third_party.pkg_resources as pkg_resources
77
from pex.interpreter import PythonInterpreter
88
from pex.iterator import Iterator
9-
from pex.package import Package, SourcePackage
9+
from pex.package import Package, SourcePackage, WheelPackage
1010
from pex.resolvable import (
1111
Resolvable,
1212
ResolvableDirectory,
@@ -121,3 +121,26 @@ def test_resolvable_is_constraint_getter_setter():
121121
assert req.is_constraint is False
122122
req.is_constraint = True
123123
assert req.is_constraint is True
124+
125+
126+
def test_deterministic_packages_from_multiple_crawlers():
127+
req = 'protobuf==3.9.1'
128+
options_builder = ResolverOptionsBuilder(precedence=(WheelPackage,))
129+
resolvable = ResolvableRequirement.from_string(req, options_builder)
130+
131+
pypi_source = Package.from_href('https://pypi.org/simple/protobuf/protobuf-3.9.1.tar.gz')
132+
pypi_wheel = Package.from_href(
133+
'https://pypi.org/simple/protobuf/protobuf-3.9.1-py2.py3-none-any.whl')
134+
internal_wheel = Package.from_href(
135+
'https://packages.internal.example/simple/protobuf/protobuf-3.9.1-py2.py3-none-any.whl')
136+
137+
# Multiple fetchers with equally-preferred packages should result in the same package every time,
138+
# regardless the order that crawlers returned them.
139+
mock_iterator = mock.create_autospec(Iterator, spec_set=True)
140+
mock_iterator.iter.return_value = iter([pypi_source, pypi_wheel, internal_wheel])
141+
url_order_one = [r.url for r in resolvable.compatible(mock_iterator)]
142+
143+
mock_iterator.iter.return_value = iter([pypi_source, internal_wheel, pypi_wheel])
144+
url_order_two = [r.url for r in resolvable.compatible(mock_iterator)]
145+
146+
assert url_order_one == url_order_two

tests/test_sorter.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def test_package_precedence():
2828
# overridden precedence
2929
PRECEDENCE = (EggPackage, WheelPackage)
3030
assert Sorter.package_precedence(source, PRECEDENCE) == (
31-
source.version, -1, 0, True) # unknown rank
31+
source.version, -1, 0, True, source.url) # unknown rank
3232
assert Sorter.package_precedence(whl, PRECEDENCE) > Sorter.package_precedence(
3333
source, PRECEDENCE)
3434
assert Sorter.package_precedence(egg, PRECEDENCE) > Sorter.package_precedence(

0 commit comments

Comments
 (0)