From: Eric Nelson <eric.nelson@boundarydevices.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH V4 2/2] i.MX: Update versions to match latest Freescale release
Date: Wed, 12 Feb 2014 12:10:37 -0700 [thread overview]
Message-ID: <52FBC72D.3090706@boundarydevices.com> (raw)
In-Reply-To: <20140212190351.GD17804@free.fr>
Hi Yann,
On 02/12/2014 12:03 PM, Yann E. MORIN wrote:
> Eric, All,
>
> A (hopefully!) better review this time...
>
> On 2013-10-08 20:05 -0700, Eric Nelson spake thusly:
>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> [--SNIP--]
>> diff --git a/package/freescale-imx/gpu-viv-bin-mx6q/gpu-viv-bin-mx6q.mk b/package/freescale-imx/gpu-viv-bin-mx6q/gpu-viv-bin-mx6q.mk
>> index 8e979ee..e799fd2 100644
>> --- a/package/freescale-imx/gpu-viv-bin-mx6q/gpu-viv-bin-mx6q.mk
>> +++ b/package/freescale-imx/gpu-viv-bin-mx6q/gpu-viv-bin-mx6q.mk
>> @@ -4,7 +4,11 @@
>> #
>> ################################################################################
>>
>> -GPU_VIV_BIN_MX6Q_VERSION = $(FREESCALE_IMX_VERSION)
>> +ifeq ($(BR2_ARM_EABIHF),y)
>> +GPU_VIV_BIN_MX6Q_VERSION = $(FREESCALE_IMX_VERSION)-hfp
>> +else
>> +GPU_VIV_BIN_MX6Q_VERSION = $(FREESCALE_IMX_VERSION)-sfp
>> +endif
>> GPU_VIV_BIN_MX6Q_SITE = $(FREESCALE_IMX_SITE)
>> GPU_VIV_BIN_MX6Q_SOURCE = gpu-viv-bin-mx6q-$(GPU_VIV_BIN_MX6Q_VERSION).bin
>
> I think we should make the source conditional on BR2_ARM_EABIHF, not the
> version:
>
> GPU_VIV_BIN_MX6Q_VERSION = $(FREESCALE_IMX_VERSION)
> ifeq ($(BR2_ARM_EABIHF),y)
> GPU_VIV_BIN_MX6Q_SOURCE = gpu-viv-bin-mx6q-$(GPU_VIV_BIN_MX6Q_VERSION)-hfp.bin
> else
> GPU_VIV_BIN_MX6Q_SOURCE = gpu-viv-bin-mx6q-$(GPU_VIV_BIN_MX6Q_VERSION)-sfp.bin
> endif
>
Agreed. This is nicer.
>> diff --git a/package/freescale-imx/imx-lib/imx-lib.mk b/package/freescale-imx/imx-lib/imx-lib.mk
>> index e1a68ef..2edfc06 100644
>> --- a/package/freescale-imx/imx-lib/imx-lib.mk
>> +++ b/package/freescale-imx/imx-lib/imx-lib.mk
>> @@ -6,8 +6,9 @@
>>
>> IMX_LIB_VERSION = $(FREESCALE_IMX_VERSION)
>> IMX_LIB_SITE = $(FREESCALE_IMX_SITE)
>> -IMX_LIB_LICENSE = LGPLv2.1+
>> -# No license file included
>> +IMX_LIB_LICENSE = IMX_LIB_LICENSE = Freescale Semiconductor Software License Agreement (vpu), LGPLv2.1+ (the rest)
>
> Double IMX_LIB_LICENSE there.
>
> This is a rather long license name. What about:
>
> IMX_LIB_LICENSE = Freescale License (vpu), LGPLv2.1+ (the rest)
>
This also works for me.
>> +IMX_LIB_LICENSE_FILES = EULA
>> +IMX_LIB_SOURCE = imx-lib-$(IMX_LIB_VERSION).bin
>>
>> IMX_LIB_INSTALL_STAGING = YES
>>
>> @@ -25,6 +26,23 @@ IMX_LIB_MAKE_ENV = \
>> PLATFORM=$(BR2_PACKAGE_IMX_LIB_PLATFORM) \
>> INCLUDE="$(IMX_LIB_INCLUDE)"
>>
>> +# The archive is a shell-self-extractor of a bzipped tar. It happens
>> +# to extract in the correct directory (imx-lib-x.y.z)
>> +# The --force makes sure it doesn't fail if the source dir already exists.
>> +# The --auto-accept skips the license check - not needed for us
>> +# because we have legal-info
>> +# Since there's a EULA in the bin file, extract it to imx-lib-x.y.z/EULA
>> +#
>> +define IMX_LIB_EXTRACT_CMDS
>> + awk 'BEGIN { start=0; } \
>> + /^EOEULA/ { start = 0; } \
>> + { if (start) print; } \
>> + /<<EOEULA/ { start=1; }'\
>> + $(DL_DIR)/$(IMX_LIB_SOURCE) > $(@D)/EULA
>> + cd $(BUILD_DIR); \
>> + sh $(DL_DIR)/$(IMX_LIB_SOURCE) --force --auto-accept
>> +endef
>
> $ make imx-lib-extract
> [...]
> Unpacking file mkdir: cannot create directory ?imx-lib-3.5.7-1.0.0?:
> File exists
>
> This is only a warning, but still: maybe extract the EULA _after_
> unpacking the archive?
>
Sure. I'll re-work.
>> diff --git a/package/libfslparser/libfslparser.mk b/package/libfslparser/libfslparser.mk
>> index 3aab3a7..d49c95c 100644
>> --- a/package/libfslparser/libfslparser.mk
>> +++ b/package/libfslparser/libfslparser.mk
>> @@ -4,7 +4,7 @@
>> #
>> ################################################################################
>>
>> -LIBFSLPARSER_VERSION = 3.0.1
>> +LIBFSLPARSER_VERSION = $(FREESCALE_IMX_VERSION)
>> LIBFSLPARSER_SITE = $(FREESCALE_IMX_SITE)
>> LIBFSLPARSER_SOURCE = libfslparser-$(LIBFSLPARSER_VERSION).bin
>> LIBFSLPARSER_LICENSE = Freescale Semiconductor Software License Agreement
>> @@ -36,4 +36,9 @@ endef
>> # The Makefile installs several versions of the libraries, but we only
>> # need one of them, depending on the platform.
>>
>> +#
>> +# without AUTORECONF, configure fails to find install-sh.
>> +#
>
> No need for leading/trailing comment lines, just the line with the
> actual comment.
>
Okay. Will fix in V5.
>> diff --git a/package/libfslvpuwrap/libfslvpuwrap.mk b/package/libfslvpuwrap/libfslvpuwrap.mk
>> index 346a20a..e93d761 100644
>> --- a/package/libfslvpuwrap/libfslvpuwrap.mk
>> +++ b/package/libfslvpuwrap/libfslvpuwrap.mk
>> @@ -4,14 +4,32 @@
>> #
>> ################################################################################
>>
>> -LIBFSLVPUWRAP_VERSION = 1.0.17
>> +LIBFSLVPUWRAP_VERSION = $(FREESCALE_IMX_VERSION)
>> LIBFSLVPUWRAP_SITE = $(FREESCALE_IMX_SITE)
>> +LIBFSLVPUWRAP_SOURCE = libfslvpuwrap-$(LIBFSLVPUWRAP_VERSION).bin
>> LIBFSLVPUWRAP_LICENSE = Freescale Semiconductor Software License Agreement
>> -LIBFSLVPUWRAP_LICENSE_FILES = EULA.txt
>> +LIBFSLVPUWRAP_LICENSE_FILES = EULA EULA.txt
>
> A comment that both EULA are not the same would be nice:
>
> # The contents of those two EULA are not the same:
> LIBFSLVPUWRAP_LICENSE_FILES = EULA EULA.txt
>
>> LIBFSLVPUWRAP_REDISTRIBUTE = NO
>>
>> LIBFSLVPUWRAP_INSTALL_STAGING = YES
>>
>> LIBFSLVPUWRAP_DEPENDENCIES += imx-lib
>>
>> +# The archive is a shell-self-extractor of a bzipped tar. It happens
>> +# to extract in the correct directory (libfslvpuwrap-x.y.z)
>> +# The --force makes sure it doesn't fail if the source dir already exists.
>> +# The --auto-accept skips the license check - not needed for us
>> +# because we have legal-info
>> +# Since the EULA in the bin file differs from the one in the tar file,
>> +# extract the one from the bin file as well.
>> +define LIBFSLVPUWRAP_EXTRACT_CMDS
>> + awk 'BEGIN { start=0; } \
>> + /^EOEULA/ { start = 0; } \
>> + { if (start) print; } \
>> + /<<EOEULA/ { start=1; }'\
>> + $(DL_DIR)/$(LIBFSLVPUWRAP_SOURCE) > $(@D)/EULA
>> + cd $(BUILD_DIR); \
>> + sh $(DL_DIR)/$(LIBFSLVPUWRAP_SOURCE) --force --auto-accept
>> +endef
>
> Ditto the mkdir warning.
> Ditto extract EULA after unpacking the archive.
>
Thanks for the thorough review.
next prev parent reply other threads:[~2014-02-12 19:10 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-09 3:05 [Buildroot] [PATCH V4 0/2] i.MX updates Eric Nelson
2013-10-09 3:05 ` [Buildroot] [PATCH V4 1/2] i.MX: Use FREESCALE_IMX_SITE for Freescale packages Eric Nelson
2014-02-12 18:10 ` Yann E. MORIN
2014-02-12 19:01 ` Eric Nelson
2013-10-09 3:05 ` [Buildroot] [PATCH V4 2/2] i.MX: Update versions to match latest Freescale release Eric Nelson
2014-02-12 18:23 ` Yann E. MORIN
2014-02-12 18:27 ` Yann E. MORIN
2014-02-12 19:03 ` Yann E. MORIN
2014-02-12 19:10 ` Eric Nelson [this message]
2014-02-12 20:20 ` Eric Nelson
2014-02-12 21:58 ` Arnout Vandecappelle
2014-02-12 22:15 ` Mike Zick
2014-02-12 22:55 ` Eric Nelson
2014-02-12 23:53 ` Eric Nelson
2014-02-12 23:59 ` Yann E. MORIN
2014-02-12 22:20 ` Peter Korsgaard
2014-02-12 22:45 ` Eric Nelson
2014-02-12 23:41 ` Yann E. MORIN
2014-02-13 2:00 ` Eric Nelson
2014-02-13 22:52 ` Yann E. MORIN
2014-02-13 22:54 ` Thomas Petazzoni
2014-02-14 0:34 ` Eric Nelson
2014-02-14 0:31 ` Eric Nelson
2014-02-12 23:25 ` Eric Nelson
2014-02-12 23:46 ` Yann E. MORIN
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52FBC72D.3090706@boundarydevices.com \
--to=eric.nelson@boundarydevices.com \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox