diff mbox series

package/pkg-golang: fix build with per-package directories

Message ID 817_1646667779_62262803_817_313_1_9705b574ea82a36b8f676e04876817b58ebb6e3c.1646667776.git.yann.morin@orange.com
State Accepted
Headers show
Series package/pkg-golang: fix build with per-package directories | expand

Commit Message

Yann E. MORIN March 7, 2022, 3:42 p.m. UTC 
From: "Yann E. MORIN" <yann.morin@orange.com>

Build with per-package directory is broken, because go is not found in
the PATH, when trying to download and vendor a go package.

This is because FOO_DL_ENV contains $(HOST_GO_COMMON_ENV), which defines
the PATH as PATH=$(BR_PATH). This is correct, except this is expanded at
the time the golang-package macro is evaluated, which means PATH
contains the 'global' BR_PATH, i.e.: $(O)/host/bin:$(O)/host/sbin:...

However, with PPD, this does not yet exist at build time; only the
per-package hoqt directory exists.

We want to have it expanded at the time the recipe is run, so like all
other variables, we need to $$-expand it.

At the same time, also $$-expand two other variables (even though those
are benign, it is better for consistency that they be $$-expanded).

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Anisse Astier <anisse@astier.eu>
Cc: Christian Stewart <christian@paral.in>
---
 package/pkg-golang.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Christian Stewart March 7, 2022, 6:42 p.m. UTC | #1 
Hi Yann,

On Mon, Mar 7, 2022 at 7:43 AM <yann.morin@orange.com> wrote:
> We want to have it expanded at the time the recipe is run, so like all
> other variables, we need to $$-expand it.
>
> At the same time, also $$-expand two other variables (even though those
> are benign, it is better for consistency that they be $$-expanded).
>
> Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Anisse Astier <anisse@astier.eu>
> Cc: Christian Stewart <christian@paral.in>
> ---
>  package/pkg-golang.mk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
> index ddefdf1214..f1c5cfd350 100644
> --- a/package/pkg-golang.mk
> +++ b/package/pkg-golang.mk
> @@ -45,7 +45,7 @@ $(2)_BUILD_OPTS += \
>         -modcacherw \
>         -tags "$$($(2)_TAGS)" \
>         -trimpath \
> -       -p $(PARALLEL_JOBS)
> +       -p $$(PARALLEL_JOBS)
>
>  # Target packages need the Go compiler on the host at download time (for
>  # vendoring), and at build and install time.
> @@ -86,7 +86,7 @@ $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
>
>  $(2)_DOWNLOAD_POST_PROCESS = go
>  $(2)_DL_ENV += \
> -       $(HOST_GO_COMMON_ENV) \
> +       $$(HOST_GO_COMMON_ENV) \
>         GOPROXY=direct \
>         BR_GOMOD=$$($(2)_GOMOD)
>
> @@ -134,7 +134,7 @@ endif
>  ifndef $(2)_INSTALL_TARGET_CMDS
>  define $(2)_INSTALL_TARGET_CMDS
>         $$(foreach d,$$($(2)_INSTALL_BINS),\
> -               $(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(TARGET_DIR)/usr/bin/$$(d)
> +               $$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(TARGET_DIR)/usr/bin/$$(d)
>         )
>  endef
>  endif
> @@ -143,7 +143,7 @@ endif
>  ifndef $(2)_INSTALL_CMDS
>  define $(2)_INSTALL_CMDS
>         $$(foreach d,$$($(2)_INSTALL_BINS),\
> -               $(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(HOST_DIR)/bin/$$(d)
> +               $$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(HOST_DIR)/bin/$$(d)
>         )

Reviewed-by: Christian Stewart <christian@paral.in>

Thanks,
Christian
Peter Korsgaard March 7, 2022, 9:10 p.m. UTC | #2 
>>>>>   <yann.morin@orange.com> writes:

 > From: "Yann E. MORIN" <yann.morin@orange.com>
 > Build with per-package directory is broken, because go is not found in
 > the PATH, when trying to download and vendor a go package.

 > This is because FOO_DL_ENV contains $(HOST_GO_COMMON_ENV), which defines
 > the PATH as PATH=$(BR_PATH). This is correct, except this is expanded at
 > the time the golang-package macro is evaluated, which means PATH
 > contains the 'global' BR_PATH, i.e.: $(O)/host/bin:$(O)/host/sbin:...

 > However, with PPD, this does not yet exist at build time; only the
 > per-package hoqt directory exists.

s/hoqt/host/

 > We want to have it expanded at the time the recipe is run, so like all
 > other variables, we need to $$-expand it.

 > At the same time, also $$-expand two other variables (even though those
 > are benign, it is better for consistency that they be $$-expanded).

 > Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Anisse Astier <anisse@astier.eu>
 > Cc: Christian Stewart <christian@paral.in>

Committed, thanks.
diff mbox series

Patch

diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index ddefdf1214..f1c5cfd350 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -45,7 +45,7 @@  $(2)_BUILD_OPTS += \
 	-modcacherw \
 	-tags "$$($(2)_TAGS)" \
 	-trimpath \
-	-p $(PARALLEL_JOBS)
+	-p $$(PARALLEL_JOBS)
 
 # Target packages need the Go compiler on the host at download time (for
 # vendoring), and at build and install time.
@@ -86,7 +86,7 @@  $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
 
 $(2)_DOWNLOAD_POST_PROCESS = go
 $(2)_DL_ENV += \
-	$(HOST_GO_COMMON_ENV) \
+	$$(HOST_GO_COMMON_ENV) \
 	GOPROXY=direct \
 	BR_GOMOD=$$($(2)_GOMOD)
 
@@ -134,7 +134,7 @@  endif
 ifndef $(2)_INSTALL_TARGET_CMDS
 define $(2)_INSTALL_TARGET_CMDS
 	$$(foreach d,$$($(2)_INSTALL_BINS),\
-		$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(TARGET_DIR)/usr/bin/$$(d)
+		$$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(TARGET_DIR)/usr/bin/$$(d)
 	)
 endef
 endif
@@ -143,7 +143,7 @@  endif
 ifndef $(2)_INSTALL_CMDS
 define $(2)_INSTALL_CMDS
 	$$(foreach d,$$($(2)_INSTALL_BINS),\
-		$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(HOST_DIR)/bin/$$(d)
+		$$(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $$(HOST_DIR)/bin/$$(d)
 	)
 endef
 endif