[PATCH 0/4] Miscellaneous fixes/improvements

[PATCH 0/4] Miscellaneous fixes/improvements

[Glenn Washburn]

Patch #1: When the HFS UUID test fails it shows two GRUB generated outputs.
  There should be one GRUB output and one host system output, like the other
  filesystems output. In this case it should show the host generated HFS UUID
  which is not matching with the GRUB generated one.
Patch #2: If tests are run as non-root user, /sbin and /usr/sbin are usually
  not in the path, which will cause the partmap test to fail because it can
  not find parted (in /usr/sbin on Debian). However, this test does not require
  root privileges to run successfully, so add those paths to the PATH to allow
  partmap to successfully run as a non-root user.
Patch #3: I believe the policy is that invocations of the N_ macro should
  have no whitespace before the '('. This fixes that in a couple invocations.
Patch #4: The crypto and cryptodisk headers are not needed by the probe user
  space utility (maybe they were at one time?). So remove them.

Glenn

Glenn Washburn (4):
  tests: Show host determined fs UUID when hfs UUID test fails
  tests: Add /sbin and /usr/sbin to path in partmap test
  formatting: Remove whitespace between N_ macro and open parenthesis
  util/probe.c: Remove unused header includes

 grub-core/commands/macbless.c | 6 ++----
 tests/partmap_test.in         | 2 ++
 tests/util/grub-fs-tester.in  | 2 +-
 util/probe.c                  | 2 --
 4 files changed, 5 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/4] tests: Show host determined fs UUID when hfs UUID test fails

[Glenn Washburn]

On failure, the hfs test should show both the host and GRUB determined fs
UUID. Prior to this change, both outputs where generated by GRUB, which is
less helpful in determining the cause of failure.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-fs-tester.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 3c1d712de..d323665d2 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1353,7 +1353,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 			:
 		    else
 			echo UUID FAIL
-			echo "$LSOUT"
+			echo "$FSUUID"
 			echo "$GRUBUUID"
 			for lodev in $LODEVICES; do
 			    blkid "$lodev"
-- 
2.34.1

[PATCH 2/4] tests: Add /sbin and /usr/sbin to path in partmap test

[Glenn Washburn]

The partmap test requires no elevated privileges. However, it uses parted
which can be used as a normal user, but is usually located in /sbin or
/usr/bin (eg. on Debian systems). Whereas the normal user does not usually
have /sbin or /usr/sbin added to their path, thus parted will not be found
causing the test to abort. Add /sbin and /usr/sbin to the path for the
partmap test so that the test can run successfully as an unprivileged user.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/partmap_test.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index a73c473b0..4138e88fe 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -19,6 +19,8 @@ set -e
 parted=parted
 grubshell=@builddir@/grub-shell
 
+PATH="$PATH:/sbin:/usr/sbin"
+
 . "@builddir@/grub-core/modinfo.sh"
 
 create_disk_image () {
-- 
2.34.1

[PATCH 3/4] formatting: Remove whitespace between N_ macro and open parenthesis

[Glenn Washburn]

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/commands/macbless.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/grub-core/commands/macbless.c b/grub-core/commands/macbless.c
index 85cefd0f7..3d761452a 100644
--- a/grub-core/commands/macbless.c
+++ b/grub-core/commands/macbless.c
@@ -220,12 +220,10 @@ GRUB_MOD_INIT(macbless)
 {
   cmd = grub_register_command ("mactelbless", grub_cmd_macbless,
 			       N_("FILE"),
-			       N_
-			       ("Bless FILE of HFS or HFS+ partition for intel macs."));
+			       N_("Bless FILE of HFS or HFS+ partition for intel macs."));
   cmd_ppc =
     grub_register_command ("macppcbless", grub_cmd_macbless, N_("DIR"),
-			   N_
-			   ("Bless DIR of HFS or HFS+ partition for PPC macs."));
+			   N_("Bless DIR of HFS or HFS+ partition for PPC macs."));
 }
 
 GRUB_MOD_FINI(macbless)
-- 
2.34.1

[PATCH 4/4] util/probe.c: Remove unused header includes

[Glenn Washburn]

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 util/probe.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/util/probe.c b/util/probe.c
index 05a6d1462..81d91cf59 100644
--- a/util/probe.c
+++ b/util/probe.c
@@ -36,8 +36,6 @@
 #include <grub/i18n.h>
 #include <grub/emu/misc.h>
 #include <grub/util/ofpath.h>
-#include <grub/crypto.h>
-#include <grub/cryptodisk.h>
 
 #include <string.h>
 
-- 
2.34.1

Re: [PATCH 1/4] tests: Show host determined fs UUID when hfs UUID test fails

[Daniel Kiper]

On Wed, May 11, 2022 at 10:19:44PM -0500, Glenn Washburn wrote:
> On failure, the hfs test should show both the host and GRUB determined fs
> UUID. Prior to this change, both outputs where generated by GRUB, which is
> less helpful in determining the cause of failure.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  tests/util/grub-fs-tester.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> index 3c1d712de..d323665d2 100644
> --- a/tests/util/grub-fs-tester.in
> +++ b/tests/util/grub-fs-tester.in
> @@ -1353,7 +1353,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
>  			:
>  		    else
>  			echo UUID FAIL
> -			echo "$LSOUT"
> +			echo "$FSUUID"

I am not sure why you drop 'echo "$LSOUT"' here.

Otherwise patches LGTM.

Daniel

Re: [PATCH 1/4] tests: Show host determined fs UUID when hfs UUID test fails

[Glenn Washburn]

On Fri, 27 May 2022 16:16:48 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Wed, May 11, 2022 at 10:19:44PM -0500, Glenn Washburn wrote:
> > On failure, the hfs test should show both the host and GRUB determined fs
> > UUID. Prior to this change, both outputs where generated by GRUB, which is
> > less helpful in determining the cause of failure.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  tests/util/grub-fs-tester.in | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> > index 3c1d712de..d323665d2 100644
> > --- a/tests/util/grub-fs-tester.in
> > +++ b/tests/util/grub-fs-tester.in
> > @@ -1353,7 +1353,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> >  			:
> >  		    else
> >  			echo UUID FAIL
> > -			echo "$LSOUT"
> > +			echo "$FSUUID"
> 
> I am not sure why you drop 'echo "$LSOUT"' here.

I dropped it because I didn't see the utility in keeping it. Perhaps
there is though, and I'm generally in favor of more output on error
than less. I'm not opposed to keeping it in. If you're like to keep
that line, do you want me to spin a new patch series or can you handle
it in the merge?

Glenn

Re: [PATCH 1/4] tests: Show host determined fs UUID when hfs UUID test fails

[Daniel Kiper]

On Fri, May 27, 2022 at 01:39:25PM -0500, Glenn Washburn wrote:
> On Fri, 27 May 2022 16:16:48 +0200
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Wed, May 11, 2022 at 10:19:44PM -0500, Glenn Washburn wrote:
> > > On failure, the hfs test should show both the host and GRUB determined fs
> > > UUID. Prior to this change, both outputs where generated by GRUB, which is
> > > less helpful in determining the cause of failure.
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > >  tests/util/grub-fs-tester.in | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> > > index 3c1d712de..d323665d2 100644
> > > --- a/tests/util/grub-fs-tester.in
> > > +++ b/tests/util/grub-fs-tester.in
> > > @@ -1353,7 +1353,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  			:
> > >  		    else
> > >  			echo UUID FAIL
> > > -			echo "$LSOUT"
> > > +			echo "$FSUUID"
> >
> > I am not sure why you drop 'echo "$LSOUT"' here.
>
> I dropped it because I didn't see the utility in keeping it. Perhaps
> there is though, and I'm generally in favor of more output on error
> than less. I'm not opposed to keeping it in. If you're like to keep

I prefer to leave it to be in par with other messages in the file.

> that line, do you want me to spin a new patch series or can you handle
> it in the merge?

I will fix it myself before the push.

Thanks,

Daniel