Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Properly cross-compile for Linux arm64 #352

Merged
merged 3 commits into from
Feb 14, 2021

Conversation

dennisameling
Copy link
Contributor

@dennisameling dennisameling commented Jan 28, 2021

Identify the Bug

See #347

Fixes #347, successor of/alternative to #349

Description of the Change

After merging this PR, arm64 Linux Electron prebuilds will be created through cross-compilation on a x64 Ubuntu host. This eliminates the need for a native arm64 host as suggested in #349.

Can confirm the generated file now has the proper architecture (tested on Ubuntu 20.04 locally):

$ file build/Release/keytar.node
build/Release/keytar.node: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, BuildID[sha1]=b8fb19367c28bccca9ce730d646fb9edb80f0048, stripped

Alternate Designs

Possible Drawbacks

None yet - the only downside of cross-compilation is that you won't be able to run automated tests for arm64 on a x64 host natively. The alternative is running tests in a QEMU-emulated arm64 Docker container on a x64 host, or leveraging Travis' native arm64 runners like lovell/sharp is doing. But the maintainers would need to request OSS credits for travis-ci.com, as .org is shutting down soon. Details in #351

Verification Process

Release Notes

Repair arm64 Linux Electron prebuilds

- |
if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then
mkdir -p prebuilds && chmod 777 prebuilds
docker build -t node-keytar/i386 docker/i386
Copy link
Contributor

Choose a reason for hiding this comment

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

Why build it every time, and not just publish it on docker hub?

Copy link
Contributor

@shiftkey shiftkey Feb 14, 2021

Choose a reason for hiding this comment

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

Having done this on other OSS projects, unless there's a need for this specific setup from the community I prefer having a fast feedback loop by using the same Dockerfile from the repository.

If this were a bigger project, with a more active CI, I'd re-evaluate and maybe get this publishing to Docker Hub.

Copy link
Contributor

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Thanks for your persistence on this @dennisameling!

Changes all look good, I'm gonna see if I can publish a release tonight with this included.

@shiftkey shiftkey merged commit c0bc478 into atom:master Feb 14, 2021
@quanglam2807
Copy link

Thank you so much, @dennisameling. I guess this fixes #318 as well. I'll try to test this out in production.

@dennisameling
Copy link
Contributor Author

@quanglam2807 that should indeed fix #318! Please do let me know in case you still run into issues 🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong prebuild architecture published for electron linux arm64
4 participants