[Buildroot] [PATCH] powerpc-utils: Bump powerpc-utils to v1.3.0

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Hello,

On Tue, 29 Mar 2016 17:41:07 +0530, Vasant Hegde wrote:
> This patch makes below changes to powerpc-utils package:
>   - Update to latest upstream version (v1.3.0)
>   - Update License (from CPL to GPLv2)
>   - Update source link (from SF to github)
>   - Disable librtas by default
>   - Finally make necessary adjustment to compile the source
>     (run autogen.sh before ./configure as we don't have configure in new tarball).
> 
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>

Thanks for this patch! It looks mostly good, but there are a few issues
here and there. Read on below.

> @@ -18,7 +18,7 @@ config BR2_PACKAGE_POWERPC_UTILS_RTAS
>  	bool "RTAS support"
>  	select BR2_PACKAGE_LIBRTAS
>  	depends on BR2_TOOLCHAIN_USES_GLIBC
> -	default y
> +	default n

Then just remove this line, because "disabled" in the default state for
an option. BTW, your commit log just says that you disable it by
default, but not why. What is the reasoning? Not that I personally care
much about powerpc-utils and specifically its rtas support, but it
might be useful to have a short explanation about this change.

> diff --git a/package/powerpc-utils/powerpc-utils.mk b/package/powerpc-utils/powerpc-utils.mk
> index ae4d662..61b0fbe 100644
> --- a/package/powerpc-utils/powerpc-utils.mk
> +++ b/package/powerpc-utils/powerpc-utils.mk
> @@ -4,15 +4,21 @@
>  #
>  ################################################################################
>  
> -POWERPC_UTILS_VERSION = 1.2.24
> -POWERPC_UTILS_SITE = http://downloads.sourceforge.net/project/powerpc-utils/powerpc-utils
> +POWERPC_UTILS_VERSION = 1.3.0
> +POWERPC_UTILS_SITE = https://github.com/nfont/powerpc-utils/archive/

Since the tarballs are not uploaded by the developer, but directly
generated by Github, please use the "github" macro in Buildroot. Check
in other packages and in the Buildroot manual for details on how to use
it.

> +POWERPC_UTILS_SOURCE = v$(POWERPC_UTILS_VERSION).tar.gz
>  POWERPC_UTILS_DEPENDENCIES = zlib
> -POWERPC_UTILS_LICENSE = Common Public License Version 1.0
> -POWERPC_UTILS_LICENSE_FILES = COPYRIGHT
> +POWERPC_UTILS_LICENSE = GPLv2

Just to verify: did you make sure it's actually GPLv2 and not GPLv2+ ?

> +POWERPC_UTILS_LICENSE_FILES = COPYING
>  
>  POWERPC_UTILS_CONF_ENV = \
>  	ax_cv_check_cflags___fstack_protector_all=$(if $(BR2_TOOLCHAIN_HAS_SSP),yes,no)
>  
> +define POWERPC_UTILS_AUTOGEN
> +	cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
> +endef
> +POWERPC_UTILS_PRE_CONFIGURE_HOOKS += POWERPC_UTILS_AUTOGEN

This is not good, because you don't depend on host-autoconf,
host-automake and host-libtool. Instead, Buildroot has a built-in
mechanism to regenerate the automake/autoconf stuff. Just do:

POWERPC_UTILS_AUTORECONF = YES

This works in 99% of the cases. There are a few cases where additional
tweaks are needed, because the autogen.sh does funky non-standard stuff.

Could you rework your patch to take into account those comments and
send an updated version?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com