Re: [Buildroot] [PATCH v1 1/3] pkg-cmake: add option to select the Ninja generator

[John Keeping <john@metanate.com>]

On Fri, Feb 24, 2023 at 12:15:07PM +0100, Thomas Devoogdt wrote:
> E.g. Commit 
>  16e5c92ff5fd2b44a1126bd7d7538c68ce838213, and
>  78d499409f71d8a22b0632c8ebc06f67ee6ae6dd 
> can now be replaced by: WEBKITGTK_CMAKE_NINJA = YES
> 
> Packages that are selecting Ninja, should also use the NINJA_{ENV,OPTS} variables iso the MAKE variables.

The current webkitgtk build commands use `cmake --build` and `cmake
--install`, wouldn't it be better to use those here?

I wonder if it makes more sense to replace anything generator-specific
(like $(PKG)_MAKE_OPTS or $(PKG)_NINJA_OPTS) with general
$(PKG)_CMAKE_BUILD_OPTS and $(PKG)_CMAKE_INSTALL_OPTS so that packages
become independent of the generator.

If those options are at the end of the command, then packages still have
the option (in the build phase) to pass built-tool specific options by
including `--` in the value.

> Signed-off-by: Thomas Devoogdt <thomas.devoogdt@barco.com>
> ---
>  package/pkg-cmake.mk | 41 +++++++++++++++++++++++++++++++++++++++--

No change in docs/? ;-)

>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 8c375779cb..4ce3e6bada 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -65,6 +65,13 @@ else
>  $(2)_BUILDDIR			= $$($(2)_SRCDIR)/buildroot-build
>  endif
>  
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +$(2)_DEPENDENCIES += host-ninja
> +$(2)_GENERATOR			= "Ninja"
> +else
> +$(2)_GENERATOR			= "Unix Makefiles"
> +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 +95,7 @@ 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_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \
>  		-DCMAKE_INSTALL_PREFIX="/usr" \
>  		-DCMAKE_INSTALL_RUNSTATEDIR="/run" \
> @@ -119,7 +126,7 @@ 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_INSTALL_SO_NO_EXE=0 \
>  		-DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \
>  		-DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \
> @@ -165,45 +172,75 @@ $(2)_DEPENDENCIES += $(BR2_CMAKE_HOST_DEPENDENCY)
>  #
>  ifndef $(2)_BUILD_CMDS
>  ifeq ($(4),target)
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +define $(2)_BUILD_CMDS
> +	$$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_BUILDDIR)
> +endef
> +else
>  define $(2)_BUILD_CMDS
>  	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) -C $$($$(PKG)_BUILDDIR)
>  endef
> +endif
> +else
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +define $(2)_BUILD_CMDS
> +	$$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_BUILDDIR)
> +endef
>  else
>  define $(2)_BUILD_CMDS
>  	$$(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) -C $$($$(PKG)_BUILDDIR)
>  endef
>  endif
>  endif
> +endif
>  
>  #
>  # Host installation step. Only define it if not already defined by the
>  # package .mk file.
>  #
>  ifndef $(2)_INSTALL_CMDS
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +define $(2)_INSTALL_CMDS
> +	$$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_BUILDDIR) $$($$(PKG)_INSTALL_OPTS)
> +endef
> +else
>  define $(2)_INSTALL_CMDS
>  	$$(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_OPTS) -C $$($$(PKG)_BUILDDIR)
>  endef
>  endif
> +endif
>  
>  #
>  # Staging installation step. Only define it if not already defined by
>  # the package .mk file.
>  #
>  ifndef $(2)_INSTALL_STAGING_CMDS
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +define $(2)_INSTALL_STAGING_CMDS
> +	$$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(STAGING_DIR) $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_BUILDDIR install
> +endef
> +else
>  define $(2)_INSTALL_STAGING_CMDS
>  	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_STAGING_OPTS) -C $$($$(PKG)_BUILDDIR)
>  endef
>  endif
> +endif
>  
>  #
>  # Target installation step. Only define it if not already defined by
>  # the package .mk file.
>  #
>  ifndef $(2)_INSTALL_TARGET_CMDS
> +ifeq ($$($(3)_CMAKE_NINJA),YES)
> +define $(2)_INSTALL_TARGET_CMDS
> +	$$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(TARGET_DIR) $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_BUILDDIR) install
> +endef
> +else
>  define $(2)_INSTALL_TARGET_CMDS
>  	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_TARGET_OPTS) -C $$($$(PKG)_BUILDDIR)
>  endef
>  endif
> +endif
>  
>  # Call the generic package infrastructure to generate the necessary
>  # make targets
> -- 
> 2.39.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot