-
Notifications
You must be signed in to change notification settings - Fork 131
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
Qt Flatpak-Builder guide #127
Conversation
Maybe @eszlari, @grulja or @nedrichards have good ideas on what it should look like beyond this. |
docs/qt.rst
Outdated
"runtime-version": "5.9", | ||
"sdk": "org.kde.Sdk", | ||
"command": "blinken", | ||
"finish-args": ["--share=ipc", "--socket=x11", "--socket=wayland" ], |
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.
--device=dri
?
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.
And --socket=wayland
only when tested?
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.
You need --socket=wayland enabled, otherwise users running on Wayland won't be able to run their app as both Plasma and Gnome require Qt Wayland plugin on wayland.
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.
But afaik there are apps that don't support wayland and crash if this enabled.
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.
The --socket=wayland just allows the app to access the wayland socket outside of it's sandbox, it shouldn't make the app crash or force it to use wayland in an X11 environment.
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.
And --socket=wayland only when tested?
For that matter xcb needs testing too. It's not like xcb always works.
Will include a comment about dri.
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.
The --socket=wayland just allows the app to access the wayland socket outside of it's sandbox, it shouldn't make the app crash or force it to use wayland in an X11 environment.
Kdenlive does crash when trying to run it under wayland natively:
$ flatpak run --socket=wayland org.kde.kdenlive
QSocketNotifier: Can only be used with threads started with QThread
Using Wayland-EGL
Using the 'xdg-shell-v6' shell integration
trying to load "/usr/lib/plugins/kf5/kio/file.so" from "/usr/lib/plugins/kf5/kio/file.so"
Attempting to create QWindow-based QOffscreenSurface outside the gui thread. Expect failures.
Attempting to create QWindow-based QOffscreenSurface outside the gui thread. Expect failures.
ASSERT failure in QCoreApplication::sendEvent: "Cannot send events to objects owned by a different thread. Current thread 0x0x2708aa0. Receiver 'QOffscreenSurface' (of type 'QWindow') was created in thread 0x0x4251b30", file kernel/qcoreapplication.cpp, line 576
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.
Then kdenlive needs fixing.
docs/qt.rst
Outdated
"modules": [ | ||
{ | ||
"name": "blinken", | ||
"buildsystem": "cmake-ninja", |
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.
"builddir": true
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.
"-DCMAKE_BUILD_TYPE=Release"
or "-DCMAKE_BUILD_TYPE=RelWithDebInfo"
?
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.
"-DCMAKE_INSTALL_LIBDIR=lib"
(workaround for flatpak+ECM bug)
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.
What bug?
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 is supposed to be fixed: flatpak/flatpak-builder#93
But projects using extra-cmake-modules still install in /lib64.
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.
The ECM bug is a bug, that only affects projects using ECM. Instead of documenting it I'd rather have it fixed.
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.
docs/qt.rst
Outdated
"sdk": "org.kde.Sdk", | ||
"command": "blinken", | ||
"finish-args": ["--share=ipc", "--socket=x11", "--socket=wayland" ], | ||
|
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.
Maybe add a
"cleanup": [
"/lib/cmake",
"/share/cmake",
"*.cmake"
]
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.
Looks noisy to me, do we really want that on every recipe?
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.
I just thought this to be a hint for people who make their first flatpak app, so they don't forget the cleanup step.
docs/qt.rst
Outdated
"buildsystem": "cmake-ninja", | ||
"sources": [ | ||
{ | ||
"type": "git", |
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.
Better use "type": "archive"
as an example? (with "branch": "stable"
?)
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.
+1 for archive as an example
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.
How does it make a difference? Documenting sources will be done elsewhere as well, no?
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.
Afaik it's Flathub's policy to prefer archives to git, so this would serve as a good example.
Is this guide necessary? Shouldn't it be enough to use the KDE runtime and follow the standard guidelines? |
I would hope it isn't necessary, I was suggested to create it and thought it probably does make sense so there's something to get started from. Also it's potentially the right place where to put any possible troubleshooting information we may have. |
It's necessary to answer the developer question "How do I flatpak a Qt/KDE app?". Even if it's a very simple answer, the answer needs to be documented. It would also be nice to have an example using qmake if that's still common among Qt apps, or whatever has superceded qmake (IIRC there was a new build tool coming) |
I think this is a good idea. As silly as it may sound, the fact that the kde runtime packages Qt dependencies may not be immediately obvious to everybody. Windows-centric Qt developers may not be familiar with the kde project. A new user following standard guidelines may try to use the freedesktop runtime and self-build Qt if this isn't clearly stated. |
Fair enough - sounds like a good idea to include this. Second question - is it possible to use the qt demo app as the example? |
Yes, that'd be even better. My understanding is that the main guide is to be rewritten using |
My thinking behind using this application was that it's useful to prove there doesn't need to be changes ad-hoc for flatpak. Is this the application you want? |
The guides branch has been merged into master. It would be great to get the Qt guide merged in there too. |
It would be good to indicate that you don't have to use the KDE runtime in order to use Qt. |
Looking at that demo, doesn't look like a great idea, it has a bunch of dependencies and makes for a worse test case IMHO.
How do you mean? The org.kde.Sdk is the one that bundles a tested and up to date Qt. One could package it within their application but that's not something I'd document. |
3d56de3
to
4833334
Compare
There are apps out there that use Qt in their own particular ways. They might well just want to bundle the libraries that they use. I think we ought to at least indicate that they can do that, if they'd like. |
@aleixpol ignore all the dependencies in the demo app, they are just for demonstration. @allanday wanted some bundled modules so he can refer to them in the documentation. This [1] is the manifest for the demo app which is actually used. [1] - https://github.com/flathub/org.flatpak.qtdemo/blob/master/org.flatpak.qtdemo.json |
Are we content with this first version? |
Have you thought about using YAML? |
All the other guides are currently json.
It would be best to keep them consistent.
I'm not against converting them all to yaml, but the pros and cons should
be weighted, and a consensus should be reached before we do anything.
El dt., 29 de maig 2018, 02:18, eszlari <notifications@github.com> va
escriure:
… Have you thought about using YAML?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#127 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALLz5DcWprRfbdoQMv3yyGLZD2QzOUIwks5t3JPwgaJpZM4UCN6p>
.
|
Hi, is there any reason why this never ended up making it in? |
The CI failure looks independent of the changes here. I'll go ahead and merge this. |
Maybe tried to be too non-verbose, please tell me what you'd like to see on it.