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

[bazel] Make pip wheels install reproducible #20872

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

pamaury
Copy link
Contributor

@pamaury pamaury commented Jan 18, 2024

When installing python packages using pip, we use a custom script that invokes python directly. By default, python will timestamp bytecode files hence making the builds non-reproducible which is bad for CI. Python also uses a random seed for hash. This commit sets two environment variables when running pip that force python to use a deterministic seed and to use a fixed timestamp instead of the current time.

Note: we should probably get rid of our custom fork of rules python and our custom pip install script. This would be a bigger change that can be done at a later stage.

When installing python packages using pip, we use a custom script
that invokes python directly. By default, python will timestamp
bytecode files hence making the builds non-reproducible which is
bad for CI. Python also uses a random seed for hash.
This commit sets two environment variables when running pip that
force python to use a deterministic seed and to use a fixed
timestamp instead of the current time.

Signed-off-by: Amaury Pouly <[email protected]>
Copy link
Contributor

@jwnrt jwnrt 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 making this change, I agree that we should think about merging our usage of rules_python in the future.

You've shown these vars make the python wheels reproducible in your experimental PR so I'm happy that these work as intended.

@jwnrt
Copy link
Contributor

jwnrt commented Jan 18, 2024

Kokoro has failed twice here and I'm reluctant to waive it because I think it might use this Python setup.

@pamaury do you know the procedure for debugging Kokoro errors?

The message is about the results file being missing:

https://reports.opentitan.org/prod/opentitan/github/presubmit/18185/20240118-035409/HEAD/reports/hw/ip/pinmux/lint/ascentlint/latest/report.html

IOError: [Errno 2] No such file or directory: '/container/opentitan-public/scratch/HEAD/pinmux-lint-ascentlint/default/results.hjson'

@pamaury
Copy link
Contributor Author

pamaury commented Jan 18, 2024

I don't know how kokoro works unfortunately, and we have no visibility in it. I doubt this change would affect it though because we it does not change what is built. Maybe let's wait for @timothytrippel to look into it and confirm that it is unrelated.

@jwnrt
Copy link
Contributor

jwnrt commented Jan 18, 2024

I see Kokoro has failed on master too, so I'm sure it's fine

@pamaury pamaury merged commit 9fe2120 into lowRISC:master Jan 18, 2024
31 of 32 checks passed
@timothytrippel
Copy link
Contributor

Sorry for the late reply. This LGTM. Not sure how this would impact kokoro. But we can keep an eye on it.

@jwnrt jwnrt added the CherryPick:earlgrey_es_sival This PR should be cherry-picked to earlgrey_es_sival label Jan 25, 2024
@nbdd0121 nbdd0121 added CherryPick:earlgrey_es_sival This PR should be cherry-picked to earlgrey_es_sival and removed CherryPick:earlgrey_es_sival This PR should be cherry-picked to earlgrey_es_sival labels Jan 31, 2024
Copy link

Successfully created backport PR for earlgrey_es_sival:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPick:earlgrey_es_sival This PR should be cherry-picked to earlgrey_es_sival
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants