Re: [LTP] [PATCH v2] isofs.sh:Run test on genisoimage, xorriso &mksisofs if not symlinks

[Petr Vorel <pvorel@suse.cz>]

Hi Subramanya,

>     1)mkisofs, genisoimage and xorriso tools are present as separate
>     tools in some distros while in others they are symlinks to one
>     another. Tests are skipped on symlinks.

>     2)mkisofs supports only -hfs option
>       genisoimage supports both -hfs & -hfsplus options
>       xorrisofs supports only -hfsplus option
=> this change actually fix xorrisofs on CentOS, see below.

First, we are just to release LTP version today and we would like to avoid last
minute changes. Looking to the original version, the attempt was to fix
xorrisofs on CentOS which is a symlink to mkisofs and because it does not
supports -hfs test IMHO fails. So I would be for merging this (with a diff, thus
I repost as v3), but depends on Cyril.

[1] https://lore.kernel.org/ltp/20240115155936.3235-1-subramanya.swamy.linux@gmail.com/

> Co-Authored-By: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Subramanya Swamy <subramanya.swamy.linux@gmail.com>
> ---
>  testcases/kernel/fs/iso9660/isofs.sh | 41 ++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 9 deletions(-)

> diff --git a/testcases/kernel/fs/iso9660/isofs.sh b/testcases/kernel/fs/iso9660/isofs.sh
> index dfa4ac73d..9aa853e67 100755
> --- a/testcases/kernel/fs/iso9660/isofs.sh
> +++ b/testcases/kernel/fs/iso9660/isofs.sh
> @@ -13,18 +13,16 @@ TST_NEEDS_CMDS="mount umount"
>  TST_NEEDS_TMPDIR=1
>  TST_SETUP=setup
>  TST_TESTFUNC=do_test
> +TST_CNT=3

>  MAX_DEPTH=3
>  MAX_DIRS=4

>  setup()
>  {
> -	if tst_cmd_available mkisofs; then
> -		MKISOFS_CMD="mkisofs"
> -	elif tst_cmd_available genisoimage; then
> -		MKISOFS_CMD="genisoimage"
> -	else
> -		tst_brk TCONF "please install mkisofs or genisoimage"
> +	if !(tst_cmd_available mkisofs \
> +		|| tst_cmd_available genisoimage || tst_cmd_available xorrisofs);then
> +			tst_brk TCONF "please install mkisofs / genisoimage / xorriso"
IMHO this is not needed, we check for command in the test itself.
>  	fi
>  }

> @@ -46,6 +44,29 @@ gen_fs_tree()

>  do_test()
>  {
> +        case $1 in
> +        1) MKISOFS_CMD="mkisofs"
> +	   HFSOPT="-hfs -D"
> +	   GREPOPT="mkisofs";;
> +        2) MKISOFS_CMD="genisoimage"
> +	   HFSOPT="-hfsplus -D -hfs -D"
> +	   GREPOPT="genisoimage";;
> +        3) MKISOFS_CMD="xorrisofs"
> +	   HFSOPT="-hfsplus -D"
> +	   GREPOPT="xorriso";;
This is clearly broken formatting (test uses tabs, but you add spaces, we care
about it as it would break readability).
> +        esac
> +
> +
> +        if ! tst_cmd_available $MKISOFS_CMD; then
> +                tst_res TCONF "Missing '$MKISOFS_CMD'"
> +                return
> +        fi
> +
> +        if ! $MKISOFS_CMD 2>&1 | head -n 2 |grep -q "$GREPOPT"; then
> +                tst_res TCONF "'$MKISOFS_CMD' is a symlink to another tool"
> +                return
> +        fi
> +
>  	local mnt_point="$PWD/mnt"
>  	local make_file_sys_dir="$PWD/files"
>  	local mkisofs_opt mount_opt
I prefer local to be at the beginning of the function.

> @@ -62,14 +83,16 @@ do_test()
>  	for mkisofs_opt in \
>  		" " \
>  		"-J" \
> -		"-hfs -D" \
> +		${HFSOPT} \
 		"$HFSOPT" \

{ } brackets are not needed, but quotes are.
Missing quotes actually causes -hfs -D being split (run separately)

Kind regards,
Petr

>  		" -R " \
>  		"-R -J" \
>  		"-f -l -D -J -allow-leading-dots -R" \
> -		"-allow-lowercase -allow-multidot -iso-level 3 -f -l -D -J -allow-leading-dots -R"
> +		"-allow-lowercase -allow-multidot -iso-level 3 -f -l -D -J \
> +			-allow-leading-dots -R"
>  	do
>  		rm -f isofs.iso
> -		EXPECT_PASS $MKISOFS_CMD -o isofs.iso -quiet $mkisofs_opt $make_file_sys_dir 2\> /dev/null \
> +		EXPECT_PASS $MKISOFS_CMD -o isofs.iso \
> +			-quiet $mkisofs_opt $make_file_sys_dir 2\> /dev/null \
>  			|| continue

>  		for mount_opt in \

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp