[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Christian Stewart]

The Go compiler needs to know the "import path" to the root of package source
repositories. Previously, this was done by creating a fake _gopath in the build
directory and symlinking the package source into that path.

Go has deprecated the GOPATH mechanism in favor of a new approach - Modules -
which specifies the root import path (and dependencies) in a "go.mod" file.

This commit moves Buildroot to use the new go.mod approach. Both host and target
Go packages compile correctly with small tweaks adjusting the build target
specifier.

Note: the Go module system will NOT download sources from the Internet due to
GOPROXY=off and -mod=vendor. All Buildroot packages currently have "vendor"
directories included with dependencies' source code.

The environment variables passed to Go during host and target package
compilation have been fixed to correctly pass CC, CXX, CFLAGS, and so on.

Reference: https://github.com/golang/go/wiki/Modules

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

v2 -> v3:

 - cjs: cleaned up spelling and moved extract hook to configure step
 - cjs: applied fixes from vincent fazio related to host packages

v3 -> v4:

 - cjs: clean up HOST and TARGET variables passed to Go
 - cjs: fix all CXXflags, cgo env vars to be consistent/correct
 - cjs: document / explain the difference from GOPATH

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/go/go.mk      | 55 +++++++++++++++++++++++++--------
 package/pkg-golang.mk | 71 +++++++++++++++++++++----------------------
 2 files changed, 76 insertions(+), 50 deletions(-)

diff --git a/package/go/go.mk b/package/go/go.mk
index 72604a250b..74e5298e66 100644
--- a/package/go/go.mk
+++ b/package/go/go.mk
@@ -12,6 +12,7 @@ GO_LICENSE = BSD-3-Clause
 GO_LICENSE_FILES = LICENSE
 
 HOST_GO_DEPENDENCIES = host-go-bootstrap
+HOST_GO_GOPATH = $(HOST_DIR)/usr/share/go-path
 HOST_GO_HOST_CACHE = $(HOST_DIR)/usr/share/host-go-cache
 HOST_GO_ROOT = $(HOST_DIR)/lib/go
 HOST_GO_TARGET_CACHE = $(HOST_DIR)/usr/share/go-cache
@@ -43,16 +44,51 @@ else ifeq ($(BR2_mips64el),y)
 GO_GOARCH = mips64le
 endif
 
-# For the convienience of target packages.
 HOST_GO_TOOLDIR = $(HOST_GO_ROOT)/pkg/tool/linux_$(GO_GOARCH)
-HOST_GO_TARGET_ENV = \
-	GO111MODULE=off \
-	GOARCH=$(GO_GOARCH) \
-	GOCACHE="$(HOST_GO_TARGET_CACHE)" \
+HOST_GO_COMMON_ENV = \
+	CC=$(HOSTCC_NOCCACHE) \
+	CGO_ENABLED=$(HOST_GO_CGO_ENABLED) \
+	CXX=$(HOSTCXX_NOCCACHE) \
+	GO111MODULE=on \
+	GOBIN= \
+	GOFLAGS=-mod=vendor \
+	GOPATH="$(HOST_GO_GOPATH)" \
+	GOPROXY=off \
 	GOROOT="$(HOST_GO_ROOT)" \
+	GOTOOLDIR="$(HOST_GO_TOOLDIR)" \
+	PATH=$(BR_PATH)
+
+# Used for compiling host packages.
+HOST_GO_HOST_ENV = \
+	$(HOST_GO_COMMON_ENV) \
+	CGO_CFLAGS="$(HOST_CFLAGS)" \
+	CGO_CXXFLAGS="$(HOST_CXXFLAGS)" \
+	CGO_FFLAGS="$(HOST_FCFLAGS)" \
+	CGO_LDFLAGS="$(HOST_LDFLAGS)" \
+	GOARCH="" \
+	GOCACHE="$(HOST_GO_HOST_CACHE)"
+
+# Used for compiling the host-go compiler and target packages.
+HOST_GO_CROSS_ENV = \
+	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
+	CC_FOR_TARGET="$(TARGET_CC)" \
+	CXX_FOR_TARGET="$(TARGET_CXX)" \
+	GOARCH=$(GO_GOARCH) \
+	GO_ASSUME_CROSSCOMPILING=1
+
+# Used for compiling target packages.
+#
+# Note: CC and CXX must be set as well as TARGET_ variants.
+HOST_GO_TARGET_ENV = \
+	$(HOST_GO_COMMON_ENV) \
+	$(HOST_GO_CROSS_ENV) \
 	CC="$(TARGET_CC)" \
 	CXX="$(TARGET_CXX)" \
-	GOTOOLDIR="$(HOST_GO_TOOLDIR)"
+	CGO_CFLAGS="$(TARGET_CFLAGS)" \
+	CGO_CXXFLAGS="$(TARGET_CXXFLAGS)" \
+	CGO_FFLAGS="$(TARGET_FCFLAGS)" \
+	CGO_LDFLAGS="$(TARGET_LDFLAGS)" \
+	GOCACHE="$(HOST_GO_TARGET_CACHE)"
 
 # The go compiler's cgo support uses threads.  If BR2_TOOLCHAIN_HAS_THREADS is
 # set, build in cgo support for any go programs that may need it.  Note that
@@ -64,13 +100,6 @@ else
 HOST_GO_CGO_ENABLED = 0
 endif
 
-HOST_GO_CROSS_ENV = \
-	CC_FOR_TARGET="$(TARGET_CC)" \
-	CXX_FOR_TARGET="$(TARGET_CXX)" \
-	GOARCH=$(GO_GOARCH) \
-	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
-	GO_ASSUME_CROSSCOMPILING=1
-
 else # !BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
 # host-go can still be used to build packages for the host. No need to set all
 # the arch stuff since we will not be cross-compiling.
diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index 2494ce028c..2d80e99619 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -23,21 +23,11 @@
 
 GO_BIN = $(HOST_DIR)/bin/go
 
-# We pass an empty GOBIN, otherwise "go install: cannot install
-# cross-compiled binaries when GOBIN is set"
-GO_COMMON_ENV = \
-	PATH=$(BR_PATH) \
-	GOBIN= \
-	CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
-
-GO_TARGET_ENV = \
-	$(HOST_GO_TARGET_ENV) \
-	$(GO_COMMON_ENV)
-
-GO_HOST_ENV = \
-	CGO_CFLAGS="$(HOST_CFLAGS)" \
-	CGO_LDFLAGS="$(HOST_LDFLAGS)" \
-	$(GO_COMMON_ENV)
+# Used when compiling host packages.
+GO_HOST_ENV = $(HOST_GO_HOST_ENV)
+
+# Used when compiling target packages.
+GO_TARGET_ENV =	$(HOST_GO_TARGET_ENV)
 
 ################################################################################
 # inner-golang-package -- defines how the configuration, compilation and
@@ -56,8 +46,6 @@ GO_HOST_ENV = \
 
 define inner-golang-package
 
-$(2)_WORKSPACE ?= _gopath
-
 $(2)_BUILD_OPTS += \
 	-ldflags "$$($(2)_LDFLAGS)" \
 	-tags "$$($(2)_TAGS)" \
@@ -79,25 +67,36 @@ endif
 
 $(2)_INSTALL_BINS ?= $(1)
 
-# Source files in Go should be extracted in a precise folder in the hierarchy
-# of GOPATH. It usually resolves around domain/vendor/software. By default, we
-# derive domain/vendor/software from the upstream URL of the project, but we
-# allow $(2)_SRC_SUBDIR to be overridden if needed.
+# Source files in Go usually use an import path resolved around
+# domain/vendor/software. We infer domain/vendor/software from the upstream URL
+# of the project.
 $(2)_SRC_DOMAIN = $$(call domain,$$($(2)_SITE))
 $(2)_SRC_VENDOR = $$(word 1,$$(subst /, ,$$(call notdomain,$$($(2)_SITE))))
 $(2)_SRC_SOFTWARE = $$(word 2,$$(subst /, ,$$(call notdomain,$$($(2)_SITE))))
 
-$(2)_SRC_SUBDIR ?= $$($(2)_SRC_DOMAIN)/$$($(2)_SRC_VENDOR)/$$($(2)_SRC_SOFTWARE)
-$(2)_SRC_PATH = $$(@D)/$$($(2)_WORKSPACE)/src/$$($(2)_SRC_SUBDIR)
-
-# Configure step. Only define it if not already defined by the package .mk
-# file.
-ifndef $(2)_CONFIGURE_CMDS
-define $(2)_CONFIGURE_CMDS
-	mkdir -p $$(dir $$($(2)_SRC_PATH))
-	ln -sf $$(@D) $$($(2)_SRC_PATH)
+# $(2)_GOMOD is the root Go module path for the project, inferred if not set.
+# If the go.mod file does not exist, one is written with this root path.
+# Replaces the old GOPATH/symlink mechanism for setting the root import path.
+$(2)_GOMOD ?= $$($(2)_SRC_DOMAIN)/$$($(2)_SRC_VENDOR)/$$($(2)_SRC_SOFTWARE)
+
+# Correctly configure the go.mod and go.sum files for the module system.
+# Note: Go is configured to use the "vendor" dir and not make network calls.
+define $(2)_APPLY_EXTRACT_GOMOD
+	if [ -f $$($(2)_PKGDIR)/go.mod ]; then \
+		cp $$($(2)_PKGDIR)/go.mod $$(@D)/go.mod; \
+		if [ -f $$(@D)/go.sum ]; then \
+			rm $$(@D)/go.sum; \
+		fi; \
+	fi; \
+	if [ -f $$($(2)_PKGDIR)/go.sum ]; then \
+		cp $$($(2)_PKGDIR)/go.sum $$(@D)/go.sum; \
+	fi
+	if [ ! -f $$(@D)/go.mod ] && [ -n "$$($(2)_GOMOD)" ]; then \
+		printf "module $$($(2)_GOMOD)\n" > $$(@D)/go.mod; \
+	fi
 endef
-endif
+
+$(2)_POST_EXTRACT_HOOKS += $(2)_APPLY_EXTRACT_GOMOD
 
 # Build step. Only define it if not already defined by the package .mk
 # file.
@@ -111,26 +110,24 @@ endif
 # Build package for target
 define $(2)_BUILD_CMDS
 	$$(foreach d,$$($(2)_BUILD_TARGETS),\
-		cd $$($(2)_SRC_PATH); \
+		cd $$(@D); \
 		$$(GO_TARGET_ENV) \
-			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
 			$$($(2)_GO_ENV) \
 			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
 			-o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
-			./$$(d)
+			$$(d)
 	)
 endef
 else
 # Build package for host
 define $(2)_BUILD_CMDS
 	$$(foreach d,$$($(2)_BUILD_TARGETS),\
-		cd $$($(2)_SRC_PATH); \
+		cd $$(@D); \
 		$$(GO_HOST_ENV) \
-			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
 			$$($(2)_GO_ENV) \
 			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
 			-o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
-			./$$(d)
+			$$(d)
 	)
 endef
 endif
-- 
2.28.0

[Buildroot] [PATCH v4 2/7] package/runc: remove unnecessary workspace identifier

[Christian Stewart]

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/runc/runc.mk | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/package/runc/runc.mk b/package/runc/runc.mk
index 4c2f84ab16..aedfb5ef9e 100644
--- a/package/runc/runc.mk
+++ b/package/runc/runc.mk
@@ -9,10 +9,7 @@ RUNC_SITE = $(call github,opencontainers,runc,v$(RUNC_VERSION))
 RUNC_LICENSE = Apache-2.0
 RUNC_LICENSE_FILES = LICENSE
 
-RUNC_WORKSPACE = Godeps/_workspace
-
 RUNC_LDFLAGS = -X main.gitCommit=$(RUNC_VERSION)
-
 RUNC_TAGS = cgo static_build
 
 ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
-- 
2.28.0

[Buildroot] [PATCH v4 3/7] package/docker-containerd: fix go-module package identifier/targets

[Christian Stewart]

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/docker-containerd/docker-containerd.mk | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/package/docker-containerd/docker-containerd.mk b/package/docker-containerd/docker-containerd.mk
index 5a44489065..8b17e6997a 100644
--- a/package/docker-containerd/docker-containerd.mk
+++ b/package/docker-containerd/docker-containerd.mk
@@ -9,13 +9,14 @@ DOCKER_CONTAINERD_SITE = $(call github,containerd,containerd,v$(DOCKER_CONTAINER
 DOCKER_CONTAINERD_LICENSE = Apache-2.0
 DOCKER_CONTAINERD_LICENSE_FILES = LICENSE
 
-DOCKER_CONTAINERD_WORKSPACE = vendor
-
+DOCKER_CONTAINERD_GOMOD = github.com/containerd/containerd
 DOCKER_CONTAINERD_LDFLAGS = \
-	-X github.com/docker/containerd.GitCommit=$(DOCKER_CONTAINERD_VERSION)
-
-DOCKER_CONTAINERD_BUILD_TARGETS = cmd/ctr cmd/containerd cmd/containerd-shim
+	-X $(DOCKER_CONTAINERD_GOMOD).GitCommit=$(DOCKER_CONTAINERD_VERSION)
 
+DOCKER_CONTAINERD_BUILD_TARGETS = \
+	$(DOCKER_CONTAINERD_GOMOD)/cmd/ctr \
+	$(DOCKER_CONTAINERD_GOMOD)/cmd/containerd \
+	$(DOCKER_CONTAINERD_GOMOD)/cmd/containerd-shim
 DOCKER_CONTAINERD_INSTALL_BINS = containerd containerd-shim
 
 ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
-- 
2.28.0

[Buildroot] [PATCH v4 4/7] package/docker-engine: fix go-module package identifier

[Christian Stewart]

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/docker-engine/docker-engine.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/docker-engine/docker-engine.mk b/package/docker-engine/docker-engine.mk
index 6bb382f5a7..5809e3e6c4 100644
--- a/package/docker-engine/docker-engine.mk
+++ b/package/docker-engine/docker-engine.mk
@@ -11,14 +11,14 @@ DOCKER_ENGINE_LICENSE = Apache-2.0
 DOCKER_ENGINE_LICENSE_FILES = LICENSE
 
 DOCKER_ENGINE_DEPENDENCIES = host-pkgconf
-DOCKER_ENGINE_SRC_SUBDIR = github.com/docker/docker
 
 DOCKER_ENGINE_LDFLAGS = \
 	-X main.GitCommit=$(DOCKER_ENGINE_VERSION) \
 	-X main.Version=$(DOCKER_ENGINE_VERSION)
-
 DOCKER_ENGINE_TAGS = cgo exclude_graphdriver_zfs autogen
-DOCKER_ENGINE_BUILD_TARGETS = cmd/dockerd
+
+DOCKER_ENGINE_GOMOD = github.com/docker/docker
+DOCKER_ENGINE_BUILD_TARGETS = $(DOCKER_ENGINE_GOMOD)/cmd/dockerd
 
 ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
 DOCKER_ENGINE_TAGS += seccomp
-- 
2.28.0

[Buildroot] [PATCH v4 5/7] package/docker-cli: fix go-module package identifier

[Christian Stewart]

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/docker-cli/docker-cli.mk | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/package/docker-cli/docker-cli.mk b/package/docker-cli/docker-cli.mk
index 1466b0afbd..07700809d5 100644
--- a/package/docker-cli/docker-cli.mk
+++ b/package/docker-cli/docker-cli.mk
@@ -13,12 +13,13 @@ DOCKER_CLI_LICENSE_FILES = LICENSE
 
 DOCKER_CLI_DEPENDENCIES = host-pkgconf
 
-DOCKER_CLI_TAGS = autogen
-DOCKER_CLI_BUILD_TARGETS = cmd/docker
-
+DOCKER_CLI_GOMOD = github.com/docker/cli
 DOCKER_CLI_LDFLAGS = \
-	-X github.com/docker/cli/cli.GitCommit=$(DOCKER_CLI_VERSION) \
-	-X github.com/docker/cli/cli.Version=$(DOCKER_CLI_VERSION)
+	-X $(DOCKER_CLI_GOMOD).GitCommit=$(DOCKER_CLI_VERSION) \
+	-X $(DOCKER_CLI_GOMOD).Version=$(DOCKER_CLI_VERSION)
+
+DOCKER_CLI_BUILD_TARGETS = $(DOCKER_CLI_GOMOD)/cmd/docker
+DOCKER_CLI_TAGS = autogen
 
 ifeq ($(BR2_PACKAGE_DOCKER_CLI_STATIC),y)
 DOCKER_CLI_LDFLAGS += -extldflags '-static'
-- 
2.28.0

[Buildroot] [PATCH v4 6/7] package/docker-proxy: fix go-module package identifier

[Christian Stewart]

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/docker-proxy/docker-proxy.mk | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/package/docker-proxy/docker-proxy.mk b/package/docker-proxy/docker-proxy.mk
index 8843266c30..41ae07da09 100644
--- a/package/docker-proxy/docker-proxy.mk
+++ b/package/docker-proxy/docker-proxy.mk
@@ -12,9 +12,7 @@ DOCKER_PROXY_LICENSE_FILES = LICENSE
 
 DOCKER_PROXY_DEPENDENCIES = host-pkgconf
 
-DOCKER_PROXY_WORKSPACE = gopath
-
-DOCKER_PROXY_BUILD_TARGETS = cmd/proxy
+DOCKER_PROXY_BUILD_TARGETS = github.com/docker/libnetwork/cmd/proxy
 
 define DOCKER_PROXY_INSTALL_TARGET_CMDS
 	$(INSTALL) -D -m 0755 $(@D)/bin/proxy $(TARGET_DIR)/usr/bin/docker-proxy
-- 
2.28.0

[Buildroot] [PATCH v4 7/7] package/mender-artifact: fix go-module package build target

[Christian Stewart]

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/mender-artifact/mender-artifact.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/mender-artifact/mender-artifact.mk b/package/mender-artifact/mender-artifact.mk
index c81ec9ba53..af78df0f61 100644
--- a/package/mender-artifact/mender-artifact.mk
+++ b/package/mender-artifact/mender-artifact.mk
@@ -33,7 +33,7 @@ HOST_MENDER_ARTIFACT_DEPENDENCIES = host-xz
 
 HOST_MENDER_ARTIFACT_LDFLAGS = -X main.Version=$(HOST_MENDER_ARTIFACT_VERSION)
 
-HOST_MENDER_ARTIFACT_BUILD_TARGETS = cli/mender-artifact
+HOST_MENDER_ARTIFACT_BUILD_TARGETS = github.com/mendersoftware/mender-artifact/cli/mender-artifact
 
 HOST_MENDER_ARTIFACT_BIN_NAME = mender-artifact
 HOST_MENDER_ARTIFACT_INSTALL_BINS = $(HOST_MENDER_ARTIFACT_BIN_NAME)
-- 
2.28.0

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Thomas Petazzoni]

Hello Christian,

I think this sort of series would greatly benefit from having a
detailed cover letter. I believe one of the reason why this series has
been stalled for so much time is because none of the Buildroot
maintainers are very familiar with the overall Go ecosystem. So in some
sense, you need to educate us: explain how it was working before, what
are the changes, what are the possible solutions, which one you have
chosen and why, etc.

That would _tremendously_ help to move forward with this series, which
we really need to get merged soon.

Thanks!

Thomas

On Thu, 13 Aug 2020 14:32:14 -0700
Christian Stewart <christian@paral.in> wrote:

> The Go compiler needs to know the "import path" to the root of package source
> repositories. Previously, this was done by creating a fake _gopath in the build
> directory and symlinking the package source into that path.
> 
> Go has deprecated the GOPATH mechanism in favor of a new approach - Modules -
> which specifies the root import path (and dependencies) in a "go.mod" file.
> 
> This commit moves Buildroot to use the new go.mod approach. Both host and target
> Go packages compile correctly with small tweaks adjusting the build target
> specifier.
> 
> Note: the Go module system will NOT download sources from the Internet due to
> GOPROXY=off and -mod=vendor. All Buildroot packages currently have "vendor"
> directories included with dependencies' source code.
> 
> The environment variables passed to Go during host and target package
> compilation have been fixed to correctly pass CC, CXX, CFLAGS, and so on.
> 
> Reference: https://github.com/golang/go/wiki/Modules
> 
> Signed-off-by: Christian Stewart <christian@paral.in>
> 
> v2 -> v3:
> 
>  - cjs: cleaned up spelling and moved extract hook to configure step
>  - cjs: applied fixes from vincent fazio related to host packages
> 
> v3 -> v4:
> 
>  - cjs: clean up HOST and TARGET variables passed to Go
>  - cjs: fix all CXXflags, cgo env vars to be consistent/correct
>  - cjs: document / explain the difference from GOPATH
> 
> Signed-off-by: Christian Stewart <christian@paral.in>
> ---
>  package/go/go.mk      | 55 +++++++++++++++++++++++++--------
>  package/pkg-golang.mk | 71 +++++++++++++++++++++----------------------
>  2 files changed, 76 insertions(+), 50 deletions(-)


Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Christian Stewart]

Hey Thomas,

On Thu, Aug 13, 2020 at 2:45 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> I think this sort of series would greatly benefit from having a
> detailed cover letter. I believe one of the reason why this series has
> been stalled for so much time is because none of the Buildroot
> maintainers are very familiar with the overall Go ecosystem. So in some
> sense, you need to educate us: explain how it was working before, what
> are the changes, what are the possible solutions, which one you have
> chosen and why, etc.

This makes sense, and I'm happy to explain.

I had hoped that the commit message in patch #1 would explain in a
succinct way.

It boils down to: Go has "packages" which have "import paths" such as
"my/package/is/here."

Previously Go would determine the path to a package by, approximately,
taking the absolute path to the package, and removing the $GOPATH
prefix.

For example: the path to "my/golang/package" would be
$GOPATH/src/my/golang/package

In Buildroot we extract the code into a build dir, not a GOPATH tree,
so we previously created a "fake gopath" to tell the Go compiler the
root import path to the code.

This change instead uses the go.mod file to determine the root import path.

Does this clear things up? Should I explain further in any part?

Best regards,
Christian

> On Thu, 13 Aug 2020 14:32:14 -0700
> Christian Stewart <christian@paral.in> wrote:
>
> > The Go compiler needs to know the "import path" to the root of package source
> > repositories. Previously, this was done by creating a fake _gopath in the build
> > directory and symlinking the package source into that path.
> >
> > Go has deprecated the GOPATH mechanism in favor of a new approach - Modules -
> > which specifies the root import path (and dependencies) in a "go.mod" file.
> >
> > This commit moves Buildroot to use the new go.mod approach. Both host and target
> > Go packages compile correctly with small tweaks adjusting the build target
> > specifier.
> >
> > Note: the Go module system will NOT download sources from the Internet due to
> > GOPROXY=off and -mod=vendor. All Buildroot packages currently have "vendor"
> > directories included with dependencies' source code.
> >
> > The environment variables passed to Go during host and target package
> > compilation have been fixed to correctly pass CC, CXX, CFLAGS, and so on.
> >
> > Reference: https://github.com/golang/go/wiki/Modules

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Christian Stewart]

Hi all,

On Thu, Aug 13, 2020 at 2:32 PM Christian Stewart <christian@paral.in> wrote:
>
> Go has deprecated the GOPATH mechanism in favor of a new approach - Modules -
> which specifies the root import path (and dependencies) in a "go.mod" file.
>
> This commit moves Buildroot to use the new go.mod approach. Both host and target
> Go packages compile correctly with small tweaks adjusting the build target
> specifier.


I'm looking to merge the Go modules change in advance of upgrading to
Go 1.14 or Go 1.15.

Patchseries available here (same as v4 in the mailing list):

https://patch-diff.githubusercontent.com/raw/paralin/buildroot/pull/8.patch

As of now I haven't had anyone else - even the Go developers in
Buildroot - test/review this.

Adam (mender), Francois (docker-engine), Angelo (go.mk) - it would be
very helpful if you could give this series a spin, particularly with
the host packages you're using.

Best regards,
Christian Stewart

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Arnout Vandecappelle]

On 13/08/2020 23:32, Christian Stewart wrote:
> -# We pass an empty GOBIN, otherwise "go install: cannot install
> -# cross-compiled binaries when GOBIN is set"
> -GO_COMMON_ENV = \
> -	PATH=$(BR_PATH) \
> -	GOBIN= \
> -	CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
> -
> -GO_TARGET_ENV = \
> -	$(HOST_GO_TARGET_ENV) \
> -	$(GO_COMMON_ENV)
> -
> -GO_HOST_ENV = \
> -	CGO_CFLAGS="$(HOST_CFLAGS)" \
> -	CGO_LDFLAGS="$(HOST_LDFLAGS)" \
> -	$(GO_COMMON_ENV)
> +# Used when compiling host packages.
> +GO_HOST_ENV = $(HOST_GO_HOST_ENV)

 Small but important comment: it's not good to use variables from a specific
package in the package infrastructure. It happens to work because those
variables are not used in any condition or immediately-expanded variable, but
it's fragile.

 Instead, it should be reversed (like it was originally): the variable should be
set in the infrastructure file and use in go.mk.

> +
> +# Used when compiling target packages.
> +GO_TARGET_ENV =	$(HOST_GO_TARGET_ENV)

 Same here of course.

 Regards,
 Arnout

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Thomas Petazzoni]

Hello Christian,

On Thu, 13 Aug 2020 14:32:14 -0700
Christian Stewart <christian@paral.in> wrote:

> The Go compiler needs to know the "import path" to the root of package source
> repositories. Previously, this was done by creating a fake _gopath in the build
> directory and symlinking the package source into that path.
> 
> Go has deprecated the GOPATH mechanism in favor of a new approach - Modules -
> which specifies the root import path (and dependencies) in a "go.mod" file.
> 
> This commit moves Buildroot to use the new go.mod approach. Both host and target
> Go packages compile correctly with small tweaks adjusting the build target
> specifier.
> 
> Note: the Go module system will NOT download sources from the Internet due to
> GOPROXY=off and -mod=vendor. All Buildroot packages currently have "vendor"
> directories included with dependencies' source code.
> 
> The environment variables passed to Go during host and target package
> compilation have been fixed to correctly pass CC, CXX, CFLAGS, and so on.
> 
> Reference: https://github.com/golang/go/wiki/Modules
> 
> Signed-off-by: Christian Stewart <christian@paral.in>

So, I have applied the whole series to our next branch, but with some
significant rework. First of all, the series was not bisectable: if you
applied PATCH 1/7 but not the others, things would be broken.

So instead, I did a number of preparation patches in the different Go
packages to set the GOMOD variable, then had the patch switching the
infrastructure to go.mod, and then patched again the packages to drop
anything that was no longer needed (such as <pkg>_WORKSPACE
definitions).

In addition, other changes were done, see below.


> -# For the convienience of target packages.
>  HOST_GO_TOOLDIR = $(HOST_GO_ROOT)/pkg/tool/linux_$(GO_GOARCH)
> -HOST_GO_TARGET_ENV = \
> -	GO111MODULE=off \
> -	GOARCH=$(GO_GOARCH) \
> -	GOCACHE="$(HOST_GO_TARGET_CACHE)" \
> +HOST_GO_COMMON_ENV = \
> +	CC=$(HOSTCC_NOCCACHE) \
> +	CGO_ENABLED=$(HOST_GO_CGO_ENABLED) \
> +	CXX=$(HOSTCXX_NOCCACHE) \
> +	GO111MODULE=on \
> +	GOBIN= \
> +	GOFLAGS=-mod=vendor \
> +	GOPATH="$(HOST_GO_GOPATH)" \
> +	GOPROXY=off \
>  	GOROOT="$(HOST_GO_ROOT)" \
> +	GOTOOLDIR="$(HOST_GO_TOOLDIR)" \
> +	PATH=$(BR_PATH)
> +
> +# Used for compiling host packages.
> +HOST_GO_HOST_ENV = \
> +	$(HOST_GO_COMMON_ENV) \
> +	CGO_CFLAGS="$(HOST_CFLAGS)" \
> +	CGO_CXXFLAGS="$(HOST_CXXFLAGS)" \
> +	CGO_FFLAGS="$(HOST_FCFLAGS)" \
> +	CGO_LDFLAGS="$(HOST_LDFLAGS)" \
> +	GOARCH="" \
> +	GOCACHE="$(HOST_GO_HOST_CACHE)"
> +
> +# Used for compiling the host-go compiler and target packages.
> +HOST_GO_CROSS_ENV = \
> +	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
> +	CC_FOR_TARGET="$(TARGET_CC)" \
> +	CXX_FOR_TARGET="$(TARGET_CXX)" \
> +	GOARCH=$(GO_GOARCH) \
> +	GO_ASSUME_CROSSCOMPILING=1
> +
> +# Used for compiling target packages.
> +#
> +# Note: CC and CXX must be set as well as TARGET_ variants.
> +HOST_GO_TARGET_ENV = \
> +	$(HOST_GO_COMMON_ENV) \
> +	$(HOST_GO_CROSS_ENV) \
>  	CC="$(TARGET_CC)" \
>  	CXX="$(TARGET_CXX)" \
> -	GOTOOLDIR="$(HOST_GO_TOOLDIR)"
> +	CGO_CFLAGS="$(TARGET_CFLAGS)" \
> +	CGO_CXXFLAGS="$(TARGET_CXXFLAGS)" \
> +	CGO_FFLAGS="$(TARGET_FCFLAGS)" \
> +	CGO_LDFLAGS="$(TARGET_LDFLAGS)" \
> +	GOCACHE="$(HOST_GO_TARGET_CACHE)"
>  
>  # The go compiler's cgo support uses threads.  If BR2_TOOLCHAIN_HAS_THREADS is
>  # set, build in cgo support for any go programs that may need it.  Note that
> @@ -64,13 +100,6 @@ else
>  HOST_GO_CGO_ENABLED = 0
>  endif
>  
> -HOST_GO_CROSS_ENV = \
> -	CC_FOR_TARGET="$(TARGET_CC)" \
> -	CXX_FOR_TARGET="$(TARGET_CXX)" \
> -	GOARCH=$(GO_GOARCH) \
> -	$(if $(GO_GOARM),GOARM=$(GO_GOARM)) \
> -	GO_ASSUME_CROSSCOMPILING=1

This was *way* too much changes in one patch, and many of those changes
are completely unrelated to the go.mod integration. In addition, I was
not happy with some variables defined in HOST_GO_COMMON_ENV (such as
CC/CXX) being overridden later on in HOST_GO_TARGET_ENV for example.

So I split that up into many more commits, unrelated to the go.mod
integration:

package/go: add a HOST_GO_HOST_ENV variable
https://git.buildroot.org/buildroot/commit/?h=next&id=7c3e3cbcf215c56c378a912a6a6ca40edc61d7ad

package/go: introduce HOST_GO_COMMON_ENV
https://git.buildroot.org/buildroot/commit/?h=next&id=557282a9c046547daade72fe18e233006716d0b2

package/go: pass CFLAGS/CXXFLAGS/LDFLAGS in HOST_GO_TARGET_ENV
https://git.buildroot.org/buildroot/commit/?h=next&id=7a89fa07ee2813a515da91d884ce5920e1ba1887

package/go: re-integrate GO_COMMON_ENV into HOST_GO_COMMON_ENV
https://git.buildroot.org/buildroot/commit/?h=next&id=1a17e4ae996c4c6727a29f049892be5ae57d7e89

package/go: drop GO_TARGET_ENV / GO_HOST_ENV
https://git.buildroot.org/buildroot/commit/?h=next&id=0d26fb58b79c6412c49287923e562ea6c8d950e0

This dramatically helps understanding what each step is doing, rather
than having one massive change in all those variables that is really
difficult to follow/review.

>  else # !BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
>  # host-go can still be used to build packages for the host. No need to set all
>  # the arch stuff since we will not be cross-compiling.
> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
> index 2494ce028c..2d80e99619 100644
> --- a/package/pkg-golang.mk
> +++ b/package/pkg-golang.mk
> @@ -23,21 +23,11 @@
>  
>  GO_BIN = $(HOST_DIR)/bin/go
>  
> -# We pass an empty GOBIN, otherwise "go install: cannot install
> -# cross-compiled binaries when GOBIN is set"
> -GO_COMMON_ENV = \
> -	PATH=$(BR_PATH) \
> -	GOBIN= \
> -	CGO_ENABLED=$(HOST_GO_CGO_ENABLED)
> -
> -GO_TARGET_ENV = \
> -	$(HOST_GO_TARGET_ENV) \
> -	$(GO_COMMON_ENV)
> -
> -GO_HOST_ENV = \
> -	CGO_CFLAGS="$(HOST_CFLAGS)" \
> -	CGO_LDFLAGS="$(HOST_LDFLAGS)" \
> -	$(GO_COMMON_ENV)
> +# Used when compiling host packages.
> +GO_HOST_ENV = $(HOST_GO_HOST_ENV)
> +
> +# Used when compiling target packages.
> +GO_TARGET_ENV =	$(HOST_GO_TARGET_ENV)

This got a bit changed by the patch mentioned above.

> +# $(2)_GOMOD is the root Go module path for the project, inferred if not set.
> +# If the go.mod file does not exist, one is written with this root path.
> +# Replaces the old GOPATH/symlink mechanism for setting the root import path.

It doesn't make much sense to have a comment about how it was before,
so I dropped this last line (suggested by Arnout).

> +$(2)_GOMOD ?= $$($(2)_SRC_DOMAIN)/$$($(2)_SRC_VENDOR)/$$($(2)_SRC_SOFTWARE)
> +
> +# Correctly configure the go.mod and go.sum files for the module system.
> +# Note: Go is configured to use the "vendor" dir and not make network calls.
> +define $(2)_APPLY_EXTRACT_GOMOD
> +	if [ -f $$($(2)_PKGDIR)/go.mod ]; then \
> +		cp $$($(2)_PKGDIR)/go.mod $$(@D)/go.mod; \
> +		if [ -f $$(@D)/go.sum ]; then \
> +			rm $$(@D)/go.sum; \
> +		fi; \
> +	fi; \

There are no packages so far that provide their go.mod in their package
directory, so I dropped this.

> +	if [ -f $$($(2)_PKGDIR)/go.sum ]; then \
> +		cp $$($(2)_PKGDIR)/go.sum $$(@D)/go.sum; \
> +	fi

There are no packages so far that provide their go.sum in their package
directory, so I dropped this.

> +	if [ ! -f $$(@D)/go.mod ] && [ -n "$$($(2)_GOMOD)" ]; then \

The $(2)_GOMOD variable cannot be empty, so this second part of the
test was not needed, I dropped it.

> +		printf "module $$($(2)_GOMOD)\n" > $$(@D)/go.mod; \
> +	fi
>  endef
> -endif
> +
> +$(2)_POST_EXTRACT_HOOKS += $(2)_APPLY_EXTRACT_GOMOD

After discussion with Arnout/Yann, we moved this to a POST_PATCH_HOOKS.

>  
>  # Build step. Only define it if not already defined by the package .mk
>  # file.
> @@ -111,26 +110,24 @@ endif
>  # Build package for target
>  define $(2)_BUILD_CMDS
>  	$$(foreach d,$$($(2)_BUILD_TARGETS),\
> -		cd $$($(2)_SRC_PATH); \
> +		cd $$(@D); \
>  		$$(GO_TARGET_ENV) \
> -			GOPATH="$$(@D)/$$($(2)_WORKSPACE)" \
>  			$$($(2)_GO_ENV) \
>  			$$(GO_BIN) build -v $$($(2)_BUILD_OPTS) \
>  			-o $$(@D)/bin/$$(or $$($(2)_BIN_NAME),$$(notdir $$(d))) \
> -			./$$(d)
> +			$$(d)

Instead of this, I did:

			$$($(2)_GOMOD)/$$(d)

as it avoids repeating the GOMOD in the BUILD_TARGETS of all packages.

Also, you had forgotten to update the Buildroot manual to take into
account the changes that happened in the infrastructure.

Overall, the series looks like this:

7c3e3cbcf215c56c378a912a6a6ca40edc61d7ad package/go: add a HOST_GO_HOST_ENV variable
557282a9c046547daade72fe18e233006716d0b2 package/go: introduce HOST_GO_COMMON_ENV
7a89fa07ee2813a515da91d884ce5920e1ba1887 package/go: pass CFLAGS/CXXFLAGS/LDFLAGS in HOST_GO_TARGET_ENV
1a17e4ae996c4c6727a29f049892be5ae57d7e89 package/go: re-integrate GO_COMMON_ENV into HOST_GO_COMMON_ENV
0d26fb58b79c6412c49287923e562ea6c8d950e0 package/go: drop GO_TARGET_ENV / GO_HOST_ENV
01c5e0ed72007433160d858a8c06cc84eafc120c package/docker-containerd: define <pkg>_GOMOD variable
edb06ecf3b378f191d5ba36ba872d8a627b2eb53 package/docker-engine: define <pkg>_GOMOD variable
b80bcd7ffa5a815334ead9c99c6b938c4d6b5a7b package/docker-cli: define <pkg>_GOMOD variable
7afd262da0e2a8fee0ebd33049619346de660cb5 package/go: implement go modules integration
302bb3347cff943977042b0bf53194c797c9a41b package/runc: drop <pkg>_WORKSPACE variable
a7ed0ae6cca1b86f03b81d461ae18fbac28df262 package/docker-containerd: drop <pkg>_WORKSPACE variable
cfcf745e10f354c419e305f3ac334764becdeff9 package/docker-engine: drop <pkg>_SRC_SUBDIR variable
913fac5bb1c9e38bd38637ea0271a1c513529a67 package/docker-proxy: drop <pkg>_WORKSPACE variable
92584815bace88b0c0548d7ab7796854c92aaa99 package/mender-artifact: drop custom GOFLAGS
a289fc8b69b71a4ab0585e23e2004c01db821e2e docs/manual/adding-packages-golang.txt: update following go.mod integration

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4 3/7] package/docker-containerd: fix go-module package identifier/target

[Thomas Petazzoni]

On Thu, 13 Aug 2020 14:32:16 -0700
Christian Stewart <christian@paral.in> wrote:

> Signed-off-by: Christian Stewart <christian@paral.in>
> ---
>  package/docker-containerd/docker-containerd.mk | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/package/docker-containerd/docker-containerd.mk b/package/docker-containerd/docker-containerd.mk
> index 5a44489065..8b17e6997a 100644
> --- a/package/docker-containerd/docker-containerd.mk
> +++ b/package/docker-containerd/docker-containerd.mk
> @@ -9,13 +9,14 @@ DOCKER_CONTAINERD_SITE = $(call github,containerd,containerd,v$(DOCKER_CONTAINER
>  DOCKER_CONTAINERD_LICENSE = Apache-2.0
>  DOCKER_CONTAINERD_LICENSE_FILES = LICENSE
>  
> -DOCKER_CONTAINERD_WORKSPACE = vendor
> -
> +DOCKER_CONTAINERD_GOMOD = github.com/containerd/containerd

So I've split this into two patches:

 - One adding the GOMOD variable, which was committed *before* the
   changes in the golang-package infrastructure.

 - One dropping the WORKSPACE variable, which was committed *after* the
   changes in the golang-package infrastructure.

This allows the series to be bisectable.

>  DOCKER_CONTAINERD_LDFLAGS = \
> -	-X github.com/docker/containerd.GitCommit=$(DOCKER_CONTAINERD_VERSION)
> -
> -DOCKER_CONTAINERD_BUILD_TARGETS = cmd/ctr cmd/containerd cmd/containerd-shim
> +	-X $(DOCKER_CONTAINERD_GOMOD).GitCommit=$(DOCKER_CONTAINERD_VERSION)
>  
> +DOCKER_CONTAINERD_BUILD_TARGETS = \
> +	$(DOCKER_CONTAINERD_GOMOD)/cmd/ctr \
> +	$(DOCKER_CONTAINERD_GOMOD)/cmd/containerd \
> +	$(DOCKER_CONTAINERD_GOMOD)/cmd/containerd-shim

I've dropped this entire changed, this the GOMOD is now pre-pended to
the BUILD_TARGETS by the golang-package infra.

I'm not going to comment on the other patches in the series, since the
changes I did were pretty much the same.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Thomas Petazzoni]

On Thu, 13 Aug 2020 14:32:14 -0700
Christian Stewart <christian@paral.in> wrote:

> +HOST_GO_GOPATH = $(HOST_DIR)/usr/share/go-path

[...]

> +	GOPATH="$(HOST_GO_GOPATH)" \

Are you sure this GOPATH is needed ? We're supposed to move away from
GOPATH, but you're adding a new GOPATH location here, and in a build
that has all our Go packages enabled, this
$(HOST_DIR)/usr/share/go-path location doesn't even exist.

Could you clarify ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Christian Stewart]

Hi Thomas,

Replies to both your emails below:

On Sat, Aug 29, 2020 at 6:00 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> So instead, I did a number of preparation patches in the different Go
> packages to set the GOMOD variable, then had the patch switching the
> infrastructure to go.mod, and then patched again the packages to drop
> anything that was no longer needed (such as <pkg>_WORKSPACE
> definitions).

OK, noted, along with the other adjustments.

> > +$(2)_GOMOD ?= $$($(2)_SRC_DOMAIN)/$$($(2)_SRC_VENDOR)/$$($(2)_SRC_SOFTWARE)
> > +
> > +# Correctly configure the go.mod and go.sum files for the module system.
> > +# Note: Go is configured to use the "vendor" dir and not make network calls.
> > +define $(2)_APPLY_EXTRACT_GOMOD
> > +     if [ -f $$($(2)_PKGDIR)/go.mod ]; then \
> > +             cp $$($(2)_PKGDIR)/go.mod $$(@D)/go.mod; \
> > +             if [ -f $$(@D)/go.sum ]; then \
> > +                     rm $$(@D)/go.sum; \
> > +             fi; \
> > +     fi; \
>
> There are no packages so far that provide their go.mod in their package
> directory, so I dropped this.
>
> > +     if [ -f $$($(2)_PKGDIR)/go.sum ]; then \
> > +             cp $$($(2)_PKGDIR)/go.sum $$(@D)/go.sum; \
> > +     fi
>
> There are no packages so far that provide their go.sum in their package
> directory, so I dropped this.
>
> > +     if [ ! -f $$(@D)/go.mod ] && [ -n "$$($(2)_GOMOD)" ]; then \
>
> The $(2)_GOMOD variable cannot be empty, so this second part of the
> test was not needed, I dropped it.

GOMOD was not required before. If the go.mod exists (which happens in
many cases) then you can infer what GOMOD is from the first line of
the file. So GOMOD absolutely can be empty and was in some of my
buildroot_ext packages.

As you're now using it as part of the build targets, requiring it to
be set is, I guess, fine.

The mechanism to copy in a go.mod file and/or a go.sum file is used in
some of my buildroot_ext packages. It's used in many Gentoo packages
as well, iirc. However, as the Buildroot mainline currently does not
use the Go compiler to download any dependencies (instead relying on a
"vendor" dir to exist in the source repository), this doesn't make any
difference yet in Buildroot.

> Overall, the series looks like this:
>
> 7c3e3cbcf215c56c378a912a6a6ca40edc61d7ad package/go: add a HOST_GO_HOST_ENV variable
> 557282a9c046547daade72fe18e233006716d0b2 package/go: introduce HOST_GO_COMMON_ENV
> 7a89fa07ee2813a515da91d884ce5920e1ba1887 package/go: pass CFLAGS/CXXFLAGS/LDFLAGS in HOST_GO_TARGET_ENV
> 1a17e4ae996c4c6727a29f049892be5ae57d7e89 package/go: re-integrate GO_COMMON_ENV into HOST_GO_COMMON_ENV
> 0d26fb58b79c6412c49287923e562ea6c8d950e0 package/go: drop GO_TARGET_ENV / GO_HOST_ENV
> 01c5e0ed72007433160d858a8c06cc84eafc120c package/docker-containerd: define <pkg>_GOMOD variable
> edb06ecf3b378f191d5ba36ba872d8a627b2eb53 package/docker-engine: define <pkg>_GOMOD variable
> b80bcd7ffa5a815334ead9c99c6b938c4d6b5a7b package/docker-cli: define <pkg>_GOMOD variable
> 7afd262da0e2a8fee0ebd33049619346de660cb5 package/go: implement go modules integration
> 302bb3347cff943977042b0bf53194c797c9a41b package/runc: drop <pkg>_WORKSPACE variable
> a7ed0ae6cca1b86f03b81d461ae18fbac28df262 package/docker-containerd: drop <pkg>_WORKSPACE variable
> cfcf745e10f354c419e305f3ac334764becdeff9 package/docker-engine: drop <pkg>_SRC_SUBDIR variable
> 913fac5bb1c9e38bd38637ea0271a1c513529a67 package/docker-proxy: drop <pkg>_WORKSPACE variable
> 92584815bace88b0c0548d7ab7796854c92aaa99 package/mender-artifact: drop custom GOFLAGS
> a289fc8b69b71a4ab0585e23e2004c01db821e2e docs/manual/adding-packages-golang.txt: update following go.mod integration

Thank you for the work to review, split, & cleanup, the attention to
detail in Buildroot is on the highest level.

> > +     GOPATH="$(HOST_GO_GOPATH)" \
>
> Are you sure this GOPATH is needed ? We're supposed to move away from
> GOPATH, but you're adding a new GOPATH location here, and in a build
> that has all our Go packages enabled, this
> $(HOST_DIR)/usr/share/go-path location doesn't even exist.
>
> Could you clarify ?

Currently the Go compiler will use $GOPATH/pkg/{mod,sumdb} for the Go
modules cache. It probably did not create any in the mainline
Buildroot builds because there is no Go module activity in use yet
(all vendor based). However, a valid GOPATH does need to be set so
that the Go compiler can use it for the "pkg" cache dir if necessary.

So, GOPATH, while not used for lookups of packages under the "src" dir
anymore, is still used for a few other directories for Go.

There is a new variable in newer versions of Go to control this
separately from GOPATH, so we can add that and eventually fully remove
GOPATH (but not yet).

GOMODCACHE="$GOPATH/pkg/mod"

Best regards,
Christian Stewart

[Buildroot] [PATCH v4 1/7] package/go: implement go modules integration

[Thomas Petazzoni]

Hello Christian,

On Sat, 29 Aug 2020 10:32:12 -0700
Christian Stewart <christian@paral.in> wrote:

> GOMOD was not required before. If the go.mod exists (which happens in
> many cases) then you can infer what GOMOD is from the first line of
> the file. So GOMOD absolutely can be empty and was in some of my
> buildroot_ext packages.

No, it cannot be empty, because pkg-golang.mk has something like this:

$(2)_GOMOD ?= $$($(2)_SRC_DOMAIN)/$$($(2)_SRC_VENDOR)/$$($(2)_SRC_SOFTWARE)

So, even if $$($(2)_SRC_DOMAIN), $$($(2)_SRC_VENDOR) and
$$($(2)_SRC_SOFTWARE) are empty, the GOMOD will be equal to //.
Obviously, that's a stupid and non-working GOMOD value, but it's still
non-empty.

> As you're now using it as part of the build targets, requiring it to
> be set is, I guess, fine.

It was already always set, as explained above.

> The mechanism to copy in a go.mod file and/or a go.sum file is used in
> some of my buildroot_ext packages. It's used in many Gentoo packages
> as well, iirc. However, as the Buildroot mainline currently does not
> use the Go compiler to download any dependencies (instead relying on a
> "vendor" dir to exist in the source repository), this doesn't make any
> difference yet in Buildroot.

Yes, in principle I don't see a problem with adding support for that,
but (1) it was not used anywhere and (2) it was not documented anywhere.

> Thank you for the work to review, split, & cleanup, the attention to
> detail in Buildroot is on the highest level.

By splitting things up, it also help understand better what was going
on.

> > Are you sure this GOPATH is needed ? We're supposed to move away from
> > GOPATH, but you're adding a new GOPATH location here, and in a build
> > that has all our Go packages enabled, this
> > $(HOST_DIR)/usr/share/go-path location doesn't even exist.
> >
> > Could you clarify ?  
> 
> Currently the Go compiler will use $GOPATH/pkg/{mod,sumdb} for the Go
> modules cache. It probably did not create any in the mainline
> Buildroot builds because there is no Go module activity in use yet
> (all vendor based). However, a valid GOPATH does need to be set so
> that the Go compiler can use it for the "pkg" cache dir if necessary.
> 
> So, GOPATH, while not used for lookups of packages under the "src" dir
> anymore, is still used for a few other directories for Go.
> 
> There is a new variable in newer versions of Go to control this
> separately from GOPATH, so we can add that and eventually fully remove
> GOPATH (but not yet).
> 
> GOMODCACHE="$GOPATH/pkg/mod"

OK, thanks for the additional explanations.

What are the next steps in terms of Go support ? Are there projects
where the "vendor" dependencies are not included as part of the
upstream project, and need to be downloaded by go at build time ? If
so, do we want to support that ? How ? Do you have examples of
prominent Go projects that are delivered like this ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com