Re: [PATCH v2] util/grub.d/linux: Improve initramfs detection

[Daniel Kiper <dkiper@net-space.pl>]

On Sun, Mar 27, 2022 at 10:41:31PM -0500, Oskari Pirhonen wrote:
> Add detection for initramfs of the form *.img.old. For example, Gentoo's
> sys-kernel/genkernel installs it as initramfs-*.img and moves any
> existing one to initramfs-*.img.old.

You are mentioning initramfs* files but the patch adds also initrd*
files. Could you explain that in the commit message?

> Signed-off-by: Oskari Pirhonen <xxc3ncoredxx@gmail.com>

You are breaking indention in the patch.

> ---
> v1 -> v2:
> - don't reorder the checks
> - include 20_linux_xen.in
>
>  util/grub.d/10_linux.in     | 17 +++++++++--------
>  util/grub.d/20_linux_xen.in | 29 +++++++++++++++--------------
>  2 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index ca068038e..cb049e943 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -215,14 +215,15 @@ while [ "x$list" != "x" ] ; do
>    done
>
>    initrd_real=
> -  for i in "initrd.img-${version}" "initrd-${version}.img" "initrd-${version}.gz" \
> -	   "initrd-${version}" "initramfs-${version}.img" \
> -	   "initrd.img-${alt_version}" "initrd-${alt_version}.img" \
> -	   "initrd-${alt_version}" "initramfs-${alt_version}.img" \
> -	   "initramfs-genkernel-${version}" \
> -	   "initramfs-genkernel-${alt_version}" \
> -	   "initramfs-genkernel-${GENKERNEL_ARCH}-${version}" \
> -	   "initramfs-genkernel-${GENKERNEL_ARCH}-${alt_version}"; do
> +  for i in "initrd.img-${version}" "initrd-${version}.img" \
> +        "initrd-${alt_version}.img.old" "initrd-${version}.gz" \
> +        "initrd-${alt_version}.gz.old" "initrd-${version}" \
> +        "initramfs-${version}.img" "initramfs-${alt_version}.img.old" \
> +        "initrd.img-${alt_version}" "initrd-${alt_version}.img" \
> +        "initrd-${alt_version}" "initramfs-${alt_version}.img" \
> +        "initramfs-genkernel-${version}" "initramfs-genkernel-${alt_version}" \
> +        "initramfs-genkernel-${GENKERNEL_ARCH}-${version}" \

Here...

> +        "initramfs-genkernel-${GENKERNEL_ARCH}-${alt_version}"; do

Ditto...

>      if test -e "${dirname}/${i}" ; then
>        initrd_real="${i}"
>        break
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index f45559ff8..5a1b7b7d4 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -283,20 +283,21 @@ while [ "x${xen_list}" != "x" ] ; do
>  	alt_version=`echo $version | sed -e "s,\.old$,,g"`
>  	linux_root_device_thisversion="${LINUX_ROOT_DEVICE}"
>
> -	initrd_real=
> -	for i in "initrd.img-${version}" "initrd-${version}.img" "initrd-${version}.gz" \
> -	   "initrd-${version}" "initramfs-${version}.img" \
> -	   "initrd.img-${alt_version}" "initrd-${alt_version}.img" \
> -	   "initrd-${alt_version}" "initramfs-${alt_version}.img" \
> -	   "initramfs-genkernel-${version}" \
> -	   "initramfs-genkernel-${alt_version}" \
> -	   "initramfs-genkernel-${GENKERNEL_ARCH}-${version}" \
> -	   "initramfs-genkernel-${GENKERNEL_ARCH}-${alt_version}" ; do
> -	    if test -e "${dirname}/${i}" ; then
> -		initrd_real="$i"
> -		break
> -	    fi
> -	done
> +    initrd_real=

And here...

> +    for i in "initrd.img-${version}" "initrd-${version}.img" \
> +        "initrd-${alt_version}.img.old" "initrd-${version}.gz" \
> +        "initrd-${alt_version}.gz.old" "initrd-${version}" \
> +        "initramfs-${version}.img" "initramfs-${alt_version}.img.old" \
> +        "initrd.img-${alt_version}" "initrd-${alt_version}.img" \
> +        "initrd-${alt_version}" "initramfs-${alt_version}.img" \
> +        "initramfs-genkernel-${version}" "initramfs-genkernel-${alt_version}" \

I would leave these two file names in separate lines. Same above...

> +        "initramfs-genkernel-${GENKERNEL_ARCH}-${version}" \
> +        "initramfs-genkernel-${GENKERNEL_ARCH}-${alt_version}"; do
> +        if test -e "${dirname}/${i}" ; then
> +            initrd_real="${i}"
> +            break
> +        fi
> +    done

Again, broken indention... It is almost in every line of this patch.

Daniel