| Message ID | 2e8ef15e2d2d729ffe1bb5f77aabeb525554c407.1428322317.git.yann.morin.1998@free.fr |
|---|---|
| State | Superseded |
| Headers | show |
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. So NACK for exporting it. 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)), -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 :-) Regards, Arnout > endif > > # In fact, we don't need to download the toolchain, since it is already >
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.
On 06/04/15 23:20, Yann E. MORIN wrote: > 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). Ah of course, that's probably why you introduced that. But it does mean that only a single file can ever be bypassed... > >> 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/ ? Untested and all :-) > >> -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); Ack that, though I don't immediately see a way to do that. > > - we pass obscur values (like a sha1), rather than the filenames, to > make it even less easy to use. I really don't see the point of that... If it's not documented, you have to look at the source code to find out which variable to set. And when you look at the source code, you immediately see how this obscure value is calculated so it makes no real difference. Regards, Arnout > > I'll respin a series taking into account your comments. Thanks! :-) > > Regards, > Yann E. MORIN. >
Arnout, All, On 2015-04-07 01:24 +0200, Arnout Vandecappelle spake thusly: > On 06/04/15 23:20, Yann E. MORIN wrote: > > On 2015-04-06 23:03 +0200, Arnout Vandecappelle spake thusly: > >> On 06/04/15 14:13, Yann E. MORIN wrote: [--SNIP--] > >>> +# 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). > > Ah of course, that's probably why you introduced that. > > But it does mean that only a single file can ever be bypassed... Right. [--SNIP--] > >> hasharg = $(if $(filter-out $(TOOLCHAIN_EXTERNAL_SOURCE),$(1)), > > You probably meant s/TOOLCHAIN_EXTERNAL_SOURCE/NOHASH_FILES/ ? > Untested and all :-) Hehe! :-) > >> -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); > Ack that, though I don't immediately see a way to do that. Don;t worry, I'll find a way... ;-] > > - we pass obscur values (like a sha1), rather than the filenames, to > > make it even less easy to use. > > I really don't see the point of that... If it's not documented, you have to > look at the source code to find out which variable to set. And when you look at > the source code, you immediately see how this obscure value is calculated so it > makes no real difference. OK, makes sense. I will rework the series accordingly. Thanks for the suggestions! Regards, Yann E. MORIN.
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)) endif # In fact, we don't need to download the toolchain, since it is already
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(+)