Re: [PATCH v2 1/4] t4xxx: don't use iconv(1) without ICONV prereq

From: Christian Couder <christian.couder@gmail.com>
Date: Feb 17, 2026, 14:48 UTC
Message-ID: <CAP8UFD23MdTF3qVFhDFBDcnqh4dqiehvFz_3c-keMhSOa92Dpw@mail.gmail.com>
In-Reply-To: <20260217-b4-pks-ci-msvc-iconv-fixes-v2-1-25491bc8dbf8@pks.im>

On Tue, Feb 17, 2026 at 2:58 PM Patrick Steinhardt <ps@pks.im> wrote:

Show 8 quoted lines

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

s/not a safe/not safe/

Show 13 quoted lines

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

Nit: when reading this, it's not clear if this commit is enough to fix
all the MSVC+Meson jobs in GitLab CI or only those related to the
t4xxx tests.

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

[...]

Show 10 quoted lines

> +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
> +'

Yeah, I think it works to actually test if iconv works and to document the small discrepancy between the ICONV prereq and NO_ICONV here.