-
Notifications
You must be signed in to change notification settings - Fork 87
Use linuxdeployqt #182
base: master
Are you sure you want to change the base?
Use linuxdeployqt #182
Conversation
linux/AppImage/build-appimage.sh
Outdated
#Copy qt plugins | ||
mkdir -p ./usr/lib/qt5/plugis | ||
cp -r /opt/qt58/plugins ./usr/lib/qt5/plugins | ||
|
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.
Not using 5.8?
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.
Why do you think so? It says qt58
.
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.
linuxdeployqt
should take care of the Qt plugins automatically
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.
Great!
@TheAssassin 58 is 5.8 without a dot. That's why.
I'll leave the merge to you guys or when you approved this. |
I'll test it thoroughly tomorrow morning, and will post feedback. |
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.
https://travis-ci.org/nextcloud/client_theming/jobs/251359362#L2256-L2281
There are several problems with this PR, it does not produce a working AppImage. There's a wrong CMake command, and appimagetool shows this $ID.desktop file not found
message that I thought we had fixed.
linux/AppImage/build-appimage.sh
Outdated
@@ -22,97 +24,58 @@ cd qtkeychain | |||
git checkout v0.8.0 | |||
mkdir build | |||
cd build | |||
cmake -D CMAKE_INSTALL_PREFIX=/app ../ | |||
cmake cmake .. -DCMAKE_INSTALL_PREFIX=/usr |
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.
This can't work for obvious reasons.
It is really annoying me that errors still result in a green Travis CI result. Something seems to ignore exit codes... |
@probonopd try adding |
Somehow I have the feeling that my edited |
The original script is called |
So indeed, I think the whole way how Travis is invoked is messy (distributed over several files) which makes it hard to see immediately what is going on. |
What's the status here? |
@probonopd while this might be true, it also means that your way of developing this feature (i.e. using the GitHub editor and Travis) doesn't work here. You might want to think about developing this feature locally. @enoch85 @probonopd has been busy recently. It's still on track as far as I can tell. |
@probonopd thnx a lot. However this AppImage doesn't work for me. It can't load the libnextcloudsync.so it seems the rpath for the binary does not contain it. Same goes actually for the rpath of the libs (so if you look at the rpath of libnextcloudsync.so it can't find the libqtkeychain.so etc). |
Have quite some extras to do just because the libnextcloud* is installed into a subdirectory of the libraries directory. Makes it much harder than necessary. What is the reason for the subdirectory? |
@rullzer please try again. https://transfer.sh/uAeVm/Nextcloud-2.3.2-beta-x86_64.AppImage works for me now. EDIT: Link corrected. |
404 |
@enoch85 Try https://transfer.sh/uAeVm/Nextcloud-2.3.2-beta-x86_64.AppImage, the second "AppImage" is not necessary. |
Aah that's better :) |
It's called "common sense", you can get it in your local convenience store. |
Didn't read, just clicked, but yeah you're right. |
Never mind, we've all been there. |
I'd love to see the switch to linuxdeployqt, better sooner than later. This project is still using the deprecated |
Work-in-progress for using linuxdeployqt. Please only merge once we have a known good AppImage.