[Buildroot] [PATCH 2/3] core/pkg-utils: check hashes of license files
Luca Ceresoli
luca at lucaceresoli.net
Fri Jun 23 21:49:41 UTC 2017
Hi Yann,
On 18/06/2017 10:01, Yann E. MORIN wrote:
> This will help catch a change of license even if the filename does
> not change.
>
> For now, a missing hash for the license files is not a fatal error, to
> let people catch up and add them. When we switch to make it mandatory,
> we can simplify the code by just removing the case statement.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998 at free.fr>
> Cc: Luca Ceresoli <luca at lucaceresoli.net>
> Cc: Rahul Bedarkar <rahulbedarkar89 at gmail.com>
> Cc: Peter Korsgaard <peter at korsgaard.com>
> ---
> package/pkg-utils.mk | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index e9ac56276f..accf48c464 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -85,5 +85,10 @@ endef
>
> define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}
> mkdir -p $(LICENSE_FILES_DIR_$(6))/$(2)/$(dir $(4)) && \
> + { \
> + support/download/check-hash $(3)/$(1).hash $(5) $(4); \
> + ret=$${?}; \
> + case $${ret} in (0|3) ;; (*) exit 1;; esac; \
> + } && \
I generally agree with the idea, even more since the implementation is
really a few-liner. But I have a few remarks.
The first is one of personal coding style taste. I don't like very much
the weird check for values 0 and 3. It does not make any sense unless
one opens check-hash and read the possible return values, which look a
bit like a randomly-ordered enum.
I'd rather prefer if we could call check-hash with a command line flag
to ask that missing hashes generate a warning and return 0 instead of
the default behavior. This would mean check-hash always returns 0 for
"go" and non-zero for "abort".
The second remark is about the output of 'make legal-info'. With this
patch the output is:
$ make legal-info
>>> Collecting legal info
WARNING: no hash file for COPYING
WARNING: no hash file for COPYING
WARNING: no hash file for COPYING3
WARNING: no hash file for COPYING.LIB
WARNING: no hash file for COPYING.LESSERv3
WARNING: no hash file for COPYINGv2
...
There's no mention of the package involved, which doesn't help
in fixing it. It should print the package name, e.g.:
$ make legal-info
>>> Collecting legal info
WARNING: binutils: no hash file for COPYING
WARNING: mpc: no hash file for COPYING
...
To achieve this I guess we'll need to pass the package name to
check-hash. It doesn't hurt if we add the package name unconditionally,
even to currently existing messages where it is not needed.
--
Luca
More information about the buildroot
mailing list