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

build: Fix build reproducibility. #3712

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jsirois
Copy link

@jsirois jsirois commented Oct 9, 2024

Without going the extra length of exporting PYTHONHASHSEED=0 or
similar, the built sdist and wheel for a given version of the project
would have inconsistent random ordering of extras_require entries
between builds.

This was observed attempting to lock a VCS requirement on a fork of this
project here: pantsbuild/pants#21145
On the bright side, it led to Pex fixing its building and locking code
to be robust to this sort of (unintended) bad behavior:
pex-tool/pex#2554

Without going the extra length of exporting `PYTHONHASHSEED=0` or
similar, the built sdist and wheel for a given version of the project
would have inconsistent random ordering of extras_require entries
between builds.

This was observed attempting to lock a VCS requirement on this project
here: pantsbuild/pants#21145
On the bright side, it led to Pex fixing its building and locking code
to be robust to this sort of (unintended) bad behavior:
pex-tool/pex#2554
@@ -56,7 +56,7 @@ def load_requirements(file_list: Optional[Union[str, List[str]]] = None) -> List
tsv_reqs = load_requirements("requirements/extra-csv.in")
xlsx_reqs = load_requirements("requirements/extra-xlsx.in")

all_doc_reqs = list(
all_doc_reqs = sorted(
Copy link
Author

@jsirois jsirois Oct 9, 2024

Choose a reason for hiding this comment

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

Although I added no reproducibility test to this repo, the Pex repo now has one inspired directly by the prior list(set(...)) construction. That test failed prior to Pex plumbing PYTHONHASHSEED=0 to all process where distributions are built: https://github.com/pex-tool/pex/pull/2554/files#diff-e405a0e10bc9facaca309df6d749de036226214e787bbcbc05ca0aa2f088bb95

@jsirois
Copy link
Author

jsirois commented Oct 10, 2024

@MthwRobinson it looks like you may be most appropriate to review. The problematic use of set appears to stem directly from #986.

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.

1 participant