feat(docker): build arm64 images again (#879) #919
Conversation
|
Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️ |
|
Do i have to check all boxes in the checklist even if i have the feeling they do not apply here? And i have the feeling the build errors regarding linting and NodeJS are not related to my change? 🤔 I will check if ignoring the toolchain has an effect on the other CI builds. Also I explain why I think ignoring the toolchain instead of waiting until it is fixed in cargo-chef or working around the issue with cargo-chef not using the toolchain in the commit message. Hopefully that is the correct place. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #919 +/- ##
=======================================
Coverage 41.74% 41.74%
=======================================
Files 21 21
Lines 1670 1670
=======================================
Hits 697 697
Misses 973 973
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Nope that seems like it is comming from somewhere else. |
LtdSauce
changed the title
This commit adds the known names of the rust-toolchain files to the .dockerignore file. This has two reasons why it makes sense: - The initial docker layer already has a set up rust toolchain that is sufficient to build the project. Thus, by providing a toolchain file, the toolchain would be installed again during docker build. - Currently cargo-chef only copies the toolchain files during cooking but it gets not used during the building of the dependencies in the cook call, see LukeMathWalker/cargo-chef#271. With this in mind, currently the dependencies were actually build twice. Once with the installed toolchain from the image itself, and then in the actual cargo build call with the toolchain speciefied in the toolchain file. Building them twice resulted in timeouts when building the arm64 images as they are emulated using qemu, which is itself already slower than building natively. Now one could argue, that as soon as the mentioned issue is solved using the toolchain again would be fine. But then it would be still needed to assemble the Dockerfile in a way that the toolchain is not build twice. Because the current structure of the Dockerfile builds the toolchain once in the cargo-chef prepare step and once during the cargo build step (and would later build it during the cargo-chef cook instead of cargo build). With all this in mind using no toolchain file but instead just using the sufficient rust installation from the base image makes sense.
LtdSauce
force-pushed
the
build-arm64-images-again
branch
from
eac0881 to
cdbada7
Compare
|
As i have seen you pushed commits fixing the clippy lints and the NodeJS checks i rebased my branch on the current main branch. Now all checks should pass :) |
LtdSauce
changed the title
|
Congrats on merging your first pull request! ⛰️ |
Description
Enable the arm64 image creation again in the docker workflow.
Motivation and Context
The arm64 image builds were previously disabled because they ran into timeouts.
The cause seemed to be, that cargo-chef accidentally build the dependencies twice. And the second build managed to hit the timeout. This solves this, by no longer using the toolchain specified in rust-toolchain.toml but instead relying on the already installed rust version from the image.
This speeds up the build twice:
This should fix #879 and make #888 no longer needed.
How Has This Been Tested?
I have tested this on my fork by changing the workflow in a way that it only builds but does not push.
In this commit i removed all unnecessary workflows and made the docker workflow not try to push. The result can be seen here
Screenshots / Logs (if applicable)
Types of Changes
Checklist: