[Buildroot] [PATCH 1/4] package/freescale-imx/imx-gpu-viv: move pkg-config files fixup to the build step

[Buildroot] [PATCH 1/4] package/freescale-imx/imx-gpu-viv: move pkg-config files fixup to the build step

[Thomas Petazzoni]

The fixup of the pkg-config files modifies files in the build
directory (@D) but is done in the staging installation step, which
doesn't make much sense, especially since the build step already has
some fixup logic. So we move the fixup logic of the pkg-config files
into the build step.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 .../freescale-imx/imx-gpu-viv/imx-gpu-viv.mk  | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
index 103c0341e3..51b04f82d4 100644
--- a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
+++ b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
@@ -42,16 +42,6 @@ define IMX_GPU_VIV_EXTRACT_CMDS
 	$(call FREESCALE_IMX_EXTRACT_HELPER,$(IMX_GPU_VIV_DL_DIR)/$(IMX_GPU_VIV_SOURCE))
 endef
 
-# Instead of building, we fix up the inconsistencies that exist
-# in the upstream archive here. We also remove unused backend files.
-# Make sure these commands are idempotent.
-define IMX_GPU_VIV_BUILD_CMDS
-	cp -dpfr $(@D)/gpu-core/usr/lib/$(IMX_GPU_VIV_LIB_TARGET)/* $(@D)/gpu-core/usr/lib/
-	for backend in fb x11 wayland; do \
-		$(RM) -r $(@D)/gpu-core/usr/lib/$$backend ; \
-	done
-endef
-
 ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb)
 define IMX_GPU_VIV_FIXUP_PKGCONFIG
 	ln -sf egl_linuxfb.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc
@@ -72,9 +62,19 @@ define IMX_GPU_VIV_FIXUP_PKGCONFIG
 endef
 endif
 
+# Instead of building, we fix up the inconsistencies that exist
+# in the upstream archive here. We also remove unused backend files.
+# Make sure these commands are idempotent.
+define IMX_GPU_VIV_BUILD_CMDS
+	cp -dpfr $(@D)/gpu-core/usr/lib/$(IMX_GPU_VIV_LIB_TARGET)/* $(@D)/gpu-core/usr/lib/
+	for backend in fb x11 wayland; do \
+		$(RM) -r $(@D)/gpu-core/usr/lib/$$backend ; \
+	done
+	$(IMX_GPU_VIV_FIXUP_PKGCONFIG)
+endef
+
 define IMX_GPU_VIV_INSTALL_STAGING_CMDS
 	cp -r $(@D)/gpu-core/usr/* $(STAGING_DIR)/usr
-	$(IMX_GPU_VIV_FIXUP_PKGCONFIG)
 	for lib in egl gbm glesv1_cm glesv2 vg; do \
 		$(INSTALL) -m 0644 -D \
 			$(@D)/gpu-core/usr/lib/pkgconfig/$${lib}.pc \
-- 
2.31.1

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/4] package/freescale-imx/imx-gpu-viv: drop pkg-config files installation

[Thomas Petazzoni]

We manually install pkg-config files from
$(@D)/gpu-core/usr/lib/pkgconfig to the STAGING_DIR right after
copying the entire $(@D)/gpu-core/usr/* files to STAGING_DIR. This
makes the manual copying of pkg-config files pretty useless.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
index 51b04f82d4..9d974cac41 100644
--- a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
+++ b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
@@ -75,11 +75,6 @@ endef
 
 define IMX_GPU_VIV_INSTALL_STAGING_CMDS
 	cp -r $(@D)/gpu-core/usr/* $(STAGING_DIR)/usr
-	for lib in egl gbm glesv1_cm glesv2 vg; do \
-		$(INSTALL) -m 0644 -D \
-			$(@D)/gpu-core/usr/lib/pkgconfig/$${lib}.pc \
-			$(STAGING_DIR)/usr/lib/pkgconfig/$${lib}.pc; \
-	done
 endef
 
 ifeq ($(BR2_PACKAGE_IMX_GPU_VIV_EXAMPLES),y)
-- 
2.31.1

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 2/4] package/freescale-imx/imx-gpu-viv: drop pkg-config files installation

[Gary Bisson]

Hi,

On Thu, Aug 05, 2021 at 11:27:33PM +0200, Thomas Petazzoni wrote:
> We manually install pkg-config files from
> $(@D)/gpu-core/usr/lib/pkgconfig to the STAGING_DIR right after
> copying the entire $(@D)/gpu-core/usr/* files to STAGING_DIR. This
> makes the manual copying of pkg-config files pretty useless.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Indeed, this step can be removed. I checked the .files-list-staging.txt
file and confirm nothing has changed with this patch.

Maybe a cleaner way would be to not copy everything first and only copy
the .pc files that are needed but this could be done later. At least
this patch doesn't change the behavior and makes the package simpler.

Reviewed-by: Gary Bisson <gary.bisson@boundarydevices.com>

Regards,
Gary
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/4] package/freescale-imx/imx-gpu-viv: make pkg-config fixups mutually exclusive

[Thomas Petazzoni]

The IMX_GPU_VIV_FIXUP_PKGCONFIG fixup is defined conditionally
depending on the value of IMX_GPU_VIV_LIB_TARGET, which means that
obviously only one of the definitions is possible. Make this clear by
using a ifeq ... else ifeq ... else ifeq ... endif logic.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
index 9d974cac41..df39d3ddc2 100644
--- a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
+++ b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
@@ -46,15 +46,11 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb)
 define IMX_GPU_VIV_FIXUP_PKGCONFIG
 	ln -sf egl_linuxfb.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc
 endef
-endif
-
-ifeq ($(IMX_GPU_VIV_LIB_TARGET),wayland)
+else ifeq ($(IMX_GPU_VIV_LIB_TARGET),wayland)
 define IMX_GPU_VIV_FIXUP_PKGCONFIG
 	ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc
 endef
-endif
-
-ifeq ($(IMX_GPU_VIV_LIB_TARGET),x11)
+else ifeq ($(IMX_GPU_VIV_LIB_TARGET),x11)
 define IMX_GPU_VIV_FIXUP_PKGCONFIG
 	for lib in egl gbm glesv1_cm glesv2 vg; do \
 		ln -sf $${lib}_x11.pc $(@D)/gpu-core/usr/lib/pkgconfig/$${lib}.pc || exit 1; \
-- 
2.31.1

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/4] package/freescale-imx/imx-gpu-viv: make pkg-config fixups mutually exclusive

[Gary Bisson]

Hi,

On Thu, Aug 05, 2021 at 11:27:34PM +0200, Thomas Petazzoni wrote:
> The IMX_GPU_VIV_FIXUP_PKGCONFIG fixup is defined conditionally
> depending on the value of IMX_GPU_VIV_LIB_TARGET, which means that
> obviously only one of the definitions is possible. Make this clear by
> using a ifeq ... else ifeq ... else ifeq ... endif logic.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Good point, not sure why it wasn't like this since day one.

Reviewed-by: Gary Bisson <gary.bisson@boundarydevices.com>

Regards,
Gary
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 4/4] package/freescale-imx/imx-gpu-viv: use make loops instead of shell loops

[Thomas Petazzoni]

We generally prefer using make loops instead of shell loops. They
bring automatic error handling, as they abort the loop when there is
an error, without the need for "|| exit 1".

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
index df39d3ddc2..fec19523a3 100644
--- a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
+++ b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk
@@ -52,9 +52,9 @@ define IMX_GPU_VIV_FIXUP_PKGCONFIG
 endef
 else ifeq ($(IMX_GPU_VIV_LIB_TARGET),x11)
 define IMX_GPU_VIV_FIXUP_PKGCONFIG
-	for lib in egl gbm glesv1_cm glesv2 vg; do \
-		ln -sf $${lib}_x11.pc $(@D)/gpu-core/usr/lib/pkgconfig/$${lib}.pc || exit 1; \
-	done
+	$(foreach lib,egl gbm glesv1_cm glesv2 vg, \
+		ln -sf $(lib)_x11.pc $(@D)/gpu-core/usr/lib/pkgconfig/$(lib).pc
+	)
 endef
 endif
 
@@ -63,9 +63,9 @@ endif
 # Make sure these commands are idempotent.
 define IMX_GPU_VIV_BUILD_CMDS
 	cp -dpfr $(@D)/gpu-core/usr/lib/$(IMX_GPU_VIV_LIB_TARGET)/* $(@D)/gpu-core/usr/lib/
-	for backend in fb x11 wayland; do \
-		$(RM) -r $(@D)/gpu-core/usr/lib/$$backend ; \
-	done
+	$(foreach backend,fb x11 wayland, \
+		$(RM) -r $(@D)/gpu-core/usr/lib/$(backend)
+	)
 	$(IMX_GPU_VIV_FIXUP_PKGCONFIG)
 endef
 
-- 
2.31.1

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 4/4] package/freescale-imx/imx-gpu-viv: use make loops instead of shell loops

[Gary Bisson]

Hi,

On Thu, Aug 05, 2021 at 11:27:35PM +0200, Thomas Petazzoni wrote:
> We generally prefer using make loops instead of shell loops. They
> bring automatic error handling, as they abort the loop when there is
> an error, without the need for "|| exit 1".
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Same, not sure why shell loops were used in the first place.

Reviewed-by: Gary Bisson <gary.bisson@boundarydevices.com>

Regards,
Gary
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/4] package/freescale-imx/imx-gpu-viv: move pkg-config files fixup to the build step

[Gary Bisson]

Hi Thomas,

On Thu, Aug 05, 2021 at 11:27:32PM +0200, Thomas Petazzoni wrote:
> The fixup of the pkg-config files modifies files in the build
> directory (@D) but is done in the staging installation step, which
> doesn't make much sense, especially since the build step already has
> some fixup logic. So we move the fixup logic of the pkg-config files
> into the build step.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Agreed, makes more sense to do the "magic" in the build step.

Reviewed-by: Gary Bisson <gary.bisson@boundarydevices.com>

Thanks,
Gary
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/4] package/freescale-imx/imx-gpu-viv: move pkg-config files fixup to the build step

[Thomas Petazzoni]

On Thu,  5 Aug 2021 23:27:32 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> The fixup of the pkg-config files modifies files in the build
> directory (@D) but is done in the staging installation step, which
> doesn't make much sense, especially since the build step already has
> some fixup logic. So we move the fixup logic of the pkg-config files
> into the build step.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  .../freescale-imx/imx-gpu-viv/imx-gpu-viv.mk  | 22 +++++++++----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Following the review of Gary, I have applied to next those 4 patches.
Thanks Gary!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot