search

asciidoctor / docker-asciidoctor

:ship: A Docker image for using the Asciidoctor toolchain to process AsciiDoc content
https://hub.docker.com/r/asciidoctor/docker-asciidoctor/
Other
324 stars 149 forks source link

arm64 support #407

Closed spkane closed 8 months ago

spkane commented 9 months ago

This supports building container images for linux/amd64 and linux/arm64.

The Makefile targets needed to change a bit, as it is impossible to push multi-arch image manifests to the standard Docker Desktop instance. (ERROR: docker exporter does not currently support exporting manifest lists).

So, make build-load will now build for the local architecture and load the images into Docker, while the standard make build will build for all platforms and then make deploy should push the images to Docker Hub as usual.

A workaround was added to the Dockerfile to fix an issue with nokogiri and asciidoctor-epub3 on arm64. The tests appear to pass fine once this workaround is in place.

The Github workflow does not currently run tests on arm64. This would likely require the GitHub ARM64 runners or doing the work to make this work via slower emulation.

dduportal commented 8 months ago

This looks like a really nice addition, many thanks!

Sorry for the delay of answer, let's see the result but first glance at your change looks good

spkane commented 8 months ago

@dduportal It looks like the GH action failed due to an error in the script, maybe because of the version of the shell that is being used. I can try and tweak it, but it might just be faster to do on your end, since I have to wait for someone to trigger the action each time. Let me know what you would prefer.

dduportal commented 8 months ago

@dduportal It looks like the GH action failed due to an error in the script, maybe because of the version of the shell that is being used. I can try and tweak it, but it might just be faster to do on your end, since I have to wait for someone to trigger the action each time. Let me know what you would prefer.

I don't mind either. I can try in the upcoming 4 days: if no updates here after, then please proceed as I might have restricted availability until Feb.

dduportal commented 8 months ago

Hello @spkane , you have removed the permissions to push to your branch. As such, I cannot update your PR to help:

It needs to be rebased against the upstream's main branch as the base Alpine OS has been bumped to 3.19 (which has some changes)

spkane commented 8 months ago

Hello @spkane , you have removed the permissions to push to your branch. As such, I cannot update your PR to help:

Sorry about that. That should be fixed now.

spkane commented 8 months ago

Could you ensure the image is loaded? I would propose to make it an opt-out: given a buildx worker is used, then --load should be used unless contributor asks it to NOT load WDYT?

The primary issue with load is that Docker can only load a single arch and --load will fail if the image was built for multiple archs, so I think the fix here, it to build and load the local arch version for the testing, and then build all the versions, for pushing to the repo.

Eventually, this should be testing each image on each arch, but that is a whole different bag of worms to deal with.

spkane commented 8 months ago

@dduportal This appears to be working now.