Fixed the autostart for AppImages. See #2504. #2739
Conversation
|
Okay. I have tested the AppImage. It works unless the file name contains a space. I am going to fix this in a few minutes. |
hrittich
force-pushed
the
master
branch
2 times, most recently
from
c085267 to
f896fae
Compare
|
So, I have tested the AppImage. The patch fixes the bug for me. |
| @@ -18,6 +18,7 @@ | |||
| */ | |||
|
|
|||
| #include <QStandardPaths> | |||
| #include <QtGlobal> | |||
There was a problem hiding this comment.
You sure this is needed? I'd expect QtGlobal to be pulled (indirectly) by QStandardPaths already.
There was a problem hiding this comment.
This header is needed according to the Qt documentation. It might not be needed, but not including it means relying on undocumented behavior and might break in future updates.
src/common/utility_unix.cpp
Outdated
| // When running inside an AppImage, we need to set the path to the | ||
| // AppImage instead of the path to the executable | ||
| QString executablePath; | ||
| QString appImagePath = qEnvironmentVariable("APPIMAGE"); |
There was a problem hiding this comment.
This should be const, so I'd go for: const auto appImagePath = qEnvironmentVariable("APPIMAGE");
src/common/utility_unix.cpp
Outdated
| // AppImage instead of the path to the executable | ||
| QString executablePath; | ||
| QString appImagePath = qEnvironmentVariable("APPIMAGE"); | ||
| if (!appImagePath.isNull() && QFile::exists(appImagePath)) { |
There was a problem hiding this comment.
Optional but since I'm a const freak I'd advise going for the ternary operator so that executablePath can be made const, it looked something like that I think:
const auto runningInsideAppImage = !appImagePath.isNull() && QFile::exists(appImagePath);
const auto executablePath = runningInsideAppImage ? appImagePath : executablePath;
There was a problem hiding this comment.
Actually a nice idea, b.c. it gets rid of the uninitialized variable, which I do not like.
There was a problem hiding this comment.
Well that's also why I tend to auto more as well, it prevents uninitialized variables. ;-)
|
/rebase |
|
Could you please rebase on latest master? Otherwise this looks good to go IMHO. |
Signed-off-by: Hannah Rittich <[email protected]>
|
AppImage file: Nextcloud-PR-2739-5fc778bfaa33b65f352e7df229bc5b69d18eb1bc-x86_64.AppImage |
|
I have tested the AppImage once more. Looks fine. |
|
/backport to stable-3.1 |
|
Thanks a lot for the contribution! |
For details please see the discussion in #2504. This fix sets the application path to the enviornment variable APPIMAGE, if the variable exists and points to an existing file. According to the discussion, this should fix #2504.
I cannot, however, check whether this fixes the bug, since I cannot build the AppImage, because the
build-appimage.shis aparently expecting to be run on a certain version of a certain Linux distribution under some CI/CD system. (And I do not know which one.)Would be great, if someone could build the AppImage and test it.