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

Avoid reordering of equivalent packages from multiple fetchers #762

Merged

Conversation

clintharrison
Copy link
Contributor

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.

(And in case it's relevant: to be honest I tried running the integration tests locally, but they were failing before this change, and it didn't look related. Are they known to fail on macOS (10.14)?)

@clintharrison
Copy link
Contributor Author

Ok, tests are green now. @jsirois do you mind taking a look?

@jsirois
Copy link
Member

jsirois commented Sep 11, 2019

Thanks @clintharrison - LGTM!

@jsirois jsirois merged commit e6aa703 into pex-tool:master Sep 11, 2019
@jsirois jsirois mentioned this pull request Sep 11, 2019
2 tasks
@jsirois
Copy link
Member

jsirois commented Sep 11, 2019

@clintharrison re:

(And in case it's relevant: to be honest I tried running the integration tests locally, but they were failing before this change, and it didn't look related. Are they known to fail on macOS (10.14)?)

CI currently runs against macos-10.13-xcode9.4.1 so we don't have the 10.14 case covered in an automated way. I don't use OSX so I can't comment either. If you want to post a seperate issue with failures or else do that over in the pantsbuild slack channel #pex, we could dig more. This should work on OSX.

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