Re: [Buildroot] [PATCH v2 1/2] fix: configs/beaglev_fire: update custom tarballs

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

Hello Jamie,

I was about to apply but had some second thoughts. See below.

First, please don't add this "fix: " at the beginning of the commit
title. The commit title should be just:

	configs/beaglev_fire: update custom tarballs

On Thu, 8 Aug 2024 11:52:20 +0100
Jamie Gibbons via buildroot <buildroot@buildroot.org> wrote:

> The Linux kernel and  U-Boot custom tarballs were updated incorrectly.

Please change to "were updated incorrectly in commit 8ce97fd550d6".

> Update these to select specific release tags instead of branches. Update
> the U-Boot hash to match changes.

Why do you mention that update the U-Boot hash here, while the Linux
hash is also updated?

> 
> Fixes:8ce97fd550d6("configs/beaglev_fire: bump Linux and U-Boot")

Space after "Fixes:", space after the commit hash.

But now, the real issue (because I had already fixed locally the above
minor details).


> index 809e00ff92..93b6aa7f26 100644
> --- a/configs/beaglev_fire_defconfig
> +++ b/configs/beaglev_fire_defconfig
> @@ -8,7 +8,7 @@ BR2_ROOTFS_OVERLAY="board/beagleboard/beaglev_fire/rootfs-overlay/"
>  BR2_ROOTFS_POST_IMAGE_SCRIPT="board/beagleboard/beaglev_fire/post-image.sh"
>  BR2_LINUX_KERNEL=y
>  BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
> -BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,linux4microchip,linux,linux-6.6-mchp+fpga)/linux4microchip+fpga-2024.06.tar.gz"
> +BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,linux4microchip,linux,linux4microchip+fpga-2024.06)/linux4microchip+fpga-2024.06.tar.gz"

Please change the name of the tarball to
linux-linux4microchip+fpga-2024.06.tar.gz, i.e this line should be:

BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,linux4microchip,linux,linux4microchip+fpga-2024.06)/linux-linux4microchip+fpga-2024.06.tar.gz"

>  BR2_LINUX_KERNEL_DEFCONFIG="mpfs"
>  BR2_LINUX_KERNEL_DTS_SUPPORT=y
>  BR2_LINUX_KERNEL_INTREE_DTS_NAME="microchip/mpfs-beaglev-fire"
> @@ -18,7 +18,7 @@ BR2_TARGET_ROOTFS_EXT2_4=y
>  BR2_TARGET_UBOOT=y
>  BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
>  BR2_TARGET_UBOOT_CUSTOM_TARBALL=y
> -BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call github,linux4microchip,u-boot-mchp,u-boot-2023.07-mchp)/linux4microchip+fpga-2024.06.tar.gz"
> +BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call github,linux4microchip,u-boot-mchp,linux4microchip+fpga-2024.06)/linux4microchip+fpga-2024.06.tar.gz"

Please change the name of the tarball to
uboot-linux4microchip+fpga-2024.06.tar.gz, so this line should become:

BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call github,linux4microchip,u-boot-mchp,linux4microchip+fpga-2024.06)/uboot-linux4microchip+fpga-2024.06.tar.gz"

The motivation for this is that otherwise we have two tarballs with
exactly the same name "linux4microchip+fpga-2024.06.tar.gz", for Linux
and U-Boot, which can be confusing.

Also, it is customary in Buildroot to name tarballe
<package>-<version>.tar.gz, which is exactly what I'm asking you to do
here.

(I could have fixed that myself, but I'm working on a not very fast DSL
connection at the moment, so downloading all those kernel tarballs takes
quite a while...)

Could you take into account those comments also for PATCH 2/2 ?

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