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

musl systems not supported #411

Closed
1 of 2 tasks
notramo opened this issue May 17, 2024 · 8 comments · Fixed by #414
Closed
1 of 2 tasks

musl systems not supported #411

notramo opened this issue May 17, 2024 · 8 comments · Fixed by #414
Labels
help wanted Extra attention is needed

Comments

@notramo
Copy link

notramo commented May 17, 2024

Check if applicable

  • I have searched the existing issues (required)
  • I'm willing to help fix the problem and contribute a pull request

Describe the bug

musl is an alternative to glibc, and is used by multiple Linux distros e.g. Alpine Linux and Void Linux.

The official builds link to glibc, which is incompatible with musl-based systems. I recommend statically linking musl as it's compatible with both glibc and musl.

How to reproduce?

  1. Install Alpine Linux.
  2. Download the built release of this software.

zk configuration

does not matter

Environment

any `musl` based Linux distro
@tjex tjex added help wanted Extra attention is needed wontfix This will not be worked on labels May 18, 2024
@tjex
Copy link
Member

tjex commented May 18, 2024

We cross compile with Docker images. If you change your mind about wanting to contribute, we'd welcome a PR to zk-xcompile (from those current images, you can easily infer what would need to be in an Alpine based image, also better than me personally as I have no experience with musl) with a golang 1.21 image based from Golang's Alpine images.

For the next release of zk we could well have migrated away from needing Docker images for cross compilation, and will be able to do it natively. As the next release is potentially a few hours away 0.14.1, I'm resistant to setting up an Alpine Docker image for this (potentially) singular minor release.

Alternatively, building zk from source is very user friendly (thanks to Go). So you can always go via that route.

Clone zk, cd zk and make install.

@tjex
Copy link
Member

tjex commented May 18, 2024

Was curious, and had a bit of a go: https://github.com/tjex/zk-xcompile/tree/alpine

But attempting to compile results in many complaints about sqlite bindings (which is the dep that requires CGO).
Happy to work with you or someone else on this or receive a direct PR, but I don't want to commit solo time to this currently.

@notramo
Copy link
Author

notramo commented May 18, 2024

Upgrading go-sqlite3 to 1.14.19 should fix the issue:
https://github.com/mattn/go-sqlite3/releases/tag/v1.14.19
(related issue and PR)

As suggested here, adding CGO_CFLAGS="-D_LARGEFILE64_SOURCE" to the environment serves as a temporary workaround, however this might break some other platforms. But I managed to build zk with the current go-sqlite3 version and this variable.

Some comments suggests that despite the fix in 1.14.19, it still fails on Alpine 3.19, but using Alpine 3.18 works. However, this might be fixed since, so I would suggest trying with latest first.

@tjex
Copy link
Member

tjex commented May 19, 2024

Ah great, thanks for working with me to get the necessary info.

Bumping sqlite version built successfully with golang 1.21 and latest alpine build.

file returns this info:

zk: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-musl-x86_64.so.1, BuildID[sha1]=e2ac8e6f536b3ae1e4c740e12ec82a601fad41bd, with debug_info, not stripped

I'm not so learned with compilation, and you did mention to statically link, but here it's stated linking was dynamic. In any case, could you download the binary and report back if it runs, @notramo ?

zk-v0.14.1-alpine-amd64.tar.gz

@tjex tjex removed the wontfix This will not be worked on label May 19, 2024
@notramo
Copy link
Author

notramo commented May 19, 2024

It will crash on glibc systems since it dynamically links musl. It's still dynamically linked, but with a different libc.
However, the compilation succeeded with the updated lib version, so we're getting closer.
According to this article, adding -extldflags=-static to the -ldflags value will switch to static compilation. Makefile snippet for this project after it's added:

# Wrapper around the go binary, to set all the default parameters.
define go
	$(ENV_PREFIX) go $(1) -tags "fts5" -ldflags "-extldflags=-static -X=main.Version=$(VERSION) -X=main.Build=$(BUILD)" $(2)
endef

I tried compiling with this modification and it resulted in a static binary (file output says statically linked, and no interpreter= is specified).

@tjex
Copy link
Member

tjex commented May 20, 2024

awesome, thanks again.

Here are two new binaries. Could you test the one for your arch? And ideally if you know someone who can run the other as a sanity check, it would be ace.

The following test should be adaquate:

cd ~
mkdir zk-test && cd zk-test
zk init

setup as desired

zk new --title "test note"

close

zk index

zk-v0.14.1-musl-amd64.tar.gz
zk-v0.14.1-musl-i386.tar.gz

@notramo
Copy link
Author

notramo commented May 21, 2024

I only have amd64 arch with musl. Tested both binaries, they work out of the box. I couldn't test the i386 on a 32 bit machine, however it works on my amd64 system.

Also tested it in a Docker container running Debian, to see if it works on a glibc system too. Works out of the box.

I think it can be considered solved.

@tjex thanks for adding this feature.

@tjex
Copy link
Member

tjex commented May 21, 2024

Fantastic. Thanks for the extra testing and information. Really appreciated 🙌
You're welcome, thanks for requesting and helping along the way. Glad that the musl peeps can get Zetteling 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants