All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robbie Harwood <rharwood@redhat.com>
To: grub-devel@gnu.org
Subject: Re: [PATCH] Correct sorting of kernel names containing '_'
Date: Tue, 18 Jan 2022 12:56:34 -0500	[thread overview]
Message-ID: <jlgbl093p2l.fsf@redhat.com> (raw)
In-Reply-To: <20220118172943.9903-1-rharwood@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2869 bytes --]

Robbie Harwood <rharwood@redhat.com> writes:

> sort(1) from GNU coreutils does not treat underscore as part of a
> version number for `sort -V.  This causes misorderings on x86_64, where
> e.g. kernel-core-3.17.6-300.11.fc21.x86_64 will incorrectly sort
> *before* kernel-core-3.17.6-300.fc21.x86_64.
>
> To cope with this behavior, replace underscores with dashes in order to
> approximate their intended meaning as version component separators.
>
> Fixes: https://savannah.gnu.org/bugs/?42844
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>  util/grub-mkconfig_lib.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 301d1ac22..23d41475f 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -243,8 +243,8 @@ version_test_numeric ()
>  
>  version_test_gt ()
>  {
> -  version_test_gt_a="`echo "$1" | sed -e "s/[^-]*-//"`"
> -  version_test_gt_b="`echo "$2" | sed -e "s/[^-]*-//"`"
> +  version_test_gt_a="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  version_test_gt_b="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
>    version_test_gt_cmp=gt
>    if [ "x$version_test_gt_b" = "x" ] ; then
>      return 0
> -- 
> 2.34.1

Before yinz bikeshed this into the ground, here are some other
approaches I considered:

- Use a C program to invoke librpm.  This has the advantage of being
  already written - it's what we currently do downstream:
  https://github.com/rhboot/grub2/commit/81e155df39c9fbc97803cf9032f4138f5d9a0310
  I dislike the approach regardless because it's heavyweight and doesn't
  generalize to non-RPM distros (especially since e.g. Debian can
  install a /usr/bin/rpm & co.).  But it is "more correct", so if you
  want to merge that instead, there's the patch :)

- Invoke rpmdev-vercmp if available.  Problem here is that it may not be
  (it's not part of the rpm package itself on Fedora, but rather
  rpmdev-tools), and again, won't generalize to non-RPM distros.

- Strip out any dot-delimited section that contains non-numerics.  This
  has also been written and is the attached solution to
  https://savannah.gnu.org/bugs/?42844 - but since it hasn't been merged
  in the six years since posting, I have to assume it's been rejected.
  It also would not correctly handle the case of
  e.g. kernel-core-2.6.32-1_test2.x86_64 vs kernel-core-2.6.32-1.x86_64

- Strip underscore entirely, either with sed or tr.  This has the same
  problem as the previous one in my mind - underscore is intended as a
  separator, and this causes it to not be treated as one.

- Ignore the sorting problem and keep the status quo as it is in
  upstream.  Attractive, but my bugzilla mailbox has plenty of angry
  people in it already :)

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

  reply	other threads:[~2022-01-18 17:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-18 17:29 [PATCH] Correct sorting of kernel names containing '_' Robbie Harwood
2022-01-18 17:56 ` Robbie Harwood [this message]
2022-01-18 21:57   ` Glenn Washburn
2022-01-19 18:52     ` Robbie Harwood
2022-01-19 21:33     ` [PATCH] mkconfig: use distro sorts when available Robbie Harwood
2022-01-20  0:36       ` Glenn Washburn
2022-01-20  1:17       ` Glenn Washburn

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=jlgbl093p2l.fsf@redhat.com \
    --to=rharwood@redhat.com \
    --cc=grub-devel@gnu.org \
    /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.