[PATCH v2 0/4] 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 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 (4):
      t4xxx: 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 v1:

1:  66ae888b34 ! 1:  0e26fa0f07 t4xxx: don't use iconv(1) without ICONV prereq
    @@ Commit message
         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.
    +
         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.
     
    -    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.
    +    [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:  d5831c230e = 2:  ae4d582c53 t4205: improve handling of ICONV prerequisite
3:  a55d3b7ed4 = 3:  3c28d86b25 t5550: add ICONV prereq to tests that use "$HTTPD_URL/error"
4:  28306e7c1e = 4:  b1f05d7725 t6006: don't use iconv(1) without ICONV prereq

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