[Buildroot] [PATCH 1/3] package/pkg-golang: add support for host target
Angelo Compagnucci
angelo at amarulasolutions.com
Wed Sep 5 13:30:06 UTC 2018
Hi Mirza,
On Wed, Sep 5, 2018 at 3:27 PM, Angelo Compagnucci
<angelo at amarulasolutions.com> wrote:
> 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.
Just found your patch repost, sorry for the noise!
Thanks!
>
> 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
More information about the buildroot
mailing list