Re: [Buildroot] [PATCH v3 1/3] package/go-bootstrap: split into two stages: go1.4 and go1.19.5

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Christian,

Thanks for working on this. I think we are not far from a good solution
that can be merged. However, I have a few comments/questions. See below.

On Tue, 14 Feb 2023 23:32:54 -0800
Christian Stewart via buildroot <buildroot@buildroot.org> wrote:

> +# Use last C-based Go compiler: v1.4.x
> +# See https://golang.org/doc/install/source#bootstrapFromSource
> +GO_BOOTSTRAP_STAGE1_VERSION = 1.4-bootstrap-20171003
> +GO_BOOTSTRAP_STAGE1_SITE = https://dl.google.com/go
> +GO_BOOTSTRAP_STAGE1_SOURCE = go$(GO_BOOTSTRAP_STAGE1_VERSION).tar.gz
> +
> +GO_BOOTSTRAP_STAGE1_LICENSE = BSD-3-Clause
> +GO_BOOTSTRAP_STAGE1_LICENSE_FILES = LICENSE
> +
> +# The toolchain is needed for HOSTCC_NOCACHE used to compile the Go compiler.

This doesn't make much sense. Why would the toolchain package be needed
for HOSTCC_NOCCACHE ? We have several packages that use HOSTCC_NOCCACHE
before the toolchain is ready.

The original comment in go-bootstrap.mk was:

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

and this made a bit more sense (even though I'm wondering why it's
host-go-bootstrap that carries this dependency, and not host-go).


> +	# Set all file timestamps to prevent the go compiler from rebuilding any
> +	# built in packages when programs are built.
> +	find $(HOST_GO_BOOTSTRAP_STAGE2_ROOT) -type f -exec touch -r $(@D)/bin/go {} \;

So we have to do this for bootstrap-stage2 but not bootstrap-stage1 ?


> -	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE2_ARCH_SUPPORTS
> +	# See https://go.dev/doc/install/source#environment
> +	# See src/go/build/syslist.go for the list of supported architectures

This comment looks good, but is unrelated. Separate patch?

>  	depends on (BR2_arm && BR2_TOOLCHAIN_SUPPORTS_PIE) || BR2_aarch64 \
>  		|| BR2_i386 || BR2_x86_64 || BR2_powerpc64le \
>  		|| BR2_mips64 || BR2_mips64el || BR2_riscv || BR2_s390x
> @@ -28,4 +30,4 @@ config BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS
>  config BR2_PACKAGE_HOST_GO_HOST_ARCH_SUPPORTS
>  	bool
>  	default y
> -	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE2_ARCH_SUPPORTS
> diff --git a/package/go/go.mk b/package/go/go.mk
> index c38ae0b99c..b0dd002712 100644
> --- a/package/go/go.mk
> +++ b/package/go/go.mk
> @@ -12,7 +12,7 @@ GO_LICENSE = BSD-3-Clause
>  GO_LICENSE_FILES = LICENSE
>  GO_CPE_ID_VENDOR = golang
>  
> -HOST_GO_DEPENDENCIES = host-go-bootstrap
> +HOST_GO_DEPENDENCIES = host-go-bootstrap-stage2
>  HOST_GO_GOPATH = $(HOST_DIR)/share/go-path
>  HOST_GO_HOST_CACHE = $(HOST_DIR)/share/host-go-cache
>  HOST_GO_ROOT = $(HOST_DIR)/lib/go
> @@ -121,12 +121,12 @@ HOST_GO_HOST_ENV = \
>  	CGO_CXXFLAGS="$(HOST_CXXFLAGS)" \
>  	CGO_LDFLAGS="$(HOST_LDFLAGS)"
>  
> -# The go build system is not compatible with ccache, so use
> -# HOSTCC_NOCCACHE.  See https://github.com/golang/go/issues/11685.
> +# The go build system is not compatable with ccache, so use HOSTCC_NOCCACHE.
> +# See https://github.com/golang/go/issues/11685.

Why is this comment being changed, with a typo added?

>  HOST_GO_MAKE_ENV = \
>  	GO111MODULE=off \
>  	GOCACHE=$(HOST_GO_HOST_CACHE) \
> -	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_ROOT) \
> +	GOROOT_BOOTSTRAP=$(HOST_GO_BOOTSTRAP_STAGE2_ROOT) \
>  	GOROOT_FINAL=$(HOST_GO_ROOT) \
>  	GOROOT="$(@D)" \
>  	GOBIN="$(@D)/bin" \
> @@ -154,7 +154,6 @@ define HOST_GO_INSTALL_CMDS
>  	cp -a $(@D)/pkg/include $(@D)/pkg/linux_* $(HOST_GO_ROOT)/pkg/
>  	cp -a $(@D)/pkg/tool $(HOST_GO_ROOT)/pkg/
>  
> -	# There is a known issue which requires the go sources to be installed

Why is this comment being removed?

>  	# https://golang.org/issue/2775
>  	cp -a $(@D)/src $(HOST_GO_ROOT)/

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot