[PATCH v2 0/2] Improve kernel sorting in grub-mkconfig

2022-01-20 Thread Robbie Harwood
This version of the patchset formally adds the distro sort support from
the previous thread, as well as attempting to address Glenn's review
suggestions around ordering and usage criteria.

Be well,
--Robbie

Robbie Harwood (2):
  Correct sorting of kernel names containing '_'
  mkconfig: use distro sorts when available

 util/grub-mkconfig_lib.in | 90 +--
 1 file changed, 49 insertions(+), 41 deletions(-)

-- 
2.34.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 2/2] mkconfig: use distro sorts when available

2022-01-20 Thread 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 
---
 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..cc0ab790e 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 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 command -v rpmdev-vercmp >/dev/null; then
+  SORT=sort_rpm
+elif command -v dpkg >/dev/null; then
+  SORT=sort_dpkg
+elif sort -V  /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


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2] grub-mount: Support libfuse 3

2022-01-20 Thread Daniel Kiper
On Mon, Jan 17, 2022 at 03:34:37PM +0100, Fabian Vogt wrote:
> libfuse 3.0.0 got released in 2016, with some API changes compared to 2.x.
> This commit introduces support for 3.x while keeping it compatible with 2.6
> as a fallback still.
>
> To detect fuse3, switch configure over to use pkg-config, which is simpler yet
> more reliable than looking for library and header manually. Also set
> FUSE_USE_VERSION that way, as it depends on the used libfuse version.
>
> Now that the CFLAGS are read from pkg-config, use just , which works
> with 2.x as well as 3.x and is recommended by libfuse upstream.
>
> One behaviour change of libfuse3 is that FUSE_ATOMIC_O_TRUNC is set by 
> default,
> which means that open with O_TRUNC is passed as-is instead of calling the
> truncate operation. With libfuse2, truncate failed with -ENOSYS and that was
> returned to the application. To make O_TRUNC fail with libfuse3, return -EROFS
> explicitly if writing was requested.
>
> Signed-off-by: Fabian Vogt 

Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 0/2] Make build more robust

2022-01-20 Thread Daniel Kiper
On Wed, Jan 12, 2022 at 09:40:19PM -0600, Glenn Washburn wrote:
> I've found these two patches to be necessary under certain build conditions
> that I've not been able to narrow down to a specific cause. I suspect it is
> related to the values of some build environment variables (like *CFLAGS).
> Either way, these patches allow a successful build finishes without error
> and where the test suite succeeds. So I believe these patches are allowing
> a usable build. Under normal conditions, these changes should be superflous
> and thus not affect the build process.

For both Reviewed-by: Daniel Kiper ...

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 2/2] mkconfig: use distro sorts when available

2022-01-20 Thread 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 
---
 util/grub-mkconfig_lib.in | 92 ++-
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 23d41475f..b858cd1a0 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -200,62 +200,72 @@ 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 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] && command -v dpkg >/dev/null; then
+  SORT=sort_dpkg
+elif command -v rpmdev-vercmp; then
+  SORT=sort_rpm
+elif command -v dpkg >/dev/null; then
+  SORT=sort_dpkg
+elif sort -V  /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


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 1/2] Correct sorting of kernel names containing '_'

2022-01-20 Thread 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 
---
 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


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 2/2] mkconfig: use distro sorts when available

2022-01-20 Thread Glenn Washburn
On Thu, 20 Jan 2022 11:45:23 -0500
Robbie Harwood  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 
> ---
>  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..cc0ab790e 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 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 command -v rpmdev-vercmp >/dev/null; then
> +  SORT=sort_rpm
> +elif command -v dpkg >/dev/null; then
> +  SORT=sort_dpkg

Sorry if I was not clear, my previous comment was suggesting to add
the above clauses below the existing clauses. So it would look like:

if [ -f /etc/debian_version ] && command -v dpkg >/dev/nul; then
  SORT=sort_dpkg
elif [ -f /etc/redhat-release ] && command -v rpmdev-vercmp >/dev/null; then
  SORT=sort_rpm
elif command -v rpmdev-vercmp >/dev/null; then
  SORT=sort_rpm
elif command -v dpkg >/dev/null; then
  SORT=sort_dpkg

The idea is if /etc/debian_version exists, you definitely want to use
dpkg, and likewise if /etc/redhat-release exists, you definitely want
to use rpmdev-vercmp. As a fallback, if either dpkg or rpmdev-vercmp
exists, use them as they are preferable to the crude sort method. And
as a last resort fallback to the sort methods. 

> +elif sort -V  /de

Re: [PATCH v3] http: Allow use of non-standard TCP/IP ports

2022-01-20 Thread Daniel Kiper
On Sun, Jan 16, 2022 at 03:46:08PM -0700, Stephen Balousek wrote:
> Allow the use of HTTP servers listening on ports other 80. This is done
> with an extension to the http notation:
>
>   (http[,server[,port]])
>
>  - or -
>
>   (http[,server[:port]])
>
> Signed-off-by: Stephen Balousek 
> ---
>
> Thanks for that, Daniel. Sorry to have so much trouble making sense of 
> grub_strtoul()! The change you suggested works like a charm.

No worries...

> I also fixed my example IPv6 address to be a proper "documentation" address.

You could do this for IPv4 too. There is 192.0.2.0/24 which is IPv6
documentation equivalent. It is a nit. I can do it for you before push.

So, Reviewed-by: Daniel Kiper 

Thank you for doing the work!

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 1/2] Correct sorting of kernel names containing '_'

2022-01-20 Thread 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 
---
 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


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 0/2] Improve kernel sorting in grub-mkconfig

2022-01-20 Thread Robbie Harwood
This version corrects a miscommunication about Glenn's review suggestions.  (I
have slightly simplified the suggested code, but it's functionally the same.)

Be well,
--Robbie

Robbie Harwood (2):
  Correct sorting of kernel names containing '_'
  mkconfig: use distro sorts when available

 util/grub-mkconfig_lib.in | 92 ++-
 1 file changed, 51 insertions(+), 41 deletions(-)

-- 
2.34.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 2/2] mkconfig: use distro sorts when available

2022-01-20 Thread Didier Spaier
Hi Robbie and All,

Do i understand correctly that this will not change anything for distributions
(like Slint, based on Slackware, that I maintain) that are neither Debian-likes
nor Fedora-likes?

Cheers,
Didier

Le 20/01/2022 à 17:45, Robbie Harwood a écrit :
> 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 
> ---
>  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..cc0ab790e 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 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 command -v rpmdev-vercmp >/dev/null; then
> +  SORT=sort_rpm
> +elif command -v dpkg >/dev/null; then
> +  SORT=sort_dpkg
> +elif sort -V  /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 ()

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v3 2/2] mkconfig: use distro sorts when available

2022-01-20 Thread Glenn Washburn
On Thu, 20 Jan 2022 14:33:20 -0500
Robbie Harwood  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 
> ---
>  util/grub-mkconfig_lib.in | 92 ++-
>  1 file changed, 51 insertions(+), 41 deletions(-)
> 
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 23d41475f..b858cd1a0 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -200,62 +200,72 @@ 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 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] && command -v dpkg >/dev/null; then

I think without the space before ']' the test won't be evaluated
correctly.

> +  SORT=sort_dpkg

At first I was wondering why you left out the elif clause that tests for
/etc/redhat-release. But on second thought it does seem to not be
needed because if rpmdev-vercmp exists then we want to use it next
regardless. Even though its not needed, I think the excluded elif clause
makes explicit the intent. If next two elif clauses get swapped, then
we will potentially preferring dpkg over rpmdev-vercmp on redhat based
distros (not good). Perhaps that's an unlikely scenario. I think we
should add a comment here saying something like, "No need to explicitly
check for Redhat-based distros because we want to use rpmdev-vercmp
next if it exists regardless of distro".

My thinking is based on the assumption that the version compare that
rpmdev-vercmp does is different that dpkg and b

Re: [PATCH v2 2/2] mkconfig: use distro sorts when available

2022-01-20 Thread Glenn Washburn
On Thu, 20 Jan 2022 23:07:10 +0100
Didier Spaier  wrote:

> Hi Robbie and All,
> 
> Do i understand correctly that this will not change anything for distributions
> (like Slint, based on Slackware, that I maintain) that are neither 
> Debian-likes
> nor Fedora-likes?

It shouldn't change anything in the sense that you as a maintainer need
to do anything. At worst, it should fall back to the current bahavior
with a slight improvement. If Slint/Slack has dpkg or rpmdev-vercmp
installed, those will be used to do the version comparison. This should
be better than the current case, unless the Slint/Slack versioning
scheme is radically different than what those packages expect.

Glenn

> 
> Cheers,
> Didier
> 
> Le 20/01/2022 à 17:45, Robbie Harwood a écrit :
> > 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 
> > ---
> >  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..cc0ab790e 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 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 ]; t