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

feat: arm64 support #87

Merged
merged 6 commits into from
Mar 31, 2022
Merged

Conversation

strophy
Copy link
Collaborator

@strophy strophy commented Mar 22, 2022

Summary

Closes #81 - this PR is not yet functional due to a bug in the Swift builder we are working to resolve over at the Swift forum here. I decided to open a draft PR to indicate that this work is mostly complete and to ask if you have any ideas how to debug this last issue building protoc for Swift.

Changes

  • Refactor Dockerfile and build.sh to be architecture-agnostic, allowing builds targetting linux/amd64 and linux/arm64 platforms.
  • Refactor GitHub Actions CI to use Docker actions for existing shell logic (necessary because building and directly pushing a multiarch image using buildx is easier and allows caching)
  • Move dependencies into deps.list to be agnostic of build system (i.e. CI or build.sh)
  • Bump Dart version (necessary to replace dart2native and build arm binaries
  • Bump Swift version (necessary for build image with arm support)

Notes for Reviewers

Please do not merge yet! The docker_meta CI step still references my Docker Hub repo, and CI does not yet complete successfully due to the bug described above.

Release Notes

  • Added arm64 support

@unkeep
Copy link

unkeep commented Mar 22, 2022

tested the image on M1 pro (generated Go, openapi) - everything works!
@rvolosatovs could we merge/tag it asap?

Copy link
Owner

@rvolosatovs rvolosatovs left a comment

Choose a reason for hiding this comment

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

Great work, thank you for contribution!

Looking forward to merging this once the issue is resolved.

I went ahead and pushed a little refactor commit for build.sh script to simplify it a bit, hope you don't mind (12f6f73)

.github/workflows/dockerimage.yml Show resolved Hide resolved
.github/workflows/dockerimage.yml Outdated Show resolved Hide resolved
@rvolosatovs
Copy link
Owner

tested the image on M1 pro (generated Go, openapi) - everything works! @rvolosatovs could we merge/tag it asap?

I think we should wait until all the issues are fixed before merging this PR, but perhaps we can extract some parts of this into a separate PR, merge and open an issue about Swift (maybe disable Swift on arm64 if it's broken?) ? This way we could close #81 albeit without Swift support for arm64
Note that there were reports of Swift build being already already problematic #77

@strophy
Copy link
Collaborator Author

strophy commented Mar 23, 2022

Splitting out languages that are currently failing makes sense to me, the guy on the Swift forum has asked me to open a bug on their compiler repo so it could take some time to fix. I've hand-built a multiarch image based on two natively built errors for now, available at strophy/protoc on Docker Hub. This image fails for me on ARM when trying to build for Java, it returns a Signal 11 when trying to execute using the Java plugin. The package I am trying to build is here. I suspect the Java plugin is being built or linked incorrectly by the g++ command here, how can I test this or try to debug? No errors are emitted when building the Java plugin...

@strophy
Copy link
Collaborator Author

strophy commented Mar 25, 2022

I had another good play with this today and cannot make progress on successfully using the image to build. The grpc-java plugin builds successfully (build log here) but when executing the below command it always fails like this:

exec protoc -I/usr/include --proto_path=/home/ubuntu/platform/packages/dapi-grpc/protos/core/v0 -I=/home/ubuntu/platform/packages/dapi-grpc/protos/core/v0 --grpc-java_out=/home/ubuntu/platform/packages/dapi-grpc/clients/core/v0/java --plugin=protoc-gen-grpc=/usr/bin/protoc-gen-grpc-java core.proto --grpc-java_out: protoc-gen-grpc-java: Plugin killed by signal 11.

I'm unable to find documentation on the exact commands to build this plugin without gradle. The build of the grpc-java repo works fine in gradle, including the plugin, but I'm not sure where to find the output artifact since I am also not familiar with gradle, so I cannot test if building this way works.

@strophy
Copy link
Collaborator Author

strophy commented Mar 29, 2022

So, after 2 more days I finally realised that the problem is not the compiler but the use of UPX that is causing the compiled plugin to crash. I have excluded a couple of plugins that failed for me when executed. UPX has a few issues open regarding compressed ARM binaries crashing, so keep this in mind for the future - this tool might be more trouble than it is worth.

I also added code to omit building Swift on ARM architecture - commits to tidy up my horrible bash skills are welcome! Everything now builds perfectly for me, but the protoc_builder is extremely slow in QEMU. Solutions to this are to implement cache mounts e.g. --mount=type=cache,... or to modify the Dockerfile further to cross-compile for ARM as described here and demonstrated here. I prefer the second solution, but I'm in way over my head here and could use some help from someone who understands compilers.

@strophy strophy marked this pull request as ready for review March 30, 2022 03:21
@strophy
Copy link
Collaborator Author

strophy commented Mar 30, 2022

Actually maybe we should merge this and add cross-compiling in a separate PR. For the moment I am having a lot of success building native x86_64 images locally and native arm64 images on AWS Graviton2 servers, then merging the two native builds together with docker manifest create.

@rvolosatovs
Copy link
Owner

So, after 2 more days I finally realised that the problem is not the compiler but the use of UPX that is causing the compiled plugin to crash. I have excluded a couple of plugins that failed for me when executed. UPX has a few issues open regarding compressed ARM binaries crashing, so keep this in mind for the future - this tool might be more trouble than it is worth.

We could also skip UPX on arm64 perhaps?

strophy and others added 6 commits March 31, 2022 11:52
- Introduce `deps.list`
- Rely on `buildx`
- Build multiarch images in CI

Co-authored-by: Roman Volosatovs <rvolosatovs@riseup.net>
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
This change should be reverted once issues are fixed

Co-authored-by: Roman Volosatovs <rvolosatovs@riseup.net>
Build only for default platform by default, if a custom platform is
required, users should pass `--platform` flag to the script.
Copy link
Owner

@rvolosatovs rvolosatovs left a comment

Choose a reason for hiding this comment

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

Rebased the commits, applied a few minor changes, mostly styling

Thanks for contribution!

@rvolosatovs rvolosatovs merged commit c8dfb03 into rvolosatovs:main Mar 31, 2022
This was referenced Mar 31, 2022
@strophy strophy deleted the feat/arm64-support branch December 1, 2022 06:36
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.

Fails to build in apple M1
3 participants