* [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure
@ 2017-10-24 21:14 Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 2/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Angelo Compagnucci @ 2017-10-24 21:14 UTC (permalink / raw)
To: buildroot
This patch adds a new infrastructure for golang based packages.
Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
Changelog:
v2 -> v3:
* Adding GO_LDFLAGS and GO_TAGS variables
* Adding GOPATH variable
* Adding option to build as static when BR2_STATIC_LIBS is enabled
* Changed building logic to take into account multiple binaries
v3 -> v4:
* Changing GOPATH default value from gopath to _gopath
package/Makefile.in | 1 +
package/pkg-golang.mk | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 141 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..2edc366
--- /dev/null
+++ b/package/pkg-golang.mk
@@ -0,0 +1,140 @@
+################################################################################
+# 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 behavior. 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 specificities 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
+
+$(2)_GOPATH ?= _gopath
+
+ifndef $(2)_MAKE_ENV
+define $(2)_MAKE_ENV
+ $$(HOST_GO_TARGET_ENV) \
+ GOPATH="$$(@D)/$$($(2)_GOPATH)" \
+ CGO_ENABLED=$$(HOST_GO_CGO_ENABLED)
+endef
+endif
+
+ifdef $(2)_GO_LDFLAGS
+ $(2)_BUILD_OPTS += -ldflags "$$($(2)_GO_LDFLAGS)"
+endif
+
+ifeq ($(BR2_STATIC_LIBS),y)
+ $(2)_BUILD_OPTS += -extldflags '-static'
+endif
+
+ifdef $(2)_GO_TAGS
+ $(2)_BUILD_OPTS += -tags "$$($(2)_GO_TAGS)"
+endif
+
+# Target packages need the Go compiler on the host.
+$(2)_DEPENDENCIES += host-go
+
+#
+# The go build/install command installs the binaries inside
+# gopath/bin/linux_GOARCH/ when cross compilation is enabled.
+# We set this variable here to be used by packages if needed.
+#
+$(2)_BINDIR = $$(@D)/$$($(2)_GOPATH)/bin/linux_$$(GO_GOARCH)
+
+#
+# Source files in Go should be uncompressed in a precise folder in the
+# hierarchy of GOPATH. It usually resolves around domain/vendor/software.
+#
+$(2)_GO_SRC_PATH ?= $$(call domain,$($(2)_SITE))/$$(firstword $$(subst /, ,$$(call notdomain,$($(2)_SITE))))
+$(2)_SRC_PATH = $$(@D)/$$($(2)_GOPATH)/src/$$($(2)_GO_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)/$$($(2)_GOPATH)/bin
+ mkdir -p $$(@D)/$$($(2)_GOPATH)/src/$$($(2)_GO_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.
+# We use the install command instead of build command here because the
+# install command also moves the package binaries in gopath/bin/linux_GOARCH/.
+# Using the install command also leverages the go build infrastructure
+# for building and installing multiple binaries.
+#
+ifndef $(2)_BUILD_CMDS
+define $(2)_BUILD_CMDS
+ cd $$($(2)_SRC_PATH) && \
+ $$($(2)_MAKE_ENV) $(HOST_DIR)/bin/go install \
+ -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)_BINDIR)/$(1) $(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)_BINDIR)/$(1) $(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] 7+ messages in thread* [Buildroot] [RFC v4 2/4] docs/manual: adding documentation for the golang infrastructure
2017-10-24 21:14 [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
@ 2017-10-24 21:14 ` Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 3/4] package/flannel: converting to " Angelo Compagnucci
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Angelo Compagnucci @ 2017-10-24 21:14 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 | 117 +++++++++++++++++++++++++++++++++
docs/manual/adding-packages.txt | 2 +
2 files changed, 119 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..a0e617f
--- /dev/null
+++ b/docs/manual/adding-packages-golang.txt
@@ -0,0 +1,117 @@
+// -*- 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.
+
+* If your package need a custom GOPATH to be compiled in, you can use the
+ +GO_FOO_GOPATH+.
+
+* +GO_FOO_GO_SRC_PATH+ is the path where your source will be compiled
+ relatively to the GOPATH.
+ The golang package infrastructure tries to guess the correct
+ sub folder to compile in but if guessing is not correct for you or your
+ package behaves differently, you can use this variable to
+ redefine the path.
+
+* +GOO_FOO_GO_LDFLAGS+ and +GO_FOO_GO_TAGS+ can be used to pass
+ respectively the LDFLAGS or the TAGS to the build command.
+ +GOO_FOO_GO_LDFLAGS+ and +GO_FOO_GO_TAGS+ will be combined into
+ the proper command line options to be passed to the compiler.
+
+* If you need to customize the install location for you binaries, you
+ can use +GOO_FOO_BINDIR+ to know where the binaries are.
+
+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] 7+ messages in thread* [Buildroot] [RFC v4 3/4] package/flannel: converting to golang infrastructure
2017-10-24 21:14 [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 2/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
@ 2017-10-24 21:14 ` Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 4/4] package/runc: " Angelo Compagnucci
2017-10-24 22:39 ` [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Arnout Vandecappelle
3 siblings, 0 replies; 7+ messages in thread
From: Angelo Compagnucci @ 2017-10-24 21:14 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 | 32 ++++----------------------------
1 file changed, 4 insertions(+), 28 deletions(-)
diff --git a/package/flannel/flannel.mk b/package/flannel/flannel.mk
index bbb2c72..6c9d9dd 100644
--- a/package/flannel/flannel.mk
+++ b/package/flannel/flannel.mk
@@ -11,36 +11,12 @@ 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)
-
-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_GO_LDFLAGS = -X github.com/coreos/flannel/version.Version=$(FLANNEL_VERSION)
+# 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_BINDIR)/flannel $(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] 7+ messages in thread
* [Buildroot] [RFC v4 4/4] package/runc: converting to golang infrastructure
2017-10-24 21:14 [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 2/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 3/4] package/flannel: converting to " Angelo Compagnucci
@ 2017-10-24 21:14 ` Angelo Compagnucci
2017-10-24 22:14 ` Arnout Vandecappelle
2017-10-24 22:39 ` [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Arnout Vandecappelle
3 siblings, 1 reply; 7+ messages in thread
From: Angelo Compagnucci @ 2017-10-24 21:14 UTC (permalink / raw)
To: buildroot
This patch converts the runc package to the new golang
infrastructure.
Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
package/runc/runc.mk | 36 +++---------------------------------
1 file changed, 3 insertions(+), 33 deletions(-)
diff --git a/package/runc/runc.mk b/package/runc/runc.mk
index 0b51e11..5d9e732 100644
--- a/package/runc/runc.mk
+++ b/package/runc/runc.mk
@@ -9,43 +9,13 @@ RUNC_SITE = $(call github,opencontainers,runc,$(RUNC_VERSION))
RUNC_LICENSE = Apache-2.0
RUNC_LICENSE_FILES = LICENSE
-RUNC_DEPENDENCIES = host-go
+RUNC_GO_LDFLAGS = -X main.gitCommit=$(RUNC_VERSION)
-RUNC_GOPATH = "$(@D)/Godeps/_workspace"
-RUNC_MAKE_ENV = $(HOST_GO_TARGET_ENV) \
- CGO_ENABLED=1 \
- GOBIN="$(@D)/bin" \
- GOPATH="$(RUNC_GOPATH)" \
- PATH=$(BR_PATH)
-
-RUNC_GLDFLAGS = \
- -X main.gitCommit=$(RUNC_VERSION)
-
-ifeq ($(BR2_STATIC_LIBS),y)
-RUNC_GLDFLAGS += -extldflags '-static'
-endif
-
-RUNC_GOTAGS = cgo static_build
+RUNC_GO_TAGS = cgo static_build
ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
RUNC_GOTAGS += seccomp
RUNC_DEPENDENCIES += libseccomp host-pkgconf
endif
-define RUNC_CONFIGURE_CMDS
- mkdir -p $(RUNC_GOPATH)/src/github.com/opencontainers
- ln -s $(@D) $(RUNC_GOPATH)/src/github.com/opencontainers/runc
-endef
-
-define RUNC_BUILD_CMDS
- cd $(RUNC_GOPATH)/src/github.com/opencontainers/runc && \
- $(RUNC_MAKE_ENV) $(HOST_DIR)/bin/go \
- build -v -o $(@D)/bin/runc \
- -tags "$(RUNC_GOTAGS)" -ldflags "$(RUNC_GLDFLAGS)" .
-endef
-
-define RUNC_INSTALL_TARGET_CMDS
- $(INSTALL) -D -m 0755 $(@D)/bin/runc $(TARGET_DIR)/usr/bin/runc
-endef
-
-$(eval $(generic-package))
+$(eval $(golang-package))
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure
2017-10-24 21:14 [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
` (2 preceding siblings ...)
2017-10-24 21:14 ` [Buildroot] [RFC v4 4/4] package/runc: " Angelo Compagnucci
@ 2017-10-24 22:39 ` Arnout Vandecappelle
2017-10-25 8:53 ` Angelo Compagnucci
3 siblings, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2017-10-24 22:39 UTC (permalink / raw)
To: buildroot
Hi Angelo,
Incomplete review but at least something you can work with :-)
On 24-10-17 23:14, Angelo Compagnucci wrote:
> This patch adds a new infrastructure for golang based packages.
>
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
[snip]> +define inner-golang-package
> +
> +$(2)_GOPATH ?= _gopath
Is there ever a need for a package to override this? docker-containerd sets it
to 'vendor' but I don't think there's a good reason for that.
> +
> +ifndef $(2)_MAKE_ENV
> +define $(2)_MAKE_ENV
> + $$(HOST_GO_TARGET_ENV) \
> + GOPATH="$$(@D)/$$($(2)_GOPATH)" \
> + CGO_ENABLED=$$(HOST_GO_CGO_ENABLED)
> +endef
> +endif
Don't do it like this, do it like in autotools: these options are always
provided in the build environment, and $(2)_MAKE_ENV (default empty but no need
to set it here) comes after it.
Also, _MAKE_ENV is not a good name since you're not calling make. Just _ENV or
_BUILD_ENV or _GO_ENV is better.
> +
> +ifdef $(2)_GO_LDFLAGS
> + $(2)_BUILD_OPTS += -ldflags "$$($(2)_GO_LDFLAGS)"
> +endif
> +
> +ifeq ($(BR2_STATIC_LIBS),y)
> + $(2)_BUILD_OPTS += -extldflags '-static'
> +endif
This is NOT what is currently done for flannel and runc, they put this in the
-ldflags argument.
> +
> +ifdef $(2)_GO_TAGS
> + $(2)_BUILD_OPTS += -tags "$$($(2)_GO_TAGS)"
> +endif
I guess it would make sense to also have
$(2)_GO_TAGS ?= $$($(3)_GO_TAGS)
(i.e. inherit the target tags for the host) though I'm not sure that is always
going to be relevant.
> +
> +# Target packages need the Go compiler on the host.
> +$(2)_DEPENDENCIES += host-go
> +
> +#
> +# The go build/install command installs the binaries inside
> +# gopath/bin/linux_GOARCH/ when cross compilation is enabled.
> +# We set this variable here to be used by packages if needed.
> +#
> +$(2)_BINDIR = $$(@D)/$$($(2)_GOPATH)/bin/linux_$$(GO_GOARCH)
What about host packages?
> +
> +#
> +# Source files in Go should be uncompressed in a precise folder in the
> +# hierarchy of GOPATH. It usually resolves around domain/vendor/software.
> +#
> +$(2)_GO_SRC_PATH ?= $$(call domain,$($(2)_SITE))/$$(firstword $$(subst /, ,$$(call notdomain,$($(2)_SITE))))
> +$(2)_SRC_PATH = $$(@D)/$$($(2)_GOPATH)/src/$$($(2)_GO_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
I don't see any differences?
> +# packages.
> +#
> +ifndef $(2)_CONFIGURE_CMDS
> +define $(2)_CONFIGURE_CMDS
> + mkdir -p $$(@D)/$$($(2)_GOPATH)/bin
> + mkdir -p $$(@D)/$$($(2)_GOPATH)/src/$$($(2)_GO_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.
Really? How is that possible that exactly the same commands sometimes build for
the host and sometimes for the target?
Since we currently don't have host-go packages, I think it's better to remove
support for host-go packages entirely. We can add it when there is an example.
> +# We use the install command instead of build command here because the
> +# install command also moves the package binaries in gopath/bin/linux_GOARCH/.
> +# Using the install command also leverages the go build infrastructure
> +# for building and installing multiple binaries.
> +#
> +ifndef $(2)_BUILD_CMDS
> +define $(2)_BUILD_CMDS
> + cd $$($(2)_SRC_PATH) && \
> + $$($(2)_MAKE_ENV) $(HOST_DIR)/bin/go install \
> + -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)_BINDIR)/$(1) $(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)_BINDIR)/$(1) $(TARGET_DIR)/usr/bin/
There is no guarantee at all that the binary name is the same as the package
name, so better let the package override it. Also, it would be convenient to
support building multiple binaries like is done now for docker-containerd and
docker-engine. But I'm all for delaying that to a future patch, let's get this
thing in first.
Regards,
Arnout
> +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)
> +
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
^ permalink raw reply [flat|nested] 7+ messages in thread* [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure
2017-10-24 22:39 ` [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Arnout Vandecappelle
@ 2017-10-25 8:53 ` Angelo Compagnucci
0 siblings, 0 replies; 7+ messages in thread
From: Angelo Compagnucci @ 2017-10-25 8:53 UTC (permalink / raw)
To: buildroot
Dear Arnout Vandecappelle,
2017-10-25 0:39 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
> Hi Angelo,
>
> Incomplete review but at least something you can work with :-)
>
> On 24-10-17 23:14, Angelo Compagnucci wrote:
>> This patch adds a new infrastructure for golang based packages.
>>
>> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> [snip]> +define inner-golang-package
>> +
>> +$(2)_GOPATH ?= _gopath
>
> Is there ever a need for a package to override this? docker-containerd sets it
> to 'vendor' but I don't think there's a good reason for that.
Yes, there's a reason for that. I converted also the docker-containerd
package and in the following respin of the patch you can see why it is
needed.
>> +
>> +ifndef $(2)_MAKE_ENV
>> +define $(2)_MAKE_ENV
>> + $$(HOST_GO_TARGET_ENV) \
>> + GOPATH="$$(@D)/$$($(2)_GOPATH)" \
>> + CGO_ENABLED=$$(HOST_GO_CGO_ENABLED)
>> +endef
>> +endif
>
> Don't do it like this, do it like in autotools: these options are always
> provided in the build environment, and $(2)_MAKE_ENV (default empty but no need
> to set it here) comes after it.
>
> Also, _MAKE_ENV is not a good name since you're not calling make. Just _ENV or
> _BUILD_ENV or _GO_ENV is better.
Will do.
>
>> +
>> +ifdef $(2)_GO_LDFLAGS
>> + $(2)_BUILD_OPTS += -ldflags "$$($(2)_GO_LDFLAGS)"
>> +endif
>> +
>> +ifeq ($(BR2_STATIC_LIBS),y)
>> + $(2)_BUILD_OPTS += -extldflags '-static'
>> +endif
>
> This is NOT what is currently done for flannel and runc, they put this in the
> -ldflags argument.
Good catch!
>
>> +
>> +ifdef $(2)_GO_TAGS
>> + $(2)_BUILD_OPTS += -tags "$$($(2)_GO_TAGS)"
>> +endif
>
> I guess it would make sense to also have
>
> $(2)_GO_TAGS ?= $$($(3)_GO_TAGS)
>
> (i.e. inherit the target tags for the host) though I'm not sure that is always
> going to be relevant.
>
>> +
>> +# Target packages need the Go compiler on the host.
>> +$(2)_DEPENDENCIES += host-go
>> +
>> +#
>> +# The go build/install command installs the binaries inside
>> +# gopath/bin/linux_GOARCH/ when cross compilation is enabled.
>> +# We set this variable here to be used by packages if needed.
>> +#
>> +$(2)_BINDIR = $$(@D)/$$($(2)_GOPATH)/bin/linux_$$(GO_GOARCH)
>
> What about host packages?
>
>> +
>> +#
>> +# Source files in Go should be uncompressed in a precise folder in the
>> +# hierarchy of GOPATH. It usually resolves around domain/vendor/software.
>> +#
>> +$(2)_GO_SRC_PATH ?= $$(call domain,$($(2)_SITE))/$$(firstword $$(subst /, ,$$(call notdomain,$($(2)_SITE))))
>> +$(2)_SRC_PATH = $$(@D)/$$($(2)_GOPATH)/src/$$($(2)_GO_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
>
> I don't see any differences?
>
>> +# packages.
>> +#
>> +ifndef $(2)_CONFIGURE_CMDS
>> +define $(2)_CONFIGURE_CMDS
>> + mkdir -p $$(@D)/$$($(2)_GOPATH)/bin
>> + mkdir -p $$(@D)/$$($(2)_GOPATH)/src/$$($(2)_GO_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.
>
> Really? How is that possible that exactly the same commands sometimes build for
> the host and sometimes for the target?
>
> Since we currently don't have host-go packages, I think it's better to remove
> support for host-go packages entirely. We can add it when there is an example.
Yes, reasoning on that I think you are right. I'll remove the host part.
>
>> +# We use the install command instead of build command here because the
>> +# install command also moves the package binaries in gopath/bin/linux_GOARCH/.
>> +# Using the install command also leverages the go build infrastructure
>> +# for building and installing multiple binaries.
>> +#
>> +ifndef $(2)_BUILD_CMDS
>> +define $(2)_BUILD_CMDS
>> + cd $$($(2)_SRC_PATH) && \
>> + $$($(2)_MAKE_ENV) $(HOST_DIR)/bin/go install \
>> + -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)_BINDIR)/$(1) $(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)_BINDIR)/$(1) $(TARGET_DIR)/usr/bin/
>
> There is no guarantee at all that the binary name is the same as the package
> name, so better let the package override it.
Nope. Go mandates[1] that if a package contains only a binary, that
binary will be named as the package unless you overwrite this behavior
with the -o option at build time.
If a package contains multiples main.go files, each binary will be
named with name of the subfolder containing it and -o option is of
source disabled.
> Also, it would be convenient to
> support building multiple binaries like is done now for docker-containerd and
> docker-engine. But I'm all for delaying that to a future patch, let's get this
> thing in first.
Honestly, it's not so common in the go world to have multiple binaries
in the same source tree. I think we should treat this as a corner case
and let the developer doing what it's best. In the follow up of the
series I added also docker-containerd to show how a non standard
package could be implemented.
>
>
> Regards,
> Arnout
>
>
>> +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)
>> +
>>
>
> --
> Arnout Vandecappelle arnout at mind be
> Senior Embedded Software Architect +32-16-286500
> Essensium/Mind http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
[1] https://golang.org/cmd/go/#hdr-Compile_packages_and_dependencies
--
Profile: http://it.linkedin.com/in/compagnucciangelo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-25 8:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-24 21:14 [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 2/4] docs/manual: adding documentation for the golang infrastructure Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 3/4] package/flannel: converting to " Angelo Compagnucci
2017-10-24 21:14 ` [Buildroot] [RFC v4 4/4] package/runc: " Angelo Compagnucci
2017-10-24 22:14 ` Arnout Vandecappelle
2017-10-24 22:39 ` [Buildroot] [RFC v4 1/4] package/pkg-golang: new package infrastructure Arnout Vandecappelle
2017-10-25 8:53 ` Angelo Compagnucci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox