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
next prev parent reply other threads:[~2024-10-22 19:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-22 18:05 [Buildroot] [PATCH] package/arm-gnu-toolchain: Add $(HOSTARCH) so it can be built with aarch64 bryce
2024-10-22 19:28 ` Thomas Petazzoni via buildroot [this message]
2024-10-22 20:36 ` Bryce Johnson
2024-10-22 21:12 ` Thomas Petazzoni via buildroot
2024-10-22 22:59 ` Bryce Johnson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241022212800.570cbcfc@windsurf \
--to=buildroot@buildroot.org \
--cc=bryce@raspberrywood.com \
--cc=bryce@redpinelabs.com \
--cc=sunil@amarulasolutions.com \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.