| Message ID | 20220929181350.1026033-1-thomas.ballasi@savoirfairelinux.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] package/qt5/qt5webkit: fix generated artifacts | expand |
Hi Thomas, On 29/09/22 20:13, Thomas Ballasi wrote: > Generated artifacts of the installation process were wrongly located, > causing packages using qt5webkit (qt-webkit-kiosk and python-pyqt5) to > fail at build time. > > Firstly, *.h files are wrongly located a directory below where supposed > (inside qt5/ directory). This is caused by using DATADIR which assumed > include files were to be located in sysroot/usr/include/. Disabling this > variable by removing it from build options leads to a correct behavior. > > Secondly, in order to locate *.pri artifacts correctly, we set the conf > option CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT on, which in turn > sets the variable KDE_INSTALL_USE_QT_SYS_PATHS on, for the following > reasons: > > 1. *.pri files are wrongly located in the host's and target's sysroot > directores while buildroot implements its own mkspecs directory. > By setting KDE_INSTALL_USE_QT_SYS_PATHS on, mkspecs modules are now > being installed in host's data dir (ECM_MKSPECS_INSTALL_DIR is set > accordingly @ line 102 in Source/cmake/ECMGeneratePriFile.cmake). > This also required to prevent using the CMake package's default > DATADIR variable, as done previously, as it enforced to install > artifacts under the sysroot directory. > > 2. *.pri files' content have hardcoded include and library paths. This > has been corrected by setting on KDE_INSTALL_USE_QT_SYS_PATHS as > their content is written according to this value (see line 514 and > 739 in file Source/WebKit/PlatformQt.cmake). > > Regression happened when qt5webkit started using cmake-package at commit > df0b0fe6919c0d0f3750f439a3cfa765232bd569. > > Fixes: > https://bugs.buildroot.org/show_bug.cgi?id=14606 > > Signed-off-by: Thomas Ballasi <thomas.ballasi@savoirfairelinux.com> > --- > package/qt5/qt5webkit/qt5webkit.mk | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/package/qt5/qt5webkit/qt5webkit.mk b/package/qt5/qt5webkit/qt5webkit.mk > index 6912359674..8310ef20c8 100644 > --- a/package/qt5/qt5webkit/qt5webkit.mk > +++ b/package/qt5/qt5webkit/qt5webkit.mk > @@ -51,10 +51,18 @@ QT5WEBKIT_CONF_OPTS += -DENABLE_SAMPLING_PROFILER=OFF > endif > > QT5WEBKIT_CONF_OPTS += \ > + -DCMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT=ON \ > -DENABLE_TOOLS=OFF \ > -DPORT=Qt \ > -DPYTHON_EXECUTABLE=$(HOST_DIR)/bin/python3 \ > -DSHARED_CORE=ON \ > -DUSE_LIBHYPHEN=OFF > > +QT5WEBKIT_INSTALL_STAGING_OPTS = install/fast > + > +define QT5WEBKIT_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --install $(QT5WEBKIT_BUILDDIR) \ > + --prefix $(TARGET_DIR)/usr > +endef > + > $(eval $(cmake-package)) Definitely a better solution, it builds fine here at me for both qt-webkit-kiosk and python-pyqt5 so: Reviewed-by: Giulio Benetti <giulio.benetti@benettiengineering.com> Thank you! Best regards
Hello Thomas, As part of an effort to clear up our backlog, I stumbled across this patch of yours, which has never been applied. However, looking at the issue, I see that both qt-webkit-kiosk and python-pyqt5 were indeed failing to build until November/December 2022, being unable to find the "webkit" Qt module. But since then, we haven't had any further build failure for those two packages: http://autobuild.buildroot.net/?reason=qt-webkit-kiosk% http://autobuild.buildroot.net/?reason=python-pyqt5% I don't immediately see what has changed in qt5webkit that could have fixed the issue. Does this issue still exists in the current Buildroot master? Thanks for your feedback, Thomas On Thu, 29 Sep 2022 14:13:50 -0400 Thomas Ballasi <thomas.ballasi@savoirfairelinux.com> wrote: > Generated artifacts of the installation process were wrongly located, > causing packages using qt5webkit (qt-webkit-kiosk and python-pyqt5) to > fail at build time. > > Firstly, *.h files are wrongly located a directory below where supposed > (inside qt5/ directory). This is caused by using DATADIR which assumed > include files were to be located in sysroot/usr/include/. Disabling this > variable by removing it from build options leads to a correct behavior. > > Secondly, in order to locate *.pri artifacts correctly, we set the conf > option CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT on, which in turn > sets the variable KDE_INSTALL_USE_QT_SYS_PATHS on, for the following > reasons: > > 1. *.pri files are wrongly located in the host's and target's sysroot > directores while buildroot implements its own mkspecs directory. > By setting KDE_INSTALL_USE_QT_SYS_PATHS on, mkspecs modules are now > being installed in host's data dir (ECM_MKSPECS_INSTALL_DIR is set > accordingly @ line 102 in Source/cmake/ECMGeneratePriFile.cmake). > This also required to prevent using the CMake package's default > DATADIR variable, as done previously, as it enforced to install > artifacts under the sysroot directory. > > 2. *.pri files' content have hardcoded include and library paths. This > has been corrected by setting on KDE_INSTALL_USE_QT_SYS_PATHS as > their content is written according to this value (see line 514 and > 739 in file Source/WebKit/PlatformQt.cmake). > > Regression happened when qt5webkit started using cmake-package at commit > df0b0fe6919c0d0f3750f439a3cfa765232bd569. > > Fixes: > https://bugs.buildroot.org/show_bug.cgi?id=14606 > > Signed-off-by: Thomas Ballasi <thomas.ballasi@savoirfairelinux.com> > --- > package/qt5/qt5webkit/qt5webkit.mk | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/package/qt5/qt5webkit/qt5webkit.mk b/package/qt5/qt5webkit/qt5webkit.mk > index 6912359674..8310ef20c8 100644 > --- a/package/qt5/qt5webkit/qt5webkit.mk > +++ b/package/qt5/qt5webkit/qt5webkit.mk > @@ -51,10 +51,18 @@ QT5WEBKIT_CONF_OPTS += -DENABLE_SAMPLING_PROFILER=OFF > endif > > QT5WEBKIT_CONF_OPTS += \ > + -DCMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT=ON \ > -DENABLE_TOOLS=OFF \ > -DPORT=Qt \ > -DPYTHON_EXECUTABLE=$(HOST_DIR)/bin/python3 \ > -DSHARED_CORE=ON \ > -DUSE_LIBHYPHEN=OFF > > +QT5WEBKIT_INSTALL_STAGING_OPTS = install/fast > + > +define QT5WEBKIT_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --install $(QT5WEBKIT_BUILDDIR) \ > + --prefix $(TARGET_DIR)/usr > +endef > + > $(eval $(cmake-package))
Thomas, All, Sorry for the awfully long delay in looking at this patch; it has all the words that make it scary to review: qt5. cmake. webkit... On 2022-09-29 14:13 -0400, Thomas Ballasi spake thusly: > Generated artifacts of the installation process were wrongly located, > causing packages using qt5webkit (qt-webkit-kiosk and python-pyqt5) to > fail at build time. It would have been good to provide an example of such a failure, and they are pretty rare: http://autobuild.buildroot.org/?reason=qt-webkit-kiosk% The last one is from 2023-09-04, pretty recent, but the one before that was in 2022-12-14, but it was more common before that (a few a month on average). We usually add a reference to such a build failure directly in the commit log: http://autobuild.buildroot.org/results/91a/91a2d87eb42bf62f8d4f2b24788deef6c5e866f6/ > Firstly, *.h files are wrongly located a directory below where supposed > (inside qt5/ directory). This is caused by using DATADIR which assumed > include files were to be located in sysroot/usr/include/. Disabling this > variable by removing it from build options leads to a correct behavior. I don't see where/how you are disabling the use of DATADIR. > Secondly, in order to locate *.pri artifacts correctly, we set the conf > option CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT on, From the cmake documentation, it looks like that variable i set by cmake, and is not to be set manually like you do: https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT.html qt5webkit looks for the value of CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT to set CMAKE_INSTALL_PREFIX: qt5webkit-5.212.0-alpha4/Source/cmake/OptionsQt.cmake 1004 query_qmake(qt_install_prefix_dir QT_INSTALL_PREFIX) 1005 if (CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT) 1006 set(CMAKE_INSTALL_PREFIX "${qt_install_prefix_dir}" CACHE PATH "Install path prefix, prepended onto install› 1007 endif () And this looks to me very close to what Looking a bit around, I only noticed jpeg-turbo doing something similar, but I don't understand why it looks if the path s set to its efault value to just then set it to its default value... I think the proper solution in pur case would be to drop that conditional block... > which in turn > sets the variable KDE_INSTALL_USE_QT_SYS_PATHS on, for the following > reasons: > > 1. *.pri files are wrongly located in the host's and target's sysroot > directores while buildroot implements its own mkspecs directory. > By setting KDE_INSTALL_USE_QT_SYS_PATHS on, mkspecs modules are now > being installed in host's data dir (ECM_MKSPECS_INSTALL_DIR is set > accordingly @ line 102 in Source/cmake/ECMGeneratePriFile.cmake). > This also required to prevent using the CMake package's default > DATADIR variable, as done previously, as it enforced to install > artifacts under the sysroot directory. > > 2. *.pri files' content have hardcoded include and library paths. This > has been corrected by setting on KDE_INSTALL_USE_QT_SYS_PATHS as > their content is written according to this value (see line 514 and > 739 in file Source/WebKit/PlatformQt.cmake). I think both issues should be fixed by removing that conditional block... [--SNIP--] > diff --git a/package/qt5/qt5webkit/qt5webkit.mk b/package/qt5/qt5webkit/qt5webkit.mk > index 6912359674..8310ef20c8 100644 > --- a/package/qt5/qt5webkit/qt5webkit.mk > +++ b/package/qt5/qt5webkit/qt5webkit.mk > @@ -51,10 +51,18 @@ QT5WEBKIT_CONF_OPTS += -DENABLE_SAMPLING_PROFILER=OFF > endif > > QT5WEBKIT_CONF_OPTS += \ > + -DCMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT=ON \ > -DENABLE_TOOLS=OFF \ > -DPORT=Qt \ > -DPYTHON_EXECUTABLE=$(HOST_DIR)/bin/python3 \ > -DSHARED_CORE=ON \ > -DUSE_LIBHYPHEN=OFF > > +QT5WEBKIT_INSTALL_STAGING_OPTS = install/fast The default _INSTALL_STAGING_OPTS contains DESTDIR and isntall/fast: https://gitlab.com/buildroot.org/buildroot/-/blob/68b68518a8cc372cd6bfb34414a91311e7266044/package/pkg-cmake.mk#L56 So what you're doing is to rem ove setting DESTDIR, which feels horribly wrong. > +define QT5WEBKIT_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --install $(QT5WEBKIT_BUILDDIR) \ > + --prefix $(TARGET_DIR)/usr Ditto, not using DESTDIR as the install step for target looks highly suspicious. Note that the current state of affairs with cmake has slightly evolved since you posted your patch. as we recently merged support for using ninja as the backend (it is opt-in to that backend, the default is still Makefiles), and we may have a bit of a fallout. Still, I was able to reproduce the issue... So, I started a build (wee, 3 hours!) that have removed the conditional block in qt5webkit, let's see what that does... Regards, Yann E. MORIN. > +endef > + > $(eval $(cmake-package))
diff --git a/package/qt5/qt5webkit/qt5webkit.mk b/package/qt5/qt5webkit/qt5webkit.mk index 6912359674..8310ef20c8 100644 --- a/package/qt5/qt5webkit/qt5webkit.mk +++ b/package/qt5/qt5webkit/qt5webkit.mk @@ -51,10 +51,18 @@ QT5WEBKIT_CONF_OPTS += -DENABLE_SAMPLING_PROFILER=OFF endif QT5WEBKIT_CONF_OPTS += \ + -DCMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT=ON \ -DENABLE_TOOLS=OFF \ -DPORT=Qt \ -DPYTHON_EXECUTABLE=$(HOST_DIR)/bin/python3 \ -DSHARED_CORE=ON \ -DUSE_LIBHYPHEN=OFF +QT5WEBKIT_INSTALL_STAGING_OPTS = install/fast + +define QT5WEBKIT_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --install $(QT5WEBKIT_BUILDDIR) \ + --prefix $(TARGET_DIR)/usr +endef + $(eval $(cmake-package))
Generated artifacts of the installation process were wrongly located, causing packages using qt5webkit (qt-webkit-kiosk and python-pyqt5) to fail at build time. Firstly, *.h files are wrongly located a directory below where supposed (inside qt5/ directory). This is caused by using DATADIR which assumed include files were to be located in sysroot/usr/include/. Disabling this variable by removing it from build options leads to a correct behavior. Secondly, in order to locate *.pri artifacts correctly, we set the conf option CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT on, which in turn sets the variable KDE_INSTALL_USE_QT_SYS_PATHS on, for the following reasons: 1. *.pri files are wrongly located in the host's and target's sysroot directores while buildroot implements its own mkspecs directory. By setting KDE_INSTALL_USE_QT_SYS_PATHS on, mkspecs modules are now being installed in host's data dir (ECM_MKSPECS_INSTALL_DIR is set accordingly @ line 102 in Source/cmake/ECMGeneratePriFile.cmake). This also required to prevent using the CMake package's default DATADIR variable, as done previously, as it enforced to install artifacts under the sysroot directory. 2. *.pri files' content have hardcoded include and library paths. This has been corrected by setting on KDE_INSTALL_USE_QT_SYS_PATHS as their content is written according to this value (see line 514 and 739 in file Source/WebKit/PlatformQt.cmake). Regression happened when qt5webkit started using cmake-package at commit df0b0fe6919c0d0f3750f439a3cfa765232bd569. Fixes: https://bugs.buildroot.org/show_bug.cgi?id=14606 Signed-off-by: Thomas Ballasi <thomas.ballasi@savoirfairelinux.com> --- package/qt5/qt5webkit/qt5webkit.mk | 8 ++++++++ 1 file changed, 8 insertions(+)