Re: [PATCH] tests: Refactor building xorriso command for iso9660 tests

[Glenn Washburn]

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