* [Buildroot] [PATCH] package/qoriq-cadence-dp-firmware: refactor with FREESCALE_IMX_EXTRACT_HELPER @ 2022-08-14 12:56 Francois Perrad 2022-08-14 15:04 ` Yann E. MORIN 0 siblings, 1 reply; 4+ messages in thread From: Francois Perrad @ 2022-08-14 12:56 UTC (permalink / raw) To: buildroot avoid code duplication 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH] package/qoriq-cadence-dp-firmware: refactor with FREESCALE_IMX_EXTRACT_HELPER 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 2022-08-14 15:56 ` Arnout Vandecappelle 0 siblings, 1 reply; 4+ messages in thread From: Yann E. MORIN @ 2022-08-14 15:04 UTC (permalink / raw) To: Francois Perrad; +Cc: buildroot 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH] package/qoriq-cadence-dp-firmware: refactor with FREESCALE_IMX_EXTRACT_HELPER 2022-08-14 15:04 ` Yann E. MORIN @ 2022-08-14 15:56 ` Arnout Vandecappelle 2022-08-14 16:11 ` Yann E. MORIN 0 siblings, 1 reply; 4+ messages in thread From: Arnout Vandecappelle @ 2022-08-14 15:56 UTC (permalink / raw) To: Yann E. MORIN, Francois Perrad; +Cc: buildroot On 14/08/2022 17:04, Yann E. MORIN wrote: > 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. Well, the name Freescale in there is legacy, it should really be called NXP_IMX_EXTRACT_HELPER - or even NXP_EXTRACT_HELPER (it's not just IMX platforms, since it's apparently also used for QorIQ). Basically everything that currently has some form of freescale in the name should be converted to NXP. But we don't like that kind of useless churn, so things stay as they are. > 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. This, on the other hand, is pretty essential IMHO. The solution in this case may be to move it to a global place, e.g. package/pkg-utils. With the number of places where we need this crappy extractor, it can be considered infra on the level of the gitlab helper. So, I propose 4 patches. 1. Fix license files as described below by Yann. 2. Move FREESCALE_IMX_EXTRACT_HELPER to package/pkg-utils.mk. 3. Rename FREESCALE_IMX_EXTRACT_HELPER to NXP_EXTRACT_HELPER. 4. Use NXP_EXTRACT_HELPER in qoriq-cadence-dp-firmware. Yann, do you agree? Regards, Arnout > > 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 > _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH] package/qoriq-cadence-dp-firmware: refactor with FREESCALE_IMX_EXTRACT_HELPER 2022-08-14 15:56 ` Arnout Vandecappelle @ 2022-08-14 16:11 ` Yann E. MORIN 0 siblings, 0 replies; 4+ messages in thread From: Yann E. MORIN @ 2022-08-14 16:11 UTC (permalink / raw) To: Arnout Vandecappelle; +Cc: buildroot, Francois Perrad Arnout, All, On 2022-08-14 17:56 +0200, Arnout Vandecappelle spake thusly: > On 14/08/2022 17:04, Yann E. MORIN wrote: > >On 2022-08-14 14:56 +0200, Francois Perrad spake thusly: > >>avoid code duplication [--SNIP--] > >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. > > This, on the other hand, is pretty essential IMHO. > > The solution in this case may be to move it to a global place, e.g. > package/pkg-utils. With the number of places where we need this crappy > extractor, it can be considered infra on the level of the gitlab helper. > > So, I propose 4 patches. > > 1. Fix license files as described below by Yann. > 2. Move FREESCALE_IMX_EXTRACT_HELPER to package/pkg-utils.mk. > 3. Rename FREESCALE_IMX_EXTRACT_HELPER to NXP_EXTRACT_HELPER. > 4. Use NXP_EXTRACT_HELPER in qoriq-cadence-dp-firmware. > > Yann, do you agree? Yes, this is fine with me. Regards, Yann E. MORIN. > Regards, > Arnout > > > > >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 > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-14 16:11 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2022-08-14 15:56 ` Arnout Vandecappelle 2022-08-14 16:11 ` Yann E. MORIN
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox