From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Francois Perrad <fperrad@gmail.com>
Cc: buildroot@busybox.net
Subject: Re: [Buildroot] [PATCH] package/qoriq-cadence-dp-firmware: refactor with FREESCALE_IMX_EXTRACT_HELPER
Date: Sun, 14 Aug 2022 17:04:44 +0200 [thread overview]
Message-ID: <20220814150444.GH2775145@scaer> (raw)
In-Reply-To: <20220814125656.1227977-1-francois.perrad@gadz.org>
François, All,
+Arnout would applied qoriq-cadence-dp-firmware.
On 2022-08-14 14:56 +0200, Francois Perrad spake thusly:
> avoid code duplication
I understand the udnerlying reason, but I am not too fond of it.
The FREESCALE_IMX_EXTRACT_HELPER really bears the name "Freescale" in
it, and if we need to change it for Freescale-related packages, it will
not be obvious that it can also be used by other, non-Freescale
packages.
Also, when we have packages that share some infrastructure, we like them
to be in a sub-directory of a common dir (like qt5 and all the Qt 5.x
packages, or like indeed freescale-imx and all relatedpackages). But
this qoriq-cadence-dp-firmware is not part of that freescale-imx
collection.
So, in this case, I prefer we keep the duplication.
Yeah, Freescale is now part of NXP, but this is not a reason yet.
Also, note that the extract helper does extract a fie named EULA, but it
it not present in LICENSE_FILES, even though the comment states it
should be.
Additionally, one extracted, there is also an EULA.txt file that is not
in LICENSE_FILES wither, which I think it should be.
Can you address those last two bits in a separate patch, please?
Regards,
Yann E. MORIN.
> Signed-off-by: Francois Perrad <francois.perrad@gadz.org>
> ---
> .../qoriq-cadence-dp-firmware.mk | 27 +------------------
> 1 file changed, 1 insertion(+), 26 deletions(-)
>
> diff --git a/package/qoriq-cadence-dp-firmware/qoriq-cadence-dp-firmware.mk b/package/qoriq-cadence-dp-firmware/qoriq-cadence-dp-firmware.mk
> index 23643d24e..1a97c5803 100644
> --- a/package/qoriq-cadence-dp-firmware/qoriq-cadence-dp-firmware.mk
> +++ b/package/qoriq-cadence-dp-firmware/qoriq-cadence-dp-firmware.mk
> @@ -13,33 +13,8 @@ QORIQ_CADENCE_DP_FIRMWARE_REDISTRIBUTE = NO
> QORIQ_CADENCE_DP_FIRMWARE_INSTALL_IMAGES = YES
> QORIQ_CADENCE_DP_FIRMWARE_INSTALL_TARGET = NO
>
> -# Helper for self-extracting binaries distributed by NXP.
> -#
> -# The --force option makes sure it doesn't fail if the source
> -# directory already exists. The --auto-accept skips the license check,
> -# as it is not needed in Buildroot because we have legal-info. Since
> -# there's a EULA in the binary file, we extract it in this macro, and
> -# it should therefore be added to the LICENSE_FILES variable of
> -# packages using this macro. Also, remember to set REDISTRIBUTE to
> -# "NO". Indeed, this is a legal minefield: the EULA specifies that the
> -# Board Support Package includes software and hardware (sic!) for
> -# which a separate license is needed...
> -#
> -# $(1): full path to the archive file
> -#
> -define QORIQ_CADENCE_DP_FIRMWARE_EXTRACT_HELPER
> - awk 'BEGIN { start = 0; } \
> - /^EOEULA/ { start = 0; } \
> - { if (start) print; } \
> - /<<EOEULA/ { start = 1; }' \
> - $(1) > $(@D)/EULA
> - cd $(@D) && sh $(1) --force --auto-accept
> - find $(@D)/$(basename $(notdir $(1))) -mindepth 1 -maxdepth 1 -exec mv {} $(@D) \;
> - rmdir $(@D)/$(basename $(notdir $(1)))
> -endef
> -
> define QORIQ_CADENCE_DP_FIRMWARE_EXTRACT_CMDS
> - $(call QORIQ_CADENCE_DP_FIRMWARE_EXTRACT_HELPER,$(QORIQ_CADENCE_DP_FIRMWARE_DL_DIR)/$(QORIQ_CADENCE_DP_FIRMWARE_SOURCE))
> + $(call FREESCALE_IMX_EXTRACT_HELPER,$(QORIQ_CADENCE_DP_FIRMWARE_DL_DIR)/$(QORIQ_CADENCE_DP_FIRMWARE_SOURCE))
> endef
>
> define QORIQ_CADENCE_DP_FIRMWARE_INSTALL_IMAGES_CMDS
> --
> 2.34.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
next prev parent reply other threads:[~2022-08-14 15:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-14 12:56 [Buildroot] [PATCH] package/qoriq-cadence-dp-firmware: refactor with FREESCALE_IMX_EXTRACT_HELPER Francois Perrad
2022-08-14 15:04 ` Yann E. MORIN [this message]
2022-08-14 15:56 ` Arnout Vandecappelle
2022-08-14 16:11 ` 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=20220814150444.GH2775145@scaer \
--to=yann.morin.1998@free.fr \
--cc=buildroot@busybox.net \
--cc=fperrad@gmail.com \
/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