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

add aarch64 build #3

Closed
wants to merge 3 commits into from
Closed

add aarch64 build #3

wants to merge 3 commits into from

Conversation

hall
Copy link

@hall hall commented Oct 17, 2021

This PR adds an aarch64 build (works great on the pinephone!).

However, the actions-rs/cargo action uses cross which doesn't include openssl by default (resulting in errors such as rust-embedded/cross#510); because of that, I've switched the native-tls package out for rustls. I'm no rust expert but it works on both x86 and aarch64.

@hall
Copy link
Author

hall commented Oct 17, 2021

Added a few more changes:

  • use the matrix build strategy to reduce CI complexity between different architectures
  • cache toolchain and cargo dependencies
  • publish musl (since they are statically linked) binaries when a tag is pushed
  • strip symbols during build through RUSTFLAGS

See the actions and releases in the source repo for examples of how this might look.

@desbma
Copy link
Owner

desbma commented Oct 18, 2021

Thanks for the contribution.

I am not sure what is the purpose of this PR though.

Does the project not already builds on aarch64?

Is the only reason to switch to rusttls to work with the CI?

I'd really like to avoid depending on rusttls, and keep native-tls, because the latter relies on the system TLS stack, which is usually maintained by the distribution/OS, instead of me having to rebuild gotify-desktop regularly to keep up with the security fixes, changed CA roots, etc.

@hall
Copy link
Author

hall commented Oct 18, 2021

It does build on aarch64 already, yeah. This is more about providing pre-built releases for distro packaging.

native-tls isn't provided by default in cross' docker image; that's the only reason I tried to go with rustls. Your argument makes sense though. I'll undo that change.

@desbma
Copy link
Owner

desbma commented Oct 18, 2021

This is more about providing pre-built releases for distro packaging.

I don't know of any distro that uses pre-built binaries from upstream in its main packages, actually for the same reason against rustlsabove: this forces static linking with the libc, and loses the advantage of the distribution updating the libc for all packages.

Upstream provides the source, downstream builds binaries, that is usually how Linux packaging work.

@hall
Copy link
Author

hall commented Oct 18, 2021

Right -- I should specify that the AUR was what I had in mind. It's a bit special since it does pull upstream binaries for packages suffixed with -bin. The hope was to avoid building directly on resource-constrained devices like the pinephone (which took, in one case, 33m17.361s just to run cargo build --release).

@desbma
Copy link
Owner

desbma commented Oct 18, 2021

I see. I made a gotify-desktop AUR package but it builds from source.

Maybe you should try building from a VM under aarch64 running Arch, so you would get faster build times (at least with many cores), and no need for cross compilation so you can use dynamic linking for libc and the TLS stack (which would make a smaller binary).

Anyway I don't think this repository is the right place to host or build such binaries.

@hall
Copy link
Author

hall commented Oct 19, 2021

Fair enough -- it's your package to maintain.

I'll close this PR but I don't see a way to submit contributions to your AUR package (let me know if that's not the case though). Will you add the aarch64 arch to the existing PKGBUILD?

@hall hall closed this Oct 19, 2021
@desbma
Copy link
Owner

desbma commented Oct 19, 2021

I don't see a way to submit contributions to your AUR package

You can just comment on https://aur.archlinux.org/packages/gotify-desktop/

Will you add the aarch64 arch to the existing PKGBUILD?

Sure, would this be enough to build on aarch64 ?

diff --git a/PKGBUILD b/PKGBUILD
index 8303a0f..47ca5ad 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -2,9 +2,9 @@
 # shellcheck disable=SC2034,SC2148,SC2154,SC2164
 pkgname=gotify-desktop
 pkgver=1.2.0
-pkgrel=1
+pkgrel=2
 pkgdesc='Small Gotify daemon to send messages as desktop notifications '
-arch=('x86_64')
+arch=('aarch64' 'x86_64')
 url="https://github.com/desbma/${pkgname}"
 license=('GPL')
 makedepends=('rust')

@hall
Copy link
Author

hall commented Oct 20, 2021

Yep -- that patch works great!

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.

2 participants