Fix arm64 cross compilation in docker build on amd64 #191
Conversation
WalkthroughThe Dockerfile has been updated to include the installation of the Changes
Poem
Warning Review ran into problemsProblems (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @@ -9,6 +9,7 @@ RUN apt-get update && apt-get install -y \ | |||
| git \ | |||
| sudo \ | |||
| zip \ | |||
| gcc-aarch64-linux-gnu \ | |||
There was a problem hiding this comment.
Ensure gcc-aarch64-linux-gnu is only installed when building for arm64.
To optimize the build process and avoid unnecessary installations on platforms that do not require this package, consider conditionally installing gcc-aarch64-linux-gnu based on the TARGETPLATFORM. Here's a suggested modification:
+ RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
+ apt-get install -y gcc-aarch64-linux-gnu; \
+ fiCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| gcc-aarch64-linux-gnu \ | |
| RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \ | |
| apt-get install -y gcc-aarch64-linux-gnu; \ | |
| fi |
| PLATFORM='unknown'; \ | ||
| case "${TARGETPLATFORM}" in \ | ||
| "linux/amd64") PLATFORM='linux_amd64' ;; \ | ||
| "linux/arm64") PLATFORM='linux_arm64' ;; \ | ||
| *) echo "Unsupported platform: '${TARGETPLATFORM}'" && exit 1 ;; \ | ||
| esac; \ | ||
| cd BirdNET-Go && make ${PLATFORM} |
There was a problem hiding this comment.
Remove redundancy in setting the PLATFORM variable.
The PLATFORM variable is set twice in the Dockerfile, which could lead to maintenance issues and makes the file harder to read. Consider setting this variable once in a shared stage or script to ensure consistency and reduce redundancy. Here's a suggested refactor:
- PLATFORM='unknown'; \
- case "${TARGETPLATFORM}" in \
- "linux/amd64") PLATFORM='linux_amd64' ;; \
- "linux/arm64") PLATFORM='linux_arm64' ;; \
- *) echo "Unsupported platform: '${TARGETPLATFORM}'" && exit 1 ;; \
- esac; \
- cd BirdNET-Go && make ${PLATFORM}
+ cd BirdNET-Go && make ${PLATFORM}Ensure that PLATFORM is correctly set in a previous stage or in a script that is included before this command.
Committable suggestion was skipped due low confidence.
As mentioned in v0.5.4 release discussion
There have been issues running the linux/arm64 image since PR #176 which fixed the linux/amd64 build and introduced improvements to the cross-compilation process by utilizing BUILDPLATFORM. Since this PR cross-compilation provided by the build system will be used rather than emulation in the docker build. Unfortunately, this change does require some more changes to the Dockerfile/makefile to work as expected.
Currently, the arm64 image contains the amd64 binaries. Due to how the Dockerfile calls the make file without an argument, leading to this piece of code to be executed:
which depends on this:
Since this part of the code no longer is being emulated by docker, uname -m will always return x86_64. Causing it to always be built even when --platform arm64 is requested.
This PR fixes that issue by instead calling the correct make target directly from the Dockerfile. Also had to install gcc-aarch64-linux-gnu for cross-compilation support in the dockerfile.
This time I went ahead and tried both arm64 and amd64 to be on the safe side after building with:
docker buildx build --platform=linux/arm64,linux/amd64 --tag test .