[PATCH v3 0/5] Fix tests with missing iconv(1) executable

Hi,

I recently noticed that the MSVC-based tests in GitLab CI started to fail. The root cause is that the iconv(1) executable cannot be found on this platform anymore. This isn't entirely surprising: we depend on the Git for Windows environment to provide necessary shell tools, and that environment of course is not a fully fledged MSYS2 installation.

In any case, this patch series fixes those issues by building on top of the ICONV prerequisite. If the prereq isn't found, then we also don't assume that the iconv(1) executable exists.

An alternative strategy would be to introduce a new ICONV_EXECUTABLE prereq. But given that Git doesn't perform any kind of reencoding itself in case the ICONV support isn't built into it I found it to not be worth the additional hassle.

In any case, this patch series causes the MSVC jobs to pass again on GitLab CI.

Changes in v3:
  - Split out the changes for the ICONV prerequisite into a standalone
    commit.
  - Link to v2: https://lore.kernel.org/r/20260217-b4-pks-ci-msvc-iconv-fixes-v2-0-25491bc8dbf8@pks.im

Changes in v2:
  - Extend the ICONV prerequisite to cover the new semantics and add a
    NEEDSWORK comment.
  - Mention the upstream issue in Git for Windows.
  - Link to v1: https://lore.kernel.org/r/20260209-b4-pks-ci-msvc-iconv-fixes-v1-0-1e3167cd8828@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (5):
      t: don't set ICONV prereq when iconv(1) is missing
      t40xx: don't use iconv(1) without ICONV prereq
      t4205: improve handling of ICONV prerequisite
      t5550: add ICONV prereq to tests that use "$HTTPD_URL/error"
      t6006: don't use iconv(1) without ICONV prereq

 t/t4041-diff-submodule-option.sh             |  8 +++--
 t/t4059-diff-submodule-not-initialized.sh    |  8 +++--
 t/t4060-diff-submodule-option-diff-format.sh |  8 +++--
 t/t4205-log-pretty-formats.sh                | 50 ++++++++++++++++------------
 t/t5550-http-fetch-dumb.sh                   | 20 +++++------
 t/t6006-rev-list-format.sh                   | 29 +++++++++++-----
 t/test-lib.sh                                | 12 ++++++-
 7 files changed, 88 insertions(+), 47 deletions(-)

Range-diff versus v2:

-:  ---------- > 1:  74b9ab2ca1 t: don't set ICONV prereq when iconv(1) is missing
1:  50bcd30389 ! 2:  f0461ca7cb t4xxx: don't use iconv(1) without ICONV prereq
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    t4xxx: don't use iconv(1) without ICONV prereq
    +    t40xx: don't use iconv(1) without ICONV prereq
     
    -    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.
    +    We've got a couple of tests related to diffs in t40xx that 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.
    -
    -    In fact, Git for Windows has recently (unintentionally) shipped a change
    -    where the iconv(1) binary is not getting installed anymore [1]. And as
    -    we use Git for Windows directly in MSVC+Meson jobs in GitLab CI this has
    -    exposed the issue. The missing iconv(1) binary is considered a bug that
    -    will be fixed in Git for Windows, but regardless of that it makes sense
    -    to not assume the binary to always exist.
    +    executable, even though it's not 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.
    -    Extend the ICONV prerequisite to cover these new semantics so that we
    -    know to skip tests in case the iconv(1) binary doesn't exist.
    -
    -    [1]: https://github.com/git-for-windows/git/issues/6083
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ t/t4060-diff-submodule-option-diff-format.sh: add_file () {
      		done >/dev/null &&
      		git rev-parse --short --verify HEAD
      	)
    -
    - ## t/test-lib.sh ##
    -@@ t/test-lib.sh: esac
    - ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
    - test -z "$NO_CURL" && test_set_prereq LIBCURL
    - test -z "$NO_GITWEB" && test_set_prereq GITWEB
    --test -z "$NO_ICONV" && test_set_prereq ICONV
    - test -z "$NO_PERL" && test_set_prereq PERL
    - test -z "$NO_PTHREADS" && test_set_prereq PTHREADS
    - test -z "$NO_PYTHON" && test_set_prereq PYTHON
    -@@ t/test-lib.sh: test -n "$SANITIZE_LEAK" && test_set_prereq SANITIZE_LEAK
    - test -n "$GIT_VALGRIND_ENABLED" && test_set_prereq VALGRIND
    - test -n "$PERL_PATH" && test_set_prereq PERL_TEST_HELPERS
    - 
    -+test_lazy_prereq ICONV '
    -+	# We require Git to be built with iconv support, and we require the
    -+	# iconv binary to exist.
    -+	#
    -+	# NEEDSWORK: We might eventually want to split this up into two
    -+	# prerequisites: one for NO_ICONV, and one for the iconv(1) binary, as
    -+	# some tests only depend on either of these.
    -+	test -z "$NO_ICONV" &&
    -+	iconv -f utf8 -t utf8 </dev/null
    -+'
    -+
    - if test -z "$GIT_TEST_CHECK_CACHE_TREE"
    - then
    - 	GIT_TEST_CHECK_CACHE_TREE=true
2:  5b4e3270bc = 3:  cdbc1ca807 t4205: improve handling of ICONV prerequisite
3:  60dfbaaf17 = 4:  3c66466ba4 t5550: add ICONV prereq to tests that use "$HTTPD_URL/error"
4:  b29618e071 = 5:  0306f9d1f8 t6006: don't use iconv(1) without ICONV prereq

--- base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc change-id: 20260209-b4-pks-ci-msvc-iconv-fixes-13de4801643f