From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Petazzoni Date: Tue, 29 Mar 2016 15:01:50 +0200 Subject: [Buildroot] [PATCH] powerpc-utils: Bump powerpc-utils to v1.3.0 In-Reply-To: <20160329121047.3804.35383.stgit@hegdevasant.in.ibm.com> References: <20160329121047.3804.35383.stgit@hegdevasant.in.ibm.com> Message-ID: <20160329150150.63d5f136@free-electrons.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: buildroot@busybox.net 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 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