diff mbox series

[1/1] pkg-golang: Allow per package/target CGO_ENABLED setting

Message ID 20180727024720.13370-1-camh@xdna.net
State Rejected
Headers show
Series [1/1] pkg-golang: Allow per package/target CGO_ENABLED setting | expand

Commit Message

Cam Hutchison July 27, 2018, 2:47 a.m. UTC
Allow the CGO_ENABLED variable to be controlled per package and per
build target, instead of having the value determined by whether or not
the toolchain has threads.

Some build targets may not build with CGO_ENABLED=1, so allowing a per
package and build target override will allow those targets to be built
with Buildroot.

Signed-off-by: Cam Hutchison <camh@xdna.net>
---
 docs/manual/adding-packages-golang.txt | 14 ++++++++++++++
 package/pkg-golang.mk                  | 17 +++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni April 7, 2019, 8:28 p.m. UTC | #1
Hello Cam,

+Christian and Anisse in Cc.

Sorry for the very slow feedback.

On Fri, 27 Jul 2018 12:47:20 +1000
Cam Hutchison <camh@xdna.net> wrote:

> Allow the CGO_ENABLED variable to be controlled per package and per
> build target, instead of having the value determined by whether or not
> the toolchain has threads.
> 
> Some build targets may not build with CGO_ENABLED=1, so allowing a per
> package and build target override will allow those targets to be built
> with Buildroot.

Could you give some specific examples instead of the fuzzy "Some build
targets" wording ?

Christian, Anisse, what do you think about this patch ? Do we need
something like this in our Go support ?

I'm leaving the full patch below. You can also find it at
http://patchwork.ozlabs.org/patch/949972/.

Thanks!

Thomas

> Signed-off-by: Cam Hutchison <camh@xdna.net>
> ---
>  docs/manual/adding-packages-golang.txt | 14 ++++++++++++++
>  package/pkg-golang.mk                  | 17 +++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-golang.txt b/docs/manual/adding-packages-golang.txt
> index efcf696867..2683774861 100644
> --- a/docs/manual/adding-packages-golang.txt
> +++ b/docs/manual/adding-packages-golang.txt
> @@ -99,6 +99,20 @@ therefore only use a few of them, or none.
>     +FOO_BUILD_TARGETS = cmd/docker cmd/dockerd+ the binaries produced
>     are +docker+ and +dockerd+.
>  
> +* +FOO_CGO_ENABLED+ overrides the default value for the +CGO_ENABLED+
> +  flag passed to the Go compiler. By default +CGO_ENABLED+ is set
> +  to 1 if the toolchain supports threads, but you can force it to
> +  0 for the package by setting this variable. If the toolchain does
> +  not support threads and you set +FOO_CGO_ENABLED+ to 1, an error
> +  will be generated.
> +
> +* +FOO_BUILD_TARGET_CGO_ENABLED_<target>+ can be used to override
> +  the +CGO_ENABLED+ flag passed to the Go compiler for a specific
> +  build target, similar to +FOO_CGO_ENABLED+. +<target>+ references
> +  a target listed in +FOO_BUILD_TARGETS+. If the toolchain does
> +  not support threads and you set this variable to 1, an error will
> +  be generated.
> +
>  * +FOO_INSTALL_BINS+ can be used to pass the list of binaries that
>    should be installed in +/usr/bin+ on the target. If
>    +FOO_INSTALL_BINS+ is not specified, it defaults to the lower-case
> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
> index bf178622b5..2be3bd4c14 100644
> --- a/package/pkg-golang.mk
> +++ b/package/pkg-golang.mk
> @@ -28,8 +28,20 @@ GO_BIN = $(HOST_DIR)/bin/go
>  GO_TARGET_ENV = \
>  	$(HOST_GO_TARGET_ENV) \
>  	PATH=$(BR_PATH) \
> -	GOBIN= \
> -	CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
> +	GOBIN=
> +
> +# Allow CGO_ENABLED to be defined per package and per build target, but if
> +# CGO is enabled for a package or a build target and the toolchain does not
> +# support that, generate an error.
> +GO_CGO_ENABLED = $(strip \
> +	$(if $(filter 10,$($(1)_CGO_ENABLED)$(HOST_GO_CGO_ENABLED)),\
> +		$(error Toolchain does not support CGO_ENABLED=1 for package $(1)),\
> +		$(or $($(1)_CGO_ENABLED),$(HOST_GO_CGO_ENABLED))))
> +
> +GO_TARGET_CGO_ENABLED = $(strip \
> +	$(if $(filter 10,$($(1)_BUILD_TARGET_CGO_ENABLED_$(2))$(HOST_GO_CGO_ENABLED)),\
> +		$(error Toolchain does not support CGO_ENABLED=1 for target $(2)),\
> +		$(or $($(1)_BUILD_TARGET_CGO_ENABLED_$(2)),$(call GO_CGO_ENABLED,$(1)))))
>  
>  ################################################################################
>  # inner-golang-package -- defines how the configuration, compilation and
> @@ -100,6 +112,7 @@ define $(2)_BUILD_CMDS
>  	$$(foreach d,$$($(2)_BUILD_TARGETS),\
>  		cd $$($(2)_SRC_PATH); \
>  		$$(GO_TARGET_ENV) \
> +			CGO_ENABLED=$$(call GO_TARGET_CGO_ENABLED,$(2),$$(d)) \
>  			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
>  			$$($(2)_GO_ENV) \
>  			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
Anisse Astier April 7, 2019, 9:49 p.m. UTC | #2
Hi Cam,

Sorry for the delay,

(+Angelo in cc)

On Fri, Jul 27, 2018 at 12:47:20PM +1000, Cam Hutchison wrote:
> Allow the CGO_ENABLED variable to be controlled per package and per
> build target, instead of having the value determined by whether or not
> the toolchain has threads.
> 
> Some build targets may not build with CGO_ENABLED=1, so allowing a per
> package and build target override will allow those targets to be built
> with Buildroot.

Just like Thomas, I'm wondering what specific use-case you have in mind;
in particular, I'm interested why you needed build-target granularity in
addition to package-level configuration.

To provide context: 

Disabling CGO isn't an issue, until you try doing DNS requests, which
are usually handled by your libc. If you have anything configured out of
the ordinary /etc/resolv.conf, a cgo-disabled binary won't be able to do
a DNS request. Same goes for user/group and user home dir resolution:
anything outside of /etc/{passwd,group} (pam, NSS, etc.), won't work.

Disabling CGO allows you to have static binaries; if you don't use any
native libraries.

I don't know in which context it might be useful in buildroot.

Regards,

Anisse
Christian Stewart April 8, 2019, 2 a.m. UTC | #3
Hi Anisse, Cam, all,

Overriding CGO_ENABLED when we know that the toolchain doesn't work
properly with cgo doesn't seem correct.

Anisse Astier <anisse@astier.eu> writes:
>> Some build targets may not build with CGO_ENABLED=1, so allowing a per
>> package and build target override will allow those targets to be built
>> with Buildroot.

Why not just add the override in the existing build flag overrides?

> Just like Thomas, I'm wondering what specific use-case you have in mind;
> in particular, I'm interested why you needed build-target granularity in
> addition to package-level configuration.

If you really need to do something this custom, I recommend just
overriding the build stage entirely.

> Disabling CGO isn't an issue, until you try doing DNS requests, which
> are usually handled by your libc. If you have anything configured out of
> the ordinary /etc/resolv.conf, a cgo-disabled binary won't be able to do
> a DNS request. Same goes for user/group and user home dir resolution:
> anything outside of /etc/{passwd,group} (pam, NSS, etc.), won't work.

Are you referring to the case where a Go-implemented DNS is used? This
should be the case with cgo disabled, and I don't know if the behavior
differs from the regular libc dns.

> I don't know in which context it might be useful in buildroot.

I don't see a reason for it either.

Best,
Christian
Thomas Petazzoni April 8, 2019, 7:04 a.m. UTC | #4
Hello,

On Sun, 07 Apr 2019 19:00:20 -0700
Christian Stewart <christian@paral.in> wrote:

> > I don't know in which context it might be useful in buildroot.  
> 
> I don't see a reason for it either.

Thanks for the feedback, I'll mark the patch as Rejected for now. We
can of course revisit if there's some further details about why this
would be needed.

Anisse, Christian: nobody is listed in the DEVELOPERS file for the
package/pkg-golang.mk file. Would you be willing to be listed as
DEVELOPERS for this file ? It would be useful as you would be Cc'ed on
patches touching this file, which would help in situations like this
one. If you're fine with this, send a patch to add yourself :)

Thanks!

Thomas
Anisse Astier April 8, 2019, 11:23 a.m. UTC | #5
On Sun, Apr 07, 2019 at 07:00:20PM -0700, Christian Stewart wrote:
> Hi Anisse, Cam, all,
> 
> Overriding CGO_ENABLED when we know that the toolchain doesn't work
> properly with cgo doesn't seem correct.
> 
> Anisse Astier <anisse@astier.eu> writes:
> >> Some build targets may not build with CGO_ENABLED=1, so allowing a per
> >> package and build target override will allow those targets to be built
> >> with Buildroot.
> 
> Why not just add the override in the existing build flag overrides?
> 
> > Just like Thomas, I'm wondering what specific use-case you have in mind;
> > in particular, I'm interested why you needed build-target granularity in
> > addition to package-level configuration.
> 
> If you really need to do something this custom, I recommend just
> overriding the build stage entirely.
> 
> > Disabling CGO isn't an issue, until you try doing DNS requests, which
> > are usually handled by your libc. If you have anything configured out of
> > the ordinary /etc/resolv.conf, a cgo-disabled binary won't be able to do
> > a DNS request. Same goes for user/group and user home dir resolution:
> > anything outside of /etc/{passwd,group} (pam, NSS, etc.), won't work.
> 
> Are you referring to the case where a Go-implemented DNS is used? This
> should be the case with cgo disabled, and I don't know if the behavior
> differs from the regular libc dns.

Yes, I'm talking about the go-implemented dns resolution when cgo is
disabled.

On a glibc system you might not simply rely on /etc/{hosts,resolv.conf}
since hostname resolution is pluggable and configurable with
nsswitch.conf.

Regards,

Anisse
Anisse Astier April 8, 2019, 11:25 a.m. UTC | #6
On Mon, Apr 08, 2019 at 09:04:58AM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 07 Apr 2019 19:00:20 -0700
> Christian Stewart <christian@paral.in> wrote:
> 
> > > I don't know in which context it might be useful in buildroot.  
> > 
> > I don't see a reason for it either.
> 
> Thanks for the feedback, I'll mark the patch as Rejected for now. We
> can of course revisit if there's some further details about why this
> would be needed.

Agreed, thanks.

> 
> Anisse, Christian: nobody is listed in the DEVELOPERS file for the
> package/pkg-golang.mk file. Would you be willing to be listed as
> DEVELOPERS for this file ? It would be useful as you would be Cc'ed on
> patches touching this file, which would help in situations like this
> one. If you're fine with this, send a patch to add yourself :)

Good catch, I was wondering why I didn't see the previous emails. I'll
send a patch to add myself and add better email filtering on my side.

Regards,

Anisse
Cam Hutchison April 9, 2019, 12:22 a.m. UTC | #7
On Mon, 8 Apr 2019 at 17:05, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Sun, 07 Apr 2019 19:00:20 -0700
> Christian Stewart <christian@paral.in> wrote:
>
> > > I don't know in which context it might be useful in buildroot.
> >
> > I don't see a reason for it either.
>
> Thanks for the feedback, I'll mark the patch as Rejected for now.

Well that's rather demoralising - waiting 9 months for some feedback
and given only 10 hours to respond before the patch is rejected.

My use case for this was for building Kubernetes, which builds some
components with CGO_ENABLED=0 and others without it. If I tried to
build some components with without it, I got linker errors. Since I
didn't feel like trying to debug/understand the whole Kubernetes build
process, I just went with building it the same way in buildroot as it
builds outside it. But for that I needed to be able to selectively
control CGO_ENABLED on a per-target basis.

But never mind. I can maintain this patch in my local buildroot patch
queue. I just thought it might be useful for others.
Christian Stewart April 9, 2019, 3:22 a.m. UTC | #8
Cam,

On Mon, Apr 8, 2019, 5:22 PM Cam Hutchison <camh@xdna.net> wrote:

> But never mind. I can maintain this patch in my local buildroot patch
> queue. I just thought it might be useful for others.
>

Rejected doesn't mean rejected permanently, just rejected as is. This means
that more information is needed in the commit message, or maybe something
should be changed in the commit itself.

Fwiw, I'm also integrating kubernetes with buildroot at the moment. I don't
see where cgo is required in this way.

I'd be happy to compare notes if you'd like?

Best,
Christian

>
<div dir="auto"><div>Cam,<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019, 5:22 PM Cam Hutchison &lt;<a href="mailto:camh@xdna.net">camh@xdna.net</a>&gt; wrote:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
But never mind. I can maintain this patch in my local buildroot patch<br>
queue. I just thought it might be useful for others.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Rejected doesn&#39;t mean rejected permanently, just rejected as is. This means that more information is needed in the commit message, or maybe something should be changed in the commit itself. </div><div dir="auto"><br></div><div dir="auto">Fwiw, I&#39;m also integrating kubernetes with buildroot at the moment. I don&#39;t see where cgo is required in this way. </div><div dir="auto"><br></div><div dir="auto">I&#39;d be happy to compare notes if you&#39;d like?</div><div dir="auto"><br></div><div dir="auto">Best,</div><div dir="auto">Christian</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
</blockquote></div></div></div>
Thomas Petazzoni April 9, 2019, 6:05 a.m. UTC | #9
Hello Cam,

On Tue, 9 Apr 2019 10:22:19 +1000
Cam Hutchison <camh@xdna.net> wrote:

> > Thanks for the feedback, I'll mark the patch as Rejected for now.  
> 
> Well that's rather demoralising - waiting 9 months for some feedback
> and given only 10 hours to respond before the patch is rejected.

Sorry for this. Taking a step back, I can understand how my very direct
decision can be disappointing.

However, as Christian explained, it's definitely not a "reject
forever", not at all. The status of patches in patchwork can always be
flipped back to whatever state we want.

As you have seen, we have a huge backlog of patches, so when the
feedback on a patch is fairly negative, I tend to immediately update
the patch state to not leave it pending forever. Otherwise, I forget
about it among the 200+ patches that are pending.

But clearly, if some additional explanation is given, either a new
iteration of the patch is sent, or alternatively we could apply the
original version after amending the commit log and/or adjusting the
implementation.

> My use case for this was for building Kubernetes, which builds some
> components with CGO_ENABLED=0 and others without it. If I tried to
> build some components with without it, I got linker errors. Since I
> didn't feel like trying to debug/understand the whole Kubernetes build
> process, I just went with building it the same way in buildroot as it
> builds outside it. But for that I needed to be able to selectively
> control CGO_ENABLED on a per-target basis.

So Christian said he would try to integrate Kubernetes, and see where
it goes. Would that work for you ?

Best regards,

Thomas
Cam Hutchison April 12, 2019, 9:29 p.m. UTC | #10
On Tue, 9 Apr 2019 at 13:22, Christian Stewart <christian@paral.in> wrote:
>
> Fwiw, I'm also integrating kubernetes with buildroot at the moment. I don't see where cgo is required in this way.
>
> I'd be happy to compare notes if you'd like?

My local BR2_EXTERNAL kubernetes config is at
https://github.com/camh-/clusterpi/tree/master/package/kubernetes.
Feel free to take anything from there that is useful.

I'm going to look at k3s instead now (https://github.com/rancher/k3s)
for its smaller size - probably more appropriate for my targets.

Regards,
Cam
Cam Hutchison April 12, 2019, 9:45 p.m. UTC | #11
On Mon, 8 Apr 2019 at 06:28, Thomas Petazzoni <thomas.petazzoni@bootlin.com>
wrote:

> Hello Cam,
>
> +Christian and Anisse in Cc.
>
> Sorry for the very slow feedback.
>
> On Fri, 27 Jul 2018 12:47:20 +1000
> Cam Hutchison <camh@xdna.net> wrote:
>
> > Allow the CGO_ENABLED variable to be controlled per package and per
> > build target, instead of having the value determined by whether or not
> > the toolchain has threads.
> >
> > Some build targets may not build with CGO_ENABLED=1, so allowing a per
> > package and build target override will allow those targets to be built
> > with Buildroot.
>
> Could you give some specific examples instead of the fuzzy "Some build
> targets" wording ?
>

Sorry for my lack of detail. Here are my notes that I made at the time:

* kube-apiserver is not linking, emitting the error `direct call too far`.
If I
  override the go.mk to force CGO_ENABLED=0, it works. The kube build script
  builds a bunch of components with CGO_ENABLED=0, but the buildroot golang
  infra does not support setting CGO_ENABLED - it is hard-coded based on
  whether the toolchain has threads (BR2_TOOLCHAIN_HAS_THREADS). I will need
  to somehow override that.
<div dir="ltr"><div dir="ltr"><div dir="ltr">On Mon, 8 Apr 2019 at 06:28, Thomas Petazzoni &lt;<a href="mailto:thomas.petazzoni@bootlin.com">thomas.petazzoni@bootlin.com</a>&gt; wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Cam,<br>
<br>
+Christian and Anisse in Cc.<br>
<br>
Sorry for the very slow feedback.<br>
<br>
On Fri, 27 Jul 2018 12:47:20 +1000<br>
Cam Hutchison &lt;<a href="mailto:camh@xdna.net" target="_blank">camh@xdna.net</a>&gt; wrote:<br>
<br>
&gt; Allow the CGO_ENABLED variable to be controlled per package and per<br>
&gt; build target, instead of having the value determined by whether or not<br>
&gt; the toolchain has threads.<br>
&gt; <br>
&gt; Some build targets may not build with CGO_ENABLED=1, so allowing a per<br>
&gt; package and build target override will allow those targets to be built<br>
&gt; with Buildroot.<br>
<br>
Could you give some specific examples instead of the fuzzy &quot;Some build<br>
targets&quot; wording ?<br></blockquote><div><br></div><div>Sorry for my lack of detail. Here are my notes that I made at the time:</div><div><br></div><div><div>* kube-apiserver is not linking, emitting the error `direct call too far`. If I</div><div>  override the <a href="http://go.mk">go.mk</a> to force CGO_ENABLED=0, it works. The kube build script</div><div>  builds a bunch of components with CGO_ENABLED=0, but the buildroot golang</div><div>  infra does not support setting CGO_ENABLED - it is hard-coded based on</div><div>  whether the toolchain has threads (BR2_TOOLCHAIN_HAS_THREADS). I will need</div><div>  to somehow override that.</div></div><div><br></div></div></div></div>
Cam Hutchison April 12, 2019, 9:48 p.m. UTC | #12
On Sat, 13 Apr 2019 at 07:29, Cam Hutchison <camh@xdna.net> wrote:

> On Tue, 9 Apr 2019 at 13:22, Christian Stewart <christian@paral.in> wrote:
> >
> > Fwiw, I'm also integrating kubernetes with buildroot at the moment. I
> don't see where cgo is required in this way.
> >
> > I'd be happy to compare notes if you'd like?
>
> My local BR2_EXTERNAL kubernetes config is at
> https://github.com/camh-/clusterpi/tree/master/package/kubernetes.
> Feel free to take anything from there that is useful.
>

Also, here are my notes I made at the time. Perhaps they might be useful:

* Success building kubelet from buildroot
  + I think I've done that before, but I will next try other components
using
    the same pattern
  + Kubernetes buildroot config directly uses go to build binaries, does not
    use kube Makefile or build scripts.
* Success with kube-proxy and kubeadm
* kube-proxy, kubelet and kubeadm are the node binaries, all successfully
built
  from a single buildroot invocation. All were built with CGO_ENABLED=1.
  kube-proxy and kubeadm would normally be build with CGO_ENABLED=0 using
the
  kube build scripts (KUBE_STATIC_LIBRARIES)
* The buildroot golang support does not allow mixed CGO_ENABLED builds of
  different binaries. The value for CGO_ENABLED seems to be hard-coded at
  toolchain build time. I should investigate what it would take to build
both
  CGO_ENABLED=0 and CGO_ENABLED=1 together in the toolchain.
* kube-apiserver build failed. It needs openapi generated files. I had
thought
  that the openapi generation stuff was for docs - not so. I will have to
see
  what needs to be done to hook in the generated files build too. Shouldn't
be
  too hard as the go compiler can build both host and target binaries, so we
  don't need a second host-targetting go compiler.
* Extending the KUBERNETES_BUILD_CMDS to run `make generated_files` worked
just
  fine.
* kube-apiserver is not linking, emitting the error `direct call too far`.
If I
  override the go.mk to force CGO_ENABLED=0, it works. The kube build script
  builds a bunch of components with CGO_ENABLED=0, but the buildroot golang
  infra does not support setting CGO_ENABLED - it is hard-coded based on
  whether the toolchain has threads (BR2_TOOLCHAIN_HAS_THREADS). I will need
  to somehow override that.
* Everything builds now (including hyperkube) using the right CGO settings
for
  the right targets.
<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Sat, 13 Apr 2019 at 07:29, Cam Hutchison &lt;<a href="mailto:camh@xdna.net">camh@xdna.net</a>&gt; wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 9 Apr 2019 at 13:22, Christian Stewart &lt;<a href="mailto:christian@paral.in" target="_blank">christian@paral.in</a>&gt; wrote:<br>
&gt;<br>
&gt; Fwiw, I&#39;m also integrating kubernetes with buildroot at the moment. I don&#39;t see where cgo is required in this way.<br>
&gt;<br>
&gt; I&#39;d be happy to compare notes if you&#39;d like?<br>
<br>
My local BR2_EXTERNAL kubernetes config is at<br>
<a href="https://github.com/camh-/clusterpi/tree/master/package/kubernetes" rel="noreferrer" target="_blank">https://github.com/camh-/clusterpi/tree/master/package/kubernetes</a>.<br>
Feel free to take anything from there that is useful.<br></blockquote><div><br></div><div>Also, here are my notes I made at the time. Perhaps they might be useful:</div><div><br></div><div>* Success building kubelet from buildroot</div><div>  + I think I&#39;ve done that before, but I will next try other components using</div><div>    the same pattern</div><div>  + Kubernetes buildroot config directly uses go to build binaries, does not</div><div>    use kube Makefile or build scripts.</div><div>* Success with kube-proxy and kubeadm</div><div>* kube-proxy, kubelet and kubeadm are the node binaries, all successfully built</div><div>  from a single buildroot invocation. All were built with CGO_ENABLED=1.</div><div>  kube-proxy and kubeadm would normally be build with CGO_ENABLED=0 using the</div><div>  kube build scripts (KUBE_STATIC_LIBRARIES)</div><div>* The buildroot golang support does not allow mixed CGO_ENABLED builds of</div><div>  different binaries. The value for CGO_ENABLED seems to be hard-coded at</div><div>  toolchain build time. I should investigate what it would take to build both</div><div>  CGO_ENABLED=0 and CGO_ENABLED=1 together in the toolchain.</div><div>* kube-apiserver build failed. It needs openapi generated files. I had thought</div><div>  that the openapi generation stuff was for docs - not so. I will have to see</div><div>  what needs to be done to hook in the generated files build too. Shouldn&#39;t be</div><div>  too hard as the go compiler can build both host and target binaries, so we</div><div>  don&#39;t need a second host-targetting go compiler.</div><div>* Extending the KUBERNETES_BUILD_CMDS to run `make generated_files` worked just</div><div>  fine.</div><div>* kube-apiserver is not linking, emitting the error `direct call too far`. If I</div><div>  override the <a href="http://go.mk">go.mk</a> to force CGO_ENABLED=0, it works. The kube build script</div><div>  builds a bunch of components with CGO_ENABLED=0, but the buildroot golang</div><div>  infra does not support setting CGO_ENABLED - it is hard-coded based on</div><div>  whether the toolchain has threads (BR2_TOOLCHAIN_HAS_THREADS). I will need</div><div>  to somehow override that.</div><div>* Everything builds now (including hyperkube) using the right CGO settings for</div><div>  the right targets.</div><div> </div></div></div></div></div>
diff mbox series

Patch

diff --git a/docs/manual/adding-packages-golang.txt b/docs/manual/adding-packages-golang.txt
index efcf696867..2683774861 100644
--- a/docs/manual/adding-packages-golang.txt
+++ b/docs/manual/adding-packages-golang.txt
@@ -99,6 +99,20 @@  therefore only use a few of them, or none.
    +FOO_BUILD_TARGETS = cmd/docker cmd/dockerd+ the binaries produced
    are +docker+ and +dockerd+.
 
+* +FOO_CGO_ENABLED+ overrides the default value for the +CGO_ENABLED+
+  flag passed to the Go compiler. By default +CGO_ENABLED+ is set
+  to 1 if the toolchain supports threads, but you can force it to
+  0 for the package by setting this variable. If the toolchain does
+  not support threads and you set +FOO_CGO_ENABLED+ to 1, an error
+  will be generated.
+
+* +FOO_BUILD_TARGET_CGO_ENABLED_<target>+ can be used to override
+  the +CGO_ENABLED+ flag passed to the Go compiler for a specific
+  build target, similar to +FOO_CGO_ENABLED+. +<target>+ references
+  a target listed in +FOO_BUILD_TARGETS+. If the toolchain does
+  not support threads and you set this variable to 1, an error will
+  be generated.
+
 * +FOO_INSTALL_BINS+ can be used to pass the list of binaries that
   should be installed in +/usr/bin+ on the target. If
   +FOO_INSTALL_BINS+ is not specified, it defaults to the lower-case
diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index bf178622b5..2be3bd4c14 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -28,8 +28,20 @@  GO_BIN = $(HOST_DIR)/bin/go
 GO_TARGET_ENV = \
 	$(HOST_GO_TARGET_ENV) \
 	PATH=$(BR_PATH) \
-	GOBIN= \
-	CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
+	GOBIN=
+
+# Allow CGO_ENABLED to be defined per package and per build target, but if
+# CGO is enabled for a package or a build target and the toolchain does not
+# support that, generate an error.
+GO_CGO_ENABLED = $(strip \
+	$(if $(filter 10,$($(1)_CGO_ENABLED)$(HOST_GO_CGO_ENABLED)),\
+		$(error Toolchain does not support CGO_ENABLED=1 for package $(1)),\
+		$(or $($(1)_CGO_ENABLED),$(HOST_GO_CGO_ENABLED))))
+
+GO_TARGET_CGO_ENABLED = $(strip \
+	$(if $(filter 10,$($(1)_BUILD_TARGET_CGO_ENABLED_$(2))$(HOST_GO_CGO_ENABLED)),\
+		$(error Toolchain does not support CGO_ENABLED=1 for target $(2)),\
+		$(or $($(1)_BUILD_TARGET_CGO_ENABLED_$(2)),$(call GO_CGO_ENABLED,$(1)))))
 
 ################################################################################
 # inner-golang-package -- defines how the configuration, compilation and
@@ -100,6 +112,7 @@  define $(2)_BUILD_CMDS
 	$$(foreach d,$$($(2)_BUILD_TARGETS),\
 		cd $$($(2)_SRC_PATH); \
 		$$(GO_TARGET_ENV) \
+			CGO_ENABLED=$$(call GO_TARGET_CGO_ENABLED,$(2),$$(d)) \
 			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
 			$$($(2)_GO_ENV) \
 			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \