Re: [Buildroot] [PATCH] package/arm-gnu-toolchain: Add $(HOSTARCH) so it can be built with aarch64

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: bryce@raspberrywood.com
Cc: buildroot@buildroot.org, Bryce Johnson <bryce@redpinelabs.com>,
	Suniel Mahesh <sunil@amarulasolutions.com>
Subject: Re: [Buildroot] [PATCH] package/arm-gnu-toolchain: Add $(HOSTARCH) so it can be built with aarch64
Date: Tue, 22 Oct 2024 21:28:00 +0200	[thread overview]
Message-ID: <20241022212800.570cbcfc@windsurf> (raw)
In-Reply-To: <20241022180542.224972-1-bryce@redpinelabs.com>

Hello Bryce!

Thanks for your patch. Some comments/questions below.

On Tue, 22 Oct 2024 12:05:40 -0600
bryce@raspberrywood.com wrote:

> From: Bryce Johnson <bryce@redpinelabs.com>
> 
> Was testing building with arm64 build server and failed because it was downloading the x86_64 version. Use $(HOSTARCH) instead so it can also downloaded

Minor nit: commit messages should have line wrapped at ~80 columns.

> 
> Tested with building configs/ti_am62x_sk_defconfig on arm64. That it uses to build the 32bit R5 MCU.

More important nit: we need you to add your Signed-off-by: line at the
end of the commit log to allow us to apply your patch.

> diff --git a/package/arm-gnu-toolchain/arm-gnu-toolchain.hash b/package/arm-gnu-toolchain/arm-gnu-toolchain.hash
> index 0800fa2168..110ec40c09 100644
> --- a/package/arm-gnu-toolchain/arm-gnu-toolchain.hash
> +++ b/package/arm-gnu-toolchain/arm-gnu-toolchain.hash
> @@ -1,2 +1,4 @@
>  # taken from https://developer.arm.com/-/media/Files/downloads/gnu/13.2.rel1/binrel/arm-gnu-toolchain-13.2.rel1-x86_64-arm-none-eabi.tar.xz.sha256asc
>  sha256  6cd1bbc1d9ae57312bcd169ae283153a9572bd6a8e4eeae2fedfbc33b115fdbb  arm-gnu-toolchain-13.2.rel1-x86_64-arm-none-eabi.tar.xz
> +# taken from https://developer.arm.com/-/media/Files/downloads/gnu/13.2.rel1/binrel/arm-gnu-toolchain-13.2.rel1-aarch64-arm-none-eabi.tar.xz.sha256asc
> +sha256  8fd8b4a0a8d44ab2e195ccfbeef42223dfb3ede29d80f14dcf2183c34b8d199a  arm-gnu-toolchain-13.2.rel1-aarch64-arm-none-eabi.tar.xz
> diff --git a/package/arm-gnu-toolchain/arm-gnu-toolchain.mk b/package/arm-gnu-toolchain/arm-gnu-toolchain.mk
> index 03bb4dc5da..fce80f2f3a 100644
> --- a/package/arm-gnu-toolchain/arm-gnu-toolchain.mk
> +++ b/package/arm-gnu-toolchain/arm-gnu-toolchain.mk
> @@ -6,7 +6,7 @@
>  
>  ARM_GNU_TOOLCHAIN_VERSION = 13.2.rel1
>  ARM_GNU_TOOLCHAIN_SITE = https://developer.arm.com/-/media/Files/downloads/gnu/$(ARM_GNU_TOOLCHAIN_VERSION)/binrel
> -ARM_GNU_TOOLCHAIN_SOURCE = arm-gnu-toolchain-$(ARM_GNU_TOOLCHAIN_VERSION)-x86_64-arm-none-eabi.tar.xz
> +ARM_GNU_TOOLCHAIN_SOURCE = arm-gnu-toolchain-$(ARM_GNU_TOOLCHAIN_VERSION)-$(HOSTARCH)-arm-none-eabi.tar.xz

The change looks correct as-is of course, as it doesn't make anything
worse. However, I believe to make things cleaner, we should perhaps
introduce a package/arm-gnu-toolchain/Config.in.host file, with a blind
option BR2_HOST_PACKAGE_ARM_GNU_TOOLCHAIN, that has a "depends on" the
host architectures that are supported. And the packages that use
host-arm-gnu-toolchain would have to be careful to propagate those
depends on, so that we do not allow using host-arm-gnu-toolchain on
configurations that do not allow using the host-arm-gnu-toolchain
package. However this can be done as a separate patch, as again your
change is not making things any worse than they already are :)

Could you send a v2 of your patch with the proposed changes?

Thanks again!

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