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

Support linux/arm64/v8 for most Ubuntu-based images #1597

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ianks
Copy link

@ianks ianks commented Dec 18, 2024

Previously, many images only provided linux/amd64 builds, which meant that Apple silicon folks were left to Rosetta or qemu emulation when cross-compiling. Although Rosetta's x86_64 emulation is a remarkable feat of engineering, nothing beats the performance of using the linux/arm64/v8 on Apple silicon.

This PR adds support to a bunch of Ubuntu-based images for linux/arm64/v8 builds. I haven't been able to build all of them yet... mostly just tested the x86_64-unknown-linux-gnu container. But I plan to squash any issues that arise from CI. Theoretically, it shouldn't be too hard since the core logic is the same across all images.

New Platforms Supported

  • aarch64-unknown-linux-gnu
  • armv5te-unknown-linux-gnueabi
  • armv7-unknown-linux-gnueabi
  • i586-unknown-linux-gnu
  • i686-unknown-linux-gnu
  • mips-unknown-linux-gnu
  • mips64-unknown-linux-gnuabi64
  • mips64el-unknown-linux-gnuabi64
  • mipsel-unknown-linux-gnu
  • powerpc-unknown-linux-gnu
  • powerpc64-unknown-linux-gnu
  • powerpc64le-unknown-linux-gnu
  • riscv64gc-unknown-linux-gnu
  • s390x-unknown-linux-gnu
  • sparc64-unknown-linux-gnu
  • thumbv7neon-unknown-linux-gnueabihf
  • x86_64-unknown-linux-gnu

@ianks ianks requested a review from a team as a code owner December 18, 2024 05:52
gfortran-aarch64-linux-gnu \
libc6-dev-arm64-cross
ENV CROSS_TOOLCHAIN_PREFIX=aarch64-linux-gnu-
ENV CROSS_SYSROOT=/usr/aarch64-linux-gnu
Copy link
Author

Choose a reason for hiding this comment

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

I decided to move these variable above apt-cross-essential.sh, since I wanted to use CROSS_TOOLCHAIN_PREFIX to test for pre-installed gfortran (which was causing some headaches).

@@ -156,7 +161,7 @@ main() {
;;
x86_64)
arch=amd64
kernel="${kversion}-amd64"
kernel="${kmajor}\.${kminor}\.${kpatch}-.*-amd64"
Copy link
Author

Choose a reason for hiding this comment

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

I don't know why, but apt was unable to find the original 5.10.0-26-amd64 package. Maybe it disappeared or something? Anyway, this fixed it and unblocked me

@@ -19,7 +19,7 @@ build_static_libffi () {
tar --strip-components=1 -xzf "v${version}.tar.gz"
./configure --prefix="$td"/lib --disable-builddir --disable-shared --enable-static
make "-j$(nproc)"
install -m 644 ./.libs/libffi.a /usr/lib64/
install -m 644 ./.libs/libffi.a /usr/local/lib/
Copy link
Author

Choose a reason for hiding this comment

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

/usr/lib64 is not a thing on arm64, so just use /usr/local/lib instead. This potentially could cause issues with folks who building of their old Pentium 4's, but I'm not sure. Either way, I'd reckon it's a small subset of the userbase anyway.

@Emilgardis Emilgardis linked an issue Dec 18, 2024 that may be closed by this pull request
2 tasks
@Emilgardis
Copy link
Member

Emilgardis commented Dec 18, 2024

This is awesome! Thank you, its all looking very good.

We have a problem though with how to publish the arm64 images, I laid out a strategy in #849 but we could also use imagetools. We'd also probably want arm64 runners, but they are only available for team/enterprise orgs :/

The current ci wont set the platform correctly, itll also overwrite the uploaded images to not be multi-arch

- name: Build Docker image
id: build-docker-image
if: steps.prepare-meta.outputs.has-image
timeout-minutes: 120
run: cargo xtask build-docker-image -v "${TARGET}${SUB:+.$SUB}" ${{ matrix.verbose && '-v' || '' }}
env:
TARGET: ${{ matrix.target }}
SUB: ${{ matrix.sub }}
LABELS: ${{ steps.docker-meta.outputs.labels }}
LATEST: ${{ needs.check.outputs.is-latest || 'false' }}
shell: bash

Ill have a look into this pr more closely

@Emilgardis
Copy link
Member

Actually, not true, it should build but would overwrite, lets do a try :D

for (platform, (target, dockerfile)) in targets

/ci try --target aarch64-unknown-linux-gnu --target x86_64-unknown-linux-gnu

This comment has been minimized.

This comment has been minimized.

@ianks
Copy link
Author

ianks commented Dec 18, 2024

@Emilgardis Ok I think I see the issue, linux-image-5.10.0-26-* is gone due to some security patch, so just bumping to 5.10.0-27 should resolve the issue. Can you try building again?

@ianks
Copy link
Author

ianks commented Dec 18, 2024

it should build but would overwrite

My (very limited) understanding was the buildx should automagically deal with pushing to the correct destination for each platform (so docker CLI can pull the right image later)... Does that sound right?

@ianks
Copy link
Author

ianks commented Dec 18, 2024

/ci try --target aarch64-unknown-linux-gnu --target x86_64-unknown-linux-gnu

@@ -424,7 +424,7 @@ EOF

# need to reinstall the removed libgcc packages, which are required for apt
if [[ "${arch}" == "${dpkg_arch}" ]]; then
apt-get install --no-install-recommends --assume-yes "${packages[@]}"
apt-get install --no-install-recommends --assume-yes "${libgcc_packages[@]}"
Copy link
Author

Choose a reason for hiding this comment

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

I stumbled upon this... which I assumed to have been a typo since packages is likely not defined (its only used for wildcard kernel image searches).

@Emilgardis
Copy link
Member

/ci try --target aarch64-unknown-linux-gnu --target x86_64-unknown-linux-gnu

This comment has been minimized.

@Emilgardis
Copy link
Member

My (very limited) understanding was the buildx should automagically deal with pushing to the correct destination for each platform (so docker CLI can pull the right image later)... Does that sound right?

That sounds right, but it doesn't mean that both the amd64 and arm64/v8 images would be available, only the last built one would be tagged with what we want.

Copy link

Try run for comment

Successful Jobs

@ianks
Copy link
Author

ianks commented Dec 19, 2024

My (very limited) understanding was the buildx should automagically deal with pushing to the correct destination for each platform (so docker CLI can pull the right image later)... Does that sound right?

That sounds right, but it doesn't mean that both the amd64 and arm64/v8 images would be available, only the last built one would be tagged with what we want.

I took a look, and I think 6428332 may do the trick. Basically, it just expands the matrix to include platform as a dimension, and passes the --platform to build-docker-image. Am I on the right track here???

@Emilgardis
Copy link
Member

Emilgardis commented Dec 19, 2024

That works, but it doesnt change anything. It doesnt solve the problem of how to consolidate the two builds into one image

@ianks
Copy link
Author

ianks commented Dec 19, 2024

That works, but it doesnt change anything. It doesnt solve the problem of how to consolidate the two builds into one image

Good point. If I'm understanding correctly, I see a couple of options for solving this:

  1. Let buildx handle it by building/pushing for all platforms in a single GHA job (--platforms linux/amd64,linux/arm64/v8). Pros: simpler to implement. Cons: slower builds.
  2. Deal with it ourselves using docker manifest create .... Pros: faster builds, more explicit. Cons: more complex orchestration of GHA jobs
  3. ... something else?

Which would you prefer???

@Emilgardis
Copy link
Member

Emilgardis commented Dec 19, 2024

I have no preference, just want something that works. I've not heard of buildx build supporting multiple platforms and then combining it, that's interesting and would be useful

@Emilgardis
Copy link
Member

Thinking about this a bit more, using the multi value --platform would work for all images, except when target=host, as we want to then use Dockerfile.native instead

@ianks
Copy link
Author

ianks commented Dec 19, 2024

Thinking about this a bit more, using the multi value --platform would work for all images, except when target=host, as we want to then use Dockerfile.native instead

Ok, I'll give the multi-platform string a whirl. The only thing I'm unsure about is if we need to supply the --push parameter for it to work. If so, that could be a nuisance because I think we want to test before actually pushing 🤔

@Emilgardis
Copy link
Member

Dont think thats needed, i only see that the docker-container driver is needed, which we should be using already. You should revert 6428332 i think

@ianks ianks changed the title Support linux/arm64/v8 for most ubuntu-based imaged Support linux/arm64/v8 for most Ubuntu-based image Dec 19, 2024
@ianks ianks changed the title Support linux/arm64/v8 for most Ubuntu-based image Support linux/arm64/v8 for most Ubuntu-based images Dec 19, 2024
@ianks
Copy link
Author

ianks commented Dec 19, 2024

Dont think thats needed, i only see that the docker-container driver is needed, which we should be using already. You should revert 6428332 i think

To clarify, you do want to use multiple platforms per job correct? Because before 6428332, we were only passing one platform (see here). If so, I can proceed with revert and work on passing multiple platforms per job afterwards.

@Emilgardis
Copy link
Member

Emilgardis commented Dec 19, 2024

To clarify, you do want to use multiple platforms per job correct? Because before 6428332, we were only passing one platform (see here). If so, I can proceed with revert and work on passing multiple platforms per job afterwards.

Interesting, that is not the intent of the code, it should build all passed platforms in one job per target in sequence. I don't think it's easy to do one job per target and platform, as we'd have to pass the built image somewhere else, however, as I mentioned here I think the most correct approach is publishing the image as <target>:<version>-<platform> and then consolidate the new <target>:<version>-* images into one image/manifest tagged <target>:<version>

@ianks ianks force-pushed the more-arm64-docker-builds branch from 22b36a4 to 6aec7b3 Compare December 27, 2024 17:12
@ianks
Copy link
Author

ianks commented Dec 27, 2024

@Emilgardis If possible, I'd like to get this PR merged as is then we can handle the CI aspect of it as a follow up if you're OK with that. I'm struggling to figure out the best way to do it and I don't want to balloon the scope of this PR any more

@Emilgardis
Copy link
Member

That's alright! The problem with merging it as is is that this will make people on aarch64 try to use an image that is not published. If you remove the changes to targets.toml and update provided_images.rs we can merge

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.

Support running natively on aarch64 / arm64 hosts / Mac M1/M2
2 participants