t7422: fix extra printf argument, eliminate loops

Commit Message

Subhaditya Nath April 9, 2025, 7:11 p.m. UTC

The POSIX man page of printf(1) mentions -
> If the format operand contains no conversion specifications and
> argument operands are present, the results are unspecified.

In practice, this means some printf implementations throw an error
when provided with extra operands, thereby causing the test to fail
erroneously. This commit fixes that issue.

This commit also eliminates the for-loops surrounding said printf
statements in favour of the built-in functionality of printf to consume
all arguments by reusing the format operand as-often-as-necessary.

This behaviour is mentioned in the POSIX man page of printf(1) under the
section titled "EXTENDED DESCRIPTION" like so -

    8. For each conversion specification that consumes an argument, the
       next argument operand shall be evaluated and converted to the
       appropriate type for the conversion as specified below.

    9. The format operand shall be reused as often as necessary to
       satisfy the argument operands.  [...]

Signed-off-by: Subhaditya Nath <[e-mail]>
---
 t/t7422-submodule-output.sh | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Junio C Hamano April 10, 2025, 12:39 p.m. UTC | #1

Subhaditya Nath <[e-mail]> 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.

Patch

diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh
index 023a5cbdc4..94a14f1c31 100755
--- a/t/t7422-submodule-output.sh
+++ b/t/t7422-submodule-output.sh
@@ -178,19 +178,13 @@  test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE'
 		test_commit initial &&
 
 		COMMIT=$(git rev-parse HEAD) &&
-		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") &&