-
Notifications
You must be signed in to change notification settings - Fork 412
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
Bundle appimagetool properly #545
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to look into this @TheAssassin. Please see my Qs.
this repository's CI is in a really bad state
What do you mean by this, how would you like to see it improved? (And no, I don't even have a local Linux machine anymore.)
@@ -17,14 +17,13 @@ sudo dpkg -i patchelf_0.8-2_amd64.deb | |||
# make -j$(nproc) | |||
# sudo make install | |||
|
|||
cd /tmp/ | |||
pushd /tmp/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any advantage of pushd
over cd
? I like cd
better because everyone understands it. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It handles recursion properly, -
is unreliable. It's well understood by regular shell users, too.
cp ./bin/linuxdeployqt linuxdeployqt.AppDir/usr/bin/ | ||
cp -r /usr/local/lib/appimagekit linuxdeployqt.AppDir/usr/lib/ | ||
cp -R /tmp/appimagekit.AppDir linuxdeployqt.AppDir/usr/appimagekit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is /tmp/appimagekit.AppDir
supposed to be coming from? I can't seem to see that being created...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the other script.
There is no effort from my side to fix the various other issues with these CI scripts and their usability. This is a minimal patch that creates a basic setup. AFAICS linuxdeployqt is doing weird things now, doesn't respect The workflow is pretty clear to the reader in my opinion. Extract the AppImage and put the entire AppDir into linuxdeployqt's AppDir. I hate to repeat myself, but it seems necessary. The reason it's done in |
I wasn't aware of the state of this tool, do you mind mentioning some recommended alternatives by name, so I can switch over to them? |
This PR illustrates how to properly bundle appimagetool by bundling its entire AppDir and using
AppRun
as the entrypoint.After playing CI ping-pong for 10 times (because this repository's CI is in a really bad state), please anyone feel free to use my work and finish it. I can't invest more time into debugging what linuxdeployqt is doing there.
LD_LIBRARY_PATH
has been set correctly IMO.This will be the last ever contribution to this tool. It's obviously close to being unmaintained, and there are better alternatives out there. Contributing stuff is hard because of the bad state of CI and code as well.
Fixes #542.