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

Use docker environment to build the actors reproducibly #1606

Merged
merged 15 commits into from
Jan 28, 2025

Conversation

Stebalien
Copy link
Member

Third attempt at fixing #171. All work on this PR was done by @lemmih in #634 and @ianconsolata in #865.

This isn't perfect (see #171 (comment)) but it does make it possible for end-users to reproduce the bundles built in CI as long as they have an x86 machine.

fixes #171

@Stebalien Stebalien requested a review from rvagg January 22, 2025 21:06
@Stebalien
Copy link
Member Author

We should get someone with a mac to reproduce this, then merge it before it gets out of date again. I've made a few changes:

  1. It pulls an image with a specific hash.
  2. I've changed how the rust binaries are installed so they layer better (only need to be updated when we change rust versions).
  3. I've removed the apt-get update etc. for better reproducibility.
  4. I'm checking out a clean copy of the repo (from the host OS, not from GitHub) instead of simply copying the repo.

This means it isn't possible to reproducibly build a dirty repo but...
nobody wants to do that anyways. It does mean that the reproducible
build won't be affected by other files in the tree.
@Stebalien Stebalien force-pushed the id/reproducible-build branch from b4d3f3a to 2e0f0e4 Compare January 22, 2025 21:10
Dockerfile Show resolved Hide resolved
@rvagg
Copy link
Member

rvagg commented Jan 23, 2025

I have opinions! See #1607

I've tested that it runs on a mac but only on an arm machine so can't get the same output, I do have an x86 mac in the house but it's a bit of an annoyance to get it set up for this, and those are going extinct now anyway.

I'm getting this when built using my branch, I think it should be the same as this one so here it is:

$ sha256sum output/builtin-actors.car 
df4957e13f356d9307bb86e345f6a4674ec671c85c5a26ffb995e8828819243e  output/builtin-actors.car

@Stebalien
Copy link
Member Author

I'm getting the same value.

In terms of macos, maybe we should just add --arch amd64 to the run commandline? Or --platform linux/amd64?

@rvagg
Copy link
Member

rvagg commented Jan 28, 2025

In terms of macos, maybe we should just add --arch amd64 to the run commandline? Or --platform linux/amd64?

Attempted but didn't get all the way; the furthest I could get was using buildx but then it failed on compiling num-traits:

  cargo:warning="  = note: cc: internal compiler error: Illegal instruction signal terminated program collect2"
  cargo:warning="          Please submit a full bug report, with preprocessed source (by using -freport-bug)."
  cargo:warning="          See <file:///usr/share/doc/gcc-12/README.Bugs> for instructions."
  cargo:warning="          "
  cargo:warning=""
  cargo:warning="error: could not compile `num-traits` (build script) due to 1 previous error"
  cargo:warning="warning: build failed, waiting for other jobs to finish..."

@rvagg
Copy link
Member

rvagg commented Jan 28, 2025

no, wait, it worked with a few tweaks!

on arm64 mac:

$ sha256sum output/builtin-actors.car
df4957e13f356d9307bb86e345f6a4674ec671c85c5a26ffb995e8828819243e  output/builtin-actors.car

needs buildx but I think that might be pretty standard on a docker install now ..? at least I don't recall ever manually installing it, and I certainly didn't on the mac which I've only ever used docker desktop on. Included in the latest commit in #1607

* exclude `target` (mine was >40G when I first ran this, not needed for a container build)
* extract common variables in Makefile
* restore missing `all-bundles` target
* make `.PHONY` readable
@Stebalien
Copy link
Member Author

Stebalien commented Jan 28, 2025

I'm having a lot of trouble with permission denied errors given the UID changes creating/chowning a target directory gets part of the way there but the real issue is that the output dir is mounted as a volume so I can't overwrite the bundle.

I believe this is a "rootless" versus "root" container setup issue:

  1. I use podman (and have used rootless docker in the past) because running docker as root grants all users in the docker group "passwordless sudo".
  2. When running in rootless mode (docker or otherwise), the outer user's ID gets mapped to 0 in the container, and the non-zero UIDs in the container get mapped to a per-user ID range. See https://man.archlinux.org/man/subuid.5.

What this means is, if you have a "rootless container" setup:

  1. If you mount a volume owned by the user outside the container into the container, that volume will be owned by root (causing the permissions issues I ran into).
  2. If you write a file as root inside the container, that file will be owned by the user outside the container (which is why the original version worked for me).
  3. If you create a user with a UID X inside the container, files created by this user inside the container will be owned by X+offset where the offset is specified by the user's "subuid" range as defined in /etc/subuid.

This works with both rootless and root docker/podman modes. Otherwise,
when running in rootless mode, the outer user's UID gets mapped to a
temporary UID on the outside, leading to permissions issues when writing
to the output directory.
@Stebalien
Copy link
Member Author

I've changed it to:

  1. Run as root.
  2. Copy the resulting file into an /output volume.
  3. Change the ownership of the output file to match the owner of that volume.

I'm hoping this'll work for your case. I'm not 100% sure the owner of said volume will be correct. If not, I may need to look at the output/.keep file and use that to determine the correct UID. The important part is that, by not passing UIDs explicitly but instead inferring them from file ownership, we don't have to think too hard about UID mapping.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

yep, works well, permissions issues solved too 👌

@Stebalien Stebalien added this pull request to the merge queue Jan 28, 2025
Merged via the queue into master with commit 5aad41b Jan 28, 2025
11 checks passed
@Stebalien Stebalien deleted the id/reproducible-build branch January 28, 2025 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎉 Done
Development

Successfully merging this pull request may close these issues.

Reproducable Build
4 participants