On Thu, Apr 07, 2022 at 05:44:18PM +0200, Daniel Kiper wrote: > 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?
Yeah, I can do that.
>
> > Signed-off-by: Oskari Pirhonen <xxc3ncore...@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...
OK.
>
> > + "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.
That is actually something I wanted to address in a separate thread. The
indentation/style feels kind of all over the place in the few files in
grub.d/ that I glanced at. Some lines are indented with spaces, others
with tabs. Sometimes variables are substituted with $var, other times
with ${var}. Sometimes if [ ... ] is used, other times if test ... is.
If there isn't already an existing style guide that I can reference, I
would like to establish one and then re-style these scripts to be
consistent with that. But it'd have to be a separate endeavor in that
case.
In the meantime, how would you like me to indent the lines in this
patch?
- Oskari
>
> Daniel
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
