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

Fix PexInfo requirements using a non-deterministic data structure #723

Merged
merged 1 commit into from
May 8, 2019

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented May 4, 2019

Problem

Even though PexInfo stores its requirements as an OrderedSet, when getting converted into a list in the dump() function, the requirements were losing their deterministic order for an unclear reason.

Specifically, the acceptance test diverged with PEX-INFO, with both files having different orders for their requirements.

# pex1
"requirements": ["six==1.12.0", "cryptography==2.6.1", "cffi!=1.11.3,>=1.8", "asn1crypto>=0.21.0", "pycparser"]

# pex2
["six==1.12.0", "cryptography==2.6.1", "asn1crypto>=0.21.0", "cffi!=1.11.3,>=1.8", "pycparser"]

Solution

Sort the requirements when being dumped to ensure we always have a consistent result.

We also make interpreter_constraints's output always be sorted, as it is internally stored as a set so by definition is not deterministic in ordering.

@Eric-Arellano Eric-Arellano requested review from stuhood and jsirois May 4, 2019 00:21
@Eric-Arellano
Copy link
Contributor Author

The meaningful commit to review is 3834a42

I do not know why requirements is not deterministic when cast into a list, as it's an OrderedSet. I tried experimenting with this and could not figure it out. All I know is that this fixes the test and that the fix to interpreter_constraints does make logical sense.

Copy link

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

I'm fine with not understanding why order is being lost internally as long as the output ends up stable. Thanks!

@Eric-Arellano Eric-Arellano changed the title WIP: Fix PexInfo requirements using a non-deterministic data structure Fix PexInfo requirements using a non-deterministic data structure May 8, 2019
@Eric-Arellano Eric-Arellano merged commit f60aa3f into pex-tool:master May 8, 2019
@Eric-Arellano Eric-Arellano deleted the reproducible-bdist branch May 8, 2019 02:32
@Eric-Arellano Eric-Arellano mentioned this pull request May 8, 2019
11 tasks
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.

2 participants