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

Make + Docker: Implement arm64 & amd64 native images #13

Merged
merged 3 commits into from
Oct 3, 2022

Conversation

markmandel
Copy link
Contributor

@markmandel markmandel commented Sep 30, 2022

This PR updates the Docker image such that it can be successfully built and used as either a linux/amd64 image or a linux/amd64 image.

The Makefile has been given a new buildx command to utilise Docker's multi-platorm building capability to build and push (!!!) both images.

This is separate from the build command as since one of the arch's will be compiled under emulation, and therefore buildx can take a long time to complete (took ~2h on my 8 core old gaming laptop).

Some other things added along the way:

  • Makefile take REPOSITORY and TAG arguments, such that building local images and testing is easier to do.

Out of scope:

  • Haven't integrated it with GitHub actions, as I'm not that familiar with it, but figured this was a good first step.

Known issues:

  • For arm64 images, can't get linux-musl test in make test to pass. Wondering if we need to explicitly install the amd64 version via apt? Have yet to experiment. Cross compiling to x86_64-apple-darwin works perfectly though on arm64 host.

Closes #12

This PR updates the Docker image such that it can be
successfully built and used as either a linux/amd64 image
or a linux/amd64 image.

The Makefile has been given a new `buildx` command to utilise
Docker's multi-platorm building capability to build and push (!!!)
both images.

This is separate from the `build` command as since one of the arch's
will be compiled under emulation, and therefore `buildx` can take a
long time to complete (took ~2h on my 8 core old gaming laptop).

Some other things added along the way:

* Makefile take REPOSITORY and TAG arguments, such that
  building local images and testing is easier to do.

Out of scope:

* Haven't integrated it with GitHub actions, as I'm not that
  familiar with it, but figured this was a good first step.

Known issues:

* For arm64 images, can't get linux-musl test in `make test` to pass.
  Wondering if we need to explicitly install the amd64 version via apt?
  Have yet to experiment. Cross compiling to `x86_64-apple-darwin`
  works perfectly though on arm64 host.

Work on joseluisq#12
Makefile Show resolved Hide resolved
joseluisq
joseluisq previously approved these changes Sep 30, 2022
Copy link
Owner

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

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

Looks just fine to me.
Just to let you know that I didn't resist giving this a try so I just built the multi-arch images via your make buildx two hours ago on a borrowed M1 and works like a charm. Of course, took 2.58h long because of qemu for the amd64 variant. 😅

I already approved the CI check to let it run in the next few minutes at least for the amd64 variant.
However, I will definitely help to incorporate the multi-arch support on Github Actions in the next few days (or if you are curious, feel free to do it, just let me know).

And let me know if a new image tag on DockerHub is needed so I could prepare one tag on the weekend.

Congrats and thanks! 👍

@joseluisq joseluisq added enhancement New feature or request arm64 labels Sep 30, 2022
@joseluisq
Copy link
Owner

  • For arm64 images, can't get linux-musl test in make test to pass. Wondering if we need to explicitly install the amd64 version via apt? Have yet to experiment.

Oh, yes I forgot that. you're right. This is something that we have to figure out and probably we need to install it.

@markmandel
Copy link
Contributor Author

Just to let you know that I didn't resist giving this a try so I just built the multi-arch images via your make buildx two hours ago on a borrowed M1 and works like a charm. Of course, took 2.58h long because of qemu for the amd64 variant. sweat_smile

Oh that's super interesting! For some reason on linux/arm64 host, I couldn't get arm64 to compile (but all worked fine on linux/amd64 🤷🏻‍♂️ weird host arch compiler issues)

However, I will definitely help to incorporate the multi-arch support on Github Actions in the next few days (or if you are curious, feel free to do it, just let me know).

I have 3% experience with Github actions, so I'm probably not the person to be playing with it - more than happy to defer to you on that one.

And let me know if a new image tag on DockerHub is needed so I could prepare one tag on the weekend.

We're not in a huge rush, happy to use it when it's ready. Also happy to have it for 1.64.0 and later -- gives us an incentive to move up our currently Rust tooling to the latest release as well.

Oh, yes I forgot that. you're right. [Muscl Test[ is something that we have to figure out and probably we need to install it.

I'm not super familiar with musl, but does it make sense to potentially change the test itself, so test a aarch64-unknown-linux-musl test instead of x86_64? 🤔

@markmandel
Copy link
Contributor Author

oooh, I wonder if it's this bit in the Dockerfile?

ENV X86_64_UNKNOWN_LINUX_MUSL_OPENSSL_DIR=/usr/local/musl/ \
    X86_64_UNKNOWN_LINUX_MUSL_OPENSSL_STATIC=1 \
    PQ_LIB_STATIC_X86_64_UNKNOWN_LINUX_MUSL=1 \
    PG_CONFIG_X86_64_UNKNOWN_LINUX_GNU=/usr/bin/pg_config \
    PKG_CONFIG_ALLOW_CROSS=true \
    PKG_CONFIG_ALL_STATIC=true \
    LIBZ_SYS_STATIC=1 \
    TARGET=musl

With all it's X86 parts?

@joseluisq
Copy link
Owner

joseluisq commented Sep 30, 2022

I'm not super familiar with musl, but does it make sense to potentially change the test itself, so test a aarch64-unknown-linux-musl test instead of x86_64?

Probably, I will do play around with this on the M1

oooh, I wonder if it's this bit in the Dockerfile?

ENV X86_64_UNKNOWN_LINUX_MUSL_OPENSSL_DIR=/usr/local/musl/ \
    X86_64_UNKNOWN_LINUX_MUSL_OPENSSL_STATIC=1 \
    PQ_LIB_STATIC_X86_64_UNKNOWN_LINUX_MUSL=1 \
    PG_CONFIG_X86_64_UNKNOWN_LINUX_GNU=/usr/bin/pg_config \
    PKG_CONFIG_ALLOW_CROSS=true \
    PKG_CONFIG_ALL_STATIC=true \
    LIBZ_SYS_STATIC=1 \
    TARGET=musl

With all it's X86 parts?

Yes, you go it. openssl and libpq

@markmandel
Copy link
Contributor Author

Wondering if we need to explicitly install the amd64 version via apt?

I was wondering if musl had different apt installs for different architectures like g++ does, but doesn't look like it, so I think that's not the right way to go.

@joseluisq
Copy link
Owner

joseluisq commented Sep 30, 2022

For example, with X86_64_UNKNOWN_LINUX_MUSL_OPENSSL_STATIC we will get static linking via musl but I think this is a second concern.
For now, I think the PR could try to focus on getting ARM64 working and also to have a test for it. So it could mean to have another make task E.g make test-ci-arm64 that could run when the image is built on an ARM64. What do you think?

@markmandel
Copy link
Contributor Author

Updates to the PR incoming, just doing final tests - I've got make test-ci using uname -m to run the appropriate tests for whichever architecture it finds itself running inside.

@joseluisq joseluisq self-requested a review September 30, 2022 22:48
@joseluisq joseluisq dismissed their stale review September 30, 2022 22:54

We need a second try

@markmandel
Copy link
Contributor Author

Okay! Here we go again 😃

@markmandel
Copy link
Contributor Author

Oh, and to be clear - make test is now passing on both arm64 and amd64!

@markmandel
Copy link
Contributor Author

Thanks for the help along the way - going to pop off for my weekend, but will keep an eye on the PR in case you see anything that needs changing. 😃

@joseluisq
Copy link
Owner

My quick tests on the M1 (first commit)

$ uname -m
# aarch64

This works

$ cargo build --release --target aarch64-unknown-linux-gnu
#   Compiling helloworld v0.0.0 (/root/src)
#    Finished release [optimized] target(s) in 4.41s

This works

$ cargo build --release --target x86_64-apple-darwin
#    Finished release [optimized] target(s) in 0.03s

This fails, but let's skip it for now

$ cargo build --release --target x86_64-unknown-linux-musl

Compiling helloworld v0.0.0 (/root/src)
error: linking with `cc` failed: exit status: 1
  |
  = note: "cc" "-m64" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/rcrt1.o" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/crti.o" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/crtbeginS.o" "/tmp/rustcp2F9Pm/symbols.o" "/root/src/target/x86_64-unknown-linux-musl/release/deps/helloworld-b450e76bc9291ecb.helloworld.d03fdc5a-cgu.0.rcgu.o" "-Wl,--as-needed" "-L" "/root/src/target/x86_64-unknown-linux-musl/release/deps" "-L" "/root/src/target/release/deps" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib" "-Wl,--start-group" "-Wl,-Bstatic" "-lunwind" "-lc" "-Wl,--end-group" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/libcompiler_builtins-7c4df512e1aa59e0.rlib" "-Wl,-Bdynamic" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-nostartfiles" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained" "-o" "/root/src/target/x86_64-unknown-linux-musl/release/deps/helloworld-b450e76bc9291ecb" "-Wl,--gc-sections" "-static-pie" "-Wl,-zrelro,-znow" "-Wl,-O1" "-nodefaultlibs" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/crtendS.o" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/crtn.o"
  = note: cc: error: unrecognized command-line option '-m64'

This fails, but we can also skip it for now

$ cargo build --release --target x86_64-unknown-linux-gnu
   Compiling helloworld v0.0.0 (/root/src)
error[E0463]: can't find crate for `std`
  |
  = note: the `x86_64-unknown-linux-gnu` target may not be installed
  = help: consider downloading the target with `rustup target add x86_64-unknown-linux-gnu`

error: cannot find macro `println` in this scope
 --> src/main.rs:2:5
  |
2 |     println!("Hello World!");
  |     ^^^^^^^

error: requires `sized` lang_item

For more information about this error, try `rustc --explain E0463`.
error: could not compile `helloworld` due to 3 previous errors

$ rustup target add x86_64-unknown-linux-gnu

info: downloading component 'rust-std' for 'x86_64-unknown-linux-gnu'
info: installing component 'rust-std' for 'x86_64-unknown-linux-gnu'
 27.4 MiB /  27.4 MiB (100 %)  16.0 MiB/s in  1s ETA:  0s
root@72bc3d79e842:~/src# cargo build --release --target x86_64-unknown-linux-gnu
   Compiling helloworld v0.0.0 (/root/src)
error: linking with `cc` failed: exit status: 1
  |
  = note: "cc" "-m64" "/tmp/rustcMPM4Nw/symbols.o" "/root/src/target/x86_64-unknown-linux-gnu/release/deps/helloworld-d3bd27b22f1481d0.helloworld.2582f9b3-cgu.0.rcgu.o" "-Wl,--as-needed" "-L" "/root/src/target/x86_64-unknown-linux-gnu/release/deps" "-L" "/root/src/target/release/deps" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,--start-group" "-Wl,--end-group" "-Wl,-Bstatic" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-ff283b4bf550fa1c.rlib" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/root/src/target/x86_64-unknown-linux-gnu/release/deps/helloworld-d3bd27b22f1481d0" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro,-znow" "-Wl,-O1" "-nodefaultlibs"
  = note: cc: error: unrecognized command-line option '-m64'

error: could not compile `helloworld` due to previous error

This also fails, but we can also skip it for now

$ rustup target add aarch64-apple-darwin
info: downloading component 'rust-std' for 'aarch64-apple-darwin'
info: installing component 'rust-std' for 'aarch64-apple-darwin'
 25.6 MiB /  25.6 MiB (100 %)  16.3 MiB/s in  1s ETA:  0s

$ cargo build --release --target aarch64-apple-darwin
   Compiling helloworld v0.0.0 (/root/src)
error: linking with `cc` failed: exit status: 1
  |
  = note: "cc" "-arch" "arm64" "/tmp/rustcgpOzMH/symbols.o" "/root/src/target/aarch64-apple-darwin/release/deps/helloworld-d73a75c155ee5f61.helloworld.20cef0d7-cgu.2.rcgu.o" "-L" "/root/src/target/aarch64-apple-darwin/release/deps" "-L" "/root/src/target/release/deps" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/aarch64-apple-darwin/lib" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/aarch64-apple-darwin/lib/libcompiler_builtins-931e7dc6a4f959e6.rlib" "-lSystem" "-lresolv" "-lc" "-lm" "-liconv" "-L" "/root/.rustup/toolchains/stable-aarch64-unknown-linux-gnu/lib/rustlib/aarch64-apple-darwin/lib" "-o" "/root/src/target/aarch64-apple-darwin/release/deps/helloworld-d73a75c155ee5f61" "-Wl,-dead_strip" "-nodefaultlibs"
  = note: cc: error: arm64: No such file or directory
          cc: error: unrecognized command-line option '-arch'; did you mean '-march='?

error: could not compile `helloworld` due to previous error

Also noting that some of them like aarch64-apple-darwin could need a target specification with the linkers like

[target.x86_64-apple-darwin]
linker = "x86_64-apple-darwin21.4-clang"
ar = "x86_64-apple-darwin21.4-ar"

@joseluisq
Copy link
Owner

Thanks for the help along the way - going to pop off for my weekend, but will keep an eye on the PR in case you see anything that needs changing. 😃

No worries, here in the EU is already Saturday so off for my weekend too. Let's resume the stuff during the next days. Nice weekend!

@joseluisq
Copy link
Owner

joseluisq commented Oct 2, 2022

Last commit tests (cargo build --release --target <TARGET>)

on ARM64 (M1)

  • aarch64-unknown-linux-gnu [works]
  • aarch64-unknown-linux-musl [works]
  • aarch64-apple-darwin [works]
  • x86_64-apple-darwin [works]
  • x86_64-unknown-linux-gnu [doesn't work] - "ok" cause' the host is an arm64 machine so we're interested in arm64 builds
  • x86_64-unknown-linux-musl [doesn't work] - "ok" cause' the host is an arm64 machine so we're interested in arm64 builds

On x86_64 (Intel)

  • x86_64-unknown-linux-gnu [works]
  • x86_64-unknown-linux-musl [works]
  • x86_64-apple-darwin [works]
  • aarch64-apple-darwin [doesn't work] - not out-the-box but it can work just by adding another entry on the cargo-config.toml specifying the linker, see dockerfile-refactoring branch
  • aarch64-unknown-linux-gnu [doesn't work] - but it can, see dockerfile-refactoring branch
  • aarch64-unknown-linux-musl [doesn't work] - but it can, see dockerfile-refactoring branch

I created this dockerfile-refactoring branch to improve the Dockerfile a bit and test the other ARM64 targets but in x86_64.
Feel free to continue with your own work here. Once it is merged, I could improve the Dockerfile as the next step with the other branch changes.

@markmandel
Copy link
Contributor Author

markmandel commented Oct 3, 2022

So a few things I added, unless there's anything you think is hugely outstanding, I think this is good to merge:

  • Inbuilt support for aarch64-apple-darwin - since *-apple-darwin` is my primary criteria (I can xcompile for other things relatively easily), I figured that would be a good stopping point for the PR
  • Added aarch64-apple-darwin to make test so there was coverage of the platform as part of ongoing possible contributions.

Sound good?

I figure then you could take over refactoring on the Dockerfile, without worrying about me stepping on your work 😄

@markmandel
Copy link
Contributor Author

Oh, a note I was going to make - linux/arm64 could become a CI platform, since ARM does have good performance for things like compilation etc -- so having aarch64-apple-darwin working on a arm64 host is definitely still useful 👍🏻

@joseluisq
Copy link
Owner

Sound good?

I figure then you could take over refactoring on the Dockerfile, without worrying about me stepping on your work smile

Absolutely, It's fine since the PR is caring about native arm64 support.

@joseluisq
Copy link
Owner

Oh, a note I was going to make - linux/arm64 could become a CI platform, since ARM does have good performance for things like compilation etc -- so having aarch64-apple-darwin working on a arm64 host is definitely still useful 👍🏻

Yes, linux/arm64 on CI should make sense now after this PR. I will definitely improve the Dockerfile and the CI workflows.

Copy link
Owner

@joseluisq joseluisq 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 the nice addition 👍

@joseluisq joseluisq merged commit 9ea2f74 into joseluisq:master Oct 3, 2022
@markmandel
Copy link
Contributor Author

Thank you for accepting!

@joseluisq
Copy link
Owner

Oh, a note I was going to make - linux/arm64 could become a CI platform, since ARM does have good performance for things like compilation etc -- so having aarch64-apple-darwin working on a arm64 host is definitely still useful 👍🏻

Yes, linux/arm64 on CI should make sense now after this PR. I will definitely improve the Dockerfile and the CI workflows.

Just for your information about linux/arm64 on CI. #16 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm64 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Native linux/arm64 image?
2 participants