-
Notifications
You must be signed in to change notification settings - Fork 12
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
Makes wheel builds reproducible, with tests #211
Conversation
Two changes to wheel build process: 1. Setting SOURCE_DATE_EPOCH env var 2. Setting a hardcoded dir path for all wheels builds That seems to do the trick. The reprotest target in the Makefile, used for debugging locally, has a platform discrepancy, building for i686 rather than x86_64.
We need to build only for x86_64 arch or else the wheel names will not match.
Using pytest with the same options as what's been confirmed locally. Tested with pip v20.1. With newer versions, specifically >=20.3, the repro check fails, due to lack of "--build" flag support.
@kushaldas Thanks for adding the platform fix! I cobbled together a pytest file to run reprotest. It works for me locally, although we're bumping up against a deprecation warning—if your |
Now
|
Drat, I did not take a look at the CI failures as I'd hoped to today, @kushaldas. Please continue to experiment and see if you can get it passing! |
I think it is some restrictions via removed capability in circleci, as I can do this on my Debian Buster system:
|
I can manually do
But, the
|
fcebf80
to
86a5928
Compare
setarch is not permittied in the docker container. Because CircleCI only supports Ubuntu VM targets [1], we must add the Bionic sources to install dh-virtualenv under Focal as no installation candidate exists for that distribution in upstream repos. [1] : https://circleci.com/docs/2.0/configuration-reference/#available-machine-images
86a5928
to
e293606
Compare
@kushaldas I've appended a commit here to use the CircleCI machine executor (a VM) instead of a docker container. It appears the setarch is restricted inside the container. With these changes, it appears to reprotests are now passing in CI. |
After @conorsch marks this as "Ready for review", we can mark it as "Approved" and merge. Next step on the packaging front:
|
"securedrop-client", | ||
"securedrop-log", | ||
"securedrop-proxy", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other repositories for which we build wheels, that should be added to this list?
Thanks, @kushaldas & @emkll. I've pushed one more commit, to enable reproducibility tests for "securedrop-log" and "securedrop-proxy" packages. Marking ready for review! |
Please open an issues to track these follow-ups! I see two (2): 1) rebuilding the wheels post-merge of reproducibility added here; and 2) removing the website index, referencing only the local files, as you describe. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with the steps mentioned in the PR. All worked as expected. Approved.
Overview
Refs #196
Makes two changes to our existing wheel build process:
With those changes, the
./scripts/build-sync-wheels
script builds wheels that are byte-for-byte reproducible. I also included a test-only Makefile target that was handy during development. It's not yet ready for CI, because I'm seeing the platform vary in the reprotest build environment. More input appreciated if anyone can resolve that problem, see initial report in #196 (comment)Testing
First, it's helpful to observe the wheel checksums changing over multiple builds, to understand the problem we're trying to solve.
git rm localwheels/*.whl
and commit../build-sync-wheels -p ../securedrop-client
, updating the dirpath as necessary. Commit the results.git rm localwheels/*.whl
, but don't commit the results../build-sync-wheels -p ../securedrop-client
again, and commit the results.git show --name-status
and confirm that many of thelocalwheels/*.whl
files changed.That's a convoluted workflow because the build-sync-wheels script will politely avoid overwriting wheel files that already exist. In this PR, a
--clobber
flag is added to the script to force overwrites, making comparison easier.OK, now let's verify the new behavior!
localwheels/*.whl
files are identical to those on the main branch../build-sync-wheels --clobber -p ../securedrop-client
, updating the dirpath as necessary. Commit the results../build-sync-wheels --clobber -p ../securedrop-client
againgit status
, confirm that there are no local modifications. The wheels were fetched, but identical.That's about it. Eventually, I'd love to drop the requirement that we store the wheels in version control, separately from the deb packages that contain them. But the work here is a step in that direction: once we have reproducible wheels, we can work on skipping artifact storage as long as the final output is deterministic.