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

cross-compile container images #138

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Conversation

ereslibre
Copy link
Member

Create multi-arch container images by first cross compiling the
binaries for the different platforms, and then creating the container
images by copying the binaries.

@ereslibre ereslibre requested a review from a team December 23, 2021 13:20
@ereslibre ereslibre marked this pull request as draft December 23, 2021 14:19
@ereslibre
Copy link
Member Author

Converting to draft because of the usage of musl.cc.

@ereslibre ereslibre force-pushed the cross-compile branch 2 times, most recently from 5bbd90a to 44c55ca Compare December 24, 2021 10:17
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Cool stuff 👏

.cargo/config Show resolved Hide resolved
.dockerignore Outdated Show resolved Hide resolved
.github/workflows/container-image.yml Show resolved Hide resolved
@viccuad viccuad self-requested a review January 10, 2022 10:53
Copy link
Member

@viccuad viccuad 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 this, this is great! I'm amazed that the compilation time went down to 15 minutes.

Did we talk about performance of running code already? I have read that Musl is not as efficient as libc on mem allocation. I suppose not a problem for our usage, but maybe worth checking.

With this, we will be moving our targets from Rust Tier 1 (guaranteed to work) to Tier 2 (guaranteed to build). For x86_64 this means a reduction in assurances. I suppose it's not a problem, but something to also have in mind.

@ereslibre ereslibre force-pushed the cross-compile branch 3 times, most recently from d068e08 to 887d683 Compare January 13, 2022 09:43
@ereslibre ereslibre marked this pull request as ready for review January 13, 2022 09:44
Dockerfile Show resolved Hide resolved
Copy link
Member

@viccuad viccuad left a comment

Choose a reason for hiding this comment

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

LGTM! yay!
Do we want to cross-compile on development too?
I tried here cargo build --target x86_64-unknown-linux-musl and it failed on the ring issue, so from what I understand it's not using .cargo/config.

Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Overall great 👏

I left some comments here and there

.github/workflows/container-image.yml Outdated Show resolved Hide resolved
- run: rustup target add aarch64-unknown-linux-musl
- name: Setup musl for aarch64
run: |
curl -O https://musl.cc/aarch64-linux-musl-cross.tgz
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@flavio
Copy link
Member

flavio commented Jan 14, 2022

Do we want to cross-compile on development too? I tried here cargo build --target x86_64-unknown-linux-musl and it failed on the ring issue, so from what I understand it's not using .cargo/config.

Actually that would be nice, just to be able to reproduce bugs locally...

@ereslibre
Copy link
Member Author

Do we want to cross-compile on development too? I tried here cargo build --target x86_64-unknown-linux-musl and it failed on the ring issue, so from what I understand it's not using .cargo/config.

Actually that would be nice, just to be able to reproduce bugs locally...

I also agree. I think is a good strategy. I would push for that right after this work. We can discuss about it. My preferred mechanism would be to have this as a nix project, so our dependencies are pinned and easily built (no need to fill pre-requirements prior to build other than having nix installed).

I think is worth checking, let me know if that's good with you and if you think we should plan for that soon.

@ereslibre
Copy link
Member Author

Do we want to cross-compile on development too?

You can manually do this already, but is not pretty. Let me know if this fixes the build & link issue for you.

$ curl https://musl.cc/x86_64-linux-musl-cross.tgz | tar -xz
$ export PATH=$PWD/x86_64-linux-musl-cross/bin:$PATH
$ cargo build --target x86_64-unknown-linux-musl

Same for targeting aarch64 from x86_64:

$ curl https://musl.cc/aarch64-linux-musl-cross.tgz | tar -xz
$ export PATH=$PWD/aarch64-linux-musl-cross/bin:$PATH
$ cargo build --target aarch64-unknown-linux-musl

This should work AFAICT.

@ereslibre ereslibre force-pushed the cross-compile branch 3 times, most recently from 6bf20a8 to ff0df95 Compare January 17, 2022 13:32
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

I think there's still some work going on, am I right? This doesn't look ready to be merged

.github/workflows/container-image.yml Outdated Show resolved Hide resolved
.github/workflows/container-image.yml Outdated Show resolved Hide resolved
@ereslibre ereslibre force-pushed the cross-compile branch 4 times, most recently from 462a08f to 08078f4 Compare January 17, 2022 15:56
@ereslibre ereslibre force-pushed the cross-compile branch 2 times, most recently from e2bba6f to b2afddd Compare January 17, 2022 17:28
@ereslibre
Copy link
Member Author

I think there's still some work going on, am I right? This doesn't look ready to be merged

This is mergeable. I was testing GitHub behaviors when building the container image. Certain parts are a bit of trial and error. It's good now.

@ereslibre ereslibre requested a review from flavio January 17, 2022 18:19
@ereslibre ereslibre force-pushed the cross-compile branch 2 times, most recently from 11a3d1a to d431a64 Compare January 18, 2022 09:25
Create multi-arch container images by first cross compiling the
binaries for the different platforms, and then creating the container
images by copying the binaries.
Copy link
Member

@jvanz jvanz left a comment

Choose a reason for hiding this comment

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

LGTM! :D

Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Great, go ahead with it 👏

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.

4 participants