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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.