All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1 of 7 v3] infra: consistently use double dollar signs inside inner-xxx-targets
Date: Wed, 11 Jun 2014 17:56:04 +0200	[thread overview]
Message-ID: <20140611175604.4e1ac2d2@free-electrons.com> (raw)
In-Reply-To: <CAAXf6LXgYEvcWKxFry8GR8sfC64K2iMEuQLZPBf9YWTXd-eVCA@mail.gmail.com>

Dear Thomas De Schampheleire,

On Sun, 8 Jun 2014 16:29:45 +0200, Thomas De Schampheleire wrote:

> In this case the (original) construct is:
> Target case:
>     FOO_BAR ?= $(FOO_BAR)
> 
> Host case:
>     HOST_FOO_BAR ?= $(FOO_BAR)
> 
> For the target case there would be a circular reference, and the
> statement doesn't make sense anyway.
> To solve this, an extra check $(ifeq $(4),host) is really needed.
> For the host case, if HOST_FOO_BAR is not yet set, it is set equal to
> the value of FOO_BAR. FOO_BAR may or may not be set previously: there
> is no default value at play here.
> 
> A very important thing to understand here, construct (a)
>     HOST_FOO_BAR ?= $(FOO_BAR)
> is not equivalent to (b)
>     ifndef HOST_FOO_BAR
>       HOST_FOO_BAR = $(FOO_BAR)
>     endif
> 
> because 'ifdef' checks for a *non-empty value* while '?=' checks for
> *set or not set*.
> Since the .mk can have a statement like:
>     FOO_PATCH = blaat.patch
>     HOST_FOO_PATCH =
> the second form (b) above will still set HOST_FOO_BAR to FOO_BAR which
> is not what we want.
> 
> 
> I hope the above is more clear to you, please let me know.
> 
> It is clear that the code is non-trivial, but I don't know if and how
> we should document all this.
> What do you think?
> Do also let me know if you expect changes in the patch or commit
> message to make any of this clear.

Thanks a lot for the detailed explanation! Definitely very useful, as
I now understand better the logic behind these additional ifeq
($(4),host) tests. We probably need to add more comments in
pkg-generic.mk, but that's clearly unrelated to your patch, and can be
handled later on.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2014-06-11 15:56 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 20:12 [Buildroot] [PATCH 0 of 7 v3] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
2014-06-06 20:12 ` [Buildroot] [PATCH 1 of 7 v3] infra: consistently use double dollar signs inside inner-xxx-targets Thomas De Schampheleire
2014-06-06 21:21   ` Yann E. MORIN
2014-06-08 14:09     ` Thomas De Schampheleire
2014-06-07  8:49   ` Thomas Petazzoni
2014-06-08 14:29     ` Thomas De Schampheleire
2014-06-11 15:56       ` Thomas Petazzoni [this message]
2014-06-06 20:12 ` [Buildroot] [PATCH 2 of 7 v3] infra: add comment describing single/double dollar-sign rules Thomas De Schampheleire
2014-06-06 21:23   ` Yann E. MORIN
2014-06-07  8:52   ` Thomas Petazzoni
2014-06-06 20:12 ` [Buildroot] [PATCH 3 of 7 v3] pkg-virtual: simplify definition of FOO_VERSION to 'virtual' Thomas De Schampheleire
2014-06-06 20:13 ` [Buildroot] [PATCH 4 of 7 v3] toolchain/toolchain-buildroot: migrate to virtual package infrastructure Thomas De Schampheleire
2014-06-07  8:56   ` Thomas Petazzoni
2014-06-06 20:13 ` [Buildroot] [PATCH 5 of 7 v3] toolchain-external: change version from 'undefined' to 'virtual' Thomas De Schampheleire
2014-06-07  8:57   ` Thomas Petazzoni
2014-06-08 15:04     ` Thomas De Schampheleire
2014-06-08 16:10       ` Thomas Petazzoni
2014-06-08 17:23         ` Thomas De Schampheleire
2014-06-16  5:17           ` Arnout Vandecappelle
2014-06-16  6:54             ` Thomas De Schampheleire
2014-06-16  7:18           ` Thomas Petazzoni
2014-06-06 20:13 ` [Buildroot] [PATCH 6 of 7 v3] makedevs: change version from 'undefined' to 'buildroot-$(BR2_VERSION)' Thomas De Schampheleire
2014-06-06 20:13 ` [Buildroot] [PATCH 7 of 7 v3] mkpasswd: " Thomas De Schampheleire

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=20140611175604.4e1ac2d2@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.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.