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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2025, 12:39 UTC
Message-ID: <xmqqcydkp4vp.fsf@gitster.g>
In-Reply-To: <20250409191139.29644-2-sn03.general@gmail.com>

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

Show 20 quoted lines

> -		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.