-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
feat/add arm64 support #613
Conversation
Thanks! What's the debian-based final container size compared to alpine? |
On |
We need to check the size diff for i64 |
Not sure I quite got this. Are we talking about potential size differences of signed integers or the image size for x86 with the Debian based image? |
lol, no, i meant x86-ia64 (amd64) |
also, you may want to rebase on #614 |
Ok, i just checked. When running
with the proposed docker image (same cargo patch), the result is
I don't think growing the image size by over 3x just to make mac docker images is a good proposition because the vast majority of these images are used on linux servers (this might change in the future). |
The size may be 3x difference, but numerically, is 100MB ever a dealbreaker over 30MB for users where you can assume their average image size is > 300MB? (We use martin and our images are way above that usually) The other thing is, this PR is not about fixing Mac images (in fact there are no such thing as Mac images, they're all If you try to build the old image on |
Can do. |
7af0101
to
64146d2
Compare
@nyurik I was able to cut 10MB by stripping the symbols release binary. |
Sorry, I meant arm64 for linux. I don't think shaving the 3MB and loosing all symbols is a good path. I would much rather have proper stack traces with symbols if something happens, rather than having hard-to-parse stacktraces -- those 3MB actually add value. What I am concerned with is that we want to add 70MB to an image without any benefit to the existing user base. 70MB might not sound like a lot, but this is a waste that I would rather avoid unless absolutely must have. I propose we figure out how to build with musl/libc on arm64 - it shouldn't be THAT difficult I would think. If we can't, lets just create a new |
Regardless - I don't think that's the issue - there seem to be some issue with dockerx and manifests - and that's the same thing I saw before with @stepankuzmin 's PR |
That is a known issue - docker/buildx#59 It can be remedied by building the ARM image as a separate step. I could work on that, as long as there is some guarantee that the working solution would be merged. Wouldn't want to work on something that maintainers don't want to go forward with. |
That I'm sure we will do, just possibly as arm64 specific docker file so to keep the image size low for others |
Got it, then I think I should go forward with arm64 specific Dockerfile and leave the current one as it is then. |
e4b7251
to
481eee6
Compare
Signed-off-by: Ismayil Mirzali <ismayilmirzeli@gmail.com>
481eee6
to
60bef90
Compare
@nyurik Seems to build fine, although slow, which is expected with emulation. For the time being, we can't run the DB tests on the ARM image yet because of unavailability of an ARM image for PostGIS. I'm looking into submitting a patch for that down the line. I think for the time being, we can assume that the DB tests that passed on x86 would also pass on arm64. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great, thanks!
@Volatus or @stepankuzmin could you verify that the arm64 image is published properly under the |
@nyurik |
that's not good :( can it be that the publish step needs to be modified as well somehow, to preserve the platform? |
Yep, should fix the manifest
|
Maybe try adding the |
Hmm, probably. I pushed it to main (no point in going via PRs because PRs don't trigger that step anyway)... will see. |
Hmm, it seems the This is not good for several reasons - most importantly because it seems it uses a different dockerfile in that step... Any thoughts of why that might be? |
@Volatus @stepankuzmin please retest - i think i fixed it, but no idea if the metadata is correct |
Weirdly enough, there are two architectures listed: arm64 and unknown. Should the x86-amd64 specify its metadata in some other way? https://github.com/maplibre/martin/pkgs/container/martin/81143769?tag=main |
And one more interesting tidbit: the image tagged as
|
I did receive a reply on stackoverflow about this https://stackoverflow.com/questions/75872905/building-both-amd64-and-arm64-with-github-workflows-from-different-files/75873049#75873049 |
I think if both of them have their own manifest step, it should end up working. |
@nyurik It's surely not the case the issue is due to order right? martin/.github/workflows/docker.yml Lines 21 to 25 in eae422a
martin/.github/workflows/docker.yml Lines 42 to 43 in eae422a
|
@Volatus definitly not because i added the QEMU configuration after I saw the issue. Before this, QEMU was configured with |
This reverts commit c358ec5, as well as any other related changes to the docker github action. It is clearly not working, while also not allowing us to build proper releases quicker. Several ways to fix: * Use 3rd party CI service to just build multi-platform docker images * Use cross-compilation wiht github actions * (???)
This reverts commit c358ec5, as well as any other related changes to the docker github action. It is clearly not working, while also not allowing us to build proper releases quicker. Several ways to fix: * Use 3rd party CI service to just build multi-platform docker images * Use cross-compilation wiht github actions * (???)
This reverts commit c358ec5, as well as any other related changes to the docker github action. It is clearly not working, while also not allowing us to build proper releases quicker. Several ways to fix: * Use 3rd party CI service to just build multi-platform docker images * Use cross-compilation wiht github actions * (???)
This reverts commit c358ec5, as well as any other related changes to the docker github action. It is clearly not working, while also not allowing us to build proper releases quicker. Several ways to fix: * Use 3rd party CI service to just build multi-platform docker images * Use cross-compilation wiht github actions * (???) See also #655, #603 and #505
This PR adds the Dockerfile to make
linux/arm64
anddarwin/arm64
builds easier/possible.closes #603
closes #505