Re: [Buildroot] [PATCH v6 01/10] package/binutils-bare-metal: new package

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

Hello,

I'm late to the party, but here are some comments.

On Fri, 20 Oct 2023 12:42:27 +0100
Neal Frager via buildroot <buildroot@buildroot.org> wrote:


> diff --git a/package/binutils-bare-metal/2.41/0001-sh-conf.patch b/package/binutils-bare-metal/2.41/0001-sh-conf.patch
> new file mode 120000
> index 0000000000..26dff99523
> --- /dev/null
> +++ b/package/binutils-bare-metal/2.41/0001-sh-conf.patch

I'm not happy with how patches are handled here. I think
package/binutils-bare-metal/2.41/ should be a symlink to
package/binutils/2.41/, and all patches you need should be added to
package/binutils/2.41/. I don't want to see a different patch stack to
be applied on binutils-bare-metal compared to binutils.


> diff --git a/package/binutils-bare-metal/Config.in.host b/package/binutils-bare-metal/Config.in.host
> new file mode 100644
> index 0000000000..e70bee8fa2
> --- /dev/null
> +++ b/package/binutils-bare-metal/Config.in.host
> @@ -0,0 +1,15 @@
> +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL
> +	bool "host binutils-bare-metal"

This visible config option is useless, as it's anyway only selected by
host-toolchain-bare-metal. I believe it can simply be dropped.

> +	help
> +	  Build GNU binutils for a bare-metal toolchain
> +
> +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL
> +
> +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION
> +	string
> +	default "2.41"

This option is invisible (it has no prompt), so it is useless. Just
hardcode 2.41 in package/binutils-bare-metal/binutils-bare-metal.mk.


> diff --git a/package/binutils-bare-metal/binutils-bare-metal.hash b/package/binutils-bare-metal/binutils-bare-metal.hash
> new file mode 100644
> index 0000000000..3aab4d433d
> --- /dev/null
> +++ b/package/binutils-bare-metal/binutils-bare-metal.hash
> @@ -0,0 +1,6 @@
> +# From https://gcc.gnu.org/pub/binutils/releases/sha512.sum
> +sha512  5df45d0bd6ddabdce4f35878c041e46a92deef01e7dea5facc97fd65cc06b59abc6fba0eb454b68e571c7e14038dc823fe7f2263843e6e627b7444eaf0fe9374  binutils-2.41.tar.xz
> +
> +# locally calculated
> +sha256  8ceb4b9ee5adedde47b31e975c1d90c73ad27b6b165a1dcd80c7c545eb65b903  COPYING3
> +sha256  56bdea73b6145ef6ac5259b3da390b981d840c24cb03b8e1cbc678de7ecfa18d  COPYING.LIB

Perhaps this should be a symlink to package/binutils/binutils.hash ?



> diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk b/package/binutils-bare-metal/binutils-bare-metal.mk
> new file mode 100644
> index 0000000000..c361a5102a
> --- /dev/null
> +++ b/package/binutils-bare-metal/binutils-bare-metal.mk
> @@ -0,0 +1,39 @@
> +################################################################################
> +#
> +# binutils-bare-metal
> +#
> +################################################################################
> +
> +HOST_BINUTILS_BARE_METAL_VERSION = \
> +	$(call qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION))
> +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),)
> +HOST_BINUTILS_BARE_METAL_VERSION = 2.41
> +endif # BINUTILS_VERSION

Just hardcode the version to 2.41, no option needed.

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