All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure
@ 2018-05-07 10:57 Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure Eric Le Bihan
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

This series adds an infrastructure for Meson-based packages and converts the
existing ones.

Eric Le Bihan (7):
  pkg-meson: new infrastructure
  docs/manual: document pkg-meson infra
  libmpdclient: convert to pkg-meson infra
  systemd: convert to pkg-meson infra
  ncmpc: convert to pkg-meson infra
  mpd-mpc: convert to pkg-meson infra
  enlightenment: convert to pkg-meson infra

 DEVELOPERS                             |   1 +
 docs/manual/adding-packages-meson.txt  | 107 ++++++++++-------------
 package/Makefile.in                    |   1 +
 package/enlightenment/enlightenment.mk |  30 ++-----
 package/libmpdclient/libmpdclient.mk   |  32 +------
 package/mpd-mpc/mpd-mpc.mk             |  26 +-----
 package/ncmpc/ncmpc.mk                 |  27 +-----
 package/pkg-meson.mk                   | 153 +++++++++++++++++++++++++++++++++
 package/systemd/systemd.mk             |  32 +------
 9 files changed, 217 insertions(+), 192 deletions(-)
 create mode 100644 package/pkg-meson.mk

--
2.14.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  2018-05-07 15:02   ` Thomas Petazzoni
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra Eric Le Bihan
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Add a new infrastructure to ease the development of packages that use Meson as
their build system.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 DEVELOPERS           |   1 +
 package/Makefile.in  |   1 +
 package/pkg-meson.mk | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 package/pkg-meson.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index 24d134cb70..2a3de6eb6c 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -577,6 +577,7 @@ F:	package/hicolor-icon-theme/
 F:	package/jemalloc/
 F:	package/meson/
 F:	package/ninja/
+F:	package/pkg-meson.mk
 F:	package/rust-bin/
 F:	package/rust/
 F:	package/s6/
diff --git a/package/Makefile.in b/package/Makefile.in
index 4325f7b3a9..a268016cdf 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -447,3 +447,4 @@ include package/pkg-rebar.mk
 include package/pkg-kernel-module.mk
 include package/pkg-waf.mk
 include package/pkg-golang.mk
+include package/pkg-meson.mk
diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
new file mode 100644
index 0000000000..e0b79abe4e
--- /dev/null
+++ b/package/pkg-meson.mk
@@ -0,0 +1,153 @@
+################################################################################
+# Meson package infrastructure
+#
+# This file implements an infrastructure that eases development of
+# package .mk files for Meson packages. It should be used for all
+# packages that use Meson as their build system.
+#
+# See the Buildroot documentation for details on the usage of this
+# infrastructure
+#
+# In terms of implementation, this Meson infrastructure requires
+# the .mk file to only specify metadata information about the
+# package: name, version, download URL, etc.
+#
+# We still allow the package .mk file to override what the different
+# steps are doing, if needed. For example, if <PKG>_BUILD_CMDS is
+# already defined, it is used as the list of commands to perform to
+# build the package, instead of the default Meson behaviour. The
+# package can also define some post operation hooks.
+#
+################################################################################
+
+
+################################################################################
+# inner-meson-package -- defines how the configuration, compilation and
+# installation of a Meson package should be done, implements a few hooks to
+# tune the build process and calls the generic package infrastructure to
+# generate the necessary make targets
+#
+#  argument 1 is the lowercase package name
+#  argument 2 is the uppercase package name, including a HOST_ prefix
+#             for host packages
+#  argument 3 is the uppercase package name, without the HOST_ prefix
+#             for host packages
+#  argument 4 is the type (target or host)
+################################################################################
+
+define inner-meson-package
+
+$(2)_CONF_ENV		?=
+$(2)_CONF_OPTS		?=
+$(2)_NINJA		= $(HOST)/bin/ninja
+$(2)_NINJA_ENV		?=
+$(2)_NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
+$(2)_SRCDIR		= $$($(2)_DIR)/$$($(2)_SUBDIR)
+
+#
+# Configure step. Only define it if not already defined by the package
+# .mk file. And take care of the differences between host and target
+# packages.
+#
+ifndef $(2)_CONFIGURE_CMDS
+ifeq ($(4),target)
+
+# Configure package for target
+#
+#
+define $(2)_CONFIGURE_CMDS
+	rm -rf $$($$(PKG)_SRCDIR)/build
+	mkdir -p $$($$(PKG)_SRCDIR)/build
+	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
+		--prefix=/usr \
+		--libdir=/usr/lib \
+		--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
+		--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
+		--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \
+		$$($$(PKG)_CONF_OPTS) \
+		$$($$(PKG)_SRCDIR) $$($$(PKG)_SRCDIR)/build
+endef
+else
+
+# Configure package for host
+define $(2)_CONFIGURE_CMDS
+	rm -rf $$($$(PKG)_SRCDIR)/build
+	mkdir -p $$($$(PKG)_SRCDIR)/build
+	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
+		--prefix="$$(HOST_DIR)" \
+		--default-library shared \
+		--buildtype release \
+		$$($$(PKG)_CONF_OPTS) \
+		$$($$(PKG)_SRCDIR) $$($$(PKG)_SRCDIR)/build
+endef
+endif
+endif
+
+$(2)_DEPENDENCIES += host-meson
+
+#
+# Build step. Only define it if not already defined by the package .mk
+# file.
+#
+ifndef $(2)_BUILD_CMDS
+ifeq ($(4),target)
+define $(2)_BUILD_CMDS
+	$$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) $$($$(PKG)_NINJA) \
+		$$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build
+endef
+else
+define $(2)_BUILD_CMDS
+	$$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) $$($$(PKG)_NINJA) \
+		$$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build
+endef
+endif
+endif
+
+#
+# Host installation step. Only define it if not already defined by the
+# package .mk file.
+#
+ifndef $(2)_INSTALL_CMDS
+define $(2)_INSTALL_CMDS
+	$$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(HOST_DIR) \
+		$$($$(PKG)_NINJA) $$($$(PKG)_NINJA_OPTS) \
+		-C $$($$(PKG)_SRCDIR)/build install
+endef
+endif
+
+#
+# Staging installation step. Only define it if not already defined by
+# the package .mk file.
+#
+ifndef $(2)_INSTALL_STAGING_CMDS
+define $(2)_INSTALL_STAGING_CMDS
+	$$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(STAGING_DIR) \
+		$$($$(PKG)_NINJA) $$($$(PKG)_NINJA_OPTS) \
+		-C $$($$(PKG)_SRCDIR)/build install
+endef
+endif
+
+#
+# Target installation step. Only define it if not already defined by
+# the package .mk file.
+#
+ifndef $(2)_INSTALL_TARGET_CMDS
+define $(2)_INSTALL_TARGET_CMDS
+	$$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(TARGET_DIR) \
+		$$($$(PKG)_NINJA) $$($$(PKG)_NINJA_OPTS) \
+		-C $$($$(PKG)_SRCDIR)/build install
+endef
+endif
+
+# Call the generic package infrastructure to generate the necessary
+# make targets
+$(call inner-generic-package,$(1),$(2),$(3),$(4))
+
+endef
+
+################################################################################
+# meson-package -- the target generator macro for Meson packages
+################################################################################
+
+meson-package = $(call inner-meson-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+host-meson-package = $(call inner-meson-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  2018-05-07 15:08   ` Thomas Petazzoni
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 3/7] libmpdclient: convert to " Eric Le Bihan
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Update documentation about adding meson-based packages with instructions for
using pkg-meson infrastructure.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 docs/manual/adding-packages-meson.txt | 107 +++++++++++++++-------------------
 1 file changed, 47 insertions(+), 60 deletions(-)

diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt
index f8aa08fa91..9863b69797 100644
--- a/docs/manual/adding-packages-meson.txt
+++ b/docs/manual/adding-packages-meson.txt
@@ -1,7 +1,7 @@
 // -*- mode:doc; -*-
 // vim: set syntax=asciidoc:
 
-=== Integration of Meson-based packages
+=== Infrastructure for Meson-based packages
 
 [[meson-package-tutorial]]
 
@@ -10,9 +10,7 @@
 http://mesonbuild.com[Meson] is an open source build system meant to be both
 extremely fast, and, even more importantly, as user friendly as possible.
 
-Buildroot does not (yet) provide a dedicated package infrastructure for
-meson-based packages. So, we will explain how to write a +.mk+ file for such a
-package. Let's start with an example:
+Let's see how to write a +.mk+ file for a Meson-based package, with an example:
 
 ------------------------------
 01: ################################################################################
@@ -28,74 +26,63 @@ package. Let's start with an example:
 11: FOO_LICENSE_FILES = COPYING
 12: FOO_INSTALL_STAGING = YES
 13:
-14: FOO_DEPENDENCIES = host-meson host-pkgconf bar
+14: FOO_DEPENDENCIES = host-pkgconf bar
 15:
-16: FOO_CONF_OPTS += \
-17: 	--prefix=/usr \
-18: 	--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
-19: 	--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf
-20:
-21: FOO_NINJA_OPTS = $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
-22:
-23: ifeq ($(BR2_PACKAGE_BAZ),y)
-24: FOO_CONF_OPTS += -Dbaz
-25: endif
-26:
-27: define FOO_CONFIGURE_CMDS
-28: 	rm -rf $(@D)/build
-29: 	mkdir -p $(@D)/build
-30: 	$(TARGET_MAKE_ENV) meson $(FOO_CONF_OPTS) $(@D) $(@D)/build
-31: endef
-32:
-33: define FOO_BUILD_CMDS
-34: 	$(TARGET_MAKE_ENV) ninja $(FOO_NINJA_OPTS) -C $(@D)/build
-35: endef
-36:
-37: define FOO_INSTALL_TARGET_CMDS
-38: 	$(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) ninja $(FOO_NINJA_OPTS) \
-39: 		-C $(@D)/build install
-40: endef
-41:
-42: define FOO_INSTALL_STAGING_CMDS
-43: 	$(TARGET_MAKE_ENV) DESTDIR=$(STAGING_DIR) ninja $(FOO_NINJA_OPTS) \
-44: 		-C $(@D)/build install
-45: endef
-46:
-47: $(eval $(generic-package))
+16: ifeq ($(BR2_PACKAGE_BAZ),y)
+17: FOO_CONF_OPTS += -Dbaz
+18: endif
+19:
+20: $(eval $(meson-package))
 --------------------------------
 
 The Makefile starts with the definition of the standard variables for package
 declaration (lines 7 to 11).
 
-As seen in line 47, it is based on the
-xref:generic-package-tutorial[+generic-package+ infrastructure]. So, it defines
-the variables required by this particular infrastructure, where Meson and its
-companion tool, Ninja, are invoked:
+On line line 20, we invoke the +meson-package+ macro that generates all the
+Makefile rules that actually allows the package to be built.
 
-* +FOO_CONFIGURE_CMDS+: the build directory required by Meson is created, and
-  Meson is invoked to generate the Ninja build file. The options required to
-  configure the cross-compilation of the package are passed via
-  +FOO_CONF_OPTS+.
+In the example, +host-pkgconf+ and +bar+ are declared as dependencies in
++FOO_DEPENDENCIES+ at line 14 because the Meson build file of +foo+ uses
+`pkg-config` to determine the compilation flags and libraries of package +bar+.
 
-* +FOO_BUILD_CMDS+: Ninja is invoked to perform the build.
+Note that it is not necessary to add +host-meson+ in the +FOO_DEPENDENCIES+
+variable of a package, since this basic dependency is automatically added as
+needed by the Meson package infrastructure.
 
-* +FOO_INSTALL_TARGET_CMDS+: Ninja is invoked to install the files generated
-  during the build step in the target directory.
-
-* +FOO_INSTALL_STAGING_CMDS+: Ninja is invoked to install the files generated
-  during the build step in the staging directory, as +FOO_INSTALL_STAGING+ is
-  set to "YES".
-
-In order to have Meson available for the build, +FOO_DEPENDENCIES+ needs to
-contain +host-meson+. In the example, +host-pkgconf+ and +bar+ are also
-declared as dependencies because the Meson build file of +foo+ uses `pkg-config`
-to determine the compilation flags and libraries of package +bar+.
-
-If the "baz" package is selected, then support for the "baz" feature in "foo"
-is activated by adding +-Dbaz+ to +FOO_CONF_OPTS+, as specified in the
+If the "baz" package is selected, then support for the "baz" feature in "foo" is
+activated by adding +-Dbaz+ to +FOO_CONF_OPTS+ at line 17, as specified in the
 +meson_options.txt+ file in "foo" source tree.
 
 To sum it up, to add a new meson-based package, the Makefile example can be
 copied verbatim then edited to replace all occurences of +FOO+ with the
 uppercase name of the new package and update the values of the standard
 variables.
+
+[[meson-package-reference]]
+
+==== +meson-package+ reference
+
+The main macro of the Meson package infrastructure is +meson-package+. It is
+similar to the +generic-package+ macro.
+
+Just like the generic infrastructure, the Meson infrastructure works by defining
+a number of variables before calling the +meson-package+ macro.
+
+First, all the package metadata information variables that exist in the generic
+infrastructure also exist in the Meson infrastructure: +FOO_VERSION+,
++FOO_SOURCE+, +FOO_PATCH+, +FOO_SITE+, +FOO_SUBDIR+, +FOO_DEPENDENCIES+,
++FOO_INSTALL_STAGING+, +FOO_INSTALL_TARGET+.
+
+A few additional variables, specific to the Meson infrastructure, can also be
+defined. Many of them are only useful in very specific cases, typical packages
+will therefore only use a few of them.
+
+* +FOO_CONF_ENV+, to specify additional environment variables to pass to
+  +meson+ for the configuration step. By default, empty.
+
+* +FOO_CONF_OPTS+, to specify additional options to pass to +meson+ for the
+  configuration step. By default, empty.
+
+* +FOO_NINJA_ENV+, to specify additional environment variables to pass to
+  +ninja+, meson companion tool in charge of the build operations. By default,
+  empty.
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 3/7] libmpdclient: convert to pkg-meson infra
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 4/7] systemd: " Eric Le Bihan
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/libmpdclient/libmpdclient.mk | 32 +-------------------------------
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git a/package/libmpdclient/libmpdclient.mk b/package/libmpdclient/libmpdclient.mk
index 6ff27241c0..c04536eb88 100644
--- a/package/libmpdclient/libmpdclient.mk
+++ b/package/libmpdclient/libmpdclient.mk
@@ -11,35 +11,5 @@ LIBMPDCLIENT_SITE = http://www.musicpd.org/download/libmpdclient/$(LIBMPDCLIENT_
 LIBMPDCLIENT_INSTALL_STAGING = YES
 LIBMPDCLIENT_LICENSE = BSD-3-Clause
 LIBMPDCLIENT_LICENSE_FILES = COPYING
-LIBMPDCLIENT_DEPENDENCIES = host-meson
 
-LIBMPDCLIENT_CONF_OPTS += \
-	--prefix=/usr \
-	--libdir=/usr/lib \
-	--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
-	--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
-	--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf
-
-LIBMPDCLIENT_NINJA_OPTS = $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
-
-define LIBMPDCLIENT_CONFIGURE_CMDS
-	rm -rf $(@D)/build
-	mkdir -p $(@D)/build
-	$(TARGET_MAKE_ENV) meson $(LIBMPDCLIENT_CONF_OPTS) $(@D) $(@D)/build
-endef
-
-define LIBMPDCLIENT_BUILD_CMDS
-	$(TARGET_MAKE_ENV) ninja $(LIBMPDCLIENT_NINJA_OPTS) -C $(@D)/build
-endef
-
-define LIBMPDCLIENT_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) \
-		ninja $(LIBMPDCLIENT_NINJA_OPTS) -C $(@D)/build install
-endef
-
-define LIBMPDCLIENT_INSTALL_STAGING_CMDS
-	$(TARGET_MAKE_ENV) DESTDIR=$(STAGING_DIR) \
-		ninja $(LIBMPDCLIENT_NINJA_OPTS) -C $(@D)/build install
-endef
-
-$(eval $(generic-package))
+$(eval $(meson-package))
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 4/7] systemd: convert to pkg-meson infra
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
                   ` (2 preceding siblings ...)
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 3/7] libmpdclient: convert to " Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 5/7] ncmpc: " Eric Le Bihan
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/systemd/systemd.mk | 32 +++-----------------------------
 1 file changed, 3 insertions(+), 29 deletions(-)

diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
index db7fb43636..c52e4923a0 100644
--- a/package/systemd/systemd.mk
+++ b/package/systemd/systemd.mk
@@ -12,7 +12,6 @@ SYSTEMD_INSTALL_STAGING = YES
 SYSTEMD_DEPENDENCIES = \
 	host-gperf \
 	host-intltool \
-	host-meson \
 	kmod \
 	libcap \
 	util-linux
@@ -26,10 +25,6 @@ SYSTEMD_DEPENDENCIES += busybox
 endif
 
 SYSTEMD_CONF_OPTS += \
-	--prefix=/usr \
-	--libdir='/usr/lib' \
-	--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
-	--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \
 	-Drootlibdir='/usr/lib' \
 	-Dblkid=true \
 	-Dman=false \
@@ -399,28 +394,7 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD
 	$(SYSTEMD_INSTALL_NETWORK_CONFS)
 endef
 
-SYSTEMD_NINJA_OPTS = $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
+SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV)
+SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV)
 
-SYSTEMD_ENV = $(TARGET_MAKE_ENV) $(HOST_UTF8_LOCALE_ENV)
-
-define SYSTEMD_CONFIGURE_CMDS
-	rm -rf $(@D)/build
-	mkdir -p $(@D)/build
-	$(SYSTEMD_ENV) meson $(SYSTEMD_CONF_OPTS) $(@D) $(@D)/build
-endef
-
-define SYSTEMD_BUILD_CMDS
-	$(SYSTEMD_ENV) ninja $(SYSTEMD_NINJA_OPTS) -C $(@D)/build
-endef
-
-define SYSTEMD_INSTALL_TARGET_CMDS
-	$(SYSTEMD_ENV) DESTDIR=$(TARGET_DIR) ninja $(SYSTEMD_NINJA_OPTS) \
-		-C $(@D)/build install
-endef
-
-define SYSTEMD_INSTALL_STAGING_CMDS
-	$(SYSTEMD_ENV) DESTDIR=$(STAGING_DIR) ninja $(SYSTEMD_NINJA_OPTS) \
-		-C $(@D)/build install
-endef
-
-$(eval $(generic-package))
+$(eval $(meson-package))
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 5/7] ncmpc: convert to pkg-meson infra
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
                   ` (3 preceding siblings ...)
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 4/7] systemd: " Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 6/7] mpd-mpc: " Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 7/7] enlightenment: " Eric Le Bihan
  6 siblings, 0 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/ncmpc/ncmpc.mk | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/package/ncmpc/ncmpc.mk b/package/ncmpc/ncmpc.mk
index 1d464b72de..d70ed46ea9 100644
--- a/package/ncmpc/ncmpc.mk
+++ b/package/ncmpc/ncmpc.mk
@@ -8,15 +8,11 @@ NCMPC_VERSION_MAJOR = 0
 NCMPC_VERSION = $(NCMPC_VERSION_MAJOR).29
 NCMPC_SOURCE = ncmpc-$(NCMPC_VERSION).tar.xz
 NCMPC_SITE = http://www.musicpd.org/download/ncmpc/$(NCMPC_VERSION_MAJOR)
-NCMPC_DEPENDENCIES = host-meson host-pkgconf libglib2 libmpdclient ncurses
+NCMPC_DEPENDENCIES = host-pkgconf libglib2 libmpdclient ncurses
 NCMPC_LICENSE = GPL-2.0+
 NCMPC_LICENSE_FILES = COPYING
 
-NCMPC_CONF_OPTS += \
-	--prefix=/usr \
-	-Dcurses=ncurses \
-	--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
-	--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf
+NCMPC_CONF_OPTS = -Dcurses=ncurses
 
 ifeq ($(BR2_PACKAGE_LIRC_TOOLS),y)
 NCMPC_DEPENDENCIES += lirc-tools
@@ -25,21 +21,4 @@ else
 NCMPC_CONF_OPTS += -Dlirc=false
 endif
 
-NCMPC_NINJA_OPTS = $(if $(VERBOSE),-v)
-
-define NCMPC_CONFIGURE_CMDS
-	rm -rf $(@D)/build
-	mkdir -p $(@D)/build
-	$(TARGET_MAKE_ENV) meson $(NCMPC_CONF_OPTS) $(@D) $(@D)/build
-endef
-
-define NCMPC_BUILD_CMDS
-	$(TARGET_MAKE_ENV) ninja $(NCMPC_NINJA_OPTS) -C $(@D)/build
-endef
-
-define NCMPC_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) \
-		ninja $(NCMPC_NINJA_OPTS) -C $(@D)/build install
-endef
-
-$(eval $(generic-package))
+$(eval $(meson-package))
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 6/7] mpd-mpc: convert to pkg-meson infra
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
                   ` (4 preceding siblings ...)
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 5/7] ncmpc: " Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 7/7] enlightenment: " Eric Le Bihan
  6 siblings, 0 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/mpd-mpc/mpd-mpc.mk | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/package/mpd-mpc/mpd-mpc.mk b/package/mpd-mpc/mpd-mpc.mk
index 942530a561..0666339ff9 100644
--- a/package/mpd-mpc/mpd-mpc.mk
+++ b/package/mpd-mpc/mpd-mpc.mk
@@ -10,28 +10,6 @@ MPD_MPC_SITE = http://www.musicpd.org/download/mpc/$(MPD_MPC_VERSION_MAJOR)
 MPD_MPC_SOURCE = mpc-$(MPD_MPC_VERSION).tar.xz
 MPD_MPC_LICENSE = GPL-2.0+
 MPD_MPC_LICENSE_FILES = COPYING
-MPD_MPC_DEPENDENCIES = host-meson host-pkgconf libmpdclient
+MPD_MPC_DEPENDENCIES = host-pkgconf libmpdclient
 
-MPD_MPC_CONF_OPTS += \
-	--prefix=/usr \
-	--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
-	--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf
-
-MPD_MPC_NINJA_OPTS = $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
-
-define MPD_MPC_CONFIGURE_CMDS
-	rm -rf $(@D)/build
-	mkdir -p $(@D)/build
-	$(TARGET_MAKE_ENV) meson $(MPD_MPC_CONF_OPTS) $(@D) $(@D)/build
-endef
-
-define MPD_MPC_BUILD_CMDS
-	$(TARGET_MAKE_ENV) ninja $(MPD_MPC_NINJA_OPTS) -C $(@D)/build
-endef
-
-define MPD_MPC_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) ninja $(MPD_MPC_NINJA_OPTS) \
-		-C $(@D)/build install
-endef
-
-$(eval $(generic-package))
+$(eval $(meson-package))
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 7/7] enlightenment: convert to pkg-meson infra
  2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
                   ` (5 preceding siblings ...)
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 6/7] mpd-mpc: " Eric Le Bihan
@ 2018-05-07 10:57 ` Eric Le Bihan
  6 siblings, 0 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 10:57 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/enlightenment/enlightenment.mk | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/package/enlightenment/enlightenment.mk b/package/enlightenment/enlightenment.mk
index 480e9e9da8..5bc1c5d6de 100644
--- a/package/enlightenment/enlightenment.mk
+++ b/package/enlightenment/enlightenment.mk
@@ -13,14 +13,10 @@ ENLIGHTENMENT_LICENSE_FILES = COPYING
 ENLIGHTENMENT_DEPENDENCIES = \
 	host-pkgconf \
 	host-efl \
-	host-meson \
 	efl \
 	xcb-util-keysyms
 
-ENLIGHTENMENT_MESON_OPTS += \
-	--prefix=/usr \
-	--buildtype=$(if $(BR2_ENABLE_DEBUG),debug,release) \
-	--cross-file=$(HOST_DIR)/etc/meson/cross-compilation.conf \
+ENLIGHTENMENT_CONF_OPTS = \
 	-Dedje-cc=$(HOST_DIR)/bin/edje_cc \
 	-Deet=$(HOST_DIR)/bin/eet \
 	-Deldbus-codegen=$(HOST_DIR)/bin/eldbus-codegen \
@@ -30,38 +26,24 @@ ENLIGHTENMENT_MESON_OPTS += \
 ENLIGHTENMENT_INSTALL_STAGING = YES
 
 ifeq ($(BR2_PACKAGE_SYSTEMD),y)
-ENLIGHTENMENT_MESON_OPTS += -Dsystemd=true
+ENLIGHTENMENT_CONF_OPTS += -Dsystemd=true
 ENLIGHTENMENT_DEPENDENCIES += systemd
 else
-ENLIGHTENMENT_MESON_OPTS += -Dsystemd=false
+ENLIGHTENMENT_CONF_OPTS += -Dsystemd=false
 endif
 
 # alsa backend needs mixer support
 ifeq ($(BR2_PACKAGE_ALSA_LIB)$(BR2_PACKAGE_ALSA_LIB_MIXER),yy)
-ENLIGHTENMENT_MESON_OPTS += -Dmixer=true
+ENLIGHTENMENT_CONF_OPTS += -Dmixer=true
 ENLIGHTENMENT_DEPENDENCIES += alsa-lib
 else
-ENLIGHTENMENT_MESON_OPTS += -Dmixer=false
+ENLIGHTENMENT_CONF_OPTS += -Dmixer=false
 endif
 
 ifeq ($(BR2_PACKAGE_XKEYBOARD_CONFIG),y)
 ENLIGHTENMENT_DEPENDENCIES += xkeyboard-config
 endif
 
-define ENLIGHTENMENT_CONFIGURE_CMDS
-	rm -rf $(@D)/build
-	mkdir -p $(@D)/build
-	$(TARGET_MAKE_ENV) meson $(ENLIGHTENMENT_MESON_OPTS) $(@D) $(@D)/build
-endef
-
-define ENLIGHTENMENT_BUILD_CMDS
-	$(TARGET_MAKE_ENV) ninja -C $(@D)/build
-endef
-
-define ENLIGHTENMENT_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) ninja -C $(@D)/build install
-endef
-
 define ENLIGHTENMENT_REMOVE_DOCUMENTATION
 	rm -rf $(TARGET_DIR)/usr/share/enlightenment/doc/
 	rm -f $(TARGET_DIR)/usr/share/enlightenment/COPYING
@@ -69,4 +51,4 @@ define ENLIGHTENMENT_REMOVE_DOCUMENTATION
 endef
 ENLIGHTENMENT_POST_INSTALL_TARGET_HOOKS += ENLIGHTENMENT_REMOVE_DOCUMENTATION
 
-$(eval $(generic-package))
+$(eval $(meson-package))
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure Eric Le Bihan
@ 2018-05-07 15:02   ` Thomas Petazzoni
  2018-05-07 19:38     ` Eric Le Bihan
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2018-05-07 15:02 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon,  7 May 2018 12:57:35 +0200, Eric Le Bihan wrote:
> Add a new infrastructure to ease the development of packages that use Meson as
> their build system.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>

Thanks for working on this!

> +define inner-meson-package
> +
> +$(2)_CONF_ENV		?=
> +$(2)_CONF_OPTS		?=
> +$(2)_NINJA		= $(HOST)/bin/ninja

Does this one really needs to be a per-package variable ? It's defined
with "=" so it doesn't even allow a package to customize this value
(but I don't think it would be necessary for packages to customize this
value anyway).

> +$(2)_NINJA_ENV		?=
> +$(2)_NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)

Same question for this one.

> +define $(2)_CONFIGURE_CMDS
> +	rm -rf $$($$(PKG)_SRCDIR)/build
> +	mkdir -p $$($$(PKG)_SRCDIR)/build
> +	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
> +		--prefix=/usr \
> +		--libdir=/usr/lib \
> +		--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
> +		--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
> +		--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \

So meson is really not consistent, and some options are "--name=value",
and some others are "--name value" ?

> +#
> +# Host installation step. Only define it if not already defined by the
> +# package .mk file.
> +#
> +ifndef $(2)_INSTALL_CMDS
> +define $(2)_INSTALL_CMDS
> +	$$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(HOST_DIR) \

DESTDIR=$$(HOST_DIR) is not correct for host installations, since you
already build for prefix=$(HOST_DIR).

Other than those questions/issues, looks good to me overall.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra
  2018-05-07 10:57 ` [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra Eric Le Bihan
@ 2018-05-07 15:08   ` Thomas Petazzoni
  2018-05-07 19:58     ` Eric Le Bihan
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2018-05-07 15:08 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon,  7 May 2018 12:57:36 +0200, Eric Le Bihan wrote:
> Update documentation about adding meson-based packages with instructions for
> using pkg-meson infrastructure.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>

Looks good overall.


> -47: $(eval $(generic-package))
> +16: ifeq ($(BR2_PACKAGE_BAZ),y)
> +17: FOO_CONF_OPTS += -Dbaz
> +18: endif

I know it wasn't in the example previously, but would it make sense to
add:

FOO_DEPENDENCIES += baz

inside this condition ? It is quite common that external packages
enabling features for a given package are build dependencies of that
package.

> +==== +meson-package+ reference
> +
> +The main macro of the Meson package infrastructure is +meson-package+. It is
> +similar to the +generic-package+ macro.
> +
> +Just like the generic infrastructure, the Meson infrastructure works by defining
> +a number of variables before calling the +meson-package+ macro.
> +
> +First, all the package metadata information variables that exist in the generic
> +infrastructure also exist in the Meson infrastructure: +FOO_VERSION+,
> ++FOO_SOURCE+, +FOO_PATCH+, +FOO_SITE+, +FOO_SUBDIR+, +FOO_DEPENDENCIES+,
> ++FOO_INSTALL_STAGING+, +FOO_INSTALL_TARGET+.
> +
> +A few additional variables, specific to the Meson infrastructure, can also be
> +defined. Many of them are only useful in very specific cases, typical packages
> +will therefore only use a few of them.
> +
> +* +FOO_CONF_ENV+, to specify additional environment variables to pass to
> +  +meson+ for the configuration step. By default, empty.
> +
> +* +FOO_CONF_OPTS+, to specify additional options to pass to +meson+ for the
> +  configuration step. By default, empty.
> +
> +* +FOO_NINJA_ENV+, to specify additional environment variables to pass to
> +  +ninja+, meson companion tool in charge of the build operations. By default,
> +  empty.

Regarding the variable naming, I'm wondering if FOO_CONF_{ENV,OPTS} and
FOO_NINJA_ENV is very consistent.

Indeed, either you name the options after the step to which they apply:
FOO_CONF_ENV, FOO_CONF_OPTS, FOO_BUILD_ENV. Or after the tool to which
they apply: FOO_MESON_ENV, FOO_MESON_OPTS, FOO_NINJA_ENV.

But:

 - We already use this inconsistent notation: CONF_ENV and MAKE_ENV for
   autotools packages

 - Ninja options are not only used at build time, but also at install
   time.

 - Renaming FOO_CONF_OPTS to FOO_MESON_OPTS would really be weird, and
   FOO_CONF_OPTS definitely makes a lot more sense.

Conclusion: don't change anything, your current naming is my preference.

However, I think you forgot to mention the existence of a host variant
for Meson packages. The other package infrastructures say: "The ability
to have target and host packages is also available, with the
host-cmake-package macro."

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-07 15:02   ` Thomas Petazzoni
@ 2018-05-07 19:38     ` Eric Le Bihan
  2018-05-07 19:43       ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 19:38 UTC (permalink / raw)
  To: buildroot

Hi!

On 18-05-07 17:02:56, Thomas Petazzoni wrote:
> Hello,
>
> On Mon,  7 May 2018 12:57:35 +0200, Eric Le Bihan wrote:
> > Add a new infrastructure to ease the development of packages that use Meson as
> > their build system.
> >
> > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
>
> Thanks for working on this!
>
> > +define inner-meson-package
> > +
> > +$(2)_CONF_ENV		?=
> > +$(2)_CONF_OPTS		?=
> > +$(2)_NINJA		= $(HOST)/bin/ninja
>
> Does this one really needs to be a per-package variable ? It's defined
> with "=" so it doesn't even allow a package to customize this value
> (but I don't think it would be necessary for packages to customize this
> value anyway).
>
> > +$(2)_NINJA_ENV		?=
> > +$(2)_NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
>
> Same question for this one.

I used variables for ninja executable path and options to avoid
cluttering the *_CMDS parts. These are, indeed, not meant to be
customized. So, I'll drop them.

> > +define $(2)_CONFIGURE_CMDS
> > +	rm -rf $$($$(PKG)_SRCDIR)/build
> > +	mkdir -p $$($$(PKG)_SRCDIR)/build
> > +	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
> > +		--prefix=/usr \
> > +		--libdir=/usr/lib \
> > +		--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
> > +		--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
> > +		--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \
>
> So meson is really not consistent, and some options are "--name=value",
> and some others are "--name value" ?

No. Both patterns are valid for any option. I'll use "--name=value" for
consistency.

> > +#
> > +# Host installation step. Only define it if not already defined by the
> > +# package .mk file.
> > +#
> > +ifndef $(2)_INSTALL_CMDS
> > +define $(2)_INSTALL_CMDS
> > +	$$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(HOST_DIR) \
>
> DESTDIR=$$(HOST_DIR) is not correct for host installations, since you
> already build for prefix=$(HOST_DIR).

OK, I'll drop it.

> Other than those questions/issues, looks good to me overall.

Thanks for the review.

Regards,

--
ELB

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-07 19:38     ` Eric Le Bihan
@ 2018-05-07 19:43       ` Thomas Petazzoni
  2018-05-07 22:12         ` Eric Le Bihan
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2018-05-07 19:43 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 7 May 2018 21:38:50 +0200, Eric Le Bihan wrote:

> > Does this one really needs to be a per-package variable ? It's defined
> > with "=" so it doesn't even allow a package to customize this value
> > (but I don't think it would be necessary for packages to customize this
> > value anyway).
> >  
> > > +$(2)_NINJA_ENV		?=
> > > +$(2)_NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)  
> >
> > Same question for this one.  
> 
> I used variables for ninja executable path and options to avoid
> cluttering the *_CMDS parts. These are, indeed, not meant to be
> customized. So, I'll drop them.

You can use variables, but global ones, not per-package ones.

For example

NINJA = $(HOST_DIR)/bin/ninja
NINJA_OPTS = $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)

or something like that.

> > > +define $(2)_CONFIGURE_CMDS
> > > +	rm -rf $$($$(PKG)_SRCDIR)/build
> > > +	mkdir -p $$($$(PKG)_SRCDIR)/build
> > > +	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
> > > +		--prefix=/usr \
> > > +		--libdir=/usr/lib \
> > > +		--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
> > > +		--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
> > > +		--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \  
> >
> > So meson is really not consistent, and some options are "--name=value",
> > and some others are "--name value" ?  
> 
> No. Both patterns are valid for any option. I'll use "--name=value" for
> consistency.

It would be great to use --name=value everywhere then, including in the
packages using the meson-package infrastructure. This should perhaps be
noted in the manual, when describing the <pkg>_CONF_OPTS variable.

> > DESTDIR=$$(HOST_DIR) is not correct for host installations, since you
> > already build for prefix=$(HOST_DIR).  
> 
> OK, I'll drop it.

OK. Did you test the host variant of the meson-package infrastructure ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra
  2018-05-07 15:08   ` Thomas Petazzoni
@ 2018-05-07 19:58     ` Eric Le Bihan
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 19:58 UTC (permalink / raw)
  To: buildroot

Hi!

On 18-05-07 17:08:55, Thomas Petazzoni wrote:
> Hello,
>
> On Mon,  7 May 2018 12:57:36 +0200, Eric Le Bihan wrote:
> > Update documentation about adding meson-based packages with instructions for
> > using pkg-meson infrastructure.
> >
> > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
>
> Looks good overall.
>
>
> > -47: $(eval $(generic-package))
> > +16: ifeq ($(BR2_PACKAGE_BAZ),y)
> > +17: FOO_CONF_OPTS += -Dbaz
> > +18: endif
>
> I know it wasn't in the example previously, but would it make sense to
> add:
>
> FOO_DEPENDENCIES += baz
>
> inside this condition ? It is quite common that external packages
> enabling features for a given package are build dependencies of that
> package.

This sounds sensible. I'll add it.

> > +==== +meson-package+ reference
> > +
> > +The main macro of the Meson package infrastructure is +meson-package+. It is
> > +similar to the +generic-package+ macro.
> > +
> > +Just like the generic infrastructure, the Meson infrastructure works by defining
> > +a number of variables before calling the +meson-package+ macro.
> > +
> > +First, all the package metadata information variables that exist in the generic
> > +infrastructure also exist in the Meson infrastructure: +FOO_VERSION+,
> > ++FOO_SOURCE+, +FOO_PATCH+, +FOO_SITE+, +FOO_SUBDIR+, +FOO_DEPENDENCIES+,
> > ++FOO_INSTALL_STAGING+, +FOO_INSTALL_TARGET+.
> > +
> > +A few additional variables, specific to the Meson infrastructure, can also be
> > +defined. Many of them are only useful in very specific cases, typical packages
> > +will therefore only use a few of them.
> > +
> > +* +FOO_CONF_ENV+, to specify additional environment variables to pass to
> > +  +meson+ for the configuration step. By default, empty.
> > +
> > +* +FOO_CONF_OPTS+, to specify additional options to pass to +meson+ for the
> > +  configuration step. By default, empty.
> > +
> > +* +FOO_NINJA_ENV+, to specify additional environment variables to pass to
> > +  +ninja+, meson companion tool in charge of the build operations. By default,
> > +  empty.
>
> Regarding the variable naming, I'm wondering if FOO_CONF_{ENV,OPTS} and
> FOO_NINJA_ENV is very consistent.
>
> Indeed, either you name the options after the step to which they apply:
> FOO_CONF_ENV, FOO_CONF_OPTS, FOO_BUILD_ENV. Or after the tool to which
> they apply: FOO_MESON_ENV, FOO_MESON_OPTS, FOO_NINJA_ENV.
>
> But:
>
>  - We already use this inconsistent notation: CONF_ENV and MAKE_ENV for
>    autotools packages
>
>  - Ninja options are not only used at build time, but also at install
>    time.
>
>  - Renaming FOO_CONF_OPTS to FOO_MESON_OPTS would really be weird, and
>    FOO_CONF_OPTS definitely makes a lot more sense.
>
> Conclusion: don't change anything, your current naming is my preference.

The need for FOO_NINJA_ENV comes from the systemd package, because of
some UTF-8 stuff. IHMO, it will not be used frequently in other
Meson-based packages.

And everybody is used to FOO_CONF_OPTS!

> However, I think you forgot to mention the existence of a host variant
> for Meson packages. The other package infrastructures say: "The ability
> to have target and host packages is also available, with the
> host-cmake-package macro."

I will add this.

Thanks for the review.

Regards,

--
ELB

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-07 19:43       ` Thomas Petazzoni
@ 2018-05-07 22:12         ` Eric Le Bihan
  2018-05-08 21:00           ` Arnout Vandecappelle
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-07 22:12 UTC (permalink / raw)
  To: buildroot

On 18-05-07 21:43:56, Thomas Petazzoni wrote:
> Hello,
>
> On Mon, 7 May 2018 21:38:50 +0200, Eric Le Bihan wrote:
>
> > > Does this one really needs to be a per-package variable ? It's defined
> > > with "=" so it doesn't even allow a package to customize this value
> > > (but I don't think it would be necessary for packages to customize this
> > > value anyway).
> > >
> > > > +$(2)_NINJA_ENV		?=
> > > > +$(2)_NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
> > >
> > > Same question for this one.
> >
> > I used variables for ninja executable path and options to avoid
> > cluttering the *_CMDS parts. These are, indeed, not meant to be
> > customized. So, I'll drop them.
>
> You can use variables, but global ones, not per-package ones.
>
> For example
>
> NINJA = $(HOST_DIR)/bin/ninja
> NINJA_OPTS = $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
>
> or something like that.

OK.

> > > > +define $(2)_CONFIGURE_CMDS
> > > > +	rm -rf $$($$(PKG)_SRCDIR)/build
> > > > +	mkdir -p $$($$(PKG)_SRCDIR)/build
> > > > +	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
> > > > +		--prefix=/usr \
> > > > +		--libdir=/usr/lib \
> > > > +		--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
> > > > +		--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
> > > > +		--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \
> > >
> > > So meson is really not consistent, and some options are "--name=value",
> > > and some others are "--name value" ?
> >
> > No. Both patterns are valid for any option. I'll use "--name=value" for
> > consistency.
>
> It would be great to use --name=value everywhere then, including in the
> packages using the meson-package infrastructure. This should perhaps be
> noted in the manual, when describing the <pkg>_CONF_OPTS variable.

The --name=value pattern is used for options related to Meson behavior
and general project configuration, whereas the -Dname=value one is used
for project feature activation/customization. So <pkg>_CONF_OPTS should
most of the time contains elements using the latter (except when
overriding sysconfdir or such locations).

In case of boolean options, Meson assumes that "-Dbaz" means
"-Dbaz=true". Should the example in the infrastructure documentation
look like this:

```
ifeq ($(BR2_PACKAGE_BAZ),y)
FOO_CONF_OPTS += -Dbaz=true
FOO_DEPENDENCIES += baz
else
FOO_CONF_OPTS += -Dbaz=false
endif
```

Or like this:

```
ifeq ($(BR2_PACKAGE_BAZ),y)
FOO_CONF_OPTS += -Dbaz
FOO_DEPENDENCIES += baz
endif
```

> > > DESTDIR=$$(HOST_DIR) is not correct for host installations, since you
> > > already build for prefix=$(HOST_DIR).
> >
> > OK, I'll drop it.
>
> OK. Did you test the host variant of the meson-package infrastructure ?

Good advice! Testing the host variant of a custom library+executable
project brought out that Meson does not automically sets RPATH [1],
which makes support/scripts/check-host-rpath unhappy, and that it
defaults --libdir to "lib64" instead of "lib" [2]. So I'll fix the host
variant accordingly.

[1] http://mesonbuild.com/Reference-manual.html#executable
[2] http://mesonbuild.com/Reference-manual.html#get_option

Regards,

--
ELB

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-07 22:12         ` Eric Le Bihan
@ 2018-05-08 21:00           ` Arnout Vandecappelle
  2018-05-09 22:41             ` Eric Le Bihan
  0 siblings, 1 reply; 17+ messages in thread
From: Arnout Vandecappelle @ 2018-05-08 21:00 UTC (permalink / raw)
  To: buildroot



On 08-05-18 00:12, Eric Le Bihan wrote:
> On 18-05-07 21:43:56, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Mon, 7 May 2018 21:38:50 +0200, Eric Le Bihan wrote:
>>

[snip]
>>>>> +	PATH=$$(BR_PATH) $$($$(PKG)_CONF_ENV) meson \
>>>>> +		--prefix=/usr \
>>>>> +		--libdir=/usr/lib \
>>>>> +		--default-library $(if $(BR2_STATIC_LIBS),static,shared) \
>>>>> +		--buildtype $(if $(BR2_ENABLE_DEBUG),debug,release) \
>>>>> +		--cross-file $(HOST_DIR)/etc/meson/cross-compilation.conf \
>>>>
>>>> So meson is really not consistent, and some options are "--name=value",
>>>> and some others are "--name value" ?
>>>
>>> No. Both patterns are valid for any option. I'll use "--name=value" for
>>> consistency.
>>
>> It would be great to use --name=value everywhere then, including in the
>> packages using the meson-package infrastructure. This should perhaps be
>> noted in the manual, when describing the <pkg>_CONF_OPTS variable.
> 
> The --name=value pattern is used for options related to Meson behavior
> and general project configuration, whereas the -Dname=value one is used

 No, it's about --name=value or --name value.

> for project feature activation/customization. So <pkg>_CONF_OPTS should
> most of the time contains elements using the latter (except when
> overriding sysconfdir or such locations).
> 
> In case of boolean options, Meson assumes that "-Dbaz" means
> "-Dbaz=true". Should the example in the infrastructure documentation

 We prefer explicit, so -Dbaz=true

> look like this:
> 
> ```
> ifeq ($(BR2_PACKAGE_BAZ),y)
> FOO_CONF_OPTS += -Dbaz=true
> FOO_DEPENDENCIES += baz
> else
> FOO_CONF_OPTS += -Dbaz=false
> endif

 Yes.

> ```
> 
> Or like this:
> 
> ```
> ifeq ($(BR2_PACKAGE_BAZ),y)
> FOO_CONF_OPTS += -Dbaz
> FOO_DEPENDENCIES += baz
> endif

 No. We certainly want the -Dbaz=false option (I guess in meson there is also a
possibility to let an option default to true, right?). So for symmetry it's
better to do -Dbaz=true as well then.

> ```
> 
>>>> DESTDIR=$$(HOST_DIR) is not correct for host installations, since you
>>>> already build for prefix=$(HOST_DIR).
>>>
>>> OK, I'll drop it.
>>
>> OK. Did you test the host variant of the meson-package infrastructure ?
> 
> Good advice! Testing the host variant of a custom library+executable
> project brought out that Meson does not automically sets RPATH [1],
> which makes support/scripts/check-host-rpath unhappy, and that it

 If that is the case, it means you don't properly pass HOST_LDFLAGS, which is A
Bad Thing (tm).

 Indeed, looking at your patch, I see none of the host options are passed. You
certainly need to pass HOST_MAKE_ENV in the environment to make pkg-config is
used correctly. And then you should also make sure that HOSTCC etc. are passed
correctly - remember that it's possible to use a different host compiler by
passing HOSTCC in the environment (and I'm actually using that feature!).

 Regards,
 Arnout

> defaults --libdir to "lib64" instead of "lib" [2]. So I'll fix the host
> variant accordingly.
> 
> [1] http://mesonbuild.com/Reference-manual.html#executable
> [2] http://mesonbuild.com/Reference-manual.html#get_option
> 
> Regards,
> 
> --
> ELB
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-08 21:00           ` Arnout Vandecappelle
@ 2018-05-09 22:41             ` Eric Le Bihan
  2018-05-10  8:40               ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Le Bihan @ 2018-05-09 22:41 UTC (permalink / raw)
  To: buildroot

Hi!
On 18-05-08 23:00:23, Arnout Vandecappelle wrote:

[snip]

> >> OK. Did you test the host variant of the meson-package infrastructure ?
> >
> > Good advice! Testing the host variant of a custom library+executable
> > project brought out that Meson does not automically sets RPATH [1],
> > which makes support/scripts/check-host-rpath unhappy, and that it
>
>  If that is the case, it means you don't properly pass HOST_LDFLAGS, which is A
> Bad Thing (tm).
>
>  Indeed, looking at your patch, I see none of the host options are passed. You
> certainly need to pass HOST_MAKE_ENV in the environment to make pkg-config is
> used correctly. And then you should also make sure that HOSTCC etc. are passed
> correctly - remember that it's possible to use a different host compiler by
> passing HOSTCC in the environment (and I'm actually using that feature!).

I added $$(HOST_CONFIGURE_OPTS), CFLAGS="$$(HOST_CFLAGS)" and
LDFLAGS="$$(HOST_LDFLAGS)" before invoking Meson (as done in
pkg-autotools.mk) and now it politely informs me that it will take these
flags into account:

```
Appending CFLAGS from environment: '-O2 -I/home/eric/build/test-meson-package/qemu/arm/vexpress/host/include'
Appending LDFLAGS from environment: '-L/home/eric/build/test-meson-package/qemu/arm/vexpress/host/lib -Wl,-rpath,/home/eric/build/test-meson-package/qemu/arm/vexpress/host/lib'
Appending CPPFLAGS from environment: '-I/home/eric/build/test-meson-package/qemu/arm/vexpress/host/include'
```

But as stated in issue 1411 [1], Meson *does strip* RPATH [2] when
installing the executable of a library+executable project [3] in
$(HOST_DIR)/bin:

```
$ readelf -a /home/eric/build/test-meson-package/qemu/arm/vexpress/build/host-hello-meson-custom/build/tools/hello-meson-tool | grep -e PATH
 0x000000000000000f (RPATH)              Library rpath: [/home/eric/build/test-meson-package/qemu/arm/vexpress/host/lib:$ORIGIN/../hello-meson]
$ readelf -a /home/eric/build/test-meson-package/qemu/arm/vexpress/host/bin/hello-meson-tool | grep -e PATH
```

The recommended workaround is to define install_rpath [4] in for the
executable target, but this requires patching the sources of the
Meson-based package when building its host variant, which does not sound
very sensible.

So it looks like the workaround used in NixOS [5], i.e. patching the
host variant of Meson itself to remove the RPATH strip step, is
preferable until upstream Meson settles on this topic. I'll send another
patch for the meson package where this step is done in a post-extract
hook of the host variant.

[1] https://github.com/mesonbuild/meson/issues/1411
[2] https://github.com/mesonbuild/meson/blob/dc91aad42009f739f9e6d73221efc071626c28a3/mesonbuild/scripts/meson_install.py#L396
[3] https://github.com/mesonbuild/meson/issues/314#issuecomment-157658562
[4] https://github.com/mesonbuild/meson/issues/314#issuecomment-326909424
[5] https://github.com/NixOS/nixpkgs/pull/28444#issuecomment-324033323

Regards,

--
ELB

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure
  2018-05-09 22:41             ` Eric Le Bihan
@ 2018-05-10  8:40               ` Thomas Petazzoni
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Petazzoni @ 2018-05-10  8:40 UTC (permalink / raw)
  To: buildroot

Hello Eric,

On Thu, 10 May 2018 00:41:28 +0200, Eric Le Bihan wrote:

> I added $$(HOST_CONFIGURE_OPTS), CFLAGS="$$(HOST_CFLAGS)" and
> LDFLAGS="$$(HOST_LDFLAGS)"

HOST_CONFIGURE_OPTS already contains CFLAGS=$(HOST_CFLAGS) and
LDFLAGS=$(HOST_LDFLAGS), so there is no need to use them in addition to
$(HOST_CONFIGURE_OPTS).

> before invoking Meson (as done in pkg-autotools.mk) and now it
> politely informs me that it will take these flags into account:

I don't understand why we are passing CFLAGS=$(HOST_CFLAGS) and
LDFLAGS=$(HOST_LDFLAGS) in pkg-autotools.mk, it seems totally useless
since they are already in $(HOST_CONFIGURE_OPTS).

They have been here in pkg-autotools.mk since we reworked the autotools
infrastructure on top of the generic package infrastructure, back in
2009. It's however later that CFLAGS and LDFLAGS have been added in
HOST_CONFIGURE_OPTS.

This should be fixed.

> The recommended workaround is to define install_rpath [4] in for the
> executable target, but this requires patching the sources of the
> Meson-based package when building its host variant, which does not sound
> very sensible.
> 
> So it looks like the workaround used in NixOS [5], i.e. patching the
> host variant of Meson itself to remove the RPATH strip step, is
> preferable until upstream Meson settles on this topic. I'll send another
> patch for the meson package where this step is done in a post-extract
> hook of the host variant.

Why not a patch against Meson itself ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-05-10  8:40 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 10:57 [Buildroot] [RFC PATCH 0/7] Add pkg-meson infrastructure Eric Le Bihan
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 1/7] pkg-meson: new infrastructure Eric Le Bihan
2018-05-07 15:02   ` Thomas Petazzoni
2018-05-07 19:38     ` Eric Le Bihan
2018-05-07 19:43       ` Thomas Petazzoni
2018-05-07 22:12         ` Eric Le Bihan
2018-05-08 21:00           ` Arnout Vandecappelle
2018-05-09 22:41             ` Eric Le Bihan
2018-05-10  8:40               ` Thomas Petazzoni
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra Eric Le Bihan
2018-05-07 15:08   ` Thomas Petazzoni
2018-05-07 19:58     ` Eric Le Bihan
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 3/7] libmpdclient: convert to " Eric Le Bihan
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 4/7] systemd: " Eric Le Bihan
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 5/7] ncmpc: " Eric Le Bihan
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 6/7] mpd-mpc: " Eric Le Bihan
2018-05-07 10:57 ` [Buildroot] [RFC PATCH 7/7] enlightenment: " Eric Le Bihan

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.