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

Fix some issues with appimage #31

Merged
merged 12 commits into from
Dec 20, 2024
Merged

Conversation

Samueru-sama
Copy link
Contributor

You were already downloading go-appimage, I just used it to deploy everything instead.

The produced appimage works on both glibc and musl systems since I used -s deploy which bundles everything, including the ld-*.so.

Also the musl appimage build is broken right now because go-appimage can't find the Qt plugins on alpine, doesn't matter anyway since the appimage made in ubuntu works on alpine as well.

I manually added the qt6ct plugin to have working dark theme, however this isn't perfect yet as it would only work for the people that use qt6ct, but maybe something similar can be done for other themes:

image

(I don't know what that Broken filename error is about btw, so far everything seems to work)

Also go-appimage already deploys the static appimage runtime, you don't need libfuse2 (and shouldn't have needed it before I changed it either since go-appimage was being used to turn the AppDir into an appimage).

You still need a fusermount or fusermount3 binary, which ubuntu should have fuse3 shipped by default.

Not sure if wayland works now, since I don't use wayland I can't test.

I also had to remove the smartctl binary from the appimage because it did not work, for some reason it only works when the appimage is extracted, I tested unsetting the env variables APPIMAGE APPDIR OWD ARGV0 which is one of the few things the appimage runtime sets, and it still didn't work. I have no idea but this seems like a fuse limitation with smartctl?

The issue is not with the smartctl binary, as I copied it to my $XDG_BIN_HOME and repackaged the appimage and it works just in case.

This issue of smartctl can be fixed by making a hook that detects if smartctl isn't in PATH, if it is not then it downloads the binary to a temp location and adds that location to path, like I do here with yt-dlp for example.

@edisionnano
Copy link
Owner

Arrow icons still seem to be missing, otherwise good work, thanks for working on the AppImage

@Samueru-sama
Copy link
Contributor Author

Samueru-sama commented Nov 7, 2024

Arrow icons still seem to be missing, otherwise good work, thanks for working on the AppImage

how does the arrow icon look like and where should it be?

@edisionnano
Copy link
Owner

Arrow icons still seem to be missing, otherwise good work, thanks for working on the AppImage

how does the arrow icon look like and where should it be?

The issue seems to happen sometimes, especially on non-qt desktop environments. Perhaps it's something I'm doing wrong, perhaps it isn't but the app uses a few XDG icons and they aren't showing in these cases. We had an issue for this #22
image
This is how they should look, also affects icons in the menus

@Samueru-sama
Copy link
Contributor Author

I wonder if the icon is an svg icon and that's why it doesn't load, I've had similar issue before where I had to deploy the gdk pixbuf loaders.

Will see if I can deploy the appimage with sharun instead of go-appimage, I've been testing it lately and it gives much better results 1

@edisionnano
Copy link
Owner

That's promising, thank you. Does Sharun still have these issues with AppImage Launcher, since a lot of people seem to be using it? Also, I wonder if it would be possible to ask for password only once and run QDiskInfo as root on Launch. I asked from my friend @qurious-pixel to review this PR since he is more knowledgeable when it comes to building AppImages. I'll be back in two weeks so if there's no review by then I will review it instead. Big thanks for your effort, I really want a better AppImage experience for the project.

@Samueru-sama
Copy link
Contributor Author

That's promising, thank you. Does Sharun still have these issues with AppImage Launcher, since a lot of people seem to be using it? Also, I wonder if it would be possible to ask for password only once and run QDiskInfo as root on Launch. I asked from my friend @qurious-pixel to review this PR since he is more knowledgeable when it comes to building AppImages. I'll be back in two weeks so if there's no review by then I will review it instead. Big thanks for your effort, I really want a better AppImage experience for the project.

The issue with appimageluancher is that it does not support the static appimage runtime which doesn't have to do with how the appimage is bundled, there isn't anything that can be done other than downgrading to the old runtime, and that means that the appimage would stop working on alpine and on glibc systems the appimage would have the dependency to libfuse2 which no longer comes installed by default.

I use AM instead as an alternative, and others use Gearlever.

@qurious-pixel
Copy link
Collaborator

@Samueru-sama I am able to load into glibc, but on alpine, I am met with error messages.

image

@Samueru-sama
Copy link
Contributor Author

Samueru-sama commented Nov 14, 2024

@Samueru-sama I am able to load into glibc, but on alpine, I am met with error messages.

That's weird, I originally tested the AppImage with distrobox but I didn't test on an vm, I just launched a voidlinux musl vm to test:

image

EDIT: Also I just noticed I don't have the broken arrow icons like I did on the original image, which means the icon is missing from my icon pack 😅

@edisionnano
Copy link
Owner

@qurious-pixel do you also get an error if you run it without doas?
@Samueru-sama does it work for you with doas?

@Samueru-sama
Copy link
Contributor Author

@qurious-pixel do you also get an error if you run it without doas? @Samueru-sama does it work for you with doas?

Oh didn't notice the doas, that's likely the issue, doas unsets all env variablas as far as I know.

Here is with sudo on the voidlinux musl vm:

image

And here is with doas:

image

Why was doas even used? does that fix the issue I talked about with smartctl before? Note that if smartctl is outside the appimage this issue does not happen, so something that downloads the binary and adds to to PATH when it detects that the user doesn't have it installed is the ideal solution.

@edisionnano
Copy link
Owner

Hello all, sorry for the big delay. @Samueru-sama I like your idea of downloading the binary when it isn't installed. Do you think you could implement it? I can host certain official binaries on a separate repo if needed. As for this PR is it ready for merge, or do you want to make more additions and/or changes?

@edisionnano edisionnano merged commit 09150a3 into edisionnano:main Dec 20, 2024
1 of 2 checks passed
@Samueru-sama
Copy link
Contributor Author

I like your idea of downloading the binary when it isn't installed. Do you think you could implement it?

The following script has to be sourced in the AppRun, it is missing the download url since ideally that has to be hosted on a separate repo:

#!/bin/sh

CACHEDIR="${XDG_CACHE_HOME:-$HOME/.cache}"
export PATH="$PATH:$CACHEDIR/qdiskinfo-appimage"
if ! command -v smartctl >/dev/null 2>&1; then
	echo "Downloading smartctl binary..."
	mkdir -p "$CACHEDIR"/qdiskinfo-appimage
	if command -v wget >/dev/null 2>&1; then
		wget -q "$DOWNLOAD_URL" -O "$CACHEDIR"/qdiskinfo-appimage/smartctl
	elif command -v curl >/dev/null 2>&1; then
		curl -Ls "$DOWNLOAD_URL" -O "$CACHEDIR"/qdiskinfo-appimage/smartctl
	else
		echo "ERROR: You need wget or curl in order to download smartctl"
		echo "You can also install smartctl using your distro's package manager"
		exit 1
	fi
	chmod +x "$CACHEDIR"/qdiskinfo-appimage/smartctl 2>/dev/null
fi

As for this PR is it ready for merge, or do you want to make more additions and/or changes?

Yeah it was ready, the only thing is that the AppImage builds on alpine have to removed since go-appimage can't find Qt for some reason on alpine.

@edisionnano
Copy link
Owner

Since the appimage built on Ubuntu shall work on non GNU distros, the Alpine build job can be removed

@edisionnano
Copy link
Owner

Thanks for the bash script, I'll look into modifying it an implementing it. Since smartctl is a command that pertains to dangerous operations like low level hard drive access some distros like OpenSUSE place the smartctl binary on /usr/sbin or /usr/local/sbin which is not in path, so we need to add it in path for example.

@Samueru-sama
Copy link
Contributor Author

/usr/sbin or /usr/local/sbin which is not in path, so we need to add it in path for example.

/usr/sbin and /usr/local/sbin are in PATH by default in most distros.

This is the PATH that I have by default on Artixlinux: /usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl

This is on alpine: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

Also some shells will set PATH to a hardcoded value similar to the alpine one if the variable for some reason is not set.

@edisionnano
Copy link
Owner

Not all distros have it on path, I'm guessing that some do. Also the appimage still doesn't run on native wayland mode, just checked.

@Samueru-sama
Copy link
Contributor Author

Not all distros have it on path,

kek this is insane:

image

Alright in that case in this line of the script: export PATH="$PATH:$CACHEDIR/qdiskinfo-appimage"

It would need to change for: export PATH="$PATH:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$CACHEDIR/qdiskinfo-appimage"

Also the appimage still doesn't run on native wayland mode, just checked.

Before I try anything can you confirm that this one works with native wayland mode? It is also a Qt app: https://github.com/pkgforge-dev/strawberry-AppImage/releases/tag/1.2.3-1

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.

3 participants