From: John Keeping <john@metanate.com>
To: Thomas Devoogdt <thomas@devoogdt.com>
Cc: Samuel Martin <s.martin49@gmail.com>,
Thomas Devoogdt <thomas.devoogdt@barco.com>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
buildroot@buildroot.org,
Adrian Perez de Castro <aperez@igalia.com>,
Fabrice Fontaine <fontaine.fabrice@gmail.com>
Subject: Re: [Buildroot] [PATCH v3 1/3] pkg-cmake: add option to select the Ninja generator
Date: Fri, 3 Mar 2023 12:15:39 +0000 [thread overview]
Message-ID: <ZAHk6987hSiKPc6I@donbot> (raw)
In-Reply-To: <20230228155528.2430203-1-thomas.devoogdt@barco.com>
On Tue, Feb 28, 2023 at 04:55:26PM +0100, Thomas Devoogdt wrote:
> E.g. Commit 16e5c92ff5fd2b44a1126bd7d7538c68ce838213 can now be replaced by:
> WEBKITGTK_CMAKE_NINJA = YES
>
> Packages that are selecting Ninja (or overtime another generator),
> should also use the _BUILD_{ENV,OPTS} variables iso the _MAKE variables.
>
> No _INSTALL{,_STAGING,_TARGET}_OPTS used so far:
>
> $ grep '_INSTALL_OPTS' $(grep -rl "cmake-package" package/*/*.mk)
> $ grep '_INSTALL_STAGING_OPTS' $(grep -rl "cmake-package" package/*/*.mk)
> $ grep '_INSTALL_TARGET_OPTS' $(grep -rl "cmake-package" package/*/*.mk)
>
> The _MAKE_{ENV,OPTS} are translated to _BUILD_{ENV,OPTS}:
>
> $ grep '_MAKE_ENV =' $(grep -rl "cmake-package" package/*/*.mk)
>
> > package/netopeer2/netopeer2.mk:NETOPEER2_MAKE_ENV = \
> > package/racehound/racehound.mk:RACEHOUND_MAKE_ENV = $(LINUX_MAKE_FLAGS)
>
> $ grep '_MAKE_OPTS =' $(grep -rl "cmake-package" package/*/*.mk)
>
> > package/mariadb/mariadb.mk:HOST_MARIADB_MAKE_OPTS = import_executables
> > package/zeek/zeek.mk:HOST_ZEEK_MAKE_OPTS = binpac bifcl
>
> Only "musepack" seems to overwrite MAKE to enforce -j1, so replace it:
>
> $ grep '_MAKE =' $(grep -rl "cmake-package" package/*/*.mk)
>
> > package/musepack/musepack.mk:MUSEPACK_MAKE = $(MAKE1)
>
> Signed-off-by: Thomas Devoogdt <thomas.devoogdt@barco.com>
> ---
> v2:
> - made generator use more generic, other generators can now easily be added if required
> v3:
> - add _GENERATOR_PROGRAM
> - add _GENERATOR_PARALLEL for make
> - dropped BUILD_OPTS
> - fix gdal.mk
> ---
> package/gdal/gdal.mk | 2 +-
> package/musepack/musepack.mk | 2 +-
> package/netopeer2/netopeer2.mk | 2 +-
> package/pkg-cmake.mk | 31 +++++++++++++++++++------------
> package/racehound/racehound.mk | 2 +-
> 5 files changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/package/gdal/gdal.mk b/package/gdal/gdal.mk
> index a3b11c53af..d8ce623283 100644
> --- a/package/gdal/gdal.mk
> +++ b/package/gdal/gdal.mk
> @@ -19,7 +19,7 @@ GDAL_SUPPORTS_IN_SOURCE_BUILD = NO
> # autotools in gdal. We need to force 'make' to use the Makefile,
> # which is generated by CMake. GNUmakefile and autoconf are dropped in
> # 3.6 so this can be dropped in future version.
> -GDAL_MAKE_OPTS += -f Makefile
> +GDAL_BUILD_OPTS += -f Makefile
Do we need "--" here before the flag?
The documentation says:
cmake --build <dir> [<options>] [-- <build-tool-options>]
implying that this is necessary.
But is the option even needed with `cmake --build`? I wonder if CMake
always uses the right Makefile anyway.
> # gdal at its core only needs host-pkgconf, libgeotiff, proj and tiff
> # but since by default mrf driver support is enabled, it also needs
> diff --git a/package/musepack/musepack.mk b/package/musepack/musepack.mk
> index fc66c684a5..d4dd08df36 100644
> --- a/package/musepack/musepack.mk
> +++ b/package/musepack/musepack.mk
> @@ -9,7 +9,7 @@ MUSEPACK_SITE = http://files.musepack.net/source
> MUSEPACK_SOURCE = musepack_src_$(MUSEPACK_VERSION).tar.gz
> MUSEPACK_DEPENDENCIES = libcuefile libreplaygain
> MUSEPACK_INSTALL_STAGING = YES
> -MUSEPACK_MAKE = $(MAKE1)
> +MUSEPACK_BUILD_OPTS = -j1
> MUSEPACK_LICENSE = BSD-3-Clause (*mpcdec), LGPL-2.1+ (*mpcenc)
> MUSEPACK_LICENSE_FILES = libmpcdec/COPYING libmpcenc/quant.c
>
> diff --git a/package/netopeer2/netopeer2.mk b/package/netopeer2/netopeer2.mk
> index 47fcd31acd..2ebb23eccd 100644
> --- a/package/netopeer2/netopeer2.mk
> +++ b/package/netopeer2/netopeer2.mk
> @@ -24,7 +24,7 @@ NETOPEER2_CONF_OPTS = \
> # affected mutualy.
> NETOPEER2_SYSREPO_SHM_PREFIX = sr_buildroot$(subst /,_,$(CONFIG_DIR))_netopeer2
>
> -NETOPEER2_MAKE_ENV = \
> +NETOPEER2_BUILD_ENV = \
> SYSREPOCTL_EXECUTABLE=$(HOST_DIR)/bin/sysrepoctl \
> SYSREPO_SHM_PREFIX=$(NETOPEER2_SYSREPO_SHM_PREFIX)
>
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 8c375779cb..5f5988beb8 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -51,11 +51,6 @@ endif
>
> define inner-cmake-package
>
> -$(2)_MAKE ?= $$(MAKE)
> -$(2)_INSTALL_OPTS ?= install
> -$(2)_INSTALL_STAGING_OPTS ?= DESTDIR=$$(STAGING_DIR) install/fast
> -$(2)_INSTALL_TARGET_OPTS ?= DESTDIR=$$(TARGET_DIR) install/fast
> -
> $(3)_SUPPORTS_IN_SOURCE_BUILD ?= YES
>
>
> @@ -65,6 +60,16 @@ else
> $(2)_BUILDDIR = $$($(2)_SRCDIR)/buildroot-build
> endif
>
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +$(2)_DEPENDENCIES += host-ninja
> +$(2)_GENERATOR = "Ninja"
> +$(2)_GENERATOR_PROGRAM = $(HOST_DIR)/bin/ninja
> +else
> +$(2)_GENERATOR = "Unix Makefiles"
> +$(2)_GENERATOR_PROGRAM = $(firstword $(BR2_MAKE))
> +$(2)_GENERATOR_PARALLEL = -j$(PARALLEL_JOBS)
Should this be passed to ninja as well? The default may be higher than
the number of parallel jobs configured.
And for make should we use:
$(wordlist 2,$(words $(BR2_MAKE)),$(BR2_MAKE))
? Although that requires passing build options after "--" which breaks
the generic $(PKG)_BUILD_OPTS handling; "-j" is special in that cmake
recognises the option itself.
> +endif
> +
> #
> # Configure step. Only define it if not already defined by the package
> # .mk file. And take care of the differences between host and target
> @@ -88,7 +93,8 @@ define $(2)_CONFIGURE_CMDS
> rm -f CMakeCache.txt && \
> PATH=$$(BR_PATH) \
> $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \
> - -G"Unix Makefiles" \
> + -G$$($$(PKG)_GENERATOR) \
> + -DCMAKE_MAKE_PROGRAM="$$($$(PKG)_GENERATOR_PROGRAM)" \
> -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \
> -DCMAKE_INSTALL_PREFIX="/usr" \
> -DCMAKE_INSTALL_RUNSTATEDIR="/run" \
> @@ -119,7 +125,8 @@ define $(2)_CONFIGURE_CMDS
> PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 \
> $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \
> - -G"Unix Makefiles" \
> + -G$$($$(PKG)_GENERATOR) \
> + -DCMAKE_MAKE_PROGRAM="$$($$(PKG)_GENERATOR_PROGRAM)" \
> -DCMAKE_INSTALL_SO_NO_EXE=0 \
> -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \
> -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \
> @@ -166,11 +173,11 @@ $(2)_DEPENDENCIES += $(BR2_CMAKE_HOST_DEPENDENCY)
> ifndef $(2)_BUILD_CMDS
> ifeq ($(4),target)
> define $(2)_BUILD_CMDS
> - $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(TARGET_MAKE_ENV) $$($$(PKG)_BUILD_ENV) $$(BR2_CMAKE) --build $$($$(PKG)_BUILDDIR) $$($$(PKG)_GENERATOR_PARALLEL) $$($$(PKG)_BUILD_OPTS)
> endef
> else
> define $(2)_BUILD_CMDS
> - $$(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(HOST_MAKE_ENV) $$($$(PKG)_BUILD_ENV) $$(BR2_CMAKE) --build $$($$(PKG)_BUILDDIR) $$($$(PKG)_GENERATOR_PARALLEL) $$($$(PKG)_BUILD_OPTS)
> endef
> endif
> endif
> @@ -181,7 +188,7 @@ endif
> #
> ifndef $(2)_INSTALL_CMDS
> define $(2)_INSTALL_CMDS
> - $$(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(HOST_MAKE_ENV) $$($$(PKG)_BUILD_ENV) $$(BR2_CMAKE) --install $$($$(PKG)_BUILDDIR) $$($$(PKG)_INSTALL_OPTS)
> endef
> endif
>
> @@ -191,7 +198,7 @@ endif
> #
> ifndef $(2)_INSTALL_STAGING_CMDS
> define $(2)_INSTALL_STAGING_CMDS
> - $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_STAGING_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(TARGET_MAKE_ENV) $$($$(PKG)_BUILD_ENV) DESTDIR=$$(STAGING_DIR) $$(BR2_CMAKE) --install $$($$(PKG)_BUILDDIR) $$($$(PKG)_INSTALL_STAGING_OPTS)
> endef
> endif
>
> @@ -201,7 +208,7 @@ endif
> #
> ifndef $(2)_INSTALL_TARGET_CMDS
> define $(2)_INSTALL_TARGET_CMDS
> - $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_TARGET_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(TARGET_MAKE_ENV) $$($$(PKG)_BUILD_ENV) DESTDIR=$$(TARGET_DIR) $$(BR2_CMAKE) --install $$($$(PKG)_BUILDDIR) $$($$(PKG)_INSTALL_TARGET_OPTS)
> endef
> endif
>
> diff --git a/package/racehound/racehound.mk b/package/racehound/racehound.mk
> index 6499e166d8..f63bb233f6 100644
> --- a/package/racehound/racehound.mk
> +++ b/package/racehound/racehound.mk
> @@ -22,6 +22,6 @@ RACEHOUND_CONF_OPTS += \
> -DKBUILD_VERSION_STRING=$(LINUX_VERSION_PROBED)
>
> # cross compile environment for linux kernel module
> -RACEHOUND_MAKE_ENV = $(LINUX_MAKE_FLAGS)
> +RACEHOUND_BUILD_ENV = $(LINUX_MAKE_FLAGS)
>
> $(eval $(cmake-package))
> --
> 2.39.2
>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
next prev parent reply other threads:[~2023-03-03 12:15 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-24 11:15 [Buildroot] [PATCH v1 1/3] pkg-cmake: add option to select the Ninja generator Thomas Devoogdt
2023-02-24 11:15 ` [Buildroot] [PATCH v1 2/3] package/webkitgtk: use the CMAKE_NINJA flag Thomas Devoogdt
2023-02-24 11:15 ` [Buildroot] [PATCH v1 3/3] package/wpewebkit: " Thomas Devoogdt
2023-02-27 10:39 ` [Buildroot] [PATCH v2 1/3] pkg-cmake: add option to select the Ninja generator Thomas Devoogdt
2023-02-27 10:39 ` [Buildroot] [PATCH v2 2/3] package/webkitgtk: use the CMAKE_NINJA flag Thomas Devoogdt
2023-02-27 10:39 ` [Buildroot] [PATCH v2 3/3] package/wpewebkit: " Thomas Devoogdt
2023-02-28 15:33 ` [Buildroot] [PATCH v2 1/3] pkg-cmake: add option to select the Ninja generator John Keeping
2023-02-28 15:55 ` [Buildroot] [PATCH v3 " Thomas Devoogdt
2023-02-28 15:55 ` [Buildroot] [PATCH v3 2/3] package/webkitgtk: use the CMAKE_NINJA flag Thomas Devoogdt
2023-02-28 15:55 ` [Buildroot] [PATCH v3 3/3] package/wpewebkit: " Thomas Devoogdt
2023-03-03 12:15 ` John Keeping [this message]
2023-03-06 11:02 ` [Buildroot] [PATCH v3 1/3] pkg-cmake: add option to select the Ninja generator Arnout Vandecappelle
2023-03-07 16:10 ` Thomas Devoogdt
2023-03-13 6:55 ` [Buildroot] [PATCH v4 " Thomas Devoogdt
2023-03-13 6:55 ` [Buildroot] [PATCH v4 2/3] package/webkitgtk: use the CMAKE_NINJA flag Thomas Devoogdt
2023-03-13 6:55 ` [Buildroot] [PATCH v4 3/3] package/wpewebkit: " Thomas Devoogdt
2023-03-13 18:48 ` [Buildroot] [PATCH v4 1/3] pkg-cmake: add option to select the Ninja generator John Keeping
2023-02-24 17:53 ` [Buildroot] [PATCH v1 " John Keeping
2023-02-24 18:12 ` Thomas Devoogdt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZAHk6987hSiKPc6I@donbot \
--to=john@metanate.com \
--cc=aperez@igalia.com \
--cc=buildroot@buildroot.org \
--cc=fontaine.fabrice@gmail.com \
--cc=s.martin49@gmail.com \
--cc=thomas.devoogdt@barco.com \
--cc=thomas.petazzoni@bootlin.com \
--cc=thomas@devoogdt.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox