All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings
@ 2017-10-16 17:08 Angelo Compagnucci
  2017-10-16 17:08 ` [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Angelo Compagnucci @ 2017-10-16 17:08 UTC (permalink / raw)
  To: buildroot

This patch fixes a bug with the BR2_TOOLCHAIN_HAS_THREADS variable
handling which causes CGO_ENABLED to be always 0.

Furthermore, it fixes the cross compilation options for the go compiler:
setting CGO_ENABLED should be done only for the target compiler not the
host one.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/go/go.mk | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/package/go/go.mk b/package/go/go.mk
index 0781dff..6035164 100644
--- a/package/go/go.mk
+++ b/package/go/go.mk
@@ -52,7 +52,7 @@ HOST_GO_TARGET_ENV = \
 # 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)
+ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
 HOST_GO_CGO_ENABLED = 1
 else
 HOST_GO_CGO_ENABLED = 0
@@ -74,8 +74,8 @@ HOST_GO_MAKE_ENV = \
 	GOARCH=$(GO_GOARCH) \
 	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
 	GOOS=linux \
-	CGO_ENABLED=$(HOST_GO_CGO_ENABLED) \
-	CC=$(HOSTCC_NOCCACHE)
+	CC=$(HOSTCC_NOCCACHE) \
+	CXX=$(HOSTCXX_NOCCACHE)
 
 HOST_GO_TARGET_CC = \
 	CC_FOR_TARGET="$(TARGET_CC)" \
@@ -83,16 +83,18 @@ HOST_GO_TARGET_CC = \
 
 HOST_GO_HOST_CC = \
 	CC_FOR_TARGET=$(HOSTCC_NOCCACHE) \
-	CXX_FOR_TARGET=$(HOSTCC_NOCCACHE)
+	CXX_FOR_TARGET=$(HOSTCXX_NOCCACHE)
 
 HOST_GO_TMP = $(@D)/host-go-tmp
 
 define HOST_GO_BUILD_CMDS
-	cd $(@D)/src && $(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) ./make.bash
+	cd $(@D)/src && \
+		$(HOST_GO_MAKE_ENV) $(HOST_GO_HOST_CC) CGO_ENABLED=0 ./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
+	cd $(@D)/src && \
+		$(HOST_GO_MAKE_ENV) $(HOST_GO_TARGET_CC) CGO_ENABLED=$(HOST_GO_CGO_ENABLED) ./make.bash
 endef
 
 define HOST_GO_INSTALL_CMDS
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure
  2017-10-16 17:08 [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Angelo Compagnucci
@ 2017-10-16 17:08 ` Angelo Compagnucci
  2017-10-21 19:55   ` Thomas Petazzoni
  2017-10-16 17:08 ` [Buildroot] [RFC v2 3/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Angelo Compagnucci @ 2017-10-16 17:08 UTC (permalink / raw)
  To: buildroot

This patch adds a new infrastructure for golang based packages.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/Makefile.in   |   1 +
 package/pkg-golang.mk | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+)
 create mode 100644 package/pkg-golang.mk

diff --git a/package/Makefile.in b/package/Makefile.in
index a1a5316..60d98d0 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -429,3 +429,4 @@ include package/pkg-kconfig.mk
 include package/pkg-rebar.mk
 include package/pkg-kernel-module.mk
 include package/pkg-waf.mk
+include package/pkg-golang.mk
diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
new file mode 100644
index 0000000..c4e6410
--- /dev/null
+++ b/package/pkg-golang.mk
@@ -0,0 +1,121 @@
+################################################################################
+# Golang package infrastructure
+#
+# This file implements an infrastructure that eases development of
+# package .mk files for Go packages. It should be used for all
+# packages that are written in go.
+#
+# See the Buildroot documentation for details on the usage of this
+# infrastructure
+#
+#
+# In terms of implementation, this golang infrastructure requires
+# the .mk file to only specify metadata information about the
+# package: name, version, download URL, etc.
+#
+# We still allow the package .mk file to override what the different
+# steps are doing, if needed. For example, if <PKG>_BUILD_CMDS is
+# already defined, it is used as the list of commands to perform to
+# build the package, instead of the default golang behaviour. The
+# package can also define some post operation hooks.
+#
+################################################################################
+
+################################################################################
+# inner-golang-package -- defines how the configuration, compilation and
+# installation of a Go package should be done, implements a few hooks to
+# tune the build process for Go specifities and calls the generic package
+# infrastructure to generate the necessary make targets
+#
+#  argument 1 is the lowercase package name
+#  argument 2 is the uppercase package name, including a HOST_ prefix
+#             for host packages
+#  argument 3 is the uppercase package name, without the HOST_ prefix
+#             for host packages
+#  argument 4 is the type (target or host)
+################################################################################
+
+define inner-golang-package
+
+ifndef $(2)_MAKE_ENV
+define $(2)_MAKE_ENV
+	$$(HOST_GO_TARGET_ENV) \
+	GOPATH="$$(@D)/gopath" \
+	CGO_ENABLED=$$(HOST_GO_CGO_ENABLED)
+endef
+endif
+
+# Target packages need the Go compiler on the host.
+$(2)_DEPENDENCIES += host-go
+
+#
+# go install command doesn't work well when cross compilation is enabled,
+# we set the executable output of the compilation to a specific location.
+# We set this variable here to be used by packages if needed.
+#
+$(2)_EXECUTABLE = $$(@D)/gopath/bin/$(1)
+
+#
+# Source files in Go should be uncompressed in a precise folder in the
+# hiearchy of GOPATH. It usually resolves around domain/vendor/software.
+#
+$(1)_src_path ?= $$(call domain,$($(2)_SITE))/$$(firstword $$(subst /, ,$$(call notdomain,$($(2)_SITE))))
+$(2)_SRC_PATH  = $$(@D)/gopath/src/$$($(1)_src_path)/$(1)
+
+#
+# Configure step. Only define it if not already defined by the package
+# .mk file. And take care of the differences between host and target
+# packages.
+#
+ifndef $(2)_CONFIGURE_CMDS
+define $(2)_CONFIGURE_CMDS
+	mkdir -p $$(@D)/gopath/bin
+	mkdir -p $$(@D)/gopath/src/$$($(1)_src_path)
+	ln -sf $$(@D) $$($(2)_SRC_PATH)
+endef
+endif
+
+#
+# Build step. Only define it if not already defined by the package .mk file.
+# There is no differences between host and target packages.
+#
+ifndef $(2)_BUILD_CMDS
+define $(2)_BUILD_CMDS
+	cd $$($(2)_SRC_PATH) && $$($(2)_MAKE_ENV) $(HOST_DIR)/bin/go build \
+	-o $$($(2)_EXECUTABLE) -v $$($(2)_BUILD_OPTS)
+endef
+endif
+
+#
+# Host installation step. Only define it if not already defined by the
+# package .mk file.
+#
+ifndef $(2)_INSTALL_CMDS
+define $(2)_INSTALL_CMDS
+	$(INSTALL) -D -m 0755 $$($(2)_EXECUTABLE) $(HOST_DIR)/usr/bin/
+endef
+endif
+
+#
+# Target installation step. Only define it if not already defined by the
+# package .mk file.
+#
+ifndef $(2)_INSTALL_TARGET_CMDS
+define $(2)_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $$($(2)_EXECUTABLE) $(TARGET_DIR)/usr/bin/
+endef
+endif
+
+# Call the generic package infrastructure to generate the necessary make
+# targets
+$(call inner-generic-package,$(1),$(2),$(3),$(4))
+
+endef # inner-golang-package
+
+################################################################################
+# golang-package -- the target generator macro for Go packages
+################################################################################
+
+golang-package = $(call inner-golang-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+host-golang-package = $(call inner-golang-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 3/4] docs/manual: adding documentation for the golang infrastructure
  2017-10-16 17:08 [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Angelo Compagnucci
  2017-10-16 17:08 ` [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
@ 2017-10-16 17:08 ` Angelo Compagnucci
  2017-10-16 17:08 ` [Buildroot] [RFC v2 4/4] package/flannel: converting to " Angelo Compagnucci
  2017-10-21 19:51 ` [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Thomas Petazzoni
  3 siblings, 0 replies; 12+ messages in thread
From: Angelo Compagnucci @ 2017-10-16 17:08 UTC (permalink / raw)
  To: buildroot

This patch adds the documentation for the golang infrastructure.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 docs/manual/adding-packages-golang.txt | 111 +++++++++++++++++++++++++++++++++
 docs/manual/adding-packages.txt        |   2 +
 2 files changed, 113 insertions(+)
 create mode 100644 docs/manual/adding-packages-golang.txt

diff --git a/docs/manual/adding-packages-golang.txt b/docs/manual/adding-packages-golang.txt
new file mode 100644
index 0000000..2ba45f0
--- /dev/null
+++ b/docs/manual/adding-packages-golang.txt
@@ -0,0 +1,111 @@
+// -*- mode:doc; -*-
+// vim: set syntax=asciidoc:
+
+=== Infrastructure for Go packages
+
+This infrastructure applies to Go packages that use the standard
+build system and use bundled dependencies.
+
+[[golang-package-tutorial]]
+
+==== +golang-package+ tutorial
+
+First, let's see how to write a +.mk+ file for a go package,
+with an example :
+
+------------------------
+01: ################################################################################
+02: #
+03: # go-foo
+04: #
+05: ################################################################################
+06:
+07: GO_FOO_VERSION = 1.0
+08: GO_FOO_SOURCE = go-foo-$(GO_FOO_VERSION).tar.xz
+09: GO_FOO_SITE = http://www.foosoftware.org/download
+10: GO_FOO_LICENSE = BSD-3-Clause
+11: GO_FOO_LICENSE_FILES = LICENSE
+12:
+13: $(eval $(golang-package))
+------------------------
+
+On line 7, we declare the version of the package.
+
+On line 8 and 9, we declare the name of the tarball (xz-ed tarball
+recommended) and the location of the tarball on the Web. Buildroot
+will automatically download the tarball from this location.
+
+On line 10 and 11, we give licensing details about the package (its
+license on line 10, and the file containing the license text on line
+11).
+
+Finally, on line 13, we invoke the +golang-package+ macro that
+generates all the Makefile rules that actually allow the package to be
+built.
+
+[[golang-package-reference]]
+
+==== +golang-package+ reference
+
+As a policy packages can freely choose their name (existing example in 
+Buildroot is +flannel+).
+
+In their +Config.in+ file, they should depend on 
++BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS+ and
++BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS+ cause host-go will compile when Buildroot will add the
+dependency automatically.
+
+The main macro of the Go package infrastructure is
++golang-package+. It is similar to the +generic-package+ macro. It is
+also possible to create Go host packages with the
++host-golang-package+ macro.
+
+Just like the generic infrastructure, the Go infrastructure works
+by defining a number of variables before calling the +golang-package+
+or +host-golang-package+ macros.
+
+All the package metadata information variables that exist in the
+xref:generic-package-reference[generic package infrastructure] also
+exist in the Go infrastructure: +GO_FOO_VERSION+,
++GO_FOO_SOURCE+, +GO_FOO_PATCH+, +GO_FOO_SITE+,
++GO_FOO_SUBDIR+, +GO_FOO_DEPENDENCIES+, +GO_FOO_LICENSE+,
++GO_FOO_LICENSE_FILES+, +GO_FOO_INSTALL_STAGING+, etc.
+
+Note that:
+
+ * It is not necessary to add +go+ or +host-go+ in the
+   +GO_FOO_DEPENDENCIES+ variable of a package, since these basic
+   dependencies are automatically added as needed by the Go
+   package infrastructure.
+
+A few additional variables, specific to the Go infrastructure, can
+optionally be defined, depending on the package's needs. Many of them
+are only useful in very specific cases, typical packages will
+therefore only use a few of them, or none.
+
+* +GO_FOO_GO_SRC_PATH+: go sources should be compiled inside GOPATH.
+  The golang package infrastructure tries to guess the correct 
+  GOPATH subfolder to compile in. If guessing is not correct or your
+  package behaves differently, you can use this variable to
+  adjust the path.
+  
+
+* +GO_FOO_BUILD_OPTS+, to specify additional options to pass to the
+  Go +setup.py+ script during the build step. For target distutils
+  packages, the +PKG_GO_DISTUTILS_BUILD_OPTS+ options are already
+  passed automatically by the infrastructure.
+
+With the Go infrastructure, all the steps required to build and
+install the packages are already defined, and they generally work well
+for most Go-based packages. However, when required, it is still
+possible to customize what is done in any particular step:
+
+* By adding a post-operation hook (after extract, patch, configure,
+  build or install). See xref:hooks[] for details.
+
+* By overriding one of the steps. For example, even if the Go
+  infrastructure is used, if the package +.mk+ file defines its own
+  +GO_FOO_BUILD_CMDS+ variable, it will be used instead of the
+  default Go one. However, using this method should be restricted
+  to very specific cases. Do not use it in the general case.
+
diff --git a/docs/manual/adding-packages.txt b/docs/manual/adding-packages.txt
index d577ff0..be7468b 100644
--- a/docs/manual/adding-packages.txt
+++ b/docs/manual/adding-packages.txt
@@ -34,6 +34,8 @@ include::adding-packages-rebar.txt[]
 
 include::adding-packages-waf.txt[]
 
+include::adding-packages-golang.txt[]
+
 include::adding-packages-kernel-module.txt[]
 
 include::adding-packages-asciidoc.txt[]
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 4/4] package/flannel: converting to golang infrastructure
  2017-10-16 17:08 [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Angelo Compagnucci
  2017-10-16 17:08 ` [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
  2017-10-16 17:08 ` [Buildroot] [RFC v2 3/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
@ 2017-10-16 17:08 ` Angelo Compagnucci
  2017-10-21 19:51 ` [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Thomas Petazzoni
  3 siblings, 0 replies; 12+ messages in thread
From: Angelo Compagnucci @ 2017-10-16 17:08 UTC (permalink / raw)
  To: buildroot

This patch converts the flannel package to the new golang
infrastructure.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/flannel/flannel.mk | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/package/flannel/flannel.mk b/package/flannel/flannel.mk
index bbb2c72..641bb97 100644
--- a/package/flannel/flannel.mk
+++ b/package/flannel/flannel.mk
@@ -11,14 +11,6 @@ FLANNEL_SOURCE = $(FLANNEL_VERSION).tar.gz
 FLANNEL_LICENSE = Apache-2.0
 FLANNEL_LICENSE_FILES = LICENSE
 
-FLANNEL_DEPENDENCIES = host-go
-
-FLANNEL_MAKE_ENV = \
-	$(HOST_GO_TARGET_ENV) \
-	GOBIN="$(@D)/bin" \
-	GOPATH="$(@D)/gopath" \
-	CGO_ENABLED=1
-
 FLANNEL_GLDFLAGS = \
 	-X github.com/coreos/flannel/version.Version=$(FLANNEL_VERSION)
 
@@ -26,21 +18,12 @@ ifeq ($(BR2_STATIC_LIBS),y)
 FLANNEL_GLDFLAGS += -extldflags '-static'
 endif
 
-define FLANNEL_CONFIGURE_CMDS
-	# Put sources at prescribed GOPATH location.
-	mkdir -p $(@D)/gopath/src/github.com/coreos
-	ln -s $(@D) $(@D)/gopath/src/github.com/coreos/flannel
-endef
-
-define FLANNEL_BUILD_CMDS
-	cd $(@D) && $(FLANNEL_MAKE_ENV) $(HOST_DIR)/bin/go \
-		build -v -o $(@D)/bin/flanneld -ldflags "$(FLANNEL_GLDFLAGS)" .
-endef
+FLANNEL_BUILD_OPTS = -ldflags "$(FLANNEL_GLDFLAGS)"
 
+# Install flannel to its well known location.
 define FLANNEL_INSTALL_TARGET_CMDS
-	# Install flannel to its well known location.
-	$(INSTALL) -D -m 0755 $(@D)/bin/flanneld $(TARGET_DIR)/opt/bin/flanneld
+	$(INSTALL) -D -m 0755 $(FLANNEL_EXECUTABLE) $(TARGET_DIR)/opt/bin/flanneld
 	$(INSTALL) -D -m 0755 $(@D)/dist/mk-docker-opts.sh $(TARGET_DIR)/opt/bin/mk-docker-opts.sh
 endef
 
-$(eval $(generic-package))
+$(eval $(golang-package))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings
  2017-10-16 17:08 [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Angelo Compagnucci
                   ` (2 preceding siblings ...)
  2017-10-16 17:08 ` [Buildroot] [RFC v2 4/4] package/flannel: converting to " Angelo Compagnucci
@ 2017-10-21 19:51 ` Thomas Petazzoni
  2017-10-22 22:48   ` Peter Korsgaard
  2017-10-25  7:41   ` Peter Korsgaard
  3 siblings, 2 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2017-10-21 19:51 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 16 Oct 2017 19:08:45 +0200, Angelo Compagnucci wrote:
> This patch fixes a bug with the BR2_TOOLCHAIN_HAS_THREADS variable
> handling which causes CGO_ENABLED to be always 0.
> 
> Furthermore, it fixes the cross compilation options for the go compiler:
> setting CGO_ENABLED should be done only for the target compiler not the
> host one.
> 
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> ---
>  package/go/go.mk | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Thanks applied the patch, with Christian's Acked-by.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure
  2017-10-16 17:08 ` [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
@ 2017-10-21 19:55   ` Thomas Petazzoni
  2017-10-23 16:36     ` Angelo Compagnucci
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2017-10-21 19:55 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 16 Oct 2017 19:08:46 +0200, Angelo Compagnucci wrote:
> This patch adds a new infrastructure for golang based packages.
> 
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>

As Christian said, it seems that this infrastructure isn't "general"
enough to really be useful. We would to see it applied to several
packages and see that there is a real benefit to several packages to
apply it.

So I'd be happy to take a go package infrastructure, but it needs to be
usable by more than one package in a useful way.

> +# Target packages need the Go compiler on the host.
> +$(2)_DEPENDENCIES += host-go
> +
> +#
> +# go install command doesn't work well when cross compilation is enabled,
> +# we set the executable output of the compilation to a specific location.
> +# We set this variable here to be used by packages if needed.
> +#
> +$(2)_EXECUTABLE = $$(@D)/gopath/bin/$(1)

This seems to make the assumption that a given package can only install
a single executable, and that this executable is named after the
package. It is true for flannel, but what about other Go packages ?

> +# Source files in Go should be uncompressed in a precise folder in the
> +# hiearchy of GOPATH. It usually resolves around domain/vendor/software.
> +#
> +$(1)_src_path ?= $$(call domain,$($(2)_SITE))/$$(firstword $$(subst /, ,$$(call notdomain,$($(2)_SITE))))
> +$(2)_SRC_PATH  = $$(@D)/gopath/src/$$($(1)_src_path)/$(1)

We don't use lower-case variables in Buildroot, especially if there is
a variable with the same name upper-case.

Best regards,

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings
  2017-10-21 19:51 ` [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Thomas Petazzoni
@ 2017-10-22 22:48   ` Peter Korsgaard
  2017-10-25  7:41   ` Peter Korsgaard
  1 sibling, 0 replies; 12+ messages in thread
From: Peter Korsgaard @ 2017-10-22 22:48 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Mon, 16 Oct 2017 19:08:45 +0200, Angelo Compagnucci wrote:
 >> This patch fixes a bug with the BR2_TOOLCHAIN_HAS_THREADS variable
 >> handling which causes CGO_ENABLED to be always 0.
 >> 
 >> Furthermore, it fixes the cross compilation options for the go compiler:
 >> setting CGO_ENABLED should be done only for the target compiler not the
 >> host one.
 >> 
 >> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
 >> ---
 >> package/go/go.mk | 14 ++++++++------
 >> 1 file changed, 8 insertions(+), 6 deletions(-)

 > Thanks applied the patch, with Christian's Acked-by.

Committed to 2017.08.x, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure
  2017-10-21 19:55   ` Thomas Petazzoni
@ 2017-10-23 16:36     ` Angelo Compagnucci
  0 siblings, 0 replies; 12+ messages in thread
From: Angelo Compagnucci @ 2017-10-23 16:36 UTC (permalink / raw)
  To: buildroot

Dear Thomas,

2017-10-21 21:55 GMT+02:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Mon, 16 Oct 2017 19:08:46 +0200, Angelo Compagnucci wrote:
>> This patch adds a new infrastructure for golang based packages.
>>
>> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>
> As Christian said, it seems that this infrastructure isn't "general"
> enough to really be useful. We would to see it applied to several
> packages and see that there is a real benefit to several packages to
> apply it.
>
> So I'd be happy to take a go package infrastructure, but it needs to be
> usable by more than one package in a useful way.
>
>> +# Target packages need the Go compiler on the host.
>> +$(2)_DEPENDENCIES += host-go
>> +
>> +#
>> +# go install command doesn't work well when cross compilation is enabled,
>> +# we set the executable output of the compilation to a specific location.
>> +# We set this variable here to be used by packages if needed.
>> +#
>> +$(2)_EXECUTABLE = $$(@D)/gopath/bin/$(1)
>
> This seems to make the assumption that a given package can only install
> a single executable, and that this executable is named after the
> package. It is true for flannel, but what about other Go packages ?

It will be addressed in the respin of the series.

>
>> +# Source files in Go should be uncompressed in a precise folder in the
>> +# hiearchy of GOPATH. It usually resolves around domain/vendor/software.
>> +#
>> +$(1)_src_path ?= $$(call domain,$($(2)_SITE))/$$(firstword $$(subst /, ,$$(call notdomain,$($(2)_SITE))))
>> +$(2)_SRC_PATH  = $$(@D)/gopath/src/$$($(1)_src_path)/$(1)
>
> We don't use lower-case variables in Buildroot, especially if there is
> a variable with the same name upper-case.

Fixed, thanks!

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



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings
  2017-10-21 19:51 ` [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Thomas Petazzoni
  2017-10-22 22:48   ` Peter Korsgaard
@ 2017-10-25  7:41   ` Peter Korsgaard
  1 sibling, 0 replies; 12+ messages in thread
From: Peter Korsgaard @ 2017-10-25  7:41 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Mon, 16 Oct 2017 19:08:45 +0200, Angelo Compagnucci wrote:
 >> This patch fixes a bug with the BR2_TOOLCHAIN_HAS_THREADS variable
 >> handling which causes CGO_ENABLED to be always 0.
 >> 
 >> Furthermore, it fixes the cross compilation options for the go compiler:
 >> setting CGO_ENABLED should be done only for the target compiler not the
 >> host one.
 >> 
 >> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
 >> ---
 >> package/go/go.mk | 14 ++++++++------
 >> 1 file changed, 8 insertions(+), 6 deletions(-)

 > Thanks applied the patch, with Christian's Acked-by.

Committed to 2017.02.x, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC, v2, 2/4] package/pkg-golang: new package infrastructure
  2017-10-23 15:50 ` Angelo Compagnucci
@ 2017-10-24  3:24   ` Christian Stewart
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Stewart @ 2017-10-24  3:24 UTC (permalink / raw)
  To: buildroot

Hi Angelo,

On Mon, Oct 23, 2017 at 11:50 AM, Angelo Compagnucci
<angelo.compagnucci@gmail.com> wrote:
>> - Is  CGO_NO_EMULATION=1  necessary? I use it in docker-engine, not sure why.
>
> Unfortunately I cannot find any information about this variable or
> google nor the manual. I don't think it makes something.

https://github.com/LibreELEC/LibreELEC.tv/pull/1669/files#diff-3fa55ea381e3943a7d8440c8adbb6bb1R40

But, you're right, I can't find it anywhere else.

> It is already provided by the GO_FOO_GO_SRC_PATH variable.
> Btw I think that there is no problem in guessing the path cause all
> the projects I worked with use the domain/vendor/software convention.

It looked to me like it was being overwritten but I may be wrong.


> I will do a respin of the series with the runc package converted to
> the golang infrastructure and you will find it much smaller!

Looking forward to it!

Best,
Christian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC, v2, 2/4] package/pkg-golang: new package infrastructure
  2017-10-19 16:06 [Buildroot] [RFC, v2, 2/4] package/pkg-golang: new package infrastructure Christian Stewart
@ 2017-10-23 15:50 ` Angelo Compagnucci
  2017-10-24  3:24   ` Christian Stewart
  0 siblings, 1 reply; 12+ messages in thread
From: Angelo Compagnucci @ 2017-10-23 15:50 UTC (permalink / raw)
  To: buildroot

Dear Christian Stewart,

2017-10-19 18:06 GMT+02:00 Christian Stewart <christian@paral.in>:
> Hi Angelo,
>
> Angelo Compagnucci wrote:
>> This patch adds a new infrastructure for golang based packages.
>
> I have tested this new infrastructure against the docker series of packages:
>
> https://github.com/paralin/buildroot/commits/docker-package-infra
>
>  - https://github.com/paralin/buildroot/commit/74a1dd8913409b65a230c7c15acfd2df8fdc3d50
>  - https://github.com/paralin/buildroot/commit/3873fdf90c4900fba2b27097eecdaadb73816ce3
>  - https://github.com/paralin/buildroot/commit/c2a73eef568f9c6f656cf3344e2f6005cec8f5ca
>
> In general though the changes seem to just be:
>
> diff --git a/package/docker-engine/docker-engine.mk
> b/package/docker-engine/docker-engine.mk
> index 8494efdce7..70ab4a531a 100644
> --- a/package/docker-engine/docker-engine.mk
> +++ b/package/docker-engine/docker-engine.mk
> @@ -11,7 +11,7 @@ DOCKER_ENGINE_SITE = $(call
> github,docker,docker-ce,$(DOCKER_ENGINE_VERSION))
>  DOCKER_ENGINE_LICENSE = Apache-2.0
>  DOCKER_ENGINE_LICENSE_FILES = LICENSE
>
> -DOCKER_ENGINE_DEPENDENCIES = host-go host-pkgconf
> +DOCKER_ENGINE_DEPENDENCIES = host-pkgconf
>
>  DOCKER_ENGINE_GOPATH = "$(@D)/gopath"
>  DOCKER_ENGINE_MAKE_ENV = $(HOST_GO_TARGET_ENV) \
> @@ -138,4 +138,4 @@ define DOCKER_ENGINE_INSTALL_TARGET_CMDS
>   )
>  endef
>
> -$(eval $(generic-package))
> +$(eval $(golang-package))
>
> While the package infra code itself works fine in this case because I
> override almost everything in these packages, I am skeptical of how
> useful it would be in the general case for the following reasons:
>
>  - The go code is already pretty succinct, aside from having a
> dependency on host-go which looks just fine to me.
>  - Go packages often do not have their main package in the root of the
> repo. You need to allow package developers to specify multiple targets
> in subdirs, like cmd/hyperkube, cmd/kubelet, etc.

Yes right. I actually changed the logic of the build phase to use the
go build/install to take advantage of the go building system.

>  - Typically executables exist under a cmd/${executable} path which is
> nice and predictable. I would almost make the default build path
> cmd/kubelet.

I think with should stick with standard go rules for packages. Usually
packages are installed in GOPATH/bin when crosscompilation is not
enabled or GOPATH/bin/$GOOS_$GOARCH/ when crosscompilation is enabled,
stating to the go manual.
I think that simply using go install should be sufficient. The only
problem remaining could be that a package that needs to install
multiple binaries in the host or target directory should implement a
host/target install step and can't use the default one.


> - Is  CGO_NO_EMULATION=1  necessary? I use it in docker-engine, not sure why.

Unfortunately I cannot find any information about this variable or
google nor the manual. I don't think it makes something.

> - Please add a mechanism to specify Go build tags

Will do.

> - In terms of gopath, determining the path to the executable / the
> location inside the gopath is scary to me. Can we please set it
> explicitly in Go packages? Like
> RUNC_GO_PATH=github.com/opencontainers/runc - please see the runc.mk
> file and make a proposed revision to that as well as flannel.

It is already provided by the GO_FOO_GO_SRC_PATH variable.
Btw I think that there is no problem in guessing the path cause all
the projects I worked with use the domain/vendor/software convention.


> Overall I think it works and may potentially be useful but at the
> moment it doesn't do enough to merit adding an entire new package
> infrastructure for it, if I'm going to override 99% of it anyway. All
> it does right now for every go package I've tried it on is remove the
> host-go dependency, which I almost like having because it's more
> explicit. Otherwise in these cases I've had to hand override most of
> the infra because Go packages do not conform to as typical of a format
> as autotools or cmake.

I will do a respin of the series with the runc package converted to
the golang infrastructure and you will find it much smaller!

>
> Thanks!
> Christian Stewart



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [RFC, v2, 2/4] package/pkg-golang: new package infrastructure
@ 2017-10-19 16:06 Christian Stewart
  2017-10-23 15:50 ` Angelo Compagnucci
  0 siblings, 1 reply; 12+ messages in thread
From: Christian Stewart @ 2017-10-19 16:06 UTC (permalink / raw)
  To: buildroot

Hi Angelo,

Angelo Compagnucci wrote:
> This patch adds a new infrastructure for golang based packages.

I have tested this new infrastructure against the docker series of packages:

https://github.com/paralin/buildroot/commits/docker-package-infra

 - https://github.com/paralin/buildroot/commit/74a1dd8913409b65a230c7c15acfd2df8fdc3d50
 - https://github.com/paralin/buildroot/commit/3873fdf90c4900fba2b27097eecdaadb73816ce3
 - https://github.com/paralin/buildroot/commit/c2a73eef568f9c6f656cf3344e2f6005cec8f5ca

In general though the changes seem to just be:

diff --git a/package/docker-engine/docker-engine.mk
b/package/docker-engine/docker-engine.mk
index 8494efdce7..70ab4a531a 100644
--- a/package/docker-engine/docker-engine.mk
+++ b/package/docker-engine/docker-engine.mk
@@ -11,7 +11,7 @@ DOCKER_ENGINE_SITE = $(call
github,docker,docker-ce,$(DOCKER_ENGINE_VERSION))
 DOCKER_ENGINE_LICENSE = Apache-2.0
 DOCKER_ENGINE_LICENSE_FILES = LICENSE

-DOCKER_ENGINE_DEPENDENCIES = host-go host-pkgconf
+DOCKER_ENGINE_DEPENDENCIES = host-pkgconf

 DOCKER_ENGINE_GOPATH = "$(@D)/gopath"
 DOCKER_ENGINE_MAKE_ENV = $(HOST_GO_TARGET_ENV) \
@@ -138,4 +138,4 @@ define DOCKER_ENGINE_INSTALL_TARGET_CMDS
  )
 endef

-$(eval $(generic-package))
+$(eval $(golang-package))

While the package infra code itself works fine in this case because I
override almost everything in these packages, I am skeptical of how
useful it would be in the general case for the following reasons:

 - The go code is already pretty succinct, aside from having a
dependency on host-go which looks just fine to me.
 - Go packages often do not have their main package in the root of the
repo. You need to allow package developers to specify multiple targets
in subdirs, like cmd/hyperkube, cmd/kubelet, etc.
 - Typically executables exist under a cmd/${executable} path which is
nice and predictable. I would almost make the default build path
cmd/kubelet.
- Is  CGO_NO_EMULATION=1  necessary? I use it in docker-engine, not sure why.
- Please add a mechanism to specify Go build tags
- In terms of gopath, determining the path to the executable / the
location inside the gopath is scary to me. Can we please set it
explicitly in Go packages? Like
RUNC_GO_PATH=github.com/opencontainers/runc - please see the runc.mk
file and make a proposed revision to that as well as flannel.

Overall I think it works and may potentially be useful but at the
moment it doesn't do enough to merit adding an entire new package
infrastructure for it, if I'm going to override 99% of it anyway. All
it does right now for every go package I've tried it on is remove the
host-go dependency, which I almost like having because it's more
explicit. Otherwise in these cases I've had to hand override most of
the infra because Go packages do not conform to as typical of a format
as autotools or cmake.

Thanks!
Christian Stewart

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-25  7:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 17:08 [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Angelo Compagnucci
2017-10-16 17:08 ` [Buildroot] [RFC v2 2/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
2017-10-21 19:55   ` Thomas Petazzoni
2017-10-23 16:36     ` Angelo Compagnucci
2017-10-16 17:08 ` [Buildroot] [RFC v2 3/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
2017-10-16 17:08 ` [Buildroot] [RFC v2 4/4] package/flannel: converting to " Angelo Compagnucci
2017-10-21 19:51 ` [Buildroot] [RFC v2 1/4] package/go: fixing crosscompilation settings Thomas Petazzoni
2017-10-22 22:48   ` Peter Korsgaard
2017-10-25  7:41   ` Peter Korsgaard
2017-10-19 16:06 [Buildroot] [RFC, v2, 2/4] package/pkg-golang: new package infrastructure Christian Stewart
2017-10-23 15:50 ` Angelo Compagnucci
2017-10-24  3:24   ` Christian Stewart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.