On Wed, 08 Dec 2021 09:11:07 +0100
"Thomas Schmitt" <scdbac...@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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
