[Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Fabio Estevam]

The 'osusergo netgo' tags need to be passed when CGO_ENABLED=1.

Fixes:

http://autobuild.buildroot.net/results/48a39341ca3dcb3675b2876e680718c3e43f55aa
http://autobuild.buildroot.net/results/897b4803f4f221c52899641a45708b49b7cb5f5c
http://autobuild.buildroot.net/results/3a75c6b73a1f583d6db666eacc45c4debd9218e9

Suggested-by: Andrea Barisani <andrea.barisani@withsecure.com>
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 package/crucible/crucible.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/crucible/crucible.mk b/package/crucible/crucible.mk
index fdad709dde..fdcda109c2 100644
--- a/package/crucible/crucible.mk
+++ b/package/crucible/crucible.mk
@@ -9,5 +9,6 @@ CRUCIBLE_SITE = $(call github,usbarmory,crucible,v$(CRUCIBLE_VERSION))
 CRUCIBLE_LICENSE = GPL-3.0
 CRUCIBLE_LICENSE_FILES = LICENSE
 CRUCIBLE_GOMOD = ./cmd/crucible
+CRUCIBLE_TAGS += osusergo netgo
 
 $(eval $(golang-package))
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Thomas Petazzoni via buildroot]

Hello Fabio, Hello Andrea,

On Wed, 20 Jul 2022 09:57:07 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> The 'osusergo netgo' tags need to be passed when CGO_ENABLED=1.
> 
> Fixes:
> 
> http://autobuild.buildroot.net/results/48a39341ca3dcb3675b2876e680718c3e43f55aa
> http://autobuild.buildroot.net/results/897b4803f4f221c52899641a45708b49b7cb5f5c
> http://autobuild.buildroot.net/results/3a75c6b73a1f583d6db666eacc45c4debd9218e9
> 
> Suggested-by: Andrea Barisani <andrea.barisani@withsecure.com>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Could you provide more background about these osusergo and netgo tags?
Indeed, they don't seem to be specific to the crucible package, but are
apparently generic Go tags, at least according to
https://www.arp242.net/static-go.html, and they seem to be needed when
statically linking Go programs.

If that is the case, then I don't think the solution is to change
crucible.mk, but probably to have a more generic solution in
package/pkg-golang.mk. Or is that package-specific in some way?

Do we understand under which conditions these tags are needed?

Thanks in advance for your feedback!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Thomas Petazzoni via buildroot]

Hello,

On Wed, 20 Jul 2022 17:05:42 +0000
"Barisani, Andrea" <andrea.barisani@withsecure.com> wrote:

> These flags are not crucible specific but they are necessary when
> compiling static Go binaries with CGO enabled (which crucible does
> not require by the way) to ensure portability as cgo versions of net
> and os/users are not portable. 

Thanks for your feedback, much appreciated!

So, I see we're passing CGO_ENABLED=1 (when thread support is
available) to all packages, and you're saying that this is incorrect as
not all Go packages need CGO? Passing CGO_ENABLED=1/0 should be a
per-package decision?

However, when CGO is enabled and we need static linking, passing
osusergo and netgo tags is needed, correct?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Arnout Vandecappelle]

On 20/07/2022 22:24, Thomas Petazzoni via buildroot wrote:
> Hello,
> 
> On Wed, 20 Jul 2022 17:05:42 +0000
> "Barisani, Andrea" <andrea.barisani@withsecure.com> wrote:
> 
>> These flags are not crucible specific but they are necessary when
>> compiling static Go binaries with CGO enabled (which crucible does
>> not require by the way) to ensure portability as cgo versions of net
>> and os/users are not portable.
> 
> Thanks for your feedback, much appreciated!
> 
> So, I see we're passing CGO_ENABLED=1 (when thread support is

  AFAICS, CGO_ENABLED is only set in HOST_GO_MAKE_ENV, which is only used in the 
host-go build commands. So it's not really possible to set it per package. 
However, apparently it is possible to override it:

DOCKER_CLI_GO_ENV = CGO_ENABLED=no

the commit message that introduced that unfortunately doesn't explain why, I'm 
not even sure if it does anything.



  Regards,
  Arnout


> available) to all packages, and you're saying that this is incorrect as
> not all Go packages need CGO? Passing CGO_ENABLED=1/0 should be a
> per-package decision?
> 
> However, when CGO is enabled and we need static linking, passing
> osusergo and netgo tags is needed, correct?
> 
> Thomas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Thomas Petazzoni via buildroot]

Hello,

On Fri, 22 Jul 2022 23:04:57 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>   AFAICS, CGO_ENABLED is only set in HOST_GO_MAKE_ENV, which is only used in the 
> host-go build commands. So it's not really possible to set it per package. 
> However, apparently it is possible to override it:

I see this:

HOST_GO_COMMON_ENV = \
        GO111MODULE=on \
        GOFLAGS=-mod=vendor \
        GOROOT="$(HOST_GO_ROOT)" \
        GOPATH="$(HOST_GO_GOPATH)" \
        GOPROXY=off \
        PATH=$(BR_PATH) \
        GOBIN= \
        CGO_ENABLED=$(HOST_GO_CGO_ENABLED)

HOST_GO_TARGET_ENV = \
        $(HOST_GO_COMMON_ENV) \
        GOOS="linux" \
        GOARCH=$(GO_GOARCH) \
        GOCACHE="$(HOST_GO_TARGET_CACHE)" \
        CC="$(TARGET_CC)" \
        CXX="$(TARGET_CXX)" \
        CGO_CFLAGS="$(TARGET_CFLAGS)" \
        CGO_CXXFLAGS="$(TARGET_CXXFLAGS)" \
        CGO_LDFLAGS="$(TARGET_LDFLAGS)" \
        GOTOOLDIR="$(HOST_GO_TOOLDIR)"

so HOST_GO_TARGET_ENV has CGO_ENABLED=$(HOST_GO_CGO_ENABLED)

and then HOST_GO_TARGET_ENV gets used in:

# Build package for target
define $(2)_BUILD_CMDS
        $$(foreach d,$$($(2)_BUILD_TARGETS),\
                cd $$(@D); \
                $$(HOST_GO_TARGET_ENV) \
                        $$($(2)_GO_ENV) \
                        $$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
                        -o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
                        $$($(2)_GOMOD)/$$(d)
        )
endef

Am I looking wrong?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Arnout Vandecappelle]

On 22/07/2022 23:15, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 22 Jul 2022 23:04:57 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>    AFAICS, CGO_ENABLED is only set in HOST_GO_MAKE_ENV, which is only used in the
>> host-go build commands. So it's not really possible to set it per package.
>> However, apparently it is possible to override it:
> 
> I see this:
> 
> HOST_GO_COMMON_ENV = \
>          GO111MODULE=on \
>          GOFLAGS=-mod=vendor \
>          GOROOT="$(HOST_GO_ROOT)" \
>          GOPATH="$(HOST_GO_GOPATH)" \
>          GOPROXY=off \
>          PATH=$(BR_PATH) \
>          GOBIN= \
>          CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
> 
> HOST_GO_TARGET_ENV = \
>          $(HOST_GO_COMMON_ENV) \
>          GOOS="linux" \
>          GOARCH=$(GO_GOARCH) \
>          GOCACHE="$(HOST_GO_TARGET_CACHE)" \
>          CC="$(TARGET_CC)" \
>          CXX="$(TARGET_CXX)" \
>          CGO_CFLAGS="$(TARGET_CFLAGS)" \
>          CGO_CXXFLAGS="$(TARGET_CXXFLAGS)" \
>          CGO_LDFLAGS="$(TARGET_LDFLAGS)" \
>          GOTOOLDIR="$(HOST_GO_TOOLDIR)"
> 
> so HOST_GO_TARGET_ENV has CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
> 
> and then HOST_GO_TARGET_ENV gets used in:
> 
> # Build package for target
> define $(2)_BUILD_CMDS
>          $$(foreach d,$$($(2)_BUILD_TARGETS),\
>                  cd $$(@D); \
>                  $$(HOST_GO_TARGET_ENV) \
>                          $$($(2)_GO_ENV) \
>                          $$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
>                          -o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
>                          $$($(2)_GOMOD)/$$(d)
>          )
> endef
> 
> Am I looking wrong?

  No, I'm looking wrong, of course. I didn't see the one in HOST_GO_COMMON_ENV 
because it comes before the definition of HOST_GO_CGO_ENABLED :-(

  So indeed, the question if we should have a per-package option to enable cgo 
is relevant.

  I'd actually expect that the package itself would specify whether it needs cgo 
or not. Can we test a build of a package that actually uses cgo to verify if we 
really need to set it in HOST_GO_COMMON_ENV?


  Regardless, we *will* need to set the 'osusergo netgo' tags for all cgo-using 
packages when they do use cgo - so either we still need a per-package "enable 
cgo" option, or we should just always add those tags if cgo is enabled and we're 
doing static linking.

  Is there any reason why this problem is only observed for the crucible package?

  Regards,
  Arnout



> 
> Thomas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Christian Stewart via buildroot]

Hi all,


On Fri, Jul 22, 2022 at 3:03 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>   I'd actually expect that the package itself would specify whether it needs cgo
> or not. Can we test a build of a package that actually uses cgo to verify if we
> really need to set it in HOST_GO_COMMON_ENV?

"CGO_ENABLED" informs the compiler that it's OK to use Cgo.

It should still be set to 1 even if the package doesn't use cgo at all.

>   Regardless, we *will* need to set the 'osusergo netgo' tags for all cgo-using
> packages when they do use cgo - so either we still need a per-package "enable
> cgo" option, or we should just always add those tags if cgo is enabled and we're
> doing static linking.
>
>   Is there any reason why this problem is only observed for the crucible package?

So, this doesn't work?

FOO_GO_ENV += CGO_ENABLED=0

.. because the package needs tags enabled conditionally if cgo is
available or not?

I don't understand why this specific package can't just check
HOST_GO_CGO_ENABLED and conditionally add the tags?

Best,
Christian Stewart
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Arnout Vandecappelle]

On 23/07/2022 02:41, Christian Stewart wrote:
> Hi all,
> 
> 
> On Fri, Jul 22, 2022 at 3:03 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>>    I'd actually expect that the package itself would specify whether it needs cgo
>> or not. Can we test a build of a package that actually uses cgo to verify if we
>> really need to set it in HOST_GO_COMMON_ENV?
> 
> "CGO_ENABLED" informs the compiler that it's OK to use Cgo.
> 
> It should still be set to 1 even if the package doesn't use cgo at all.
> 
>>    Regardless, we *will* need to set the 'osusergo netgo' tags for all cgo-using
>> packages when they do use cgo - so either we still need a per-package "enable
>> cgo" option, or we should just always add those tags if cgo is enabled and we're
>> doing static linking.
>>
>>    Is there any reason why this problem is only observed for the crucible package?
> 
> So, this doesn't work?
> 
> FOO_GO_ENV += CGO_ENABLED=0
> 
> .. because the package needs tags enabled conditionally if cgo is
> available or not?
> 
> I don't understand why this specific package can't just check
> HOST_GO_CGO_ENABLED and conditionally add the tags?

  Yes it can. However, Andrea wrote:

> These flags are not crucible specific but they are necessary when
> compiling static Go binaries with CGO enabled (which crucible does
> not require by the way) to ensure portability as cgo versions of net
> and os/users are not portable. 

and we are wondering why only crucible needs those tags, and other Go packages 
don't. And what is especially weird is the part "which crucible does not 
require" which makes it sound as if crucible isn't using cgo at all...


  Regards,
  Arnout



> 
> Best,
> Christian Stewart
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Arnout Vandecappelle]

On 23/07/2022 10:54, Arnout Vandecappelle wrote:
>
>
> On 23/07/2022 02:41, Christian Stewart wrote:
>> Hi all,
>>
>>
>> On Fri, Jul 22, 2022 at 3:03 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>>>    I'd actually expect that the package itself would specify whether it 
>>> needs cgo
>>> or not. Can we test a build of a package that actually uses cgo to verify if we
>>> really need to set it in HOST_GO_COMMON_ENV?
>>
>> "CGO_ENABLED" informs the compiler that it's OK to use Cgo.
>>
>> It should still be set to 1 even if the package doesn't use cgo at all.
>>
>>>    Regardless, we *will* need to set the 'osusergo netgo' tags for all 
>>> cgo-using
>>> packages when they do use cgo - so either we still need a per-package "enable
>>> cgo" option, or we should just always add those tags if cgo is enabled and 
>>> we're
>>> doing static linking.
>>>
>>>    Is there any reason why this problem is only observed for the crucible 
>>> package?
>>
>> So, this doesn't work?
>>
>> FOO_GO_ENV += CGO_ENABLED=0
>>
>> .. because the package needs tags enabled conditionally if cgo is
>> available or not?
>>
>> I don't understand why this specific package can't just check
>> HOST_GO_CGO_ENABLED and conditionally add the tags?
>
>  Yes it can. However, Andrea wrote:
>
>> These flags are not crucible specific but they are necessary when
>> compiling static Go binaries with CGO enabled (which crucible does
>> not require by the way) to ensure portability as cgo versions of net
>> and os/users are not portable. 
>
> and we are wondering why only crucible needs those tags, and other Go packages 
> don't. And what is especially weird is the part "which crucible does not 
> require" which makes it sound as if crucible isn't using cgo at all...

  It turns out that other packages also do need it, e.g. containerd and delve.

  So I moved this to the pkg-golang infra [1], and marked this patch as Superseded.


  Regards,
  Arnout

[1] 
https://patchwork.ozlabs.org/project/buildroot/patch/20220918122239.189147-1-arnout@mind.be/


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/crucible: Pass 'osusergo netgo' tags

[Fabio Estevam]

Hi Arnout,

On Sun, Sep 18, 2022 at 9:50 AM Arnout Vandecappelle <arnout@mind.be> wrote:

>   It turns out that other packages also do need it, e.g. containerd and delve.
>
>   So I moved this to the pkg-golang infra [1], and marked this patch as Superseded.

Thanks for taking care of this.

Regards,

Fabio Estevam
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot