All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra
Date: Mon, 7 May 2018 17:08:55 +0200	[thread overview]
Message-ID: <20180507170855.3bf884fa@windsurf> (raw)
In-Reply-To: <20180507105741.31747-3-eric.le.bihan.dev@free.fr>

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

  reply	other threads:[~2018-05-07 15:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

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=20180507170855.3bf884fa@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=buildroot@busybox.net \
    /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.