[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Geoff Levand]

Fixes build erorros like these:

  cgo.a(_all.o): unknown relocation type 42; compiled without -fpic?

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go-bootstrap/go-bootstrap.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go-bootstrap/go-bootstrap.mk
index bcf7c0b..1ced355 100644
--- a/package/go-bootstrap/go-bootstrap.mk
+++ b/package/go-bootstrap/go-bootstrap.mk
@@ -19,7 +19,8 @@ HOST_GO_BOOTSTRAP_MAKE_ENV = \
 	GOOS=linux \
 	GOROOT_FINAL="$(HOST_GO_BOOTSTRAP_ROOT)" \
 	GOROOT="$(@D)" \
-	GOBIN="$(@D)/bin"
+	GOBIN="$(@D)/bin" \
+	CGO_ENABLED=0
 
 define HOST_GO_BOOTSTRAP_BUILD_CMDS
 	cd $(@D)/src && $(HOST_GO_BOOTSTRAP_MAKE_ENV) ./make.bash
-- 
2.5.0

[Buildroot] [PATCH 4/9] package/go: Add BR2_TOOLCHAIN_HAS_THREADS

[Geoff Levand]

CGO needs thread support.  Fixes build errors like these:

  warning requested reentrant code, but thread support was disabled

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go/Config.in.host | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/go/Config.in.host b/package/go/Config.in.host
index 094e402..e988483 100644
--- a/package/go/Config.in.host
+++ b/package/go/Config.in.host
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
 	bool
 	default y
+	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc
 	depends on !BR2_ARM_CPU_ARMV4
-- 
2.5.0

[Buildroot] [PATCH 3/9] package/go-bootstrap: Set CC to host CC

[Geoff Levand]

Use the host compiler when building host tools.

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go-bootstrap/go-bootstrap.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go-bootstrap/go-bootstrap.mk
index 1ced355..5dc839a 100644
--- a/package/go-bootstrap/go-bootstrap.mk
+++ b/package/go-bootstrap/go-bootstrap.mk
@@ -20,6 +20,7 @@ HOST_GO_BOOTSTRAP_MAKE_ENV = \
 	GOROOT_FINAL="$(HOST_GO_BOOTSTRAP_ROOT)" \
 	GOROOT="$(@D)" \
 	GOBIN="$(@D)/bin" \
+	CC=$(HOSTCC_NOCCACHE) \
 	CGO_ENABLED=0
 
 define HOST_GO_BOOTSTRAP_BUILD_CMDS
-- 
2.5.0

[Buildroot] [PATCH 0/9] Fixes for go language support

[Geoff Levand]

Hi,

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

[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159336.html

-Geoff

The following changes since commit 30706f9252021283374903c078e4380e7ee0c985:

  android-tools: adb/adbd sub options use fork(), need MMU (2016-05-11 22:47:28 +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 9caa1646d3da7936fb25fd4e4cd2dcd8b8387374:

  package/flannel: Set go env variables (2016-05-11 16:48:02 -0700)

----------------------------------------------------------------
Geoff Levand (9):
      package/go-bootstrap: Add toolchain dependency
      package/go-bootstrap: Set CGO_ENABLED=0
      package/go-bootstrap: Set CC to host CC
      package/go: Add BR2_TOOLCHAIN_HAS_THREADS
      package/go: Fix powerpc64 config typo
      package/go: Enable MIPS support
      package/go: Add HOST_GO_TOOLDIR
      package/go: Build special host binaries
      package/flannel: Set go env variables

 package/flannel/flannel.mk           |  6 ++++-
 package/go-bootstrap/go-bootstrap.mk |  6 ++++-
 package/go/Config.in.host            |  3 ++-
 package/go/go.mk                     | 43 ++++++++++++++++++++++++++++++++----
 4 files changed, 51 insertions(+), 7 deletions(-)

-- 
2.5.0

[Buildroot] [PATCH 1/9] package/go-bootstrap: Add toolchain dependency

[Geoff Levand]

To build programs that need cgo support the toolchain needs to
be available.

Cc: Christian Stewart <christian@paral.in>
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go-bootstrap/go-bootstrap.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go-bootstrap/go-bootstrap.mk
index e404cb6..bcf7c0b 100644
--- a/package/go-bootstrap/go-bootstrap.mk
+++ b/package/go-bootstrap/go-bootstrap.mk
@@ -11,6 +11,8 @@ GO_BOOTSTRAP_SOURCE = go$(GO_BOOTSTRAP_VERSION).src.tar.gz
 GO_BOOTSTRAP_LICENSE = BSD-3c
 GO_BOOTSTRAP_LICENSE_FILES = LICENSE
 
+HOST_GO_BOOTSTRAP_DEPENDENCIES = toolchain
+
 HOST_GO_BOOTSTRAP_ROOT = $(HOST_DIR)/usr/lib/go-$(GO_BOOTSTRAP_VERSION)
 
 HOST_GO_BOOTSTRAP_MAKE_ENV = \
-- 
2.5.0

[Buildroot] [PATCH 7/9] package/go: Add HOST_GO_TOOLDIR

[Geoff Levand]

For the convenience of package makefiles define the
new variable HOST_GO_TOOLDIR.

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go/go.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/go/go.mk b/package/go/go.mk
index 00b1d21..4a99144 100644
--- a/package/go/go.mk
+++ b/package/go/go.mk
@@ -38,6 +38,7 @@ endif
 
 HOST_GO_DEPENDENCIES = host-go-bootstrap
 HOST_GO_ROOT = $(HOST_DIR)/usr/lib/go
+HOST_GO_TOOLDIR = $(HOST_GO_ROOT)/pkg/tool/linux_$(GO_GOARCH)
 
 HOST_GO_MAKE_ENV = \
 	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
-- 
2.5.0

[Buildroot] [PATCH 5/9] package/go: Fix powerpc64 config typo

[Geoff Levand]

Fix typo in config powerpc64 depends.  Go language only supports 64 bit powerpc.

Also add BR2_powerpc64le to depends list.

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go/Config.in.host | 2 +-
 package/go/go.mk          | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/package/go/Config.in.host b/package/go/Config.in.host
index e988483..5833bd9 100644
--- a/package/go/Config.in.host
+++ b/package/go/Config.in.host
@@ -2,5 +2,5 @@ config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
 	bool
 	default y
 	depends on BR2_TOOLCHAIN_HAS_THREADS
-	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc
+	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le
 	depends on !BR2_ARM_CPU_ARMV4
diff --git a/package/go/go.mk b/package/go/go.mk
index 720c75a..a9e16dd 100644
--- a/package/go/go.mk
+++ b/package/go/go.mk
@@ -26,8 +26,10 @@ else ifeq ($(BR2_i386),y)
 GO_GOARCH = 386
 else ifeq ($(BR2_x86_64),y)
 GO_GOARCH = amd64
-else ifeq ($(BR2_powerpc),y)
+else ifeq ($(BR2_powerpc64),y)
 GO_GOARCH = ppc64
+else ifeq ($(BR2_powerpc64le),y)
+GO_GOARCH = ppc64le
 endif
 
 HOST_GO_DEPENDENCIES = host-go-bootstrap
-- 
2.5.0

[Buildroot] [PATCH 6/9] package/go: Enable MIPS support

[Geoff Levand]

Enable go language support for mips64 and mips64el (mips64le), which
were added in go-1.6.2.

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go/Config.in.host | 2 +-
 package/go/go.mk          | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/go/Config.in.host b/package/go/Config.in.host
index 5833bd9..3d8fe45 100644
--- a/package/go/Config.in.host
+++ b/package/go/Config.in.host
@@ -2,5 +2,5 @@ config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
 	bool
 	default y
 	depends on BR2_TOOLCHAIN_HAS_THREADS
-	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le
+	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el
 	depends on !BR2_ARM_CPU_ARMV4
diff --git a/package/go/go.mk b/package/go/go.mk
index a9e16dd..00b1d21 100644
--- a/package/go/go.mk
+++ b/package/go/go.mk
@@ -30,6 +30,10 @@ else ifeq ($(BR2_powerpc64),y)
 GO_GOARCH = ppc64
 else ifeq ($(BR2_powerpc64le),y)
 GO_GOARCH = ppc64le
+else ifeq ($(BR2_mips64),y)
+GO_GOARCH = mips64
+else ifeq ($(BR2_mips64el),y)
+GO_GOARCH = mips64le
 endif
 
 HOST_GO_DEPENDENCIES = host-go-bootstrap
-- 
2.5.0

[Buildroot] [PATCH 9/9] package/flannel: Set go env variables

[Geoff Levand]

To get cross compile builds working correctly.

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/flannel/flannel.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/flannel/flannel.mk b/package/flannel/flannel.mk
index 876efe6..dd500bb 100644
--- a/package/flannel/flannel.mk
+++ b/package/flannel/flannel.mk
@@ -17,7 +17,11 @@ FLANNEL_MAKE_ENV = \
 	GOBIN="$(@D)/bin" \
 	GOPATH="$(@D)/gopath" \
 	GOARCH=$(GO_GOARCH) \
-	CGO_ENABLED=1
+	CGO_ENABLED=1 \
+	CC=$(TARGET_CC) \
+	CXX=$(TARGET_CXX) \
+	GOROOT="$(HOST_GO_ROOT)" \
+	GOTOOLDIR="$(HOST_GO_TOOLDIR)"
 
 FLANNEL_GLDFLAGS = \
 	-X github.com/coreos/flannel/version.Version=$(FLANNEL_VERSION) \
-- 
2.5.0

[Buildroot] [PATCH 8/9] package/go: Build special host binaries

[Geoff Levand]

To work around cross compile limitations of go's build scripts, when the
host and target arches are the same build the host go binaries with
CC_FOR_TARGET set to the host compiler.

Fixes build errors like these:

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

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 package/go/go.mk | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/package/go/go.mk b/package/go/go.mk
index 4a99144..e2ad864 100644
--- a/package/go/go.mk
+++ b/package/go/go.mk
@@ -40,20 +40,47 @@ HOST_GO_DEPENDENCIES = host-go-bootstrap
 HOST_GO_ROOT = $(HOST_DIR)/usr/lib/go
 HOST_GO_TOOLDIR = $(HOST_GO_ROOT)/pkg/tool/linux_$(GO_GOARCH)
 
-HOST_GO_MAKE_ENV = \
+# To work around cross compile limitations of go's build scripts, when the
+# host and target arches are the same build the host go binaries with
+# CC_FOR_TARGET set to the host compiler.
+ifeq ($(ARCH),$(HOSTARCH))
+	HOST_GO_BUILD_SPECIAL = y
+endif
+
+HOST_GO_ENV_COMMON = \
 	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
 	GOROOT_FINAL=$(HOST_GO_ROOT) \
-	GOROOT="$(@D)" \
 	GOBIN="$(@D)/bin" \
 	GOARCH=$(GO_GOARCH) \
 	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
 	GOOS=linux \
 	CGO_ENABLED=1 \
+	CC=$(HOSTCC_NOCCACHE)
+
+HOST_GO_MAKE_ENV = \
+	$(HOST_GO_ENV_COMMON) \
+	GOROOT="$(@D)" \
 	CC_FOR_TARGET=$(TARGET_CC) \
 	CXX_FOR_TARGET=$(TARGET_CXX)
 
+HOST_GO_MAKE_ENV_SPECIAL = \
+	$(HOST_GO_ENV_COMMON) \
+	GOROOT="$(@D).host" \
+	CC_FOR_TARGET=$(HOSTCC_NOCCACHE) \
+	CXX_FOR_TARGET=$(HOSTCC_NOCCACHE) \
+
+define HOST_GO_CONFIGURE_CMDS
+	if [ "$(HOST_GO_BUILD_SPECIAL)" ]; then \
+		cp -a $(@D) $(@D).host; \
+	fi
+endef
+
 define HOST_GO_BUILD_CMDS
 	cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
+	if [ "$(HOST_GO_BUILD_SPECIAL)" ]; then \
+		rm -f $(@D)/bin/go $(@D)/bin/gofmt; \
+		cd $(@D).host/src && $(HOST_GO_MAKE_ENV_SPECIAL) ./make.bash; \
+	fi
 endef
 
 define HOST_GO_INSTALL_CMDS
@@ -66,7 +93,8 @@ define HOST_GO_INSTALL_CMDS
 	cp -a $(@D)/lib $(HOST_GO_ROOT)/
 
 	mkdir -p $(HOST_GO_ROOT)/pkg
-	cp -a $(@D)/pkg/include $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
+	cp -a $(@D)/pkg/include $(HOST_GO_ROOT)/pkg/
+	cp -a $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
 	cp -a $(@D)/pkg/tool $(HOST_GO_ROOT)/pkg/
 
 	# There is a known issue which requires the go sources to be installed
-- 
2.5.0

[Buildroot] [PATCH 0/9] Fixes for go language support

[Christian Stewart]

All,

On Wed, May 11, 2016, 5:09 PM Geoff Levand <geoff@infradead.org> wrote:

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


Looks good to me.

Acked-by: Christian Stewart <christian@paral.in>

~ Christian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160512/df81f2a9/attachment.html>

[Buildroot] [PATCH 3/9] package/go-bootstrap: Set CC to host CC

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:

> diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go-bootstrap/go-bootstrap.mk
> index 1ced355..5dc839a 100644
> --- a/package/go-bootstrap/go-bootstrap.mk
> +++ b/package/go-bootstrap/go-bootstrap.mk
> @@ -20,6 +20,7 @@ HOST_GO_BOOTSTRAP_MAKE_ENV = \
>  	GOROOT_FINAL="$(HOST_GO_BOOTSTRAP_ROOT)" \
>  	GOROOT="$(@D)" \
>  	GOBIN="$(@D)/bin" \
> +	CC=$(HOSTCC_NOCCACHE) \

Why do you use HOSTCC_NOCCACHE instead of just HOSTCC ? Is the usage of
ccache a problem?

Thanks,

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

[Buildroot] [PATCH 4/9] package/go: Add BR2_TOOLCHAIN_HAS_THREADS

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:
> CGO needs thread support.  Fixes build errors like these:
> 
>   warning requested reentrant code, but thread support was disabled
> 
> Signed-off-by: Geoff Levand <geoff@infradead.org>

When a commit fixes some build issues reported by the autobuilders,
please add something like:

Fixes:

   http://autobuild.buildroot.net/results/0f0f5892e474bdf9145242cee836b8acc383734f/

in the commit log.

Thanks!

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

[Buildroot] [PATCH 1/9] package/go-bootstrap: Add toolchain dependency

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:

> +HOST_GO_BOOTSTRAP_DEPENDENCIES = toolchain

Since it's far from being obvious, I've added a comment above this
line, which says:

+# To build programs that need cgo support the toolchain needs to be
+# available, so the toolchain is not needed to build host-go-bootstrap
+# itself, but needed by other packages that depend on
+# host-go-bootstrap.

And applied your patch to master, thanks!

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

[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:
> Fixes build erorros like these:
> 
>   cgo.a(_all.o): unknown relocation type 42; compiled without -fpic?
> 
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  package/go-bootstrap/go-bootstrap.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to master, thanks.

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

[Buildroot] [PATCH 4/9] package/go: Add BR2_TOOLCHAIN_HAS_THREADS

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:
> CGO needs thread support.  Fixes build errors like these:
> 
>   warning requested reentrant code, but thread support was disabled
> 
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  package/go/Config.in.host | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/go/Config.in.host b/package/go/Config.in.host
> index 094e402..e988483 100644
> --- a/package/go/Config.in.host
> +++ b/package/go/Config.in.host
> @@ -1,5 +1,6 @@
>  config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
>  	bool
>  	default y
> +	depends on BR2_TOOLCHAIN_HAS_THREADS

This is not an architecture dependency, it's a toolchain dependency. We
normally show a comment about such a dependency so that users know they
need to enable thread support in their toolchain.

When you say "CGO needs thread support", is CGO the go runtime on the
target? Does this means that any Go program needs threads?

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

[Buildroot] [PATCH 5/9] package/go: Fix powerpc64 config typo

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:47 +0000, Geoff Levand wrote:
> Fix typo in config powerpc64 depends.  Go language only supports 64 bit powerpc.
> 
> Also add BR2_powerpc64le to depends list.
> 
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  package/go/Config.in.host | 2 +-
>  package/go/go.mk          | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Applied to master, thanks.

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

[Buildroot] [PATCH 0/9] Fixes for go language support

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:

> Geoff Levand (9):
>       package/go-bootstrap: Add toolchain dependency
>       package/go-bootstrap: Set CGO_ENABLED=0
>       package/go-bootstrap: Set CC to host CC
>       package/go: Add BR2_TOOLCHAIN_HAS_THREADS
>       package/go: Fix powerpc64 config typo
>       package/go: Enable MIPS support
>       package/go: Add HOST_GO_TOOLDIR
>       package/go: Build special host binaries
>       package/flannel: Set go env variables

I've applied a few patches, and made comments on a few others. However,
there's one key issue with this series: it mixes new features (like
"Enable MIPS support") with build/bug fixes, without saying which patch
is a build/bug fix and which patch is a feature addition. For example,
I have no idea if "package/go: Add HOST_GO_TOOLDIR" or "package/go:
Build special host binaries" are needed to fix the build issues or not.

Also, as I said as a reply to one of the patches, please add references
to the autobuilder failures.

Could you resend a new series, on top of the latest master (since I've
applied some of your patches), which contains as the first patches the
build/bug fixes and then as the last patches the feature
additions/improvements? The former will be committed in the master
branch, while the latter will be committed in the next branch.

Thanks!

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

[Buildroot] [PATCH 8/9] package/go: Build special host binaries

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 00:08:48 +0000, Geoff Levand wrote:
> To work around cross compile limitations of go's build scripts, when the
> host and target arches are the same build the host go binaries with
> CC_FOR_TARGET set to the host compiler.

What happens when the host and target architectures are different? It
all works fine?

Is the problem just that Go doesn't "cross compile" properly when the
host and target architectures are the same ?

> +# To work around cross compile limitations of go's build scripts, when the

Please give more details about those "limitations".

> +# host and target arches are the same build the host go binaries with
> +# CC_FOR_TARGET set to the host compiler.
> +ifeq ($(ARCH),$(HOSTARCH))
> +	HOST_GO_BUILD_SPECIAL = y

Don't indent this line.

> +endif
> +
> +HOST_GO_ENV_COMMON = \
>  	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
>  	GOROOT_FINAL=$(HOST_GO_ROOT) \
> -	GOROOT="$(@D)" \
>  	GOBIN="$(@D)/bin" \
>  	GOARCH=$(GO_GOARCH) \
>  	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
>  	GOOS=linux \
>  	CGO_ENABLED=1 \
> +	CC=$(HOSTCC_NOCCACHE)

This addition seems unrelated to the patch. And why are you using
HOSTCC_NOCCACHE and not just HOSTCC ?

> +
> +HOST_GO_MAKE_ENV = \
> +	$(HOST_GO_ENV_COMMON) \
> +	GOROOT="$(@D)" \
>  	CC_FOR_TARGET=$(TARGET_CC) \
>  	CXX_FOR_TARGET=$(TARGET_CXX)
>  
> +HOST_GO_MAKE_ENV_SPECIAL = \
> +	$(HOST_GO_ENV_COMMON) \
> +	GOROOT="$(@D).host" \
> +	CC_FOR_TARGET=$(HOSTCC_NOCCACHE) \
> +	CXX_FOR_TARGET=$(HOSTCC_NOCCACHE) \

Please no final backslash on the last line.

> +define HOST_GO_CONFIGURE_CMDS
> +	if [ "$(HOST_GO_BUILD_SPECIAL)" ]; then \
> +		cp -a $(@D) $(@D).host; \
> +	fi
> +endef

Please use make conditionals, i.e:

ifeq ($(HOST_GO_BUILD_SPECIAL),y)
define HOST_GO_CONFIGURE_CMDS
	cp -a $(@D) $(@D).host
endef
endif

> +
>  define HOST_GO_BUILD_CMDS
>  	cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash

You really need a different set of environment variables for here?

> +	if [ "$(HOST_GO_BUILD_SPECIAL)" ]; then \
> +		rm -f $(@D)/bin/go $(@D)/bin/gofmt; \
> +		cd $(@D).host/src && $(HOST_GO_MAKE_ENV_SPECIAL) ./make.bash; \

And here ?

Also, same here: use make conditionals, i.e:

ifeq ($(HOST_GO_BUILD_SPECIAL),y)
define HOST_GO_BUILD_SPECIAL_CMDS
	...
endef
endif

define HOST_GO_BUILD_CMDS
	.... build go normally ...
	$(HOST_GO_BUILD_SPECIAL_CMDS)
endef

> +	fi
>  endef
>  
>  define HOST_GO_INSTALL_CMDS
> @@ -66,7 +93,8 @@ define HOST_GO_INSTALL_CMDS
>  	cp -a $(@D)/lib $(HOST_GO_ROOT)/
>  
>  	mkdir -p $(HOST_GO_ROOT)/pkg
> -	cp -a $(@D)/pkg/include $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
> +	cp -a $(@D)/pkg/include $(HOST_GO_ROOT)/pkg/
> +	cp -a $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/

This change seems useless and unrelated.

Thanks!

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

[Buildroot] [PATCH 3/9] package/go-bootstrap: Set CC to host CC

[Geoff Levand]

On Thu, 2016-05-12 at 15:28 +0200, Thomas Petazzoni wrote:
> On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:
> 
> > diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go
> > -bootstrap/go-bootstrap.mk
> > index 1ced355..5dc839a 100644
> > --- a/package/go-bootstrap/go-bootstrap.mk
> > +++ b/package/go-bootstrap/go-bootstrap.mk
> > @@ -20,6 +20,7 @@ HOST_GO_BOOTSTRAP_MAKE_ENV = \
> >  	GOROOT_FINAL="$(HOST_GO_BOOTSTRAP_ROOT)" \
> >  	GOROOT="$(@D)" \
> >  	GOBIN="$(@D)/bin" \
> > +	CC=$(HOSTCC_NOCCACHE) \
> 
> Why do you use HOSTCC_NOCCACHE instead of just HOSTCC ? Is the usage
> of
> ccache a problem?
> 

Yes, it is generally known that the go build system is not compatible
with the use of ccache.

  https://github.com/golang/go/issues/11685

-Geoff

[Buildroot] [PATCH 8/9] package/go: Build special host binaries

[Geoff Levand]

Hi,

On Thu, 2016-05-12 at 16:34 +0200, Thomas Petazzoni wrote:
> On Thu, 12 May 2016 00:08:48 +0000, Geoff Levand wrote:
> > To work around cross compile limitations of go's build scripts, when the
> > host and target arches are the same build the host go binaries with
> > CC_FOR_TARGET set to the host compiler.
> 
> What happens when the host and target architectures are different? It
> all works fine?

Yes, it works fine.

> Is the problem just that Go doesn't "cross compile" properly when the
> host and target architectures are the same ?

Yes, the 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 compiler for the
target.  When the architectures are the same it assumes it will use
the same compiler for both host and target.

> > +# To work around cross compile limitations of go's build scripts, when the
> 
> Please give more details about those "limitations".

OK.

> > +# host and target arches are the same build the host go binaries with
> > +# CC_FOR_TARGET set to the host compiler.
> > +ifeq ($(ARCH),$(HOSTARCH))
> > +> > 	> > HOST_GO_BUILD_SPECIAL = y
> 
> Don't indent this line.
> 
> > +endif
> > +
> > +HOST_GO_ENV_COMMON = \
> >  > > 	> > GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
> >  > > 	> > GOROOT_FINAL=$(HOST_GO_ROOT) \
> > -> > 	> > GOROOT="$(@D)" \
> >  > > 	> > GOBIN="$(@D)/bin" \
> >  > > 	> > GOARCH=$(GO_GOARCH) \
> >  > > 	> > $(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
> >  > > 	> > GOOS=linux \
> >  > > 	> > CGO_ENABLED=1 \
> > +> > 	> > CC=$(HOSTCC_NOCCACHE)
> 
> This addition seems unrelated to the patch. And why are you using
> HOSTCC_NOCCACHE and not just HOSTCC ?

As mentioned in my reply to Patch 3/9, it is generally known that
the go build system is not compatible with the use of ccache.  See

  https://github.com/golang/go/issues/11685

> > +
> > +HOST_GO_MAKE_ENV = \
> > +> > 	> > $(HOST_GO_ENV_COMMON) \
> > +> > 	> > GOROOT="$(@D)" \
> >  > > 	> > CC_FOR_TARGET=$(TARGET_CC) \
> >  > > 	> > CXX_FOR_TARGET=$(TARGET_CXX)
> >  
> > +HOST_GO_MAKE_ENV_SPECIAL = \
> > +> > 	> > $(HOST_GO_ENV_COMMON) \
> > +> > 	> > GOROOT="$(@D).host" \
> > +> > 	> > CC_FOR_TARGET=$(HOSTCC_NOCCACHE) \
> > +> > 	> > CXX_FOR_TARGET=$(HOSTCC_NOCCACHE) \
> 
> Please no final backslash on the last line.

Sure, missed that.

> > +define HOST_GO_CONFIGURE_CMDS
> > +> > 	> > if [ "$(HOST_GO_BUILD_SPECIAL)" ]; then \
> > +> > 	> > 	> > cp -a $(@D) $(@D).host; \
> > +> > 	> > fi
> > +endef
> 
> Please use make conditionals, i.e:
> 
> ifeq ($(HOST_GO_BUILD_SPECIAL),y)
> define HOST_GO_CONFIGURE_CMDS
> 	> cp -a $(@D) $(@D).host
> endef
> endif

OK.

> 
> > +
> >  define HOST_GO_BUILD_CMDS
> >  > > 	> > cd $(@D)/src && $(HOST_GO_MAKE_ENV) ./make.bash
> 
> You really need a different set of environment variables for here?
> 
> > +> > 	> > if [ "$(HOST_GO_BUILD_SPECIAL)" ]; then \
> > +> > 	> > 	> > rm -f $(@D)/bin/go $(@D)/bin/gofmt; \
> > +> > 	> > 	> > cd $(@D).host/src && $(HOST_GO_MAKE_ENV_SPECIAL) ./make.bash; \
> 
> And here ?

Yes, to make the build scripts build the target bits with a
cross compiler of the same architecture as the host.  Another
option, which I have not seriously looked into, is to possibly
patch the go build scripts.

> Also, same here: use make conditionals, i.e:
> 
> ifeq ($(HOST_GO_BUILD_SPECIAL),y)
> define HOST_GO_BUILD_SPECIAL_CMDS
> 	> ...
> endef
> endif
> 
> define HOST_GO_BUILD_CMDS
> 	> .... build go normally ...
> 	> $(HOST_GO_BUILD_SPECIAL_CMDS)
> endef
> 
> > +> > 	> > fi
> >  endef
> >  
> >  define HOST_GO_INSTALL_CMDS
> > @@ -66,7 +93,8 @@ define HOST_GO_INSTALL_CMDS
> >  > > 	> > cp -a $(@D)/lib $(HOST_GO_ROOT)/
> >  
> >  > > 	> > mkdir -p $(HOST_GO_ROOT)/pkg
> > -> > 	> > cp -a $(@D)/pkg/include $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
> > +> > 	> > cp -a $(@D)/pkg/include $(HOST_GO_ROOT)/pkg/
> > +> > 	> > cp -a $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
> 
> This change seems useless and unrelated.

Sure, I'll remove it.

-Geoff

[Buildroot] [PATCH 4/9] package/go: Add BR2_TOOLCHAIN_HAS_THREADS

[Geoff Levand]

Hi,

On Thu, 2016-05-12 at 16:12 +0200, Thomas Petazzoni wrote:
> On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote:
> > CGO needs thread support.  Fixes build errors like these:
> > 
> >   warning requested reentrant code, but thread support was disabled
> > 
> > Signed-off-by: Geoff Levand <geoff@infradead.org>
> > ---
> >  package/go/Config.in.host | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/package/go/Config.in.host b/package/go/Config.in.host
> > index 094e402..e988483 100644
> > --- a/package/go/Config.in.host
> > +++ b/package/go/Config.in.host
> > @@ -1,5 +1,6 @@
> >  config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
> >  > > 	> > bool
> >  > > 	> > default y
> > +> > 	> > depends on BR2_TOOLCHAIN_HAS_THREADS
> 
> This is not an architecture dependency, it's a toolchain dependency. We
> normally show a comment about such a dependency so that users know they
> need to enable thread support in their toolchain.

OK, so a Config.host.in like this?

config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
	bool
	default y
	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el
	depends on !BR2_ARM_CPU_ARMV4

comment "go needs a toolchain w/ threads"
	depends on !BR2_TOOLCHAIN_HAS_THREADS
	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS

> When you say "CGO needs thread support", is CGO the go runtime on the
> target? Does this means that any Go program needs threads?

cgo is the C language call support and has a runtime component (the
cgo package).

The host go compiler and some host tools use pthreads, and so are
linked with libpthread.

If a target go program uses the cgo package, it will need to be
linked with libpthread.

-Geoff

[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Peter Korsgaard]

>>>>> "Geoff" == Geoff Levand <geoff@infradead.org> writes:

 > Fixes build erorros like these:
 >   cgo.a(_all.o): unknown relocation type 42; compiled without -fpic?

 > Signed-off-by: Geoff Levand <geoff@infradead.org>
 > ---
 >  package/go-bootstrap/go-bootstrap.mk | 3 ++-
 >  1 file changed, 2 insertions(+), 1 deletion(-)

 > diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go-bootstrap/go-bootstrap.mk
 > index bcf7c0b..1ced355 100644
 > --- a/package/go-bootstrap/go-bootstrap.mk
 > +++ b/package/go-bootstrap/go-bootstrap.mk
 > @@ -19,7 +19,8 @@ HOST_GO_BOOTSTRAP_MAKE_ENV = \
 >  	GOOS=linux \
 >  	GOROOT_FINAL="$(HOST_GO_BOOTSTRAP_ROOT)" \
 >  	GOROOT="$(@D)" \
 > -	GOBIN="$(@D)/bin"
 > +	GOBIN="$(@D)/bin" \
 > +	CGO_ENABLED=0

That is not a very detailed description. I know next to nothing about
go, but according to google this doesn't have anything directly to do
with -fpic or not. From https://golang.org/cmd/cgo/

Cgo enables the creation of Go packages that call C code.
..
The cgo tool is enabled by default for native builds on systems where it
is expected to work. It is disabled by default when cross-compiling. You
can control this by setting the CGO_ENABLED environment variable when
running the go tool: set it to 1 to enable the use of cgo, and to 0 to
disable it. The go tool will set the build constraint "cgo" if cgo is
enabled.

When cross-compiling, you must specify a C cross-compiler for cgo to
use. You can do this by setting the CC_FOR_TARGET environment variable
when building the toolchain using make.bash, or by setting the CC
environment variable any time you run the go tool. The CXX_FOR_TARGET
and CXX environment variables work in a similar way for C++ code.


So it sounds like this change disables the cgo tool. How does that match
with your other change to pull in the toolchain dependencies which was
"To build programs that need cgo support"?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 3/9] package/go-bootstrap: Set CC to host CC

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 09:53:32 -0700, Geoff Levand wrote:

> Yes, it is generally known that the go build system is not compatible
> with the use of ccache.
> 
>   https://github.com/golang/go/issues/11685

OK. Then please improve the commit log to explain this, and add a
comment in the code that it is intentional to not use ccache because it
doesn't play well with Go's build system.

Thanks!

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

[Buildroot] [PATCH 4/9] package/go: Add BR2_TOOLCHAIN_HAS_THREADS

[Thomas Petazzoni]

Hello,

On Thu, 12 May 2016 11:32:04 -0700, Geoff Levand wrote:

> OK, so a Config.host.in like this?
> 
> config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
> 	bool
> 	default y
> 	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el
> 	depends on !BR2_ARM_CPU_ARMV4
> 
> comment "go needs a toolchain w/ threads"
> 	depends on !BR2_TOOLCHAIN_HAS_THREADS
> 	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS

No, because we don't show comments for host packages (since most of
them are not selectable in menuconfig).

Unfortunately, the only solution that I see now is that *target* go
packages (such as flannel) should have this dependency on
BR2_TOOLCHAIN_HAS_THREADS + the comment.

All in all, I am in fact not super happy with how we're handling this,
but I don't really see how to do it better. One option would be to make
the go package a target package rather than a host package. But it's
also weird because it basically installs nothing to the target.

> > When you say "CGO needs thread support", is CGO the go runtime on the
> > target? Does this means that any Go program needs threads?  
> 
> cgo is the C language call support and has a runtime component (the
> cgo package).

Are all Go packages going to use "cgo" ? Or are pure Go software
potentially not using cgo ?

> The host go compiler and some host tools use pthreads, and so are
> linked with libpthread.

The fact that the host go compiler uses threads is irrelevant. The
BR2_TOOLCHAIN_HAS_THREADS boolean indicate whether there is thread
support or not on the *target*. You can basically assume that thread
support is always available on the host.

> If a target go program uses the cgo package, it will need to be
> linked with libpthread.

So if I understand correctly, it's only a subset of target Go programs
that need thread support?

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

[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Geoff Levand]

Hi,

On Fri, 2016-05-13 at 13:56 +0200, Peter Korsgaard wrote:
> > > > > > "Geoff" == Geoff Levand <geoff@infradead.org> writes:
> 
>  > Fixes build erorros like these:
>  >   cgo.a(_all.o): unknown relocation type 42; compiled without 
> -fpic?
> 
>  > Signed-off-by: Geoff Levand <geoff@infradead.org>
>  > ---
>  >  package/go-bootstrap/go-bootstrap.mk | 3 ++-
>  >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
>  > diff --git a/package/go-bootstrap/go-bootstrap.mk b/package/go
> -bootstrap/go-bootstrap.mk
>  > index bcf7c0b..1ced355 100644
>  > --- a/package/go-bootstrap/go-bootstrap.mk
>  > +++ b/package/go-bootstrap/go-bootstrap.mk
>  > @@ -19,7 +19,8 @@ HOST_GO_BOOTSTRAP_MAKE_ENV = \
>  >  	GOOS=linux \
>  >  	GOROOT_FINAL="$(HOST_GO_BOOTSTRAP_ROOT)" \
>  >  	GOROOT="$(@D)" \
>  > -	GOBIN="$(@D)/bin"
>  > +	GOBIN="$(@D)/bin" \
>  > +	CGO_ENABLED=0
> 
> That is not a very detailed description. I know next to nothing about
> go, but according to google this doesn't have anything directly to do
> with -fpic or not. From https://golang.org/cmd/cgo/

Sure, I'll add some more details to the comment.

> Cgo enables the creation of Go packages that call C code.
> ..
> The cgo tool is enabled by default for native builds on systems where
> it
> is expected to work. It is disabled by default when cross-compiling.
> You
> can control this by setting the CGO_ENABLED environment variable when
> running the go tool: set it to 1 to enable the use of cgo, and to 0
> to
> disable it. The go tool will set the build constraint "cgo" if cgo is
> enabled.
> 
> When cross-compiling, you must specify a C cross-compiler for cgo to
> use. You can do this by setting the CC_FOR_TARGET environment
> variable
> when building the toolchain using make.bash, or by setting the CC
> environment variable any time you run the go tool. The CXX_FOR_TARGET
> and CXX environment variables work in a similar way for C++ code.
> 
> 
> So it sounds like this change disables the cgo tool. How does that
> match
> with your other change to pull in the toolchain dependencies which
> was
> "To build programs that need cgo support"?

This change disables cgo support for the bootstrap compiler.  It is not
needed.  The bootstrap compiler is just used to build the final go
compiler.  We need full cgo support in the final go compiler to support
any go programs that may need cgo support.

I hope this makes more sense.

-Geoff

[Buildroot] [PATCH 4/9] package/go: Add BR2_TOOLCHAIN_HAS_THREADS

[Geoff Levand]

On Fri, 2016-05-13 at 15:35 +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 12 May 2016 11:32:04 -0700, Geoff Levand wrote:
> 
> > OK, so a Config.host.in like this?
> > 
> > config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
> > 	bool
> > 	default y
> > 	depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 ||
> > BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el
> > 	depends on !BR2_ARM_CPU_ARMV4
> > 
> > comment "go needs a toolchain w/ threads"
> > 	depends on !BR2_TOOLCHAIN_HAS_THREADS
> > 	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
> 
> No, because we don't show comments for host packages (since most of
> them are not selectable in menuconfig).

OK.

> Unfortunately, the only solution that I see now is that *target* go
> packages (such as flannel) should have this dependency on
> BR2_TOOLCHAIN_HAS_THREADS + the comment.

That seems like it will work.

> All in all, I am in fact not super happy with how we're handling
> this,
> but I don't really see how to do it better. One option would be to
> make
> the go package a target package rather than a host package. But it's
> also weird because it basically installs nothing to the target.
> 
> > > When you say "CGO needs thread support", is CGO the go runtime on
> > > the
> > > target? Does this means that any Go program needs threads?  
> > 
> > cgo is the C language call support and has a runtime component (the
> > cgo package).
> 
> Are all Go packages going to use "cgo" ? Or are pure Go software
> potentially not using cgo ?

No, see below.

> The host go compiler and some host tools use pthreads, and so are
> > linked with libpthread.
> 
> The fact that the host go compiler uses threads is irrelevant. The
> BR2_TOOLCHAIN_HAS_THREADS boolean indicate whether there is thread
> support or not on the *target*. You can basically assume that thread
> support is always available on the host.
> 
> > If a target go program uses the cgo package, it will need to be
> > linked with libpthread.
> 
> So if I understand correctly, it's only a subset of target Go
> programs
> that need thread support?

Yes, if they don't use the cgo package, they shouldn't need thread
support, so the solution seems to be to just leave it to the target
package to specify thread support is needed.

I'll make a non-cgo test package to verify.

-Geoff

[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Peter Korsgaard]

>>>>> "Geoff" == Geoff Levand <geoff@infradead.org> writes:

Hi,

 >> So it sounds like this change disables the cgo tool. How does that
 >> match
 >> with your other change to pull in the toolchain dependencies which
 >> was
 >> "To build programs that need cgo support"?

 > This change disables cgo support for the bootstrap compiler.  It is not
 > needed.  The bootstrap compiler is just used to build the final go
 > compiler.  We need full cgo support in the final go compiler to support
 > any go programs that may need cgo support.

 > I hope this makes more sense.

Ok, thanks - But why did you then add toolchain to
HOST_GO_BOOTSTRAP_DEPENDENCIES and not to HOST_GO_DEPENDENCIES with the
comment that it was about cgo?

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH 2/9] package/go-bootstrap: Set CGO_ENABLED=0

[Geoff Levand]

On Fri, 2016-05-13 at 19:12 +0200, Peter Korsgaard wrote:
> > > > >  > This change disables cgo support for the bootstrap compiler.  It is not
>  > needed.  The bootstrap compiler is just used to build the final go
>  > compiler.  We need full cgo support in the final go compiler to support
>  > any go programs that may need cgo support.
> 
>  > I hope this makes more sense.
> 
> Ok, thanks - But why did you then add toolchain to
> HOST_GO_BOOTSTRAP_DEPENDENCIES and not to HOST_GO_DEPENDENCIES with the
> comment that it was about cgo?

Yes, we could do it that way.  I'll update the patch.

-Geoff