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
next prev parent 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 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.