Re: [PATCH] t7422: fix extra printf argument, eliminate loops

[Junio C Hamano <gitster@pobox.com>]

Subhaditya Nath <sn03.general@gmail.com> writes:

> -		for i in $(test_seq 2000)
> -		do
> -			printf "[submodule \"sm-$i\"]\npath = recursive-submodule-path-$i\n" "$i" ||
> -			return 1
> -		done >gitmodules &&
> +		printf "[submodule \"sm-%d\"]\npath = recursive-submodule-path-%d\n" \
> +			$(test_seq 2000 | sed p) >gitmodules &&
>  		BLOB=$(git hash-object -w --stdin <gitmodules) &&
>  
>  		printf "100644 blob $BLOB\t.gitmodules\n" >tree &&
> -		for i in $(test_seq 2000)
> -		do
> -			printf "160000 commit $COMMIT\trecursive-submodule-path-%d\n" "$i" ||
> -			return 1
> -		done >>tree &&
> +		printf "160000 commit $COMMIT\trecursive-submodule-path-%d\n" \
> +			$(test_seq 2000) >>tree &&
>  		TREE=$(git mktree <tree) &&
>  
>  		COMMIT=$(git commit-tree "$TREE") &&

Other than the cuteness value (in other words, "by rewriting this
way, I can use this shiny fun feature `printf` has that I just
learned about"), I do not see in what way(s) the updated code is
better than what Eric picked as "most natural" among the four
candidates you presented earlier.

If I am not mistaken, the `printf` utility tends to be implemented
as a built-in in modern shells, so it is not like the above rewrite
replaced 2000 fork+exec with a single fork+exec of /usr/bin/printf,
so for those shells, there is no performance based argument to
prefer it.  And with shells that do have to fork+exec
/usr/bin/printf, the command line to invoke it once now uses about
18k bytes with the current code that uses 2-thousand submodules.

When somebody wants to extend the test to try with more submodules,
at some point they need to start worring about hitting argv[] limit
of the userspace-kernel interface, and at that point, it is likely
that they have to go back to a for loop, doing something like

	i=0
	while test "$i" -le 200000
	do
		printf ... "$i" "$i"
		i=$((i+1))
	done

Sorry for not spelling "I would not recommend going in that
direction" in all caps in red letters in my earlier message.