Fixes for go language support

PATCH Superseded Geoff Levand 3 participants 18 messages May 19, 2016 – May 26, 2016

lore

Share & tools

lore

Geoff Levand

author

2016-05-19 22:25 UTC #0 lore raw

v2 cover Fixes for go language support

Hi,

This series includes a number of fixup for the go language support. All the buildbot problems pointed out by Thomas [1] should be resolved by this series.

As requested, I pulled out the enhancment patches to post as a seperate series [2]. Please consider all patches in this series to apply to master.

[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159336.html
[2] https://github.com/glevand/buildroot--buildroot/tree/for-merge-go-enhancements

-Geoff

The following changes since commit 3f3342a581970977f28620f17391d8aaf2204fec:

Update for 2016.05-rc2 (2016-05-17 16:11:24 +0200)

are available in the git repository at:

git at github.com:glevand/buildroot--buildroot.git for-merge-go-fixes

for you to fetch changes up to ec83a5927025fc759d5bfc26ee01aabde3658a7c:

package/go: Set file timestamp (2016-05-19 15:17:20 -0700)

----------------------------------------------------------------
Geoff Levand (6):
package/go: Build host tools with host CC package/go: Add HOST_GO_TARGET_ENV package/flannel: Use HOST_GO_TARGET_ENV package/go: Add HOST_GO_CGO_ENABLED package/flannel: Add BR2_TOOLCHAIN_HAS_THREADS package/go: Set file timestamp

 package/flannel/Config.in  |  4 +++
 package/flannel/flannel.mk |  2 +-
 package/go/go.mk           | 66 ++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 66 insertions(+), 6 deletions(-)

Signature · 1 line

2.5.0

Geoff Levand

author

2016-05-19 22:25 UTC #1 lore raw

PATCH v2 1/6 package/go: Build host tools with host CC

The go build system doesn't have the notion of cross compiling, but just the notion of architecture. When the host and target architectures are different it expects to be given a target cross compiler in CC_FOR_TARGET. When the architectures are the same it will use CC_FOR_TARGET for both host and target compilation. To work around this limitation, when the host and target architectures are the same build a set of host binaries and tools with CC_FOR_TARGET set to the host compiler. Also, the go build system is not compatible with ccache, so use HOSTCC_NOCCACHE. See
https://github.com/golang/go/issues/11685.

Fixes build errors like these:

host/usr/bin/go: No such file or directory

http://autobuild.buildroot.net/results/6664189a6f3a815978e8d0a1d7ef408ca47e2874/

Signed-off-by Geoff Levand <[email protected]>

---
 package/go/go.mk | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

package/go/go.mk +37 -4

index a9e16dd..5acd75f 100644
@@ -35,6 +35,16 @@ endif
 HOST_GO_DEPENDENCIES = host-go-bootstrap
 HOST_GO_ROOT = $(HOST_DIR)/usr/lib/go
 
+# The go build system doesn't have the notion of cross compiling, but just the
+# notion of architecture.  When the host and target architectures are different
+# it expects to be given a target cross compiler in CC_FOR_TARGET.  When the
+# architectures are the same it will use CC_FOR_TARGET for both host and target
+# compilation.  To work around this limitation, when the host and target
+# architectures are the same build a set of host binaries and tools with
+# CC_FOR_TARGET set to the host compiler.  Also, the go build system is not
+# compatible with ccache, so use HOSTCC_NOCCACHE.  See
+# https://github.com/golang/go/issues/11685.
+
 HOST_GO_MAKE_ENV = \
 	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
 	GOROOT_FINAL=$(HOST_GO_ROOT) \

@@ -44,16 +54,39 @@ HOST_GO_MAKE_ENV = \
 	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
 	GOOS=linux \
 	CGO_ENABLED=1 \
+	CC=$(HOSTCC_NOCCACHE)
+
+HOST_GO_TARGET_CC = \
 	CC_FOR_TARGET=$(TARGET_CC) \
 	CXX_FOR_TARGET=$(TARGET_CXX)
 
+HOST_GO_HOST_CC = \
+	CC_FOR_TARGET=$(HOSTCC_NOCCACHE) \
+	CXX_FOR_TARGET=$(HOSTCC_NOCCACHE)
+
+HOST_GO_TMP = $(@D)/host-go-tmp
+
+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-	cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+	mv $(@D)/bin/ $(HOST_GO_TMP)/
 endef
+else
+define HOST_GO_BUILD_CMDS
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	cp -a $(@D)/pkg/tool $(HOST_GO_TMP)/
+	cp -a $(@D)/bin/ $(HOST_GO_TMP)/
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+	cp -a $(@D)/pkg/tool $(HOST_GO_TMP)/
+endef
+endif
 
 define HOST_GO_INSTALL_CMDS
-	$(INSTALL) -D -m 0755 $(@D)/bin/go $(HOST_GO_ROOT)/bin/go
-	$(INSTALL) -D -m 0755 $(@D)/bin/gofmt $(HOST_GO_ROOT)/bin/gofmt
+	$(INSTALL) -D -m 0755 $(HOST_GO_TMP)/bin/go $(HOST_GO_ROOT)/bin/go
+	$(INSTALL) -D -m 0755 $(HOST_GO_TMP)/bin/gofmt $(HOST_GO_ROOT)/bin/gofmt
 
 	ln -sf ../lib/go/bin/go $(HOST_DIR)/usr/bin/
 	ln -sf ../lib/go/bin/gofmt $(HOST_DIR)/usr/bin/

@@ -62,7 +95,7 @@ define HOST_GO_INSTALL_CMDS
 
 	mkdir -p $(HOST_GO_ROOT)/pkg
 	cp -a $(@D)/pkg/include $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
-	cp -a $(@D)/pkg/tool $(HOST_GO_ROOT)/pkg/
+	cp -a $(HOST_GO_TMP)/tool $(HOST_GO_ROOT)/pkg/
 
 	# There is a known issue which requires the go sources to be installed
 	# https://golang.org/issue/2775

Signature · 1 line

2.5.0

Thomas Petazzoni

2016-05-24 19:11 UTC #2 lore raw

PATCH v2 1/6 package/go: Build host tools with host CC

Hello,

Thanks for this new version.

package/go/go.mk HOST_GO_MAKE_ENV = \ · line 74

Quoted 9 lines · Geoff Levand

package/go/go.mk +7 -1

+HOST_GO_TMP = $(@D)/host-go-tmp
+
+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-	cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+	mv $(@D)/bin/ $(HOST_GO_TMP)/

So here you are moving what was built to HOST_GO_TMP.

package/go/go.mk HOST_GO_MAKE_ENV = \ · line 83

Quoted 9 lines

endef

package/go/go.mk +8 -0

+else
+define HOST_GO_BUILD_CMDS
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	cp -a $(@D)/pkg/tool $(HOST_GO_TMP)/
+	cp -a $(@D)/bin/ $(HOST_GO_TMP)/
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+	cp -a $(@D)/pkg/tool $(HOST_GO_TMP)/

And here you're using a copy. Why?

Also, in this last case, you are copying pkg/tool/ twice, once after the build with HOST_GO_HOST_CC, and once after the build with HOST_GO_TARGET_CC. It seems like the first copy is useless.

Could you comment on this?

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Geoff Levand

author

2016-05-24 21:31 UTC #3 lore raw

PATCH v2 1/6 package/go: Build host tools with host CC

Hi,

Quoted 14 lines · Thomas Petazzoni

Review

Quoted 9 lines · Geoff Levand

(quoted diff) +7 -1

+HOST_GO_TMP = $(@D)/host-go-tmp
+
+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+> > 	> > mkdir -p $(HOST_GO_TMP)
+> > 	> > mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+> > 	> > mv $(@D)/bin/ $(HOST_GO_TMP)/

So here you are moving what was built to HOST_GO_TMP.

Quoted 11 lines

Review

Quoted 9 lines

endef

(quoted diff) +8 -0

+else
+define HOST_GO_BUILD_CMDS
+> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+> > 	> > mkdir -p $(HOST_GO_TMP)
+> > 	> > cp -a $(@D)/pkg/tool $(HOST_GO_TMP)/
+> > 	> > cp -a $(@D)/bin/ $(HOST_GO_TMP)/
+> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+> > 	> > cp -a $(@D)/pkg/tool $(HOST_GO_TMP)/

And here you're using a copy. Why?

We can use mv here also.

Quoted 3 lines

Also, in this last case, you are copying pkg/tool/ twice, once after the build with HOST_GO_HOST_CC, and once after the build with HOST_GO_TARGET_CC. It seems like the first copy is useless.

This got messed up, this last cp line should not be there. We want the binaries and tools build with the host compiler to be installed. Everything else installed should be build with the target compiler.

I'll post a follow up patch.

-Geoff

Geoff Levand

author

2016-05-24 21:34 UTC #4 lore raw

PATCH v2 1/6 package/go: Build host tools with host CC

In reply to Geoff Levand

Fixes build errors like these:

host/usr/bin/go: No such file or directory

http://autobuild.buildroot.net/results/6664189a6f3a815978e8d0a1d7ef408ca47e2874/

Signed-off-by Geoff Levand <[email protected]>

---
 package/go/go.mk | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

package/go/go.mk +36 -4

index a9e16dd..45c29fe 100644
@@ -35,6 +35,16 @@ endif
 HOST_GO_DEPENDENCIES = host-go-bootstrap
 HOST_GO_ROOT = $(HOST_DIR)/usr/lib/go
 
+# The go build system doesn't have the notion of cross compiling, but just the
+# notion of architecture.  When the host and target architectures are different
+# it expects to be given a target cross compiler in CC_FOR_TARGET.  When the
+# architectures are the same it will use CC_FOR_TARGET for both host and target
+# compilation.  To work around this limitation, when the host and target
+# architectures are the same build a set of host binaries and tools with
+# CC_FOR_TARGET set to the host compiler.  Also, the go build system is not
+# compatible with ccache, so use HOSTCC_NOCCACHE.  See
+# https://github.com/golang/go/issues/11685.
+
 HOST_GO_MAKE_ENV = \
 	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
 	GOROOT_FINAL=$(HOST_GO_ROOT) \

@@ -44,16 +54,38 @@ HOST_GO_MAKE_ENV = \
 	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
 	GOOS=linux \
 	CGO_ENABLED=1 \
+	CC=$(HOSTCC_NOCCACHE)
+
+HOST_GO_TARGET_CC = \
 	CC_FOR_TARGET=$(TARGET_CC) \
 	CXX_FOR_TARGET=$(TARGET_CXX)
 
+HOST_GO_HOST_CC = \
+	CC_FOR_TARGET=$(HOSTCC_NOCCACHE) \
+	CXX_FOR_TARGET=$(HOSTCC_NOCCACHE)
+
+HOST_GO_TMP = $(@D)/host-go-tmp
+
+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-	cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+	mv $(@D)/bin/ $(HOST_GO_TMP)/
 endef
+else
+define HOST_GO_BUILD_CMDS
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+	mv $(@D)/bin/ $(HOST_GO_TMP)/
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+endef
+endif
 
 define HOST_GO_INSTALL_CMDS
-	$(INSTALL) -D -m 0755 $(@D)/bin/go $(HOST_GO_ROOT)/bin/go
-	$(INSTALL) -D -m 0755 $(@D)/bin/gofmt $(HOST_GO_ROOT)/bin/gofmt
+	$(INSTALL) -D -m 0755 $(HOST_GO_TMP)/bin/go $(HOST_GO_ROOT)/bin/go
+	$(INSTALL) -D -m 0755 $(HOST_GO_TMP)/bin/gofmt $(HOST_GO_ROOT)/bin/gofmt
 
 	ln -sf ../lib/go/bin/go $(HOST_DIR)/usr/bin/
 	ln -sf ../lib/go/bin/gofmt $(HOST_DIR)/usr/bin/

@@ -62,7 +94,7 @@ define HOST_GO_INSTALL_CMDS
 
 	mkdir -p $(HOST_GO_ROOT)/pkg
 	cp -a $(@D)/pkg/include $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
-	cp -a $(@D)/pkg/tool $(HOST_GO_ROOT)/pkg/
+	cp -a $(HOST_GO_TMP)/tool $(HOST_GO_ROOT)/pkg/
 
 	# There is a known issue which requires the go sources to be installed
 	# https://golang.org/issue/2775

Signature · 1 line

2.5.0

Thomas Petazzoni

2016-05-25 14:05 UTC #5 lore raw

PATCH 1/6 package/go: Build host tools with host CC

Hello,

Quoted 5 lines · Geoff Levand

package/go/go.mk +3 -1

+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-	cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)

package/go/go.mk HOST_GO_MAKE_ENV = \ · line 82

Quoted 10 lines

package/go/go.mk +9 -0

+	mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+	mv $(@D)/bin/ $(HOST_GO_TMP)/
 endef
+else
+define HOST_GO_BUILD_CMDS
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+	mkdir -p $(HOST_GO_TMP)
+	mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+	mv $(@D)/bin/ $(HOST_GO_TMP)/
+	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash

Just to make sure I understand correctly: you build everything with HOST_GO_HOST_CC first, and put the files generated in pkg/tool and bin/ on the side in HOST_GO_TMP, and then rebuild everything again with HOST_GO_TARGET_CC. Then later on, the installation step will pick up bin/ and pkg/tool from HOST_GO_TMP, and the rest from $(@D) directly ?

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Geoff Levand

author

2016-05-25 16:41 UTC #6 lore raw

PATCH v2 1/6 package/go: Build host tools with host CC

Quoted 17 lines · Thomas Petazzoni

Hello,

Quoted 13 lines · Geoff Levand

(quoted diff) +10 -1

+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+> > 	> > mkdir -p $(HOST_GO_TMP)
+> > 	> > mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+> > 	> > mv $(@D)/bin/ $(HOST_GO_TMP)/
 endef
+else
+define HOST_GO_BUILD_CMDS
+> > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+> > 	> > mkdir -p $(HOST_GO_TMP)
+> > 	> > mv $(@D)/pkg/tool $(HOST_GO_TMP)/

Quoted 8 lines

Quoted 2 lines
+> > > > mv $(@D)/bin/ $(HOST_GO_TMP)/
+> > > > cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
Just to make sure I understand correctly: you build everything with HOST_GO_HOST_CC first, and put the files generated in pkg/tool and bin/ on the side in HOST_GO_TMP, and then rebuild everything again with HOST_GO_TARGET_CC. Then later on, the installation step will pick up bin/ and pkg/tool from HOST_GO_TMP, and the rest from $(@D) directly ?

Yes, that is correct.

bin and tool with HOST_GO_HOST_CC, because those are host binaries. The rest with HOST_GO_TARGET_CC because those are target binaries.

If you can think of a better way to do this two step build I can try it.

-Geoff

Thomas Petazzoni

2016-05-25 20:59 UTC #7 lore raw

PATCH 1/6 package/go: Build host tools with host CC

Hello,

Quoted 7 lines · Geoff Levand

Quoted 5 lines
Just to make sure I understand correctly: you build everything with HOST_GO_HOST_CC first, and put the files generated in pkg/tool and bin/ on the side in HOST_GO_TMP, and then rebuild everything again with HOST_GO_TARGET_CC. Then later on, the installation step will pick up bin/ and pkg/tool from HOST_GO_TMP, and the rest from $(@D) directly ?
Yes, that is correct.

OK, thanks for confirming!

Quoted 4 lines

bin and tool with HOST_GO_HOST_CC, because those are host binaries. The rest with HOST_GO_TARGET_CC because those are target binaries.
If you can think of a better way to do this two step build I can try it.

I was thinking of having the host parts build by host-go and the target parts by the "go" package, but I'm not sure it's easy to achieve nor that it will make the stuff clearer.

Now that I think of it, why do we have to have those two cases:

+ifneq ($(ARCH),$(HOSTARCH))
define HOST_GO_BUILD_CMDS
- cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+ cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+ mkdir -p $(HOST_GO_TMP)
+ mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+ mv $(@D)/bin/ $(HOST_GO_TMP)/
endef
+else
+define HOST_GO_BUILD_CMDS
+ cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+ mkdir -p $(HOST_GO_TMP)
+ mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+ mv $(@D)/bin/ $(HOST_GO_TMP)/
+ cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+endef
+endif

Aren't the host binaries always built for the host, and the target binaries always built for the target? So can't we replace this by just:

+define HOST_GO_BUILD_CMDS
+ cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+ mkdir -p $(HOST_GO_TMP)
+ mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+ mv $(@D)/bin/ $(HOST_GO_TMP)/
+ cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+endef

and get in fact the exact same behavior?

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Geoff Levand

author

2016-05-26 17:24 UTC #8 lore raw

PATCH v2 1/6 package/go: Build host tools with host CC

Hi,

Quoted 9 lines · Thomas Petazzoni

On Wed, 25 May 2016 09:41:54 -0700, Geoff Levand wrote:
bin and tool with HOST_GO_HOST_CC, because those are host binaries.
Quoted 3 lines
The rest with HOST_GO_TARGET_CC because those are target binaries.
If you can think of a better way to do this two step build I can try it.
I was thinking of having the host parts build by host-go and the target parts by the "go" package, but I'm not sure it's easy to achieve nor that it will make the stuff clearer.

Seems to be more complicated.

Quoted 31 lines

Now that I think of it, why do we have to have those two cases:

(quoted diff) +14 -1

+ifneq ($(ARCH),$(HOSTARCH))
 define HOST_GO_BUILD_CMDS
-> 	> cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+> 	> cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+> 	> mkdir -p $(HOST_GO_TMP)
+> 	> mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+> 	> mv $(@D)/bin/ $(HOST_GO_TMP)/
 endef
+else
+define HOST_GO_BUILD_CMDS
+> 	> cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+> 	> mkdir -p $(HOST_GO_TMP)
+> 	> mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+> 	> mv $(@D)/bin/ $(HOST_GO_TMP)/
+> 	> cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+endef
+endif

Aren't the host binaries always built for the host, and the target binaries always built for the target? So can't we replace this by just:

(quoted diff) +7 -0

+define HOST_GO_BUILD_CMDS
+> 	> cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+> 	> mkdir -p $(HOST_GO_TMP)
+> 	> mv $(@D)/pkg/tool $(HOST_GO_TMP)/
+> 	> mv $(@D)/bin/ $(HOST_GO_TMP)/
+> 	> cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) ./make.bash
+endef

Quoted 1 line

and get in fact the exact same behavior?

There is a subtle difference, when we build with HOST_GO_HOST_CC the resulting host compiler binaries have the go environment variables CC and CXX set to the host compilers. They should be set to the target cross compilers so that by default go programs are built with the target cross compiler. That was one motivation for my follow up patch 'package/go: Add HOST_GO_TARGET_ENV', which sets those to the proper values for target package builds.

I had planned to look into how to fix this environment problem. One idea was to use a custom version of ./make.bash that would do the right thing.

On thinking about it, it seems it would be best for now to do the two step build for all configs and expect package maintainers to either use HOST_GO_TARGET_ENV or set CC and CXX. That would avoid the situation where the maintainer does not test with host arch equal to target arch.

-Geoff

Thomas Petazzoni

2016-05-26 17:30 UTC #9 lore raw

PATCH 1/6 package/go: Build host tools with host CC

Hello,

Quoted 7 lines · Geoff Levand

There is a subtle difference, when we build with HOST_GO_HOST_CC the resulting host compiler binaries have the go environment variables CC and CXX set to the host compilers. They should be set to the target cross compilers so that by default go programs are built with the target cross compiler. That was one motivation for my follow up patch 'package/go: Add HOST_GO_TARGET_ENV', which sets those to the proper values for target package builds.

Ah, indeed.

Quoted 9 lines

I had planned to look into how to fix this environment problem. One idea was to use a custom version of ./make.bash that would do the right thing.
On thinking about it, it seems it would be best for now to do the two step build for all configs and expect package maintainers to either use HOST_GO_TARGET_ENV or set CC and CXX. That would avoid the situation where the maintainer does not test with host arch equal to target arch.

OK. Can you respin the patch quickly to address this? the host-go and flannel issues are the main remaining build issues we have, and it would be good to fix them for 2016.05.

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Geoff Levand

author

2016-05-19 22:25 UTC #10 lore raw

PATCH v2 2/6 package/go: Add HOST_GO_TARGET_ENV

In reply to Geoff Levand

For the convenience of package makefiles define the new make variables HOST_GO_TOOLDIR and HOST_GO_TARGET_ENV.

Signed-off-by Geoff Levand <[email protected]>

---
 package/go/go.mk | 9 +++++++++
 1 file changed, 9 insertions(+)

package/go/go.mk +9 -0

index 5acd75f..5e4ee09 100644
@@ -35,6 +35,15 @@ endif
 HOST_GO_DEPENDENCIES = host-go-bootstrap
 HOST_GO_ROOT = $(HOST_DIR)/usr/lib/go
 
+# For the convienience of target packages.
+HOST_GO_TOOLDIR = $(HOST_GO_ROOT)/pkg/tool/linux_$(GO_GOARCH)
+HOST_GO_TARGET_ENV = \
+	GOARCH=$(GO_GOARCH) \
+	GOROOT="$(HOST_GO_ROOT)" \
+	CC=$(TARGET_CC) \
+	CXX=$(TARGET_CXX) \
+	GOTOOLDIR="$(HOST_GO_TOOLDIR)"
+
 # The go build system doesn't have the notion of cross compiling, but just the
 # notion of architecture.  When the host and target architectures are different
 # it expects to be given a target cross compiler in CC_FOR_TARGET.  When the

Signature · 1 line

2.5.0

Geoff Levand

author

2016-05-19 22:25 UTC #11 lore raw

PATCH v2 3/6 package/flannel: Use HOST_GO_TARGET_ENV

In reply to Geoff Levand

Use the newly added HOST_GO_TARGET_ENV variable to pickup the correct go environment for package builds.

Signed-off-by Geoff Levand <[email protected]>

---
 package/flannel/flannel.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/flannel/flannel.mk +1 -1

index 876efe6..b5f61a0 100644
@@ -14,9 +14,9 @@ FLANNEL_LICENSE_FILES = LICENSE
 FLANNEL_DEPENDENCIES = host-go
 
 FLANNEL_MAKE_ENV = \
+	$(HOST_GO_TARGET_ENV) \
 	GOBIN="$(@D)/bin" \
 	GOPATH="$(@D)/gopath" \
-	GOARCH=$(GO_GOARCH) \
 	CGO_ENABLED=1
 
 FLANNEL_GLDFLAGS = \

Signature · 1 line

2.5.0

Geoff Levand

author

2016-05-19 22:25 UTC #12 lore raw

PATCH v2 4/6 package/go: Add HOST_GO_CGO_ENABLED

In reply to Geoff Levand

The go compiler's cgo support uses threads. If BR2_TOOLCHAIN_HAS_THREADS is set, build in cgo support for any go programs that may need it. Note that any target package needing cgo support must include
'depends on BR2_TOOLCHAIN_HAS_THREADS' in its config file.

Fixes build errors like these:

error: #warning requested reentrant code

http://autobuild.buildroot.net/results/42a8d07101d8d954511d1c884ecb66e8d861899e

Signed-off-by Geoff Levand <[email protected]>

---
 package/go/go.mk | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

package/go/go.mk +11 -1

index 5e4ee09..8565a86 100644
@@ -44,6 +44,16 @@ HOST_GO_TARGET_ENV = \
 	CXX=$(TARGET_CXX) \
 	GOTOOLDIR="$(HOST_GO_TOOLDIR)"
 
+# The go compiler's cgo support uses threads.  If BR2_TOOLCHAIN_HAS_THREADS is
+# set, build in cgo support for any go programs that may need it.  Note that
+# any target package needing cgo support must include
+# 'depends on BR2_TOOLCHAIN_HAS_THREADS' in its config file.
+ifeq (BR2_TOOLCHAIN_HAS_THREADS,y)
+	HOST_GO_CGO_ENABLED = 1
+else
+	HOST_GO_CGO_ENABLED = 0
+endif
+
 # The go build system doesn't have the notion of cross compiling, but just the
 # notion of architecture.  When the host and target architectures are different
 # it expects to be given a target cross compiler in CC_FOR_TARGET.  When the

@@ -62,7 +72,7 @@ HOST_GO_MAKE_ENV = \
 	GOARCH=$(GO_GOARCH) \
 	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
 	GOOS=linux \
-	CGO_ENABLED=1 \
+	CGO_ENABLED=$(HOST_GO_CGO_ENABLED) \
 	CC=$(HOSTCC_NOCCACHE)
 
 HOST_GO_TARGET_CC = \

Signature · 1 line

2.5.0

Geoff Levand

author

2016-05-19 22:25 UTC #13 lore raw

PATCH v2 5/6 package/flannel: Add BR2_TOOLCHAIN_HAS_THREADS

In reply to Geoff Levand

flannel uses the cgo package, so needs a toolchain with thread support.

Signed-off-by Geoff Levand <[email protected]>

---
 package/flannel/Config.in | 4 ++++
 1 file changed, 4 insertions(+)

package/flannel/Config.in +4 -0

index c6a84ef..32d9f66 100644
@@ -1,6 +1,10 @@
+comment "flannel needs a toolchain w/ threads"
+	depends on !BR2_TOOLCHAIN_HAS_THREADS
+
 config BR2_PACKAGE_FLANNEL
 	bool "flannel"
 	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
+	depends on BR2_TOOLCHAIN_HAS_THREADS
 	help
 	  Flannel is a virtual network that gives a subnet to each
 	  host for use with container runtimes.

Signature · 1 line

2.5.0

Geoff Levand

author

2016-05-19 22:25 UTC #14 lore raw

PATCH v2 6/6 package/go: Set file timestamp

In reply to Geoff Levand

Set all file timestamps to prevent the go compiler from rebuilding any built in packages when programs are built.

Signed-off-by Geoff Levand <[email protected]>

---
 package/go/go.mk | 4 ++++
 1 file changed, 4 insertions(+)

package/go/go.mk +4 -0

index 8565a86..a2babab 100644
@@ -119,6 +119,10 @@ define HOST_GO_INSTALL_CMDS
 	# There is a known issue which requires the go sources to be installed
 	# https://golang.org/issue/2775
 	cp -a $(@D)/src $(HOST_GO_ROOT)/
+
+	# Set all file timestamps to prevent the go compiler from rebuilding any
+	# built in packages when programs are built.
+	find $(HOST_GO_ROOT) -type f -exec touch -r $(HOST_GO_TMP)/bin/go {} \;
 endef
 
 $(eval $(host-generic-package))

Signature · 1 line

2.5.0

Thomas Petazzoni

2016-05-24 19:13 UTC #15 lore raw

PATCH v2 6/6 package/go: Set file timestamp

Hello,

Quoted 13 lines · Geoff Levand

Set all file timestamps to prevent the go compiler from rebuilding any built in packages when programs are built.
Signed-off-by Geoff Levand <[email protected]>
---
 package/go/go.mk | 4 ++++
 1 file changed, 4 insertions(+)
package/go/go.mk +0 -0
index 8565a86..a2babab 100644

package/go/go.mk define HOST_GO_INSTALL_CMDS · line 126

Quoted 8 lines

package/go/go.mk +4 -0

@@ -119,6 +119,10 @@ define HOST_GO_INSTALL_CMDS
 	# There is a known issue which requires the go sources to be installed
 	# https://golang.org/issue/2775
 	cp -a $(@D)/src $(HOST_GO_ROOT)/
+
+	# Set all file timestamps to prevent the go compiler from rebuilding any
+	# built in packages when programs are built.
+	find $(HOST_GO_ROOT) -type f -exec touch -r $(HOST_GO_TMP)/bin/go {} \;
 endef

That's weird, why is it needed? Why are timestamps wrong in the first place?

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Christian Stewart

2016-05-24 20:12 UTC #16 lore raw

PATCH v2 6/6 package/go: Set file timestamp

Geoff,

Same question. I'm not sure if the timestamps are even considered by the compiler. If so, do you have a reference to how this works?

Thanks,
Christian

Quoted 28 lines · Thomas Petazzoni

Hello,
Quoted 13 lines · Geoff Levand
Set all file timestamps to prevent the go compiler from rebuilding any built in packages when programs are built.
Signed-off-by Geoff Levand <[email protected]>
---
 package/go/go.mk | 4 ++++
 1 file changed, 4 insertions(+)
package/go/go.mk +0 -0
index 8565a86..a2babab 100644
package/go/go.mk define HOST_GO_INSTALL_CMDS · line 119
Quoted 1 line
package/go/go.mk +0 -0
@@ -119,6 +119,10 @@ define HOST_GO_INSTALL_CMDS
      # There is a known issue which requires the go sources to be
installed
package/go/go.mk define HOST_GO_INSTALL_CMDS
Quoted 4 lines
package/go/go.mk +2 -0
      # https://golang.org/issue/2775
      cp -a $(@D)/src $(HOST_GO_ROOT)/
+
+     # Set all file timestamps to prevent the go compiler from
rebuilding any
package/go/go.mk define HOST_GO_INSTALL_CMDS · line 125
Quoted 2 lines
package/go/go.mk +2 -0
+     # built in packages when programs are built.
+     find $(HOST_GO_ROOT) -type f -exec touch -r $(HOST_GO_TMP)/bin/go
{} \;
package/go/go.mk define HOST_GO_INSTALL_CMDS · line 126
Quoted 1 line
package/go/go.mk +0 -0
 endef

Quoted 9 lines

That's weird, why is it needed? Why are timestamps wrong in the first place?
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160524/dbcac3da/attachment.html>

Geoff Levand

author

2016-05-24 21:35 UTC #17 lore raw

PATCH v2 6/6 package/go: Set file timestamp

In reply to Thomas Petazzoni

Hi,

Quoted 5 lines · Thomas Petazzoni

Quoted 2 lines · Geoff Levand
Set all file timestamps to prevent the go compiler from rebuilding any built in packages when programs are built.
That's weird, why is it needed?

If the times are not set, when a go program is built go may rebuild some of the installed packages. This is the same kind of time based dependecy checking make does.

Without this change I can observe installed packages being rebuilt when flannel is built on my machine.

Quoted 2 lines

Why are timestamps wrong in the first
place?

I guess go is built in parallel, and so the artifacts have different times. Gentoo does a similar timestamp operation in the pkg_postinst of its ebuild:

https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/dev-lang/go/go-9999.ebuild?logsort=cvs&revision=1.30&view=markup

-Geoff