Qml modules #13304
Qml modules #13304
Conversation
chubinou
force-pushed
the
qml_modules
branch
2 times, most recently
from
5a309af to
2c9b9eb
Compare
|
arch tests are failing because the CI doesn't provide QtQml for qt 6 which is a new requirement for the test |
mesonbuild/modules/_qt.py
Outdated
| for check in checklist: | ||
| if check not in choices: |
There was a problem hiding this comment.
I think you can simplify (and make a better error message) this to something like:
invalid = set(checklist).difference(choices)
if invalid:
return f"invalid selections {', '.join(sorted(invalid))}, valid elements are {', '.join(sorted(choices))}."
mesonbuild/modules/_qt.py
Outdated
| preserve_paths: bool | ||
|
|
||
| class HasToolKwArgs(kwargs.ExtractRequired): | ||
|
|
||
| method: str | ||
| tools: T.List[str] |
There was a problem hiding this comment.
you can make this a little more powerful (and maybe remove an assert later on) with:
tools: T.List[Literal['moc, 'lrelease', <insert reset of valid tools>]]
mesonbuild/modules/_qt.py
Outdated
| else: | ||
| assert False, "unreachable" |
There was a problem hiding this comment.
I'm happy to trust the type annotations for the final release, though this might be useful for bringup
There was a problem hiding this comment.
I changed this function quite a bit, I took inspiration from the one in gnome.py
mesonbuild/modules/_qt.py
Outdated
| basename: str = os.path.basename(sourcepath) | ||
| classname: str = basename.rsplit('.', maxsplit=1)[0] | ||
|
|
||
| if not any(basename.endswith(ext) for ext in ['.qml', '.js', '.mjs']): |
There was a problem hiding this comment.
| if not any(basename.endswith(ext) for ext in ['.qml', '.js', '.mjs']): | |
| if not basename.endswith(('.qml', '.js', '.mjs')): |
mesonbuild/modules/_qt.py
Outdated
| if not any(basename.endswith(ext) for ext in ['.qml', '.js', '.mjs']): | ||
| raise MesonException(f'unexpected file type declared in qml sources {s}') | ||
|
|
||
| if len(classname) == 0 or '.' in classname or classname[0].islower(): |
There was a problem hiding this comment.
| if len(classname) == 0 or '.' in classname or classname[0].islower(): | |
| if not classname or '.' in classname or classname[0].islower(): |
mesonbuild/modules/_qt.py
Outdated
|
|
||
| cmd.extend(kwargs['extra_args']) | ||
|
|
||
| if namespace != '': |
There was a problem hiding this comment.
| if namespace != '': | |
| if namespace: |
mesonbuild/modules/_qt.py
Outdated
| description=f'Qml type registration for {target_name}', | ||
| ) | ||
|
|
||
| @FeatureNew('qt.qml_module', '1.0') |
There was a problem hiding this comment.
| @FeatureNew('qt.qml_module', '1.0') | |
| @FeatureNew('qt.qml_module', '1.6') |
mesonbuild/modules/_qt.py
Outdated
| qrc_resouces: T.List[T.Union[FileOrString, build.CustomTarget, build.CustomTargetIndex, build.GeneratedList]] = [] | ||
| all_qml_files: T.Sequence[T.Union[FileOrString, build.CustomTarget]] = list(kwargs['qml_sources']) + list(kwargs['qml_singletons']) + list(kwargs['qml_internals']) |
There was a problem hiding this comment.
I'm wondering if it would be beneficial to use the Interpreter.source_strings_to_files method here to remove the strings and just have list[File | CustomTarget | CustomTargetIndex | GeneratedList] here, and then not have to do string handling internally.
|
some self comments on the PR: I tried to follow Qt module directory structure at the moment this is still a bit clumpsy:
|
|
Thank you for undertaking this work ! I wanted to give a try to registering QML modules but I am afraid I've hit against a regression introduced in this MR before I could get to it. Here's the reproducer: git clone --depth=1 https://github.com/AdelKS/ZeGrapher.git
cd ZeGrapher
meson setup build
cd build
meson compilewhich outputs this error it looks like there's a naming convention that isn't respected between Or maybe it's related to the fact that I currently pass all headers that need a |
|
Hi, thanks for your interest
your repository doesn't use the feature from this MR, so I don't really see how this is a regression. I tried to compile you project but I ended up with a lot of compilation issues (template issues in your math classes, compiled on ubuntu 22.04 with GCC 11.4), So I can't really help you there
qt.qml_module will moc the file itself as it needs to pass specific options to moc to extract type information, so |
|
Hey, thanks for trying to look into it
Yeah I stopped trying to make "old" GCC or clang versions happy x)
Sorry, I haven't been clear enough: the master branch of my repo builds fine with latest release of meson 1.5.0, but not with this branch, this is before I make changes to
I understand. I have a noob suggestion on this aspect: I have this (maybe wrong ?) approach where I use a bash script to make a meson |
Okay I just tried on meson commit Would you be willing to install add-apt-repository -y ppa:ubuntu-toolchain-r/test
apt install -y gcc-13
git clone --depth=1 https://github.com/AdelKS/ZeGrapher.git
cd ZeGrapher
CXX=g++13 meson setup build
cd build
meson compile |
`outfiles` is the output list of the individual commands Bug: mesonbuild/pull/13304#issuecomment-2226398671
`outfilelist` is the output list of the target, while `outfiles` is the output list of the individual commands Bug: mesonbuild/pull/13304#issuecomment-2226398671
`outfilelist` is the output list of the target, while `outfiles` is the output list of the individual commands Bug: mesonbuild/pull/13304#issuecomment-2226398671
|
I updated the PR, it no longer requires the sources to be generated with a precise directory structure which should match meson philosophy. I also added a 'devel' install target to allow installing the module definition (for qmllint or and qml tools) |
chubinou
force-pushed
the
qml_modules
branch
2 times, most recently
from
3e31346 to
e8e8465
Compare
`outfilelist` is the output list of the target, while `outfiles` is the output list of the individual commands Bug: /pull/13304#issuecomment-2226398671
mesonbuild/modules/_qt.py
Outdated
| module_prefix_full: str = os.path.join(*(kwargs['resources_prefix'].split('/') + module_prefix_list)) | ||
|
|
||
| qrc_resouces: T.List[T.Union[FileOrString, build.GeneratedTypes]] = [] | ||
| all_qml: T.Sequence[T.Union[FileOrString, build.GeneratedTypes]] = list(kwargs['qml_sources']) + list(kwargs['qml_singletons']) + list(kwargs['qml_internals']) |
There was a problem hiding this comment.
Aren't these already lists? Why cast them again to list?
There was a problem hiding this comment.
mypy fails otherwise
Unsupported left operand type for + ("Sequence[File | str | CustomTarget | CustomTargetIndex | GeneratedList]")
There was a problem hiding this comment.
Maybe the original definition of kwargs should annotate these as T.List instead of the generic T.Sequence, if we need them to be a List type?
There was a problem hiding this comment.
I changed it for a T.List, this seems to be enough
|
The next release is creeping closer so it would be nice to get this in. FWICT there are only some minor fixes + a rebase and this should be good to merge. |
in `func_install_data`, `rename` parameter is `ContainerTypeInfo(list, str)` in `module/python.py`, rename is `None` `rename` is stored in `build.Data` object under the type `T.List[str]`
chubinou
force-pushed
the
qml_modules
branch
from
e8e8465 to
4c2af31
Compare
|
Sorry for the late update, I was first waiting for #13526 to get merged then I somehow missed the review notification late October I made a few updates:
|
chubinou
force-pushed
the
qml_modules
branch
from
4c2af31 to
ba2c988
Compare
This might only be possible by adding a new frameworks test specifically to handle qml, which can raise a SKIP if qt6 isn't installed. As opposed to the current test which simply tests all currently-installed Qt versions. The alternative would be to somehow update the test harness to allow specifying expect_skip_on_jobname for installed files, which sounds neat albeit fiddly.
Right, so that should mean the CI failure for that specific runner can be ignored. (The CI image builder for Arch does succeed, which is all we need.) From a very superficial look it seems like all the other CI failures are image builders (always ignore these) and CI runners that don't test Qt6 and fall under the previous category. |
|
Hey, thanks for #13526 and for this MR. I tried to move one of the modules of my project to
Error outputThe commit just before, that doesn't use I'm probably doing something wrong, but I couldn't find something that could help me out in the documentation and test cases of this MR. How to reproduce needs |
chubinou
force-pushed
the
qml_modules
branch
from
ba2c988 to
1f04921
Compare
|
I removed the 'moc_source' argument as moc will complain about Qml definition in non-header files, and will likely fail to build (qmltyperegistrar will include the .cpp)
I went for this
I handle the case for qml modules without qml files and added a unit test about this
Unfortunately this is a qt issue (see QTBUG-93443 and QTBUG-87221). From what I understand the generated JSON doesn't store the relative header path as they want their installed files to be relocatable, as a result the generated code for qmltyperegistrar doesn't know the path of the headers. This is mentioned in Qt documentation here the solution for this is to explicitly add the header path to the include directories of the target. I added a note in the documentation about this
your branch seems to be missing |
so now, the CI fails because the tests are skipped. Should I :
are the images updated before the CI runs? |
Probably the latter is better because it provides more coverage, though in at least the case of Bionic I suspect the only option is to blacklist it. ;)
No, but the image builder runs the testsuite too. Typically we manually ignore failures for Linux jobs when the corresponding image builder succeeds in the same commit. |
chubinou
force-pushed
the
qml_modules
branch
from
1f04921 to
4b0718c
Compare
|
Thanks for your input @chubinou
You probably re-used the clone you had before, in which case you need to do a little
I tried here to put a
But, if I move the QML C++ header file to the main include folder (in my case |
This aims to bring the support of QML modules to meson, the goal is to provide something similar to CMake `qt_add_qml_module` function provided by Qt (see https://doc.qt.io/qt-6/qt-add-qml-module.html ) Fixes: mesonbuild#6988, mesonbuild#9683
These packages are required to test `qml_module` from qt
chubinou
force-pushed
the
qml_modules
branch
from
4b0718c to
a39f29e
Compare
|
you need to add the include dir where the file from executable('ZeGrapher', 'main.cpp', compiled_ui, compiled_ressources, highlighter_qmlmod,
link_with: zg_static_lib,
include_directories: [inc, include_directories('Utils')],
dependencies: [qt6_dep, boost_dep, zecalculator_dep],
install: true)also, if you build a static library as in you first branch |
|
Moved one of our simpler projects to this implementation of modules - so far so good. There is probably a bug in qmlcachegen on handling paths with symlinks, but this is an edge case of my setup. Thanks for the great work! |
docs/markdown/Qt6-module.md
Outdated
| A list of sources to be transpiled into .moc files for manual inclusion. | ||
| - `headers` (File | string | custom_target | custom_target index | generator_output)[]: | ||
| A list of headers to be transpiled into .cpp files | ||
| - `extra_args` string[]: Extra arguments to pass directly to `qt-moc` |
There was a problem hiding this comment.
You have moc_extra_args and don't define this parameter in QmlModuleKwArgs
|
Thank you very much for your input @chubinou. I could transfer my whole project to using It may be worth adding these two links to the documentation:
Cherry-on-top: any plans with adding support for qmltc ? 🤩 |
chubinou
force-pushed
the
qml_modules
branch
from
a39f29e to
8042dff
Compare
|
I updated documentation to reflect @klokik & @AdelKS remarks
maybe some missing includes/dep ? moc tend to to be lenient about this. Q_MOC_INCLUDE may help about this also don't you need public inheritance rather than default (private)
not in short term, I'm more interested in providing shader compilation (qsb) support |
Thanks! Tried this
#pragma once
#include <QSyntaxHighlighter>
#include <QQuickTextDocument>
#include "Utils/state.h"
/// @brief backend that highlights parsing errors in math expressions in LineEdit
class Highlighter: public QSyntaxHighlighter
{
Q_OBJECT
QML_ELEMENT
// ...
Q_MOC_INCLUDE("QSyntaxHighlighter")same warning message, I don't know if this is a
That will also be very cool ! I may try to get the |
This MR aims to bring the support of QML modules to meson, the goal is to
provide something similar to CMake
qt_add_qml_modulefunction provided by Qt(see https://doc.qt.io/qt-6/qt-add-qml-module.html)
fixes: #6988 #9683
what's provided
the QML module
what's missing:
the scope of this PR is already large, I voluntary put aside some aspect of the module generation.
tools (or options) are not available with older version of Qt,
specific Qml files, which is not very practical in terms of API
tooling, I haven't investigated this
VERSIONPAST_MAJOR_VERSIONSSTATIC / SHAREDPLUGIN_TARGETOUTPUT_DIRECTORYRESOURCE_PREFIXCLASS_NAMETYPEINFOIMPORTSOPTIONAL_IMPORTSDEFAULT_IMPORTSDEPENDENCIESIMPORT_PATHSOURCESQML_FILESRESOURCESOUTPUT_TARGETSDESIGNER_SUPPORTEDFOLLOW_FOREIGN_VERSIONINGNAMESPACENO_PLUGINNO_PLUGIN_OPTIONALNO_CREATE_PLUGIN_TARGETNO_GENERATE_PLUGIN_SOURCENO_GENERATE_QMLTYPESNO_GENERATE_QMLDIRNO_LINTNO_CACHEGENNO_RESOURCE_TARGET_PATHNO_IMPORT_SCANENABLE_TYPE_COMPILERTYPE_COMPILER_NAMESPACEQMLTC_EXPORT_DIRECTIVEQMLTC_EXPORT_FILE_NAME