Re: [Buildroot] [PATCH v2] package/go: go-bin host-go provider implies target support

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

Hello Tom,

Thanks for your patch! I must admit for this change I really would like
to have the feedback from Christian. Christian, could you review and
provide your feedback? This patch allows to use Go packages on ARM64
hosts, where the go-bootstrap1 -> go-boostrap2 -> go-bootstrap3 ->
go-src solution isn't available as go-bootstrap1 is only supported on
i386, x86-64 and arm (32-bit).

Tom: I however have a comment below.

On Mon, 31 Mar 2025 11:34:37 -0400
Tom Wambold <tom5760@gmail.com> wrote:

> The BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS configuration symbol is
> used to determine if we can build Go packages for the target
> architecture.  Previously, this depended *only* on the
> BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE3_ARCH_SUPPORTS symbol, which is true
> only if the host system can bootstrap the Go compiler.
> 
> The go-bin package makes it unnecessary to bootstrap the Go compiler.
> Also, some host architectures (i.e. arm64/aarch64) don't support
> bootstrapping the compiler anyway.
> 
> Add a new symbol BR2_PACKAGE_HOST_GO_BIN_TARGET_ARCH_SUPPORTS to let
> BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS depend on either being
> able to bootstrap the compiler *OR* using the go-bin package.  This
> fixes being able to use the Go compiler to build target packages on an
> arm64/aarch64 host system.
> 
> Signed-off-by: Tom Wambold <tom5760@gmail.com>
> 
> ---
> Changes v1 -> v2:
>   - Added missing Signed-off-by
> ---
>  package/go/Config.in.host        | 2 +-
>  package/go/go-bin/Config.in.host | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/package/go/Config.in.host b/package/go/Config.in.host
> index 7e354b3298..f9b27cc4d8 100644
> --- a/package/go/Config.in.host
> +++ b/package/go/Config.in.host
> @@ -2,7 +2,7 @@
>  config BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
>  	bool
>  	default y
> -	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE3_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE3_ARCH_SUPPORTS || BR2_PACKAGE_HOST_GO_BIN_TARGET_ARCH_SUPPORTS
>  	# See https://go.dev/doc/install/source#environment
>  	# See src/go/build/syslist.go for the list of supported architectures
>  	depends on (BR2_arm && BR2_TOOLCHAIN_SUPPORTS_PIE) || BR2_aarch64 \
> diff --git a/package/go/go-bin/Config.in.host b/package/go/go-bin/Config.in.host
> index 6312c1b950..5fe1686fef 100644
> --- a/package/go/go-bin/Config.in.host
> +++ b/package/go/go-bin/Config.in.host
> @@ -12,3 +12,8 @@ config BR2_PACKAGE_HOST_GO_BIN_HOST_ARCH_SUPPORTS
>  	bool
>  	default y
>  	depends on BR2_PACKAGE_HOST_GO_BIN_HOST_ARCH != ""
> +
> +config BR2_PACKAGE_HOST_GO_BIN_TARGET_ARCH_SUPPORTS
> +	bool
> +	default y
> +	depends on BR2_PACKAGE_HOST_GO_BIN_HOST_ARCH != ""

I think you don't need the new
BR2_PACKAGE_HOST_GO_BIN_TARGET_ARCH_SUPPORTS, which is actually
misleading as this option doesn't represent which target architecture
are represented.

And in fact if you look at BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
it's defined like this:

config BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
        bool
        default y
        depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE3_ARCH_SUPPORTS
        # See https://go.dev/doc/install/source#environment
        # See src/go/build/syslist.go for the list of supported architectures
        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
        depends on !BR2_ARM_CPU_ARMV4
        # MIPS R6 support in Go has not yet been developed.
        depends on !BR2_MIPS_CPU_MIPS64R6
        # Go doesn't support Risc-v 32-bit.
        depends on !BR2_RISCV_32
        # Go requires the following Risc-v General (G) features:
        depends on !BR2_riscv || (BR2_RISCV_ISA_RVI && \
                BR2_RISCV_ISA_RVM && BR2_RISCV_ISA_RVA && \
                BR2_RISCV_ISA_RVF && BR2_RISCV_ISA_RVD)

The "depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE3_ARCH_SUPPORTS"
allows to verify that we are on a host where bootstraping is possible.
And then the remaining dependencies are about making sure the target
architecture is supported.

So I think I would rather do:

config BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
        bool
        default y
        depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_STAGE3_ARCH_SUPPORTS || BR2_PACKAGE_HOST_GO_BIN_HOST_ARCH_SUPPORTS
        # See https://go.dev/doc/install/source#environment
        # See src/go/build/syslist.go for the list of supported architectures
        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
        depends on !BR2_ARM_CPU_ARMV4
        # MIPS R6 support in Go has not yet been developed.
        depends on !BR2_MIPS_CPU_MIPS64R6
        # Go doesn't support Risc-v 32-bit.
        depends on !BR2_RISCV_32
        # Go requires the following Risc-v General (G) features:
        depends on !BR2_riscv || (BR2_RISCV_ISA_RVI && \
                BR2_RISCV_ISA_RVM && BR2_RISCV_ISA_RVA && \
                BR2_RISCV_ISA_RVF && BR2_RISCV_ISA_RVD)

This basically extends the thing to say: "In order to build Go packages
for the target, we need:

1. A compiler that runs on our host: either bootstrapped, or pre-compiled (go-bin)

2. A target architecture that's supported
"

Tom, Christian, what do you think?

Best regards,

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