Use plain Debian images in CI #372
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eloquence
reviewed
Aug 30, 2022
gonzalo-bulnes
approved these changes
Aug 30, 2022
Currently running build-sync-wheels as root will cause issues for different, subtle reasons. First, you can't unpack idna-3.2.tar.gz as root with tar. For some reason it uses very large uids/gids. When tar is run as root, it tries to restore the original file owners, which doesn't work because those large uids/gids are bigger than an unsigned 32-bit integer. Note that repacking the upstream tarball isn't an option because we are testing reproducibility against the source tarball hosted on PyPI. I've asked upstream if they can fix this going forward at <kjd/idna#123>. The workaround for tar is to use `--no-same-owner`, so the files will end up owned by the current user, aka root:root. This allows unpacking to succeed, but for some reason, it causes very subtle reproducibility differences in the built Python wheels. I've posted diffoscope output to <https://gist.github.com/legoktm/d8a9209dbf94bb3a8939828a609fd2c4>, various metadata files have the group writable bit set when previously it wasn't. Theoretically we could rebuild all of our wheels to be in this configuration, except that's a lot of unnecessary work and it would likely mean that everyone has to build wheels as root going forward. That's risky unless you're properly using rootless containers, so it's easiest to just tell people they need to run this script as a non-root user.
For the most part this is a very straightforward switch, except for the
reprotest-wheels job.
As noted in the previous commit, we cannot run the build-sync-wheels
script as root (which the Debian images default to). We add a "ci" user,
chown the repository for them, and then run the test command. One subtle
catch is that the repository needs to be checked out in a place other
than /root, so we set the working_directory to /srv.
I also noticed that we were installing all of the 5GB of diffoscope
dependencies without actually ever using diffoscope (and if we were
using diffoscope, we don't need extra utilities to diff debs and
wheels), so using `--no-install-recommends` shaves off about 3 minutes
of runtime. We do need to manually install faketime since it's only a
recommends, but that seems like a fair tradeoff.
And we can now delete the custom `packaging-debian-{buster,bullseye}`
images we were maintaining!
legoktm
force-pushed
the
plain-debian-images
branch
from
9727077 to
29ccbe0
Compare
gonzalo-bulnes
approved these changes
Aug 31, 2022
There was a problem hiding this comment.
Nice, thanks for the explanations @legoktm (and @eloquence). I'm shipping this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I wrote detailed commit messages for these, so please read those. In summary: