[Buildroot] [PATCH] package/pkg-utils: add 'hash_files' to show-info

[Buildroot] [PATCH] package/pkg-utils: add 'hash_files' to show-info

[Brandon Maier via buildroot]

Signed-off-by: Brandon Maier <brandon.maier@collins.com>
---
 package/pkg-utils.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index d1964299af..8970a2a8b9 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -176,6 +176,13 @@ define _json-info-pkg-details
 		},
 	)
 	],
+	"hash_files": [
+		$(foreach f, $($(1)_HASH_FILES),$(call mk-json-str,$(f))$(comma))
+	],
+	"no_check_hash_for": \
+		$(if $(BR2_DOWNLOAD_FORCE_CHECK_HASHES), \
+			false, \
+			$(if $(filter $($(1)_SOURCE),$(BR_NO_CHECK_HASH_FOR)),true,false)),
 endef
 
 define _json-info-fs
-- 
2.45.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/pkg-utils: add 'hash_files' to show-info

[Thomas Petazzoni via buildroot]

Hello Brandon,

Thanks for your patch! Questions/comments below.

On Tue, 28 May 2024 15:29:48 +0000
Brandon Maier via buildroot <buildroot@buildroot.org> wrote:

> Signed-off-by: Brandon Maier <brandon.maier@collins.com>

Could you expand a bit the commit log to describe the motivation for
this change? What are the use-cases?

> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index d1964299af..8970a2a8b9 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -176,6 +176,13 @@ define _json-info-pkg-details
>  		},
>  	)
>  	],
> +	"hash_files": [
> +		$(foreach f, $($(1)_HASH_FILES),$(call mk-json-str,$(f))$(comma))
> +	],

One thing that isn't that "useful" here is that this is going to list
*all* hash files for that package, not the one hash file that contained
the appropriate hash used to check the package (if there are multiple
hash files).

> +	"no_check_hash_for": \
> +		$(if $(BR2_DOWNLOAD_FORCE_CHECK_HASHES), \
> +			false, \
> +			$(if $(filter $($(1)_SOURCE),$(BR_NO_CHECK_HASH_FOR)),true,false)),

Could we have positive logic instead, such as "hash-checked": true?

Also, here you're basing the value of this property solely on whether
<pkg>_SOURCE was hash-checked. But what about <pkg>_PATCH and
<pkg>_EXTRA_DOWNLOADS? So basically this should be true only if all
files in <pkg>_ALL_DOWNLOADS have been hash-checked.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/pkg-utils: add 'hash_files' to show-info

[Brandon Maier via buildroot]

Hi Thomas,

On Sat Jul 13, 2024 at 5:10 PM UTC, Thomas Petazzoni via buildroot wrote:
> Hello Brandon,
>
> Thanks for your patch! Questions/comments below.
>
> On Tue, 28 May 2024 15:29:48 +0000
> Brandon Maier via buildroot <buildroot@buildroot.org> wrote:
>
> > Signed-off-by: Brandon Maier <brandon.maier@collins.com>
>
> Could you expand a bit the commit log to describe the motivation for
> this change? What are the use-cases?

Sure, I was working on utils/add-custom-hashes and it extracts all the
info it needs using `make show-info` in JSON. Except the TOPDIR, DL_DIR,
BR_NO_CHECK_HASH_FOR, and BR2_GLOBAL_PATCH_DIR. I wanted to expose more
of those values through `make show-info` as the JSON is easier to work
with.

BR2_GLOBAL_PATCH_DIR is only needed to infer where each package stores
its hash files. So by printing the "hash_files" for each package we get
the BR2_GLOBAL_PATCH_DIR and we also detect any other hash files in use
like the $(TOPDIR)/package/<pkg>/<pkg>.hash

>
> > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> > index d1964299af..8970a2a8b9 100644
> > --- a/package/pkg-utils.mk
> > +++ b/package/pkg-utils.mk
> > @@ -176,6 +176,13 @@ define _json-info-pkg-details
> >  		},
> >  	)
> >  	],
> > +	"hash_files": [
> > +		$(foreach f, $($(1)_HASH_FILES),$(call mk-json-str,$(f))$(comma))
> > +	],
>
> One thing that isn't that "useful" here is that this is going to list
> *all* hash files for that package, not the one hash file that contained
> the appropriate hash used to check the package (if there are multiple
> hash files).

Right, it is more difficult to get that. As the Makefile doesn't
actually do that hash checking, it's handled by
support/download/check-hash.

It may also be desirable to know all the hash lookup directories, for
example to write out a new file in the $BR2_GLOBAL_PATCH_DIR/<pkg>/ that
overrides the one in $TOPDIR/package/<pkg>/

>
> > +	"no_check_hash_for": \
> > +		$(if $(BR2_DOWNLOAD_FORCE_CHECK_HASHES), \
> > +			false, \
> > +			$(if $(filter $($(1)_SOURCE),$(BR_NO_CHECK_HASH_FOR)),true,false)),
>
> Could we have positive logic instead, such as "hash-checked": true?

I was trying to be consistent with BR_NO_CHECK_HASH_FOR, but it's easier
to understand in positive logic, so I will resend with that.

>
> Also, here you're basing the value of this property solely on whether
> <pkg>_SOURCE was hash-checked. But what about <pkg>_PATCH and
> <pkg>_EXTRA_DOWNLOADS? So basically this should be true only if all
> files in <pkg>_ALL_DOWNLOADS have been hash-checked.

I will need to look at this more. But I believe this 'check-hash' for
logic matches how BR_NO_CHECK_HASH_FOR works. I don't think it's
possible to override NO_CHECK_HASH_FOR for specific items in
_ALL_DOWNLOADS?

I will need to look at the above some more, will send a v2 with the
updated commit message and positive 'check-hash'.

Thanks,
Brandon

>
> Thanks!
>
> Thomas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot