package/qt5location: handle plugins being conditionally built
PATCH 1/1 package/qt5location: handle plugins being conditionally built
This fixes a build issue where the qt5location plugins are not built, but are attempted to be installed.
The qt5location plugins have dependencies that are automatically resolved causing plugins to conditionaly be built. This change checks for the existance of the plugins prior to trying to install them.
Fixes:
http://autobuild.buildroot.net/results/bc1/bc13abf3bb2fe1c991aec2334ee658c9641d1fd5/build-end.log
--- package/qt5/qt5location/qt5location.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
package/qt5/qt5location/qt5location.mk +3 -1
@@ -48,7 +48,9 @@ endif define QT5LOCATION_INSTALL_TARGET_POSITION cp -dpf $(STAGING_DIR)/usr/lib/libQt5Positioning.so.* $(TARGET_DIR)/usr/lib - cp -dpfr $(STAGING_DIR)/usr/lib/qt/plugins/position $(TARGET_DIR)/usr/lib/qt/plugins/ + if [ -d $(STAGING_DIR)/usr/lib/qt/plugins/position ] ; then \ + cp -dpfr $(STAGING_DIR)/usr/lib/qt/plugins/position $(TARGET_DIR)/usr/lib/qt/plugins/ ; \ + fi endef define QT5LOCATION_INSTALL_TARGET_CMDS
Signature · 1 line
2.7.4
PATCH 1/1 package/qt5location: handle plugins being conditionally built
Hello,
Quoted 6 lines · Joshua Henderson
This fixes a build issue where the qt5location plugins are not built, but are attempted to be installed.
The qt5location plugins have dependencies that are automatically resolved causing plugins to conditionaly be built. This change checks for the existance of the plugins prior to trying to install them.
Thanks for the fix. Do you know under which condition the plugins will be built vs. not built ?
If it's related to one or several dependencies being available, then I would prefer something like:
ifeq ($(BR2...dependencies needed for plugins),y)
define QT5LOCATION_INSTALL_PLUGINS
cp -dpfr $(STAGING_DIR)/usr/lib/qt/plugins/position $(TARGET_DIR)/usr/lib/qt/plugins/
endef
endif
define QT5LOCATION_INSTALL_TARGET_POSITION
cp -dpf $(STAGING_DIR)/usr/lib/libQt5Positioning.so.* $(TARGET_DIR)/usr/lib $(QT5LOCATION_INSTALL_PLUGINS)
endef
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 package/qt5location: handle plugins being conditionally built
Thomas,
Quoted 6 lines · Thomas Petazzoni
Hello,
Quoted 3 lines · Joshua Henderson
This fixes a build issue where the qt5location plugins are not built, but are attempted to be installed.
Quoted 7 lines
Quoted 3 lines
The qt5location plugins have dependencies that are automatically resolved causing plugins to conditionaly be built. This change checks for the existance of the plugins prior to trying to install them.
Thanks for the fix. Do you know under which condition the plugins will be built vs. not built ?
See my reply to Peter on this issue and v2 of this patch. This is not the right fix. It appears that qtlocation improperly has a dependency on the quick module in order to establish build dependencies for the plugin in question. It is probably better to hit this issue head on instead of just making buildroot install whatever is built.
Quoted 5 lines
If it's related to one or several dependencies being available, then I would prefer something like:
ifeq ($(BR2...dependencies needed for plugins),y)
defineQT5LOCATION_INSTALL_PLUGINS
Quoted 9 lines
cp -dpfr $(STAGING_DIR)/usr/lib/qt/plugins/position $(TARGET_DIR)/usr/lib/qt/plugins/endef
endifdefine
QT5LOCATION_INSTALL_TARGET_POSITIONcp -dpf $(STAGING_DIR)/usr/lib/libQt5Positioning.so.* $(TARGET_DIR)/usr/lib $(QT5LOCATION_INSTALL_PLUGINS)endef
Good to know. I actually looked into doing this, but then I started questioning having dependency tracking of this sort in both buildroot and the application itself.
Food for thought on this topic of copying installed files from staging to target. Why not abstract it out and simply provide a list of files and directories to a single package variable that would handle this automatically? It won't work for every case, but it seems like it would work most of the time and be a bit simpler to create and clearer to read what is being installed.
Josh
PATCH 1/1 package/qt5location: handle plugins being conditionally built
In reply to Joshua HendersonHello Joshua,
Quoted 6 lines
Gesendet: Freitag, 21. Juli 2017 um 01:33 Uhr
Von: "Joshua Henderson" <[email protected]>
An: buildroot at buildroot.org
Cc: "Peter Seiderer" <[email protected]>, "Julien Corjon" <[email protected]>, "Thomas Petazzoni" <[email protected]>
Betreff: [PATCH] package/qt5location: handle plugins being conditionally built
Quoted 7 lines
This fixes a build issue where the qt5location plugins are not built, but are attempted to be installed.
The qt5location plugins have dependencies that are automatically resolved causing plugins to conditionaly be built. This change checks for the existance of the plugins prior to trying to install them.
No, the dependencies of the position plugins are only on module positioning (one purpose of the qt5location package):
$ cat src/plugins/plugins.pro TEMPLATE = subdirs qtHaveModule(positioning): SUBDIRS += position qtHaveModule(location): SUBDIRS += geoservices
The problem is a build-ordering, the right fix is to enable ordering in src/src.pro:
src/src.pro +2 -1
@@ -1,5 +1,6 @@ TEMPLATE = subdirs - +CONFIG += ordered + SUBDIRS += 3rdparty/clip2tri 3rdparty/clipper 3rdparty/poly2tri 3rdparty/clip2tri.depends = 3rdparty/clipper 3rdparty/poly2tri
Buildroot patch follows...
Regards,
Peter
Quoted 31 lines
Fixes:
http://autobuild.buildroot.net/results/bc1/bc13abf3bb2fe1c991aec2334ee658c9641d1fd5/build-end.logCc Julien Corjon <[email protected]>Cc Peter Seiderer <[email protected]>Cc Thomas Petazzoni <[email protected]>Signed-off-by Joshua Henderson <[email protected]>--- package/qt5/qt5location/qt5location.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)package/qt5/qt5location/qt5location.mk +3
-1@@ -48,7 +48,9 @@ endif define QT5LOCATION_INSTALL_TARGET_POSITION cp -dpf $(STAGING_DIR)/usr/lib/libQt5Positioning.so.* $(TARGET_DIR)/usr/lib - cp -dpfr $(STAGING_DIR)/usr/lib/qt/plugins/position $(TARGET_DIR)/usr/lib/qt/plugins/ + if [ -d $(STAGING_DIR)/usr/lib/qt/plugins/position ] ; then \ + cp -dpfr $(STAGING_DIR)/usr/lib/qt/plugins/position $(TARGET_DIR)/usr/lib/qt/plugins/ ; \ + fi endef define QT5LOCATION_INSTALL_TARGET_CMDSSignature · 1 line
2.7.4
PATCH 1/1 package/qt5location: handle plugins being conditionally built
Peter, Thomas,
Quoted 11 lines · Peter Seiderer
Hello Joshua,
Quoted 9 lines
Gesendet: Freitag, 21. Juli 2017 um 01:33 Uhr
Von: "Joshua Henderson" <[email protected]>
An: buildroot at buildroot.org
Cc: "Peter Seiderer" <[email protected]>, "Julien Corjon" <[email protected]>, "Thomas Petazzoni" <[email protected]>
Betreff: [PATCH] package/qt5location: handle plugins being conditionally builtThis fixes a build issue where the qt5location plugins are not built, but are attempted to be installed.
Quoted 7 lines
Quoted 4 lines
The qt5location plugins have dependencies that are automatically resolved causing plugins to conditionaly be built. This change checks for the existance of the plugins prior to trying to install them.
No, the dependencies of the position plugins are only on module positioning (one purpose of the qt5location package):
Ahhh. You are correct and that statement means I should see some dependency on position for positioning. Something like this covers it:
plugins.depends += positioning
And I do see that. The problem is that the above line only happens in src/src.pro if we have the quick module with qtHaveModule(quick). This is a qt5location change [1] that happened between qt 5.8.0 and qt 5.9.1, which moved the above line as a dependency on quick, which is why this issue did not show up in 5.8.0.
This is also why I see the root problem come and go only with a different buildroot config (availability of quick or not). I was led down this erroneous path when I took the src.pro for its word and assumed a dependency on quick in v1 of this patch.
[1] https://code.qt.io/cgit/qt/qtlocation.git/commit/?id=c54ee74acdb9757989004005baf79e99be4c9417
Quoted 21 lines
$ cat src/plugins/plugins.pro TEMPLATE = subdirs qtHaveModule(positioning): SUBDIRS += position qtHaveModule(location): SUBDIRS += geoservicesThe problem is a build-ordering, the right fix is to enable ordering in src/src.pro:
src/src.pro +2
-1@@ -1,5 +1,6 @@ TEMPLATE = subdirs - +CONFIG += ordered + SUBDIRS += 3rdparty/clip2tri 3rdparty/clipper 3rdparty/poly2tri 3rdparty/clip2tri.depends = 3rdparty/clipper 3rdparty/poly2tri
Quoted 1 line
Buildroot patch follows...
Isn't there some saying about every time "ordered" is used a kitten dies? Wouldn't the proper fix be to address the dependency instead? I did not see your patch. So, I have submitted a v2 to this patch and also submitted the patch to the Qt bug tracker going about fixing the dependency instead.
Josh
PATCH 1/1 package/qt5location: handle plugins being conditionally built
Hello Joshua,
Quoted 59 lines · Joshua Henderson
Peter, Thomas,
Quoted 11 lines · Peter Seiderer
Hello Joshua,
Quoted 9 lines
Gesendet: Freitag, 21. Juli 2017 um 01:33 Uhr
Von: "Joshua Henderson" <[email protected]>
An: buildroot at buildroot.org
Cc: "Peter Seiderer" <[email protected]>, "Julien Corjon" <[email protected]>, "Thomas Petazzoni" <[email protected]>
Betreff: [PATCH] package/qt5location: handle plugins being conditionally builtThis fixes a build issue where the qt5location plugins are not built, but are attempted to be installed.
Quoted 7 lines
Quoted 4 lines
The qt5location plugins have dependencies that are automatically resolved causing plugins to conditionaly be built. This change checks for the existance of the plugins prior to trying to install them.
No, the dependencies of the position plugins are only on module positioning (one purpose of the qt5location package):
Ahhh. You are correct and that statement means I should see some dependency on position for positioning. Something like this covers it:
plugins.depends += positioningAnd I do see that. The problem is that the above line only happens in src/src.pro if we have the quick module with qtHaveModule(quick). This is a qt5location change [1] that happened between qt 5.8.0 and qt 5.9.1, which moved the above line as a dependency on quick, which is why this issue did not show up in 5.8.0.
This is also why I see the root problem come and go only with a different buildroot config (availability of quick or not). I was led down this erroneous path when I took the src.pro for its word and assumed a dependency on quick in v1 of this patch.
[1] https://code.qt.io/cgit/qt/qtlocation.git/commit/?id=c54ee74acdb9757989004005baf79e99be4c9417
Quoted 21 lines
$ cat src/plugins/plugins.pro TEMPLATE = subdirs qtHaveModule(positioning): SUBDIRS += position qtHaveModule(location): SUBDIRS += geoservicesThe problem is a build-ordering, the right fix is to enable ordering in src/src.pro:
src/src.pro +2
-1@@ -1,5 +1,6 @@ TEMPLATE = subdirs - +CONFIG += ordered + SUBDIRS += 3rdparty/clip2tri 3rdparty/clipper 3rdparty/poly2tri 3rdparty/clip2tri.depends = 3rdparty/clipper 3rdparty/poly2tri
Quoted 5 lines
Quoted 1 line
Buildroot patch follows...
Isn't there some saying about every time "ordered" is used a kitten dies? Wouldn't the proper fix be to address the dependency instead? I did not see your patch. So, I have submitted a v2 to this patch and also submitted the patch to the Qt bug tracker going about fixing the dependency instead.
Did not yet send the patch (lack of time for a qt bug report (thanks to you for doing it) and proper gerrit patch submission)...
Fixing it by a dependency is a nice solution (if it works, see review of your v2 patch), 'ordered' is used by some other qt submodules...
Regards,
Peter
Quoted 2 lines
Josh