All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.