Merge lp:~rpadovani/oxide/type-info into lp:~oxide-developers/oxide/oxide.trunk
| Status: | Merged |
|---|---|
| Approved by: | Olivier Tilloy on 2015-10-20 |
| Approved revision: | 1108 |
| Merged at revision: | 1226 |
| Proposed branch: | lp:~rpadovani/oxide/type-info |
| Merge into: | lp:~oxide-developers/oxide/oxide.trunk |
| Diff against target: |
982 lines (+931/-3) 5 files modified
CMakeLists.txt (+3/-2) qt/CMakeLists.txt (+1/-1) qt/qmlplugin/CMakeLists.txt (+17/-0) qt/qmlplugin/oxide.qmltypes (+909/-0) qt/qmlplugin/qmldir (+1/-0) |
| To merge this branch: | bzr merge lp:~rpadovani/oxide/type-info |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Olivier Tilloy | Approve on 2015-10-20 | ||
| Chris Coulson | 2015-05-25 | Pending | |
|
Review via email:
|
|||
Commit Message
Add oxide.qmltypes in qmlplugin directory
Description of the Change
Add oxide.qmltypes in qmlplugin directory as explained in http://
- 1090. By Riccardo Padovani on 2015-05-25
-
Add oxide.qmltypes file
| Olivier Tilloy (osomon) wrote : | # |
- 1091. By Riccardo Padovani on 2015-05-27
-
Add build target to CMake
| Olivier Tilloy (osomon) wrote : | # |
On the paper that looks ok, but when I run cmake with -DGENERATE_
I’m not sure whether this has something to do with it, but I’m seeing this message when running cmake:
QQmlComponent: Component is not ready
Also, instead of "${CMAKE_
- 1092. By Riccardo Padovani on 2015-05-28
-
Fix CMake var
- 1093. By Riccardo Padovani on 2015-05-28
-
Merge from upstream
| Riccardo Padovani (rpadovani) wrote : | # |
I'm a bit confused. I leave a comment here just to remember tomorrow, when I'll
investigate a bit more.
So, it works only with installed version of Oxide.
I didn't find any documentation, the only one is the output you've when you run
qmlplugindump without arguments.
There are two way to use it:
Usage: qmlplugindump [-v] [-noinstantiate] [-defaultplatform] [-[non]relocatable] module.uri version [module/
The first one doesn't work: I tried with all possible combination of
[module/
file://
I've no idea of what typelist.qml is supposed to be, but anyway it seems it
doesn't try to load the path but first checks the module.
So I tried the second command, and as path flag I set this one:
/home/rpadovani
Now the error is different:
file://
Now I have to investigare what's this fail
| Olivier Tilloy (osomon) wrote : | # |
I’ve got it to work (manually) by setting LD_LIBRARY_PATH and QML2_IMPORT_PATH to allow qmlplugindump to locate libOxideQtCore.so.0 and libqmloxideplug
From my local build dir:
LD_
However this shouldn’t be run at configure time (when running cmake), but rather at build time, after building the two .so.
I guess modifying the checked-in version in the source tree is ok, it will actually be a useful tool to help developers visualize API changes.
- 1094. By Riccardo Padovani on 2015-06-03
-
Work on qmlplugindump
- 1095. By Riccardo Padovani on 2015-06-03
-
Merge from upstream
| Riccardo Padovani (rpadovani) wrote : | # |
Mhh, I wasn't able to have it working with Oxide 1.9 (while it works with 1.8).
Anyway, does the approach make any sense? (well, I just copied your code and replaved directories with vars).
Is right to use execute_process()? I tried also add_custom_
| Olivier Tilloy (osomon) wrote : | # |
According to http://
« The execute_process() command is a newer more powerful version of exec_program(), but the old command has been kept for compatibility. Both commands run while CMake is processing the project prior to build system generation. Use add_custom_target() and add_custom_
So what you want is to use is add_custom_
Note that it is fine to have oxide.qmltypes initially generated and checked in the branch (your last revision removed it).
It doesn’t work for 1.9 because there are no new APIs in 1.9 yet (if you check the contents of registerTypes() in qt/qmlplugin/
- 1096. By Riccardo Padovani on 2015-06-06
-
Add 1.8 qmltypes
- 1097. By Riccardo Padovani on 2015-06-07
-
First working build
| Riccardo Padovani (rpadovani) wrote : | # |
Ok, I did some progresses: now the file is generated, but
- the build fails if there isn't anything new in the release (like with 1.9 right now). If you use 1.8 it works like a charm.
- I'm sure it could be written in a better way, but I found nothing
- In the header it uses an absolute path: // This file was auto-generated by:
// 'qmlplugindump -v -noinstantiate com.canonical.Oxide 1.8 /home/rpadovani
This means every commit will have a diff on that line.
Any suggestion?
| Olivier Tilloy (osomon) wrote : | # |
I’m testing this on the latest trunk, and it doesn’t look like the file is correctly re-generated: there is now a new API version of WebContext (1.9), however when running cmake and make again, oxide.qmltypes is not updated.
Regarding the absolute path, if we could arrange for qmlplugindump to be called with a QMLPLUGIN_
- 1098. By Riccardo Padovani on 2015-06-22
-
Merge from upstream
- 1099. By Riccardo Padovani on 2015-06-22
-
Fix qmltype file generation and add file for 1.9
| Riccardo Padovani (rpadovani) wrote : | # |
I finally managed to have it working, sorry for the long wait!
| Olivier Tilloy (osomon) wrote : | # |
This works well indeed.
However, thinking about it again, I’m not sure that we want this file checked in the source tree. Wouldn’t it be enough to have it generated at build time under ${QMLPLUGIN_
It would also need to be installed to ${QT_INSTALL_
- 1100. By Riccardo Padovani on 2015-06-28
-
Install qmltype file
| Riccardo Padovani (rpadovani) wrote : | # |
Well, I think could be useful having it because, as you said, 'modifying the checked-in version in the source tree is ok, it will actually be a useful tool to help developers visualize API changes'.
I added the INSTALL command for the file
| Olivier Tilloy (osomon) wrote : | # |
According to http://
set(
set(
I haven’t tested it myself, but it’s worth a shot.
| Olivier Tilloy (osomon) wrote : | # |
> Well, I think could be useful having it because, as you said, 'modifying the
> checked-in version in the source tree is ok, it will actually be a useful tool
> to help developers visualize API changes'.
True. I’m turning schizophrenic in this review…
- 1101. By Riccardo Padovani on 2015-06-29
-
Use set command instead of export
| Riccardo Padovani (rpadovani) wrote : | # |
> According to http://
> use the following syntax to set LD_LIBRARY_PATH and QML2_IMPORT_PATH to run
> qmlplugindump:
>
> set(ENV{
> ${CHROMIUM_
> set(ENV{
>
> I haven’t tested it myself, but it’s worth a shot.
Indeed it works, but it needs the "
Thanks for pointing it to me!
| Olivier Tilloy (osomon) wrote : | # |
I’m merging into the latest trunk, running cmake and then make, and I’m getting the following error:
make[2]: Entering directory '/home/
cd /home/osomon/
QQmlComponent: Component is not ready
qt/qmlplugin/
make[2]: *** [qt/qmlplugin/
make[2]: Leaving directory '/home/
CMakeFiles/
make[1]: *** [qt/qmlplugin/
- 1102. By Riccardo Padovani on 2015-06-30
-
Return to export command due http://
www.cmake. org/Bug/ view.php? id=5145 - 1103. By Riccardo Padovani on 2015-06-30
-
Use env instead of export
| Olivier Tilloy (osomon) wrote : | # |
Please add a dependency (https:/
Also, I don’t think that installing oxide.qmltypes should be conditioned by whether we’re cross-compiling or not, given that the file already exists in trunk anyway. Can you remind me (and explain in a comment maybe) why we’re not generating the file when cross-compiling by the way?
| Riccardo Padovani (rpadovani) wrote : | # |
> Please add a dependency (https:/
> ncies.html#
> ${OXIDE_QMLPLUGIN}, to ensure the targets are built in the right order.
Done.
> Also, I don’t think that installing oxide.qmltypes should be conditioned by
> whether we’re cross-compiling or not, given that the file already exists in
> trunk anyway. Can you remind me (and explain in a comment maybe) why we’re not
> generating the file when cross-compiling by the way?
To be honest, I don't remember at all, and I don't find any valid reason right now, so I removed it
- 1104. By Riccardo Padovani on 2015-10-18
-
Add dependency of generate_
oxide_qmltypes on ${OXIDE_QMLPLUGIN}
| Olivier Tilloy (osomon) wrote : | # |
45 +# Only generate typeinfo if not cross compiling
This comment should be removed if the conditional on cross-compilation is removed. BUT, I just ran a build of your branch in a chroot to cross-build it, and got this error: "qmlplugindump: could not find a Qt installation of ''". We might be able to fix it by installing additional dependencies in the chroot, but there’s little use to it, as cross-building is only used for local testing (building for armhf on an x86 laptop). So I’d suggest adding back the conditional that you just removed, and that should be good to merge.
Can you please update the local copy of oxide.qmltypes to the latest changes in trunk (trunk is now at 1.11 and there have been some API changes/additions)?
- 1105. By Riccardo Padovani on 2015-10-19
-
Readd check for crosscompiling
- 1106. By Riccardo Padovani on 2015-10-19
-
Merge from trunk
- 1107. By Riccardo Padovani on 2015-10-19
-
Update oxide.qmltypes to 1.11
| Riccardo Padovani (rpadovani) wrote : | # |
Done :-)
| Olivier Tilloy (osomon) wrote : | # |
Thanks Riccardo, this looks good. I wonder if the install() stance should be conditional? I’d say it shouldn’t, because the file should always be installed. But it doesn’ẗ really matter.
When merging in to the latest trunk, the generate_
| Olivier Tilloy (osomon) wrote : | # |
> Ideally, this kind of error should be caught and the file should be
> restored to its current version if that happens, to avoid failing the
> build entirely. Not sure if that’s possible. If not, I’m okay with
> merging this as is.
qmlplugindump exits with a return code of 3 (EXIT_IMPORTERROR) in this case. It should be possible to test the value of the exit code and act accordingly.
| Olivier Tilloy (osomon) wrote : | # |
Okay, I think I found a way to do what I was suggesting above.
Since the output file gets overwritten in case of an error, we cannot redirect the output of the command directly to ${CMAKE_
We also want the custom target to always succeed, because not having new APIs should not be considered a valid condition for failing the build.
I’m pasting a diff that should apply cleanly on top of your branch and that implements that suggestion. Please take a look and let me know what you think.
=== modified file 'qt/qmlplugin/
--- qt/qmlplugin/
+++ qt/qmlplugin/
@@ -87,10 +87,12 @@
add_
COMMAND env "LD_LIBRARY_
env "QML2_IMPORT_
- qmlplugindump -v -noinstantiate com.canonical.Oxide ${OXIDE_
- WORKING_DIRECTORY ${QMLPLUGIN_
- )
+ qmlplugindump -v -noinstantiate com.canonical.Oxide ${OXIDE_
+ && cp ${CMAKE_
+ || true
+ WORKING_DIRECTORY ${QMLPLUGIN_
add_
+endif()
- install(FILES ${CMAKE_
-endif()
+install(FILES ${CMAKE_
+ DESTINATION ${QT_INSTALL_
- 1108. By Riccardo Padovani on 2015-10-20
-
Don't wipe oxide.qmltypes file if for any reason qmlplugindump fails

Could we maybe have a build target that re-generates this file? That would help ensuring that it doesn’t get outdated too easily.