[PATCH] Correct sorting of kernel names containing '_'

[PATCH] Correct sorting of kernel names containing '_'

[Robbie Harwood]

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

Re: [PATCH] Correct sorting of kernel names containing '_'

[Robbie Harwood]

[-- 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 --]

Re: [PATCH] Correct sorting of kernel names containing '_'

[Glenn Washburn]

On Tue, 18 Jan 2022 12:56:34 -0500
Robbie Harwood <rharwood@redhat.com> wrote:

> 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:

Let the bikeshedding begin! Just kidding :)
This seems like a simple solution. Not being in the weeds on this, can
you comment on the short coming of changing the '_' to '-'? Are there
any known version string formats where this would be worse? At the
moment, it seems like a good incremental solution.

> 
> - 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 :)

Agreed, not a fan of this approach.

> - 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.

I like this as a supplement to this patch. I'm thinking first identify
the distro (using /etc/os-release?), if known RPM distro look for
rpmdev-vercmp, else look for dpkg (to use dpkg --compare-versions), and
if neither is found fall back to this approach. This could be extended
to other package manager compare logic as desired.

> - 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

This patch actually removes the last part of the version string
starting with the first dot segment whose first character is
non-numeric. Regardless, this doesn't seem like a great approach.

> - 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.

Agreed.

> - 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 :)

I think at a minimum this patch seems like a step in the right
direction. It would be nice to have it accompanied by my suggestions
above, but also recognize its a non-insignificant amount of effort.

Glenn

Re: [PATCH] Correct sorting of kernel names containing '_'

[Robbie Harwood]

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

Glenn Washburn <development@efficientek.com> writes:

> This seems like a simple solution. Not being in the weeds on this, can
> you comment on the short coming of changing the '_' to '-'? Are there
> any known version string formats where this would be worse? At the
> moment, it seems like a good incremental solution.

I think in order for it to be a problem, someone / some packaging system
would have to be relying on '_' to not be part of the version.  All
package managers I'm aware of have the version at the end - i.e., after
the package name.[1]

The only real "drawback" I'm aware of is that this still leaves
https://savannah.gnu.org/bugs/?42921 (and any other inconsistencies
between distro sort and `sort -V`) unaddressed - but none of the other
proposals listed would help with that either.  (Fedora avoids this
particular issue by mandating prerelease versions start with 0 - for
instance, rather than making the 3.16.0-rc6 as gentoo did in that bug,
we make 3.16.0-0.rc6 .)

>> - 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.
>
> I like this as a supplement to this patch. I'm thinking first identify
> the distro (using /etc/os-release?), if known RPM distro look for
> rpmdev-vercmp, else look for dpkg (to use dpkg --compare-versions), and
> if neither is found fall back to this approach. This could be extended
> to other package manager compare logic as desired.

Interesting.  Doing this for rpmdev-vercmp alone still doesn't seem
worth the effort to me, but dpkg --compare-versions is more attractive
since it'll be installed for sure.  I'll see if I can make something
tolerable.

>> - 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
>
> This patch actually removes the last part of the version string
> starting with the first dot segment whose first character is
> non-numeric. Regardless, this doesn't seem like a great approach.

Oops, yes.

Be well,
--Robbie

1: This ignores things like epoch, but I'm not aware of any distro using
   an epoch-like versioning for the kernel, nor does it seem likely that
   Linux will change its versioning scheme in a way that breaks "normal"
   version sorting.  It would also be broken right now, though, which is
   why I've buried mention of it it in a footnote :)

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

[PATCH] mkconfig: use distro sorts when available

[Robbie Harwood]

For Debian-likes and Fedora-likes, use the distribution's sorting tools
to determine the latest package before falling back to sort(1).  While
Fedora's rpmdev-vercmp(1) is likely unavailable, Debian's is built into
dpkg itself, and hence likely present.

Refactor to remove unused code and make it easy to add other package
managers as needed.

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
 util/grub-mkconfig_lib.in | 90 +++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 23d41475f..6b0d0327b 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -200,62 +200,70 @@ grub_file_is_not_garbage ()
   return 0
 }
 
-version_sort ()
+# A $SORT function returns 0 if $1 is newer than $2, and 1 otherwise.  Other
+# package managers can be plugged in here as needed with their own functions.
+sort_dpkg ()
 {
-  case $version_sort_sort_has_v in
-    yes)
-      LC_ALL=C sort -V;;
-    no)
-      LC_ALL=C sort -n;;
-    *)
-      if sort -V </dev/null > /dev/null 2>&1; then
-        version_sort_sort_has_v=yes
-	LC_ALL=C sort -V
-      else
-        version_sort_sort_has_v=no
-        LC_ALL=C sort -n
-      fi;;
-   esac
+  left="`echo "$1" | sed -e "s/^[^0-9]*//"`"
+  right="`echo "$2" | sed -e "s/^[^0-9]*//"`"
+  dpkg --compare-versions "$left" gt "$right"
 }
 
-version_test_numeric ()
+sort_rpm ()
 {
-  version_test_numeric_a="$1"
-  version_test_numeric_cmp="$2"
-  version_test_numeric_b="$3"
-  if [ "$version_test_numeric_a" = "$version_test_numeric_b" ] ; then
-    case "$version_test_numeric_cmp" in
-      ge|eq|le) return 0 ;;
-      gt|lt) return 1 ;;
-    esac
-  fi
-  if [ "$version_test_numeric_cmp" = "lt" ] ; then
-    version_test_numeric_c="$version_test_numeric_a"
-    version_test_numeric_a="$version_test_numeric_b"
-    version_test_numeric_b="$version_test_numeric_c"
-  fi
-  if (echo "$version_test_numeric_a" ; echo "$version_test_numeric_b") | version_sort | head -n 1 | grep -qx "$version_test_numeric_b" ; then
-    return 0
-  else
-    return 1
+  left="`echo "$1" | sed -e "s/^[^0-9]*//"`"
+  right="`echo "$2" | sed -e "s/^[^0-9]*//"`"
+  rpmdev-vercmp "$left" "$right" >/dev/null
+  if [ $? -eq 12 ]; then
+    return 0;
   fi
+  return 1;
+}
+
+sort_V ()
+{
+  left="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
+  right="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
+  printf "$left\n$right\n" | LC_ALL=C sort -V | head -n1 | grep -qx "$right"
+}
+
+sort_n ()
+{
+  left="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
+  right="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
+  printf "$left\n$right\n" | LC_ALL=C sort -n | head -n1 | grep -qx "$right"
 }
 
 version_test_gt ()
 {
-  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
+  version_test_gt_a="$1"
+  version_test_gt_b="$2"
+
   if [ "x$version_test_gt_b" = "x" ] ; then
     return 0
   fi
   case "$version_test_gt_a:$version_test_gt_b" in
     *.old:*.old) ;;
-    *.old:*) version_test_gt_a="`echo "$version_test_gt_a" | sed -e 's/\.old$//'`" ; version_test_gt_cmp=gt ;;
-    *:*.old) version_test_gt_b="`echo "$version_test_gt_b" | sed -e 's/\.old$//'`" ; version_test_gt_cmp=ge ;;
+    *.old:*) version_test_gt_a="`echo "$version_test_gt_a" | sed -e 's/\.old$//'`" ;;
+    *:*.old) version_test_gt_b="`echo "$version_test_gt_b" | sed -e 's/\.old$//'`" ;;
   esac
-  version_test_numeric "$version_test_gt_a" "$version_test_gt_cmp" "$version_test_gt_b"
-  return "$?"
+
+  if [ "$version_test_gt_a" = "$version_test_gt_b" ]; then
+    return 1;
+  fi
+
+  if [ x"$SORT" = x ]; then
+    if [ -f /etc/debian_version -a -f /usr/bin/dpkg ]; then
+      SORT=sort_dpkg
+    elif [ -f /etc/redhat-release -a -f /usr/bin/rpmdev-vercmp ]; then
+      SORT=sort_rpm
+    elif sort -V </dev/null > /dev/null 2>&1; then
+      SORT=sort_V
+    else
+      SORT=sort_n
+    fi
+  fi
+  $SORT "$version_test_gt_a" "$version_test_gt_b"
 }
 
 version_find_latest ()
-- 
2.34.1

Re: [PATCH] mkconfig: use distro sorts when available

[Glenn Washburn]

On Wed, 19 Jan 2022 16:33:57 -0500
Robbie Harwood <rharwood@redhat.com> wrote:

> For Debian-likes and Fedora-likes, use the distribution's sorting tools
> to determine the latest package before falling back to sort(1).  While
> Fedora's rpmdev-vercmp(1) is likely unavailable, Debian's is built into
> dpkg itself, and hence likely present.
> 
> Refactor to remove unused code and make it easy to add other package
> managers as needed.
> 
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>  util/grub-mkconfig_lib.in | 90 +++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 41 deletions(-)
> 
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 23d41475f..6b0d0327b 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -200,62 +200,70 @@ grub_file_is_not_garbage ()
>    return 0
>  }
>  
> -version_sort ()
> +# A $SORT function returns 0 if $1 is newer than $2, and 1 otherwise.  Other
> +# package managers can be plugged in here as needed with their own functions.
> +sort_dpkg ()
>  {
> -  case $version_sort_sort_has_v in
> -    yes)
> -      LC_ALL=C sort -V;;
> -    no)
> -      LC_ALL=C sort -n;;
> -    *)
> -      if sort -V </dev/null > /dev/null 2>&1; then
> -        version_sort_sort_has_v=yes
> -	LC_ALL=C sort -V
> -      else
> -        version_sort_sort_has_v=no
> -        LC_ALL=C sort -n
> -      fi;;
> -   esac
> +  left="`echo "$1" | sed -e "s/^[^0-9]*//"`"
> +  right="`echo "$2" | sed -e "s/^[^0-9]*//"`"
> +  dpkg --compare-versions "$left" gt "$right"
>  }
>  
> -version_test_numeric ()
> +sort_rpm ()
>  {
> -  version_test_numeric_a="$1"
> -  version_test_numeric_cmp="$2"
> -  version_test_numeric_b="$3"
> -  if [ "$version_test_numeric_a" = "$version_test_numeric_b" ] ; then
> -    case "$version_test_numeric_cmp" in
> -      ge|eq|le) return 0 ;;
> -      gt|lt) return 1 ;;
> -    esac
> -  fi
> -  if [ "$version_test_numeric_cmp" = "lt" ] ; then
> -    version_test_numeric_c="$version_test_numeric_a"
> -    version_test_numeric_a="$version_test_numeric_b"
> -    version_test_numeric_b="$version_test_numeric_c"
> -  fi
> -  if (echo "$version_test_numeric_a" ; echo "$version_test_numeric_b") | version_sort | head -n 1 | grep -qx "$version_test_numeric_b" ; then
> -    return 0
> -  else
> -    return 1
> +  left="`echo "$1" | sed -e "s/^[^0-9]*//"`"
> +  right="`echo "$2" | sed -e "s/^[^0-9]*//"`"
> +  rpmdev-vercmp "$left" "$right" >/dev/null
> +  if [ $? -eq 12 ]; then
> +    return 0;
>    fi
> +  return 1;
> +}
> +
> +sort_V ()
> +{
> +  left="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  right="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  printf "$left\n$right\n" | LC_ALL=C sort -V | head -n1 | grep -qx "$right"
> +}
> +
> +sort_n ()
> +{
> +  left="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  right="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  printf "$left\n$right\n" | LC_ALL=C sort -n | head -n1 | grep -qx "$right"
>  }
>  
>  version_test_gt ()
>  {
> -  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
> +  version_test_gt_a="$1"
> +  version_test_gt_b="$2"
> +
>    if [ "x$version_test_gt_b" = "x" ] ; then
>      return 0
>    fi
>    case "$version_test_gt_a:$version_test_gt_b" in
>      *.old:*.old) ;;
> -    *.old:*) version_test_gt_a="`echo "$version_test_gt_a" | sed -e 's/\.old$//'`" ; version_test_gt_cmp=gt ;;
> -    *:*.old) version_test_gt_b="`echo "$version_test_gt_b" | sed -e 's/\.old$//'`" ; version_test_gt_cmp=ge ;;
> +    *.old:*) version_test_gt_a="`echo "$version_test_gt_a" | sed -e 's/\.old$//'`" ;;
> +    *:*.old) version_test_gt_b="`echo "$version_test_gt_b" | sed -e 's/\.old$//'`" ;;
>    esac
> -  version_test_numeric "$version_test_gt_a" "$version_test_gt_cmp" "$version_test_gt_b"
> -  return "$?"
> +
> +  if [ "$version_test_gt_a" = "$version_test_gt_b" ]; then
> +    return 1;
> +  fi
> +
> +  if [ x"$SORT" = x ]; then
> +    if [ -f /etc/debian_version -a -f /usr/bin/dpkg ]; then
> +      SORT=sort_dpkg
> +    elif [ -f /etc/redhat-release -a -f /usr/bin/rpmdev-vercmp ]; then
> +      SORT=sort_rpm
> +    elif sort -V </dev/null > /dev/null 2>&1; then
> +      SORT=sort_V
> +    else
> +      SORT=sort_n
> +    fi
> +  fi
> +  $SORT "$version_test_gt_a" "$version_test_gt_b"
>  }
>  
>  version_find_latest ()

I like that this makes the previous implementation simpler too. Thanks
Robbie, LGTM.

Reviewed-by: Glenn Washburn <development@efficientek.com>

Glenn

Re: [PATCH] mkconfig: use distro sorts when available

[Glenn Washburn]

Hi Robbie,

Looks like I sent my first response a tad too soon.

On Wed, 19 Jan 2022 16:33:57 -0500
Robbie Harwood <rharwood@redhat.com> wrote:

> For Debian-likes and Fedora-likes, use the distribution's sorting tools
> to determine the latest package before falling back to sort(1).  While
> Fedora's rpmdev-vercmp(1) is likely unavailable, Debian's is built into
> dpkg itself, and hence likely present.
> 
> Refactor to remove unused code and make it easy to add other package
> managers as needed.
> 
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>  util/grub-mkconfig_lib.in | 90 +++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 41 deletions(-)
> 
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 23d41475f..6b0d0327b 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -200,62 +200,70 @@ grub_file_is_not_garbage ()
>    return 0
>  }
>  
> -version_sort ()
> +# A $SORT function returns 0 if $1 is newer than $2, and 1 otherwise.  Other
> +# package managers can be plugged in here as needed with their own functions.
> +sort_dpkg ()
>  {
> -  case $version_sort_sort_has_v in
> -    yes)
> -      LC_ALL=C sort -V;;
> -    no)
> -      LC_ALL=C sort -n;;
> -    *)
> -      if sort -V </dev/null > /dev/null 2>&1; then
> -        version_sort_sort_has_v=yes
> -	LC_ALL=C sort -V
> -      else
> -        version_sort_sort_has_v=no
> -        LC_ALL=C sort -n
> -      fi;;
> -   esac
> +  left="`echo "$1" | sed -e "s/^[^0-9]*//"`"
> +  right="`echo "$2" | sed -e "s/^[^0-9]*//"`"
> +  dpkg --compare-versions "$left" gt "$right"
>  }
>  
> -version_test_numeric ()
> +sort_rpm ()
>  {
> -  version_test_numeric_a="$1"
> -  version_test_numeric_cmp="$2"
> -  version_test_numeric_b="$3"
> -  if [ "$version_test_numeric_a" = "$version_test_numeric_b" ] ; then
> -    case "$version_test_numeric_cmp" in
> -      ge|eq|le) return 0 ;;
> -      gt|lt) return 1 ;;
> -    esac
> -  fi
> -  if [ "$version_test_numeric_cmp" = "lt" ] ; then
> -    version_test_numeric_c="$version_test_numeric_a"
> -    version_test_numeric_a="$version_test_numeric_b"
> -    version_test_numeric_b="$version_test_numeric_c"
> -  fi
> -  if (echo "$version_test_numeric_a" ; echo "$version_test_numeric_b") | version_sort | head -n 1 | grep -qx "$version_test_numeric_b" ; then
> -    return 0
> -  else
> -    return 1
> +  left="`echo "$1" | sed -e "s/^[^0-9]*//"`"
> +  right="`echo "$2" | sed -e "s/^[^0-9]*//"`"
> +  rpmdev-vercmp "$left" "$right" >/dev/null
> +  if [ $? -eq 12 ]; then
> +    return 0;
>    fi
> +  return 1;
> +}
> +
> +sort_V ()
> +{
> +  left="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  right="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  printf "$left\n$right\n" | LC_ALL=C sort -V | head -n1 | grep -qx "$right"
> +}
> +
> +sort_n ()
> +{
> +  left="`echo "$1" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  right="`echo "$2" | sed -e "s/[^-]*-//" -e "s/_/-/g"`"
> +  printf "$left\n$right\n" | LC_ALL=C sort -n | head -n1 | grep -qx "$right"
>  }
>  
>  version_test_gt ()
>  {
> -  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
> +  version_test_gt_a="$1"
> +  version_test_gt_b="$2"
> +
>    if [ "x$version_test_gt_b" = "x" ] ; then
>      return 0
>    fi
>    case "$version_test_gt_a:$version_test_gt_b" in
>      *.old:*.old) ;;
> -    *.old:*) version_test_gt_a="`echo "$version_test_gt_a" | sed -e 's/\.old$//'`" ; version_test_gt_cmp=gt ;;
> -    *:*.old) version_test_gt_b="`echo "$version_test_gt_b" | sed -e 's/\.old$//'`" ; version_test_gt_cmp=ge ;;
> +    *.old:*) version_test_gt_a="`echo "$version_test_gt_a" | sed -e 's/\.old$//'`" ;;
> +    *:*.old) version_test_gt_b="`echo "$version_test_gt_b" | sed -e 's/\.old$//'`" ;;
>    esac
> -  version_test_numeric "$version_test_gt_a" "$version_test_gt_cmp" "$version_test_gt_b"
> -  return "$?"
> +
> +  if [ "$version_test_gt_a" = "$version_test_gt_b" ]; then
> +    return 1;
> +  fi
> +
> +  if [ x"$SORT" = x ]; then
> +    if [ -f /etc/debian_version -a -f /usr/bin/dpkg ]; then

I think we should be checking for binary presence using "comamnd -v
dpkg >/dev/null". This way dpkg only needs to be in PATH, not some
specific directory. This construct works in bash, sh, and dash. Same for
the rpmdev-vercmp.

> +      SORT=sort_dpkg
> +    elif [ -f /etc/redhat-release -a -f /usr/bin/rpmdev-vercmp ]; then
> +      SORT=sort_rpm

I was thinking about this some more and I'm thinking there might be
distros out there that have dpkg or rpmdev-vercmp (Slack or SuSE?) but
not the etc files. Perhaps we should add two more elif clauses.


    elif command -v rpmdev-vercmp >/dev/null; then
      SORT=sort_rpm
    elif command -v dpkg >/dev/null; then
      SORT=sort_dpkg

I suggest checking for rpmdev-vercmp first because its less commonly
installed and it seems more likely to me that an RPM based distro
(where you want to prefer using rpmdev-vercmp) has dpkg installed than
a DEB distro having rpmdev-vercmp installed.

Glenn