Re: [PATCH 1/4] t4xxx: don't use iconv(1) without ICONV prereq
From: Junio C Hamano <hidden>
Date: 2026-02-09 17:55:13
Patrick Steinhardt [off-list ref] writes:
We've got a couple of tests that all use the iconv(1) executable to convert the encoding of a commit message. All of these tests are prepared to handle a missing ICONV prereq, in which case they will simply use UTF-8 encoding. But even if the ICONV prerequisite has failed we try to use the iconv(1) executable. But it's not a safe to assume that the executable exists in that case. And besides that, it's also unnecessary to use iconv(1) in the first place, as we would only use it to convert from UTF-8 to UTF-8, which should be equivalent to a no-op. Fix the issue and skip the call to iconv(1) in case the prerequisite is not set. This makes tests work on systems that don't have iconv at all. Note that arguably, it's even unsafe to assume that the iconv(1) executable exists only because Git has been built with support for it. A more wholistic approach would thus be to split up the ICONV prereq into two prereqs: one that tells us whether Git has been built with ICONV support, and one that tells us whether the iconv(1) executable exists. But that would lead to a bunch of changes throughout our tests, and for arguably negligible benefit. Signed-off-by: Patrick Steinhardt <redacted> --- t/t4041-diff-submodule-option.sh | 8 ++++++-- t/t4059-diff-submodule-not-initialized.sh | 8 ++++++-- t/t4060-diff-submodule-option-diff-format.sh | 8 ++++++-- 3 files changed, 18 insertions(+), 6 deletions(-)
The repetition across three files look a bit disturbing X-<.
quoted hunk
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index 4d4aa1650f..4dd4954260 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh@@ -37,8 +37,12 @@ add_file () { test_tick && # "git commit -m" would break MinGW, as Windows refuse to pass # $test_encoding encoded parameter to git. - echo "Add $name ($added $name)" | iconv -f utf-8 -t $test_encoding | - git -c "i18n.commitEncoding=$test_encoding" commit -F - + message="Add $name ($added $name)" && + if test_have_prereq ICONV + then + message=$(echo "$message" | iconv -f utf-8 -t $test_encoding) + fi && + echo "$message" | git -c "i18n.commitEncoding=$test_encoding" commit -F -
This was a bit unexpected. Do we give any guarantee to builds that lack iconv support that "git -c i18n.commitEncoding=... commit" will pass the payload verbatim? I would have expected ICONV prerequisite is used on the whole test_expect_success to exclude the tests that are affected, not at such a low level.