Subject: Re: [PATCH v2 02/10] t/test-lib: wire up NO_ICONV prerequisite

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 17 Oct 2024 06:03:37 +0200

On Wed, Oct 16, 2024 at 05:02:59PM -0400, Taylor Blau wrote:
> On Wed, Oct 16, 2024 at 04:59:26PM -0400, Taylor Blau wrote:
> > On Wed, Oct 16, 2024 at 05:57:10AM +0200, Patrick Steinhardt wrote:
> > > On Tue, Oct 15, 2024 at 07:53:12PM -0400, Taylor Blau wrote:
> > > > On Tue, Oct 15, 2024 at 01:45:11PM +0200, Patrick Steinhardt wrote:
> > > > > Further note that there are several "!MINGW" conditions in t4201, and
> > > > > all of these fail due to iconv-related errors. This is quite likely a
> > > > > leftover from times before dce7d29551 (msvc: support building Git using
> > > > > MS Visual C++, 2019-06-25), which switched Windows-based builds over
> > > > > from "NO_ICONV=YesPlease" to "NEEDS_LIBICONV=YesPlease". Consequently,
> > > > > adapt those tests to also use the new ICONV prerequisite.
> > > >
> > > > This appears to break CI on Windows when I merged this into 'jch':
> > > >
> > > >     https://github.com/ttaylorr/git/actions/runs/11355564982/job/31585450667
> > > >
> > > > I'm going to temporarily eject this from 'jch' and 'seen' until we can
> > > > properly deal with this.
> > >
> > > Ugh, I'm looking forward to the Windows jobs for GitLab CI being merged
> > > down to next so that I can finally see such regressions before they hit
> > > our trees. Anyway, thanks for the heads up, will have a look.
> >
> > It's OK. Ejecting a topic out of 'seen' is relatively easy as it
> > requires the following (after removing the line out of Meta/redo-seen.sh):
> >
> >     git checkout -B seen jch
> >     sh Meta/redo-seen.sh
> >
> > I was mostly confused why my build of 'seen' passed 'make test' locally,
> > but failed CI when pushed to GitHub. Of course, I'm not testing on a
> > Windows machine, and you didn't have easy access to Windows CI runs on
> > GitLab (for now), so the result makes sense.
> 
> Speaking of... I have 'ps/ci-gitlab-windows' tagged for 'next' in the
> next integration round. There is some duplication of patches between
> that topic and this one (as well as ps/build).
> 
> How do you want me to handle the dependency?

I'll soon reroll ps/build to depend on all the other series, and will
evict the duplicate patches from there. I'm happy to send another
version of this series here that depends on ps/ci-gitlab-windows, but
wouldn't mind either if the patch landed via both series.

Patrick