| Message ID | 20230209170707.4066903-1-peter@korsgaard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] package/webkitgtk: Build with ninja | expand |
Hi Peter, On Thu, Feb 09, 2023 at 06:07:05PM +0100, Peter Korsgaard wrote: > +define WEBKITGTK_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --build $(WEBKITGTK_BUILDDIR) > +endef > + > +define WEBKITGTK_INSTALL_STAGING_CMDS > + $(TARGET_MAKE_ENV) DESTDIR=$(STAGING_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) > +endef > + > +define WEBKITGTK_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) > +endef > + Would it make sense to apply these changes in pkg-cmake.mk? Using `cmake --build` and `cmake --install` works with all generators and it would avoid needing to repeat this block for both of the packages in this series. Regards, John
>>>>> "John" == John Keeping <john@metanate.com> writes: > Hi Peter, > On Thu, Feb 09, 2023 at 06:07:05PM +0100, Peter Korsgaard wrote: >> +define WEBKITGTK_BUILD_CMDS >> + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --build $(WEBKITGTK_BUILDDIR) >> +endef >> + >> +define WEBKITGTK_INSTALL_STAGING_CMDS >> + $(TARGET_MAKE_ENV) DESTDIR=$(STAGING_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) >> +endef >> + >> +define WEBKITGTK_INSTALL_TARGET_CMDS >> + $(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) >> +endef >> + > Would it make sense to apply these changes in pkg-cmake.mk? > Using `cmake --build` and `cmake --install` works with all generators > and it would avoid needing to repeat this block for both of the packages > in this series. Longer term that is indeed the direction we want to move to (and generally build cmake packages with the ninja backend). This is meant as a minimal patch that is easy to backport to the stable branches. We have a number of cmake packages that use custom FOO_MAKE_OPTS, how are those passed to make when using cmake --build / --install?
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes: > Webkitgtk needs cmake >= 3.20 when building with the make backend since > webkitgtk 3.8.0. > Cmake 3.20 is above our minimal version in > support/dependencies/check-host-cmake.mk, so this breaks builds on hosts > with cmake >= 3.18 < 3.20 - So use the ninja backend instead. > https://github.com/WebKit/WebKit/commit/6cd89696b5d406c1a3d9a7a9bbb18fda9284fa1f > Signed-off-by: Peter Korsgaard <peter@korsgaard.com> Committed, thanks.
On Fri, Feb 10, 2023 at 12:45:24PM +0100, Peter Korsgaard wrote: > >>>>> "John" == John Keeping <john@metanate.com> writes: > > > Hi Peter, > > On Thu, Feb 09, 2023 at 06:07:05PM +0100, Peter Korsgaard wrote: > >> +define WEBKITGTK_BUILD_CMDS > >> + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --build $(WEBKITGTK_BUILDDIR) > >> +endef > >> + > >> +define WEBKITGTK_INSTALL_STAGING_CMDS > >> + $(TARGET_MAKE_ENV) DESTDIR=$(STAGING_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) > >> +endef > >> + > >> +define WEBKITGTK_INSTALL_TARGET_CMDS > >> + $(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) > >> +endef > >> + > > > Would it make sense to apply these changes in pkg-cmake.mk? > > > Using `cmake --build` and `cmake --install` works with all generators > > and it would avoid needing to repeat this block for both of the packages > > in this series. > > Longer term that is indeed the direction we want to move to (and > generally build cmake packages with the ninja backend). > > This is meant as a minimal patch that is easy to backport to the stable > branches. Makes sense. > We have a number of cmake packages that use custom FOO_MAKE_OPTS, how > are those passed to make when using cmake --build / --install? For building, we have: cmake --build <dir> [<options>] [-- <build-tool-options>] so it should be easy to pass _MAKE_OPTS through. But I can't see anything similar for --install, although I wonder if anything actually needs to pass _MAKE_OPTS for installation. From a quick look only 3 CMake packages use _MAKE_OPTS at all: $ git grep -l cmake-package package/ | grep -vF pkg-cmake.mk | xargs grep MAKE_OPTS package/gdal/gdal.mk:GDAL_MAKE_OPTS += -f Makefile package/mariadb/mariadb.mk:HOST_MARIADB_MAKE_OPTS = import_executables package/zeek/zeek.mk:HOST_ZEEK_MAKE_OPTS = binpac bifcl Of those, gdal's use is probably unnecessary when using `cmake --build` and the other two could us a more specific option to set targets and use the --targets option (but should also be okay with passing options after `--`). Regards, John
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes: > Webkitgtk needs cmake >= 3.20 when building with the make backend since > webkitgtk 3.8.0. > Cmake 3.20 is above our minimal version in > support/dependencies/check-host-cmake.mk, so this breaks builds on hosts > with cmake >= 3.18 < 3.20 - So use the ninja backend instead. > https://github.com/WebKit/WebKit/commit/6cd89696b5d406c1a3d9a7a9bbb18fda9284fa1f > Signed-off-by: Peter Korsgaard <peter@korsgaard.com> Committed to 2022.11.x and 2022.02.x, thanks.
diff --git a/package/webkitgtk/webkitgtk.mk b/package/webkitgtk/webkitgtk.mk index b42ac3b644..a6974db926 100644 --- a/package/webkitgtk/webkitgtk.mk +++ b/package/webkitgtk/webkitgtk.mk @@ -139,4 +139,23 @@ ifeq ($(BR2_ARM_CPU_ARMV5)$(BR2_ARM_CPU_ARMV6)$(BR2_MIPS_CPU_MIPS32R6)$(BR2_MIPS WEBKITGTK_CONF_OPTS += -DENABLE_JIT=OFF -DENABLE_C_LOOP=ON -DENABLE_SAMPLING_PROFILER=OFF endif +# webkitgtk needs cmake >= 3.20 when not building with ninja, which is +# above our minimal version in +# support/dependencies/check-host-cmake.mk, so use the ninja backend: +# https://github.com/WebKit/WebKit/commit/6cd89696b5d406c1a3d9a7a9bbb18fda9284fa1f +WEBKITGTK_CONF_OPTS += -GNinja +WEBKITGTK_DEPENDENCIES += host-ninja + +define WEBKITGTK_BUILD_CMDS + $(TARGET_MAKE_ENV) $(BR2_CMAKE) --build $(WEBKITGTK_BUILDDIR) +endef + +define WEBKITGTK_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) DESTDIR=$(STAGING_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) +endef + +define WEBKITGTK_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) $(BR2_CMAKE) --install $(WEBKITGTK_BUILDDIR) +endef + $(eval $(cmake-package))
Webkitgtk needs cmake >= 3.20 when building with the make backend since webkitgtk 3.8.0. Cmake 3.20 is above our minimal version in support/dependencies/check-host-cmake.mk, so this breaks builds on hosts with cmake >= 3.18 < 3.20 - So use the ninja backend instead. https://github.com/WebKit/WebKit/commit/6cd89696b5d406c1a3d9a7a9bbb18fda9284fa1f Signed-off-by: Peter Korsgaard <peter@korsgaard.com> --- package/webkitgtk/webkitgtk.mk | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)