Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 2/7] pkg-infra: assign BR_NO_CHECK_HASH_FOR so it is recursively-expanded
Date: Fri, 1 May 2015 17:26:48 +0200	[thread overview]
Message-ID: <20150501152648.GA4206@free.fr> (raw)
In-Reply-To: <55437A92.3060007@mind.be>

arnout, All,

On 2015-05-01 15:07 +0200, Arnout Vandecappelle spake thusly:
> On 29/04/15 00:39, Yann E. MORIN wrote:
> > Currently, assigning to BR_NO_CHECK_HASH_FOR in conditionals, but
> > referencing vairaible assigned to later, fails. Here's a failing
>               variable
> 
> > test-case, which is the reduced test-case of how we handle
> > BR_NO_CHECK_HASH_FOR for now:
> > 
> >     export FOO
> > 
> >     ifeq ($(BAR_V),)
> >     BAR_V=1
> >     else
> >     FOO += $(BAR_S)
> >     endif
> 
>  I think the test case is too complicated - in fact the conditional construct
> has nothing to do with it.

Yup, right, I know. However, I wanted a reduced test-case that still
ressembled our current code base.

> The culprit is appending to it, which means we never
> explicitly set it to recursively expanded. Exported variables that are not
> defined explicitly for some reason default to be immediately expanded rather
> than recursively.

Exactly. However, there's nothing like so explained in the make manual.

> export FOO
> FOO += $(BAR)
> $(warning FOO='$(FOO)' BAR='$(BAR)')
> BAR = BAR
> $(warning FOO='$(FOO)' BAR='$(BAR)')
> default:
> 
> Will give
> 
> /tmp/Makefile:3: FOO=' ' BAR=''
> /tmp/Makefile:5: FOO=' ' BAR='BAR'
> 
> while explicitly setting FOO to be recursively expanded:
> 
> export FOO =
> 
> will give
> 
> /tmp/Makefile:3: FOO=' ' BAR=''
> /tmp/Makefile:5: FOO=' BAR' BAR='BAR'

Do you prefere I use your even-more-simplified test-case, or may I keep
the reduced test-case I provided that ressemble our current code-path?

> > 
> >     BAR_S = bar-$(BAR_V).tar
> > 
> >     all:
> >         @echo "BAR_S=\"$(BAR_S)\""
> >         @$(SHELL) -c 'echo "FOO=\"$${FOO}\""'
> > 
> > Run it with:
> > 
> >     $ make
> >     BAR_S="bar-1.tar"
> >     FOO=""
> >     $ make BAR_V=2
> >     BAR_S="bar-2.tar"
> >     FOO=" "
> > 
> > Now, change the firstline to read:
> > 
> >     export FOO =
> > 
> > And we now get:
> > 
> >     $ make
> >     BAR_S="bar-1.tar"
> >     FOO=""
> >     $ make BAR_V=2
> >     BAR_S="bar-2.tar"
> >     FOO=" bar-2.tar"
> > 
> > This new behaviour will be needed later for Xenomai, which handles the
> > version string in an unusual way.
> > 
> > (Note how there's a leading space in both cases? In Buildroot, we do not
> > care that extra space, it is simply ignored).
> 
>  The leading space is there because += always inserts a space, even if the
> variable was empty before.

Yes, I know. I'll ditch that comment.

> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  package/pkg-download.mk | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index a38062e..f0651db 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -56,7 +56,9 @@ domainseparator = $(if $(1),$(1),/)
> >  github = https://github.com/$(1)/$(2)/archive/$(3)
> >  
> >  # Expressly do not check hashes for those files
> > -export BR_NO_CHECK_HASH_FOR
> > +# It needs to be assigned so as to be a recursively-expanded variable, and
> > +# so that it can be assigned inside conditionals.
> 
> # Exported variables default to immediately expanded,

I would avoid stating that. That's the behaviour we observed, but
nothing is said about that in the make manual. If a future version of
make changes that behaviour, the comment would be wrong.

However, I'll ditch the comment about conditionals.

> # but we need it to be
> # recursively-epxanded, so explicitly define it.
> 
> 
>  With all that,
> 
>   Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

  reply	other threads:[~2015-05-01 15:26 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-28 22:39 [Buildroot] [PATCH 0/7] hashes: add more custom versions to the exclusion list (branch yem/no-hash-custom) Yann E. MORIN
2015-04-28 22:39 ` [Buildroot] [PATCH 1/7] boot/gummiboot: switch to http for git clone Yann E. MORIN
2015-05-01  8:32   ` Thomas Petazzoni
2015-04-28 22:39 ` [Buildroot] [PATCH 2/7] pkg-infra: assign BR_NO_CHECK_HASH_FOR so it is recursively-expanded Yann E. MORIN
2015-05-01 13:07   ` Arnout Vandecappelle
2015-05-01 15:26     ` Yann E. MORIN [this message]
2015-05-01 15:45       ` Arnout Vandecappelle
2015-05-01 16:22         ` Yann E. MORIN
2015-04-28 22:40 ` [Buildroot] [PATCH 3/7] boot/uboot: do not check hash for custom versions Yann E. MORIN
2015-04-29  5:49   ` Vincent Stehlé
2015-05-01 13:14   ` Arnout Vandecappelle
2015-05-01 15:31     ` Yann E. MORIN
2015-05-01 15:48       ` Arnout Vandecappelle
2015-05-01 16:21         ` Yann E. MORIN
2015-04-28 22:40 ` [Buildroot] [PATCH 4/7] boot/barebox: " Yann E. MORIN
2015-05-01 13:15   ` Arnout Vandecappelle
2015-05-01 15:32     ` Yann E. MORIN
2015-05-01 15:52       ` Arnout Vandecappelle
2015-04-28 22:40 ` [Buildroot] [PATCH 5/7] boot/mxs-bootlets: do not check hash of custom tarball Yann E. MORIN
2015-05-01 13:25   ` Arnout Vandecappelle
2015-04-28 22:40 ` [Buildroot] [PATCH 6/7] linux: do not check hashes for custom versions and tarballs Yann E. MORIN
2015-05-01 13:34   ` Arnout Vandecappelle
2015-05-01 15:38     ` Yann E. MORIN
2015-05-01 15:50       ` Arnout Vandecappelle
2015-05-01 21:35     ` Yann E. MORIN
2015-04-28 22:40 ` [Buildroot] [PATCH 7/7] package/xenomai: ignore custom versions Yann E. MORIN
2015-05-01 13:37   ` Arnout Vandecappelle
2015-04-28 22:46 ` [Buildroot] [PATCH 8/7] package/xenomai: add hash file Yann E. MORIN
2015-04-29  4:16   ` Baruch Siach
2015-04-29  6:33     ` Yann E. MORIN
2015-05-01 13:54   ` Arnout Vandecappelle

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=20150501152648.GA4206@free.fr \
    --to=yann.morin.1998@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