probonopd / go-appimage

Go implementation of AppImage tools
MIT License
818 stars 71 forks source link

Do not run patchelf on ld-musl #310

Closed probonopd closed 3 weeks ago

probonopd commented 3 weeks ago

https://git.sr.ht/~xordspar0/packages/tree/main/item/AppImage/srb2/build.sh seems to suggest that we should not run patchelf on ld-musl (which we don't want to anyway; see below).

Reference:

probonopd commented 3 weeks ago

Turns out that our code should already NOT run patchelf on ld-...

https://github.com/probonopd/go-appimage/blob/09bf4559842865935e361339d7b3de42654cf6c9/src/appimagetool/appdirtool.go#L782-L786

So I wonder why @xordspar0 needs

# Demangle ld-musl, which appimagetool messes up with patchelf.
cp /lib/ld-musl-$(uname -m).so.1 "$appdir"/lib/
Samueru-sama commented 3 weeks ago

So I wonder why @xordspar0 needs

It might be because some other patch that go-appimage makes to the ld-***.so

Back in #298 I found that if I built on ubuntu 20.04 or 22.04 I had to recopy the ld-linux.so because otherwise yt-dlp would break, oddly enough that doesn't happen on 24.04 though.

probonopd commented 3 weeks ago

I can reproduce the issue:

When I comment out https://github.com/probonopd/xordspar0-packages/blob/09174c3458056399d5d485b36fb21dd821ef27f3/AppImage/appstream/build.sh#L54 then it fails.

It definitely seems like the rpath patching is going on. This is a bug.

probonopd commented 3 weeks ago

In Alpine Linux, ld-musl-... links to libc.musl-... - we only checked for ld-... so far, causing the issue.

probonopd commented 3 weeks ago

Fixed by https://github.com/probonopd/go-appimage/commit/6bb97135d6d79e2d22d1de72c30c319b66ffce07.

Samueru-sama commented 3 weeks ago

In Alpine Linux, ld-musl-... links to libc.musl-... - we only checked for ld-... so far, causing the issue.

Interesting, but I have patched thelibc.musl without issues before, going to avoid that in the future just in case anyway.

probonopd commented 3 weeks ago

@xordspar0 you should be able to remove your workaround, similar to

https://github.com/probonopd/xordspar0-packages/commit/0b82bfc84f20dadb49ffe3a7002316ea8407091f

xordspar0 commented 3 weeks ago

Excellent, thanks for fixing that so quickly.

It definitely seems like the rpath patching is going on. This is a bug.

Yes, that's what I saw. ld-musl would segfault, and the only difference I could see from usual was the rpath changes. I didn't have a chance to look into it any deeper than that.