[Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target

[Angelo Compagnucci]

Hi Mirza,

On Mon, Aug 27, 2018 at 11:02 AM, Thomas Petazzoni
<thomas.petazzoni at bootlin.com> wrote:
> Hello Mirza,
>
> On Sun, 26 Aug 2018 23:41:44 +0200, Mirza Krak wrote:
>> With this you can add:
>>
>>     $(eval $(host-golang-package))
>>
>> to a package .mk file to build for host.
>>
>> Signed-off-by: Mirza Krak <mirza.krak at northern.tech>
>
> Thanks for this contribution! I believe the title should be improved,
> because "host target" is a bit confusion. Perhaps:
>
> package/pkg-golang: add support for building host packages
>
> This commit also needs a small update to the Buildroot manual, to add
> something like "The ability to build host packages is also available,
> with the host-golang-package macro."

Could you repost this patch? I found it very useful and I think you
could post it sooner than the rest of the series.

Thanks!

>
>> ---
>>  package/pkg-golang.mk | 38 +++++++++++++++++++++++++++++++++-----
>>  1 file changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
>> index 6eacd14180..7d9956378f 100644
>> --- a/package/pkg-golang.mk
>> +++ b/package/pkg-golang.mk
>> @@ -23,10 +23,13 @@
>>
>>  GO_BIN = $(HOST_DIR)/bin/go
>>
>> -# We pass an empty GOBIN, otherwise "go install: cannot install
>> -# cross-compiled binaries when GOBIN is set"
>>  GO_TARGET_ENV = \
>>       $(HOST_GO_TARGET_ENV) \
>> +     $(GO_HOST_ENV)
>
> I think it's a bit weird and dangerous to re-use GO_HOST_ENV in
> GO_TARGET_ENV. Indeed, there is the risk that we add something
> host-specific, and it will be re-used for both target and host.
>
> So either your duplicate the definitions (which is reasonable if they
> aren't a lot of them), or you add something like GO_COMMON_ENV, re-used
> between target and host.
>
>>  define inner-golang-package
>> @@ -96,6 +98,9 @@ endif
>>  # Build step. Only define it if not already defined by the package .mk
>>  # file.
>>  ifndef $(2)_BUILD_CMDS
>> +ifeq ($(4),target)
>> +
>
> Drop this empty line.
>
>> +# Build package for target
>>  define $(2)_BUILD_CMDS
>>       $$(foreach d,$$($(2)_BUILD_TARGETS),\
>>               cd $$($(2)_SRC_PATH); \
>> @@ -107,10 +112,23 @@ define $(2)_BUILD_CMDS
>>                       ./$$(d)
>>       )
>>  endef
>> +else
>> +     # Build package for host
>
> Don't indent this comment.
>
>> +define $(2)_BUILD_CMDS
>> +     $$(foreach d,$$($(2)_BUILD_TARGETS),\
>> +             cd $$($(2)_SRC_PATH); \
>> +             $$(GO_HOST_ENV) \
>> +                     GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
>> +                     $$($(2)_GO_ENV) \
>> +                     $$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
>> +                     -o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
>> +                     ./$$(d)
>> +     )
>> +endef
>> +endif
>>  endif
>>
>> -# Target installation step. Only define it if not already defined by the
>> -# package .mk file.
>
> Why are you changing this comment ?
>
>> +# Target installation step
>>  ifndef $(2)_INSTALL_TARGET_CMDS
>>  define $(2)_INSTALL_TARGET_CMDS
>>       $$(foreach d,$$($(2)_INSTALL_BINS),\
>> @@ -119,6 +137,15 @@ define $(2)_INSTALL_TARGET_CMDS
>>  endef
>>  endif
>>
>> +# Host installation step
>> +ifndef $(2)_INSTALL_CMDS
>> +define $(2)_INSTALL_CMDS
>> +     $$(foreach d,$$($(2)_INSTALL_BINS),\
>> +             $(INSTALL) -D -m 0755 $$(@D)/bin/$$(d) $(HOST_DIR)/usr/bin/$$(d)
>
>
> Install in $(HOST_DIR)/bin, not $(HOST_DIR)/usr/bin. We no longer use
> $(HOST_DIR)/usr/bin in fact, and $(HOST_DIR)/usr is a symlink to
> $(HOST_DIR).
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot