All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tests: Refactor building xorriso command for iso9660 tests
@ 2021-12-08  6:40 Glenn Washburn
  2021-12-08  8:11 ` Thomas Schmitt
  0 siblings, 1 reply; 3+ messages in thread
From: Glenn Washburn @ 2021-12-08  6:40 UTC (permalink / raw)
  To: Daniel Kiper, grub-devel; +Cc: Thomas Schmitt, Glenn Washburn

The iso9660 tests test creating isos with different combinations of joliet,
rockridge, and iso9660 conformance level. Refactor xorriso argument
generation for more readability and extensibility.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-fs-tester.in | 43 +++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 2dcd09f5e..959d0694d 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1027,30 +1027,27 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		x"ziso9660")
 		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
 		    xorriso -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -R -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" -- -add /="$MASTER" -- -zisofs default -set_filter_r --zisofs / -- ;;
-		x"iso9660")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge off -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"joliet")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge off  -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -J -joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"rockridge")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge on -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"rockridge_joliet")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge on -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -J -joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"iso9660_1999")
+		x"iso9660" | xjoliet | xrockridge | xrockridge_joliet | x"iso9660_1999" | xjoliet_1999 | xrockridge_1999 | xrockridge_joliet_1999)
 		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge off -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 4 -graft-points -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"joliet_1999")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge off  -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 4 -graft-points -J -joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"rockridge_1999")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge on -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 4 -graft-points -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
-		x"rockridge_joliet_1999")
-		    FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
-		    xorriso --rockridge on -compliance rec_mtime -as mkisofs $XORRISOFS_CHARSET -iso-level 4 -graft-points -J -joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER"  ;;
+		    XORRISO_ARGS="-as mkisofs $XORRISOFS_CHARSET -graft-points"
+
+		    if [ -z "${fs##*rockridge*}" ]; then
+			XORRISO_ARGS="-rockridge on $XORRISO_ARGS"
+		    else
+			XORRISO_ARGS="-rockridge off $XORRISO_ARGS"
+		    fi
+
+		    if [ -z "${fs##*1999*}" ]; then
+			XORRISO_ARGS="$XORRISO_ARGS -iso-level 4"
+		    else
+			XORRISO_ARGS="$XORRISO_ARGS -iso-level 3"
+		    fi
+
+		    if [ -z "${fs##*joliet*}" ]; then
+			XORRISO_ARGS="$XORRISO_ARGS -J -joliet-long"
+		    fi
+
+		    xorriso -compliance rec_mtime $XORRISO_ARGS -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER" ;;
 		x"romfs")
 		    genromfs -V "$FSLABEL" -f "${FSIMAGEP}0.img" -d "$MASTER" ;;
 		xsquash4_*)
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tests: Refactor building xorriso command for iso9660 tests
  2021-12-08  6:40 [PATCH] tests: Refactor building xorriso command for iso9660 tests Glenn Washburn
@ 2021-12-08  8:11 ` Thomas Schmitt
  2021-12-10  8:43   ` Glenn Washburn
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Schmitt @ 2021-12-08  8:11 UTC (permalink / raw)
  To: grub-devel; +Cc: dkiper, development

Hi,

i think this change is beneficial for the maintainability of the test.

But this sequence looks a bit confusing, albeit it is ok on the second
glimpse:

+                   XORRISO_ARGS="-as mkisofs $XORRISOFS_CHARSET -graft-points"
+
+                   if [ -z "${fs##*rockridge*}" ]; then
+                       XORRISO_ARGS="-rockridge on $XORRISO_ARGS"
+                   else
+                       XORRISO_ARGS="-rockridge off $XORRISO_ARGS"
+                   fi
+
+                   if [ -z "${fs##*1999*}" ]; then
+                       XORRISO_ARGS="$XORRISO_ARGS -iso-level 4"
+                   else
+                       XORRISO_ARGS="$XORRISO_ARGS -iso-level 3"
+                   fi

It is essential here, but not really obvious, that the native command
-rockridge "on"|"off" must be _prepended_ to XORRISO_ARGS whereas the
mkisofs emulation options must be _appended_ to the variable content.

If not, then the xorriso run would fail:

  $ xorriso -as mkisofs -rockridge on
  ...
  xorriso : FAILURE : -as mkisofs: Unrecognized option '-rockridge'
  xorriso : aborting : -abort_on 'FAILURE' encountered 'FAILURE'

  $ xorriso -iso-level 4 -as mkisofs
  ...
  xorriso : FAILURE : Not a known command:  '-iso-level'

  xorriso : FAILURE : Not a known command:  '4'

  xorriso : aborting : -abort_on 'FAILURE' encountered 'FAILURE'
  $

It would be more intuitive to build XORRISO_ARGS in the sequence that
will be seen by xorriso.

So consider to pull -compliance "rec_mtime" into XORRISO_ARGS too, and
to set the native commands before the -as "mkisofs" command.
Like:

                   XORRISO_ARGS="-compliance rec_mtime"

                   if [ -z "${fs##*rockridge*}" ]; then
                       XORRISO_ARGS="$XORRISO_ARGS -rockridge on"
                   else
                       XORRISO_ARGS="$XORRISO_ARGS -rockridge off"
                   fi

                   XORRISO_ARGS="$XORRISO_ARGS -as mkisofs $XORRISOFS_CHARSET -graft-points"

                   if [ -z "${fs##*1999*}" ]; then
                       XORRISO_ARGS="$XORRISO_ARGS -iso-level 4"
                   else
                       XORRISO_ARGS="$XORRISO_ARGS -iso-level 3"
                   fi

                   if [ -z "${fs##*joliet*}" ]; then
                       XORRISO_ARGS="$XORRISO_ARGS -J -joliet-long"
                   fi

                   xorriso $XORRISO_ARGS -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER" ;;


Have a nice day :)

Thomas



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tests: Refactor building xorriso command for iso9660 tests
  2021-12-08  8:11 ` Thomas Schmitt
@ 2021-12-10  8:43   ` Glenn Washburn
  0 siblings, 0 replies; 3+ messages in thread
From: Glenn Washburn @ 2021-12-10  8:43 UTC (permalink / raw)
  To: Thomas Schmitt; +Cc: grub-devel, dkiper

On Wed, 08 Dec 2021 09:11:07 +0100
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> i think this change is beneficial for the maintainability of the test.
> 
> But this sequence looks a bit confusing, albeit it is ok on the second
> glimpse:
> 
> +                   XORRISO_ARGS="-as mkisofs $XORRISOFS_CHARSET -graft-points"
> +
> +                   if [ -z "${fs##*rockridge*}" ]; then
> +                       XORRISO_ARGS="-rockridge on $XORRISO_ARGS"
> +                   else
> +                       XORRISO_ARGS="-rockridge off $XORRISO_ARGS"
> +                   fi
> +
> +                   if [ -z "${fs##*1999*}" ]; then
> +                       XORRISO_ARGS="$XORRISO_ARGS -iso-level 4"
> +                   else
> +                       XORRISO_ARGS="$XORRISO_ARGS -iso-level 3"
> +                   fi
> 
> It is essential here, but not really obvious, that the native command
> -rockridge "on"|"off" must be _prepended_ to XORRISO_ARGS whereas the
> mkisofs emulation options must be _appended_ to the variable content.
> 
> If not, then the xorriso run would fail:
> 
>   $ xorriso -as mkisofs -rockridge on
>   ...
>   xorriso : FAILURE : -as mkisofs: Unrecognized option '-rockridge'
>   xorriso : aborting : -abort_on 'FAILURE' encountered 'FAILURE'
> 
>   $ xorriso -iso-level 4 -as mkisofs
>   ...
>   xorriso : FAILURE : Not a known command:  '-iso-level'
> 
>   xorriso : FAILURE : Not a known command:  '4'
> 
>   xorriso : aborting : -abort_on 'FAILURE' encountered 'FAILURE'
>   $
> 
> It would be more intuitive to build XORRISO_ARGS in the sequence that
> will be seen by xorriso.
> 
> So consider to pull -compliance "rec_mtime" into XORRISO_ARGS too, and
> to set the native commands before the -as "mkisofs" command.
> Like:
> 
>                    XORRISO_ARGS="-compliance rec_mtime"
> 
>                    if [ -z "${fs##*rockridge*}" ]; then
>                        XORRISO_ARGS="$XORRISO_ARGS -rockridge on"
>                    else
>                        XORRISO_ARGS="$XORRISO_ARGS -rockridge off"
>                    fi
> 
>                    XORRISO_ARGS="$XORRISO_ARGS -as mkisofs $XORRISOFS_CHARSET -graft-points"
> 
>                    if [ -z "${fs##*1999*}" ]; then
>                        XORRISO_ARGS="$XORRISO_ARGS -iso-level 4"
>                    else
>                        XORRISO_ARGS="$XORRISO_ARGS -iso-level 3"
>                    fi
> 
>                    if [ -z "${fs##*joliet*}" ]; then
>                        XORRISO_ARGS="$XORRISO_ARGS -J -joliet-long"
>                    fi
> 
>                    xorriso $XORRISO_ARGS -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" /="$MASTER" ;;
> 
> 
> Have a nice day :)
> 
> Thomas
> 

Thanks for the suggestion Thomas. I've updated the patch to add this.

Glenn


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-12-10  8:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08  6:40 [PATCH] tests: Refactor building xorriso command for iso9660 tests Glenn Washburn
2021-12-08  8:11 ` Thomas Schmitt
2021-12-10  8:43   ` Glenn Washburn

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.