Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Le Bihan <eric.le.bihan.dev@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [RFC PATCH 2/7] docs/manual: document pkg-meson infra
Date: Mon, 7 May 2018 21:58:08 +0200	[thread overview]
Message-ID: <20180507195808.GB15375@ned> (raw)
In-Reply-To: <20180507170855.3bf884fa@windsurf>

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

  reply	other threads:[~2018-05-07 19:58 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
2018-05-07 19:58     ` Eric Le Bihan [this message]
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=20180507195808.GB15375@ned \
    --to=eric.le.bihan.dev@free.fr \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox