Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
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.

  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