-
Notifications
You must be signed in to change notification settings - Fork 60
Add QML module #895
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
base: dev
Are you sure you want to change the base?
Add QML module #895
Conversation
Thanks, that would avoid doing this in the client code in Itinerary as well :) One additional type we have exported there is |
4d60c99
to
69b605e
Compare
This adds a dependency on QtQml and makes the dependency on extra-cmake-modules non-optional. We could make the module optional to build (in this case, i would prefer building it by default and having an opt-out option, to make it harder for distros to build without qml by mistake). We could also keep extra-cmake-modules entirely optional by bundling the necessary files, but i wouldn't like that. @KitsuneRal what do you think? |
QtQmlIntegration rather than QtQml? Or is QtQml a dependency of the ECM module? |
I actually started doing something similar without ECM but the finicky QML module machinery have always put me off. If ECM can help with it (as I can see), great - but I don't quite get why it would need entire QtQml for that. |
right, it's QtQmlIntegration, not QtQml |
QtQmlIntegration is tiny, literally just a few macros in a header file. And I already endorsed ECM previously, so let's make it. |
Cool. In that case, I think this is ready. It doesn't register all the types that we use anywhere in QML, but that doesn't block us from using it |
Add ECM to CI as well, please :) |
|
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.
Petty things.
On linux yes, but not on win / mac |
And of course ECMQmlModule needs qt6_policy, which is only available from qt6.5 |
We could probably look into bumping the Qt version requirement to 6.6 or even 6.8 - Ubuntu 24.04 was the primary reason I sat on Qt 6.4. By now it's relatively old and I think we should move on anyway. |
ba84d27
to
f3c22f6
Compare
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
f3c22f6
to
d022580
Compare
a3af000
to
3795960
Compare
3795960
to
a213b24
Compare
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 unfortunate:
❯ ldd libQuotientQt6.so.0.10.0 | rg Qt6Qml
libQt6Qml.so.6 => /lib64/libQt6Qml.so.6 (0x00007f2cb9600000)
I know that the library is de-facto used with QML-based applications but I still hope we can keep the library rely on as little on-screen API as possible. And as I understand it, the reason is merely the fact that the ECM macro pulls in Qml
and not QmlIntegration
. Can we somehow go without that? Qt's own QML integration macros don't require Qml
.
a213b24
to
ff72589
Compare
Yes; done now. |
This is required for some tooling - qmlls, qmllint, etc. - to work correctly. It also improves qml compilation in the clients
ff72589
to
0f06617
Compare
Okay, my packager's self is happy - I can understand having a separate library linking to But I was still under impression that it's possible to have a QML integration plugin only linking to |
I was under that impression as well, but I don't know what I'd need to do to achieve that |
Heh. Guess we'll have to go with this for now. |
git clone https://invent.kde.org/frameworks/extra-cmake-modules.git | ||
cmake -S extra-cmake-modules -B extra-cmake-modules/build $CMAKE_ARGS | ||
cmake --build extra-cmake-modules/build --target install | ||
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.
Can we also have some basic test that the installed QML integration files are there and have something reasonable inside? Like grep a line in qmldir
and *qmltypes
... Just to make sure it doesn't silently fall apart, because right now QML integration is not checked at all.
The QML integration plugin needs the full QML library. What |
That's also my understanding. And I hoped to have it this way (i.e. with macros inline - it's actually more practical this way, harder to forget exporting a newly added class); but it seems that |
The CMake code should stay pretty much as it is right now, apart from linking the main library to |
@TobiasFella sorry but can we get back to using |
This is required for some tooling - qmlls, qmllint, etc. - to work correctly. It also improves qml compilation in the clients