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 5/6 v3] toolchain/external: ignore missing hash for custom downloaded toolchain
Date: Mon, 6 Apr 2015 23:20:01 +0200	[thread overview]
Message-ID: <20150406212001.GI4369@free.fr> (raw)
In-Reply-To: <5522F4AF.7000409@mind.be>

Arnout, All,

On 2015-04-06 23:03 +0200, Arnout Vandecappelle spake thusly:
> On 06/04/15 14:13, Yann E. MORIN wrote:
> > We will *always* be missing a hash file for custom external toolchains
> > that are downloaded.
> > 
> > So, just ignore that failure.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
> > 
> > ---
> > Changes v1 -> v2:
> >   - fix typoes in title
> > ---
> >  toolchain/toolchain-external/toolchain-external.mk | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> > index c0429bb..3659511 100644
> > --- a/toolchain/toolchain-external/toolchain-external.mk
> > +++ b/toolchain/toolchain-external/toolchain-external.mk
> > @@ -400,6 +400,8 @@ else
> >  # Custom toolchain
> >  TOOLCHAIN_EXTERNAL_SITE = $(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
> >  TOOLCHAIN_EXTERNAL_SOURCE = $(notdir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
> > +# Magic value to tell the download helper to avoid failling on missing hash
> > +export BR_MISSING_HASH_OK := $(call ignore-missing-hash-magic,$(TOOLCHAIN_EXTERNAL_SOURCE))
> 
>  Urgh - this disables the hash check for all packages, not just the toolchain.

No, that disable the check only for that one file, because we're not
chcking whether it is set, but whether it is set to the correct value
(i.e. the sha1 of the filename).

> So NACK for exporting it.

OK, this is indeed fishy.

>  But I see that it's not so simple to get it into the environment of the
> download step...
> 
> 
>  Perhaps instead we should add a global variable with the files that should not
> be hash-checked:
> 
> NOHASH_FILES += $(TOOLCHAIN_EXTERNAL_SOURCE)
> 
> and in pkg-download.mk
> 
> hasharg = $(if $(filter-out $(TOOLCHAIN_EXTERNAL_SOURCE),$(1)),

You probably meant s/TOOLCHAIN_EXTERNAL_SOURCE/NOHASH_FILES/ ?

> 	-H $(PKGDIR)/$($(PKG)_RAWNAME).hash)
> 
> define DOWNLOAD_WGET
>         $(EXTRA_ENV) $(DL_WRAPPER) -b wget \
>                 -o $(DL_DIR)/$(2) \
>                 $(call hasharg,$(2)) \
>                 $(QUIET) \
>                 -- \
>                 '$(call qstrip,$(1))'
> endef
> 
> (all completely untested, obviously :-)

Well, that sounds a bit better than my proposal.

Still, I prefer:

  - we offload the check in the dl-wrapper so we can do the check in a
    single place, rather than re-add extra code in each download macros
    (the dl-wrapper was added because the Makefile macros were too
    complex to handle);

  - we pass obscur values (like a sha1), rather than the filenames, to
    make it even less easy to use.

I'll respin a series taking into account your comments. Thanks! :-)

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-04-06 21:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-06 12:13 [Buildroot] [PATCH 0/6 v3] toolchain: better handle hashes (branch yem/dl-hash-toolchains) Yann E. MORIN
2015-04-06 12:13 ` [Buildroot] [PATCH 1/6 v3] support/download: do not warn about missing hash file for repositories Yann E. MORIN
2015-04-06 14:39   ` Arnout Vandecappelle
2015-04-06 21:29   ` Thomas Petazzoni
2015-04-06 12:13 ` [Buildroot] [PATCH 2/6 v3] package/gcc: add hashes Yann E. MORIN
2015-04-06 12:13 ` [Buildroot] [PATCH 3/6 v3] package/binutils: " Yann E. MORIN
2015-04-06 12:13 ` [Buildroot] [PATCH 4/6 v3] support/download: add possibility to not fail on missing hash Yann E. MORIN
2015-04-06 20:47   ` Arnout Vandecappelle
2015-04-06 12:13 ` [Buildroot] [PATCH 5/6 v3] toolchain/external: ignore missing hash for custom downloaded toolchain Yann E. MORIN
2015-04-06 21:03   ` Arnout Vandecappelle
2015-04-06 21:20     ` Yann E. MORIN [this message]
2015-04-06 23:24       ` Arnout Vandecappelle
2015-04-07 21:51         ` Yann E. MORIN
2015-04-06 12:13 ` [Buildroot] [PATCH 6/6 v3] support/download: restore mandatory check of hashes Yann E. MORIN
2015-04-06 21:05   ` Arnout Vandecappelle
2015-04-07 22:04     ` Yann E. MORIN

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=20150406212001.GI4369@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