win+Meson test failures due to ps/build-meson-fixes-0130 topic?

win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Junio C Hamano]

As of yesterday, 'next' started failing all the "win+Meson test"
jobs.

https://github.com/git/git/actions/runs/13506034920

I had to do one of my least favourite activities working on Git:
bisecting CI breakage on a platform I have no idea how well it
works.

Here is the full "stack" of changes, it contains all the topics
merged between 'master' and 'next', plus a single change that
butchers .github/workflows/main.yml to leave only three jobs
(ci-config, windows-meson-build, and windows-meson-test).

    3a7e83b527 Merge branch 'ps/build-meson-fixes-0130'
    e59193e984 Merge branch 'ek/mingw-rename-symlink'
    a209cf1427 Merge branch 'po/meson-perl-fix'
    9f1e73677c Merge branch 'ua/os-version-capability'
    82f01cb375 Merge branch 'ms/rename-match-name-with-pattern'
    8b04381049 Merge branch 'ad/set-default-target-in-makefiles'
    2cab1bd7d0 Merge branch 'pw/merge-tree-stdin-deadlock-fix'
    92b43de9df Merge branch 'mh/doc-commit-title-not-subject'
    685c150623 Merge branch 'bc/diff-reject-empty-arg-to-pickaxe'
    c5093fc2d8 Merge branch 'tb/new-make-fix'
    9091607e95 Merge branch 'da/xdiff-w-sign-compare-workaround'
    01e86410fe GitHub CI: trim and slim for win+Meson test breakage hunting
    2d2a71ce85 The thirteenth batch

At 01e86410fe and e59193e984, CI seems to pass.

https://github.com/git/git/actions/runs/13528755524

With ps/build-meson-fixes-0130 at 3a7e83b527, all the "win+Meson
test" jobs fail.  I suspect that the series does not break
individual tests but the breakage is in somewhere more
framework-ish?  I didn't look into individual test job failures.

https://github.com/git/git/actions/runs/13529323827

I'll also note that in one of my earlier attempts (which had
ek/mingw-rename-symlink merged before po/meson-perl-fix), one of the
10 "win+Meson test" jobs failed ("win+Meson test (7)" to be exact).
Rerunning the failed test succeeds, so there must be something flaky
in t0610-reftable-basics test that is exposed by this change,
perhaps?

  https://github.com/git/git/actions/runs/13528071468/attempts/1
  https://github.com/git/git/actions/runs/13528071468/attempts/2

As the ps/build-meson-fixes-0130 topic has 13 changes, and each
iteration in the above exercise took somewhere around 20 minutes, I
won't dive into the topic myself to identify which one of these
individual topics causes the breakage.  This however gave me enough
to go on to today's 'master' integration (mainly, by mark the topic
to be held in 'next' until this gets resolved, and deal with the
other topics).

Thanks.

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Patrick Steinhardt]

On Tue, Feb 25, 2025 at 11:45:47AM -0800, Junio C Hamano wrote:
> As of yesterday, 'next' started failing all the "win+Meson test"
> jobs.
> 
> https://github.com/git/git/actions/runs/13506034920
> 
> I had to do one of my least favourite activities working on Git:
> bisecting CI breakage on a platform I have no idea how well it
> works.
> 
> Here is the full "stack" of changes, it contains all the topics
> merged between 'master' and 'next', plus a single change that
> butchers .github/workflows/main.yml to leave only three jobs
> (ci-config, windows-meson-build, and windows-meson-test).
> 
>     3a7e83b527 Merge branch 'ps/build-meson-fixes-0130'
>     e59193e984 Merge branch 'ek/mingw-rename-symlink'
>     a209cf1427 Merge branch 'po/meson-perl-fix'
>     9f1e73677c Merge branch 'ua/os-version-capability'
>     82f01cb375 Merge branch 'ms/rename-match-name-with-pattern'
>     8b04381049 Merge branch 'ad/set-default-target-in-makefiles'
>     2cab1bd7d0 Merge branch 'pw/merge-tree-stdin-deadlock-fix'
>     92b43de9df Merge branch 'mh/doc-commit-title-not-subject'
>     685c150623 Merge branch 'bc/diff-reject-empty-arg-to-pickaxe'
>     c5093fc2d8 Merge branch 'tb/new-make-fix'
>     9091607e95 Merge branch 'da/xdiff-w-sign-compare-workaround'
>     01e86410fe GitHub CI: trim and slim for win+Meson test breakage hunting
>     2d2a71ce85 The thirteenth batch
> 
> At 01e86410fe and e59193e984, CI seems to pass.
> 
> https://github.com/git/git/actions/runs/13528755524
> 
> With ps/build-meson-fixes-0130 at 3a7e83b527, all the "win+Meson
> test" jobs fail.  I suspect that the series does not break
> individual tests but the breakage is in somewhere more
> framework-ish?  I didn't look into individual test job failures.
> 
> https://github.com/git/git/actions/runs/13529323827
> 
> I'll also note that in one of my earlier attempts (which had
> ek/mingw-rename-symlink merged before po/meson-perl-fix), one of the
> 10 "win+Meson test" jobs failed ("win+Meson test (7)" to be exact).
> Rerunning the failed test succeeds, so there must be something flaky
> in t0610-reftable-basics test that is exposed by this change,
> perhaps?
> 
>   https://github.com/git/git/actions/runs/13528071468/attempts/1
>   https://github.com/git/git/actions/runs/13528071468/attempts/2
> 
> As the ps/build-meson-fixes-0130 topic has 13 changes, and each
> iteration in the above exercise took somewhere around 20 minutes, I
> won't dive into the topic myself to identify which one of these
> individual topics causes the breakage.  This however gave me enough
> to go on to today's 'master' integration (mainly, by mark the topic
> to be held in 'next' until this gets resolved, and deal with the
> other topics).

Hm, interesting. I didn't notice those issues in our CI. Thanks for
diving into it, I'll figure out what has happened here.

Patrick

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Jeff King]

On Wed, Feb 26, 2025 at 06:45:06AM +0100, Patrick Steinhardt wrote:

> > I'll also note that in one of my earlier attempts (which had
> > ek/mingw-rename-symlink merged before po/meson-perl-fix), one of the
> > 10 "win+Meson test" jobs failed ("win+Meson test (7)" to be exact).
> > Rerunning the failed test succeeds, so there must be something flaky
> > in t0610-reftable-basics test that is exposed by this change,
> > perhaps?
> > 
> >   https://github.com/git/git/actions/runs/13528071468/attempts/1
> >   https://github.com/git/git/actions/runs/13528071468/attempts/2
> > 
> > As the ps/build-meson-fixes-0130 topic has 13 changes, and each
> > iteration in the above exercise took somewhere around 20 minutes, I
> > won't dive into the topic myself to identify which one of these
> > individual topics causes the breakage.  This however gave me enough
> > to go on to today's 'master' integration (mainly, by mark the topic
> > to be held in 'next' until this gets resolved, and deal with the
> > other topics).
> 
> Hm, interesting. I didn't notice those issues in our CI. Thanks for
> diving into it, I'll figure out what has happened here.

I spent a little time poking at this yesterday. Here's what I found in
case it helps.

I arbitrarily picked t0210 to poke at because it's short and it was
failing. Interestingly, the failing command was clone, with the client
complaining that it did not understand the server's protocol version
response.

Running with GIT_TRACE_PACKET shows that we got a line:

  version 2\r\n

And the extra "\r" confuses the parser. So perhaps something in the
build changed that is affecting how CRLF is output by standard
functions? That would explain why we see failures all over the test
suite. I didn't dig into any other cases, though.

-Peff

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Patrick Steinhardt]

On Wed, Feb 26, 2025 at 06:45:06AM +0100, Patrick Steinhardt wrote:
> On Tue, Feb 25, 2025 at 11:45:47AM -0800, Junio C Hamano wrote:
> > As of yesterday, 'next' started failing all the "win+Meson test"
> > jobs.
> > 
> > https://github.com/git/git/actions/runs/13506034920
> > 
> > I had to do one of my least favourite activities working on Git:
> > bisecting CI breakage on a platform I have no idea how well it
> > works.
> > 
> > Here is the full "stack" of changes, it contains all the topics
> > merged between 'master' and 'next', plus a single change that
> > butchers .github/workflows/main.yml to leave only three jobs
> > (ci-config, windows-meson-build, and windows-meson-test).
> > 
> >     3a7e83b527 Merge branch 'ps/build-meson-fixes-0130'
> >     e59193e984 Merge branch 'ek/mingw-rename-symlink'
> >     a209cf1427 Merge branch 'po/meson-perl-fix'
> >     9f1e73677c Merge branch 'ua/os-version-capability'
> >     82f01cb375 Merge branch 'ms/rename-match-name-with-pattern'
> >     8b04381049 Merge branch 'ad/set-default-target-in-makefiles'
> >     2cab1bd7d0 Merge branch 'pw/merge-tree-stdin-deadlock-fix'
> >     92b43de9df Merge branch 'mh/doc-commit-title-not-subject'
> >     685c150623 Merge branch 'bc/diff-reject-empty-arg-to-pickaxe'
> >     c5093fc2d8 Merge branch 'tb/new-make-fix'
> >     9091607e95 Merge branch 'da/xdiff-w-sign-compare-workaround'
> >     01e86410fe GitHub CI: trim and slim for win+Meson test breakage hunting
> >     2d2a71ce85 The thirteenth batch
> > 
> > At 01e86410fe and e59193e984, CI seems to pass.
> > 
> > https://github.com/git/git/actions/runs/13528755524
> > 
> > With ps/build-meson-fixes-0130 at 3a7e83b527, all the "win+Meson
> > test" jobs fail.  I suspect that the series does not break
> > individual tests but the breakage is in somewhere more
> > framework-ish?  I didn't look into individual test job failures.
> > 
> > https://github.com/git/git/actions/runs/13529323827
> > 
> > I'll also note that in one of my earlier attempts (which had
> > ek/mingw-rename-symlink merged before po/meson-perl-fix), one of the
> > 10 "win+Meson test" jobs failed ("win+Meson test (7)" to be exact).
> > Rerunning the failed test succeeds, so there must be something flaky
> > in t0610-reftable-basics test that is exposed by this change,
> > perhaps?
> > 
> >   https://github.com/git/git/actions/runs/13528071468/attempts/1
> >   https://github.com/git/git/actions/runs/13528071468/attempts/2
> > 
> > As the ps/build-meson-fixes-0130 topic has 13 changes, and each
> > iteration in the above exercise took somewhere around 20 minutes, I
> > won't dive into the topic myself to identify which one of these
> > individual topics causes the breakage.  This however gave me enough
> > to go on to today's 'master' integration (mainly, by mark the topic
> > to be held in 'next' until this gets resolved, and deal with the
> > other topics).
> 
> Hm, interesting. I didn't notice those issues in our CI. Thanks for
> diving into it, I'll figure out what has happened here.

Okay, the issue was in 163a3e97ac1 (meson: simplify use of the
common-main library, 2025-01-30). In that commit we stopped using the
common-main dependency for a couple of binaries, including
git-remote-https and others. What I didn't realize though is that this
also made us not use the linker arguments anymore, but those are
required on Windows when linking an executable.

I've kicked off CI jobs on GitHub to verify that the revised version of
this patch series works alright. Once those tests pass I'll send another
version of this patch series.

Patrick

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Jeff King]

On Wed, Feb 26, 2025 at 08:35:01AM +0100, Patrick Steinhardt wrote:

> > Hm, interesting. I didn't notice those issues in our CI. Thanks for
> > diving into it, I'll figure out what has happened here.
> 
> Okay, the issue was in 163a3e97ac1 (meson: simplify use of the
> common-main library, 2025-01-30). In that commit we stopped using the
> common-main dependency for a couple of binaries, including
> git-remote-https and others. What I didn't realize though is that this
> also made us not use the linker arguments anymore, but those are
> required on Windows when linking an executable.

Ah, thanks, I was just closing in on that commit, too, by reverting
backwards from the tip of the series

There's one other oddity I found that you might be interested in. After
each revert doing "meson test ..." mostly worked. But when I reverted
bf4a16b84a (meson: drop separate version library, 2025-01-30), the build
then fails with:

  $ GIT_TRACE_PACKET=1 meson test -C build t0210-* --interactive --test-args=-vi
  ...
  [305/571] Linking static target libgit.a
  FAILED: libgit.a
  "gcc-ar" "csrDT" libgit.a @libgit.a.rsp
  C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ar.exe: libgit.a: No such file or directory

I'd imagine that if I did a complete "rm -rf build && meson setup build"
it would work. But it's pretty lousy to have to do that preemptively
during a bisection.

I didn't dig into the actual source of the problem in that commit, so I
don't know if there's a way to write it that is happy working with both
the before and after states.

-Peff

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Jeff King]

On Wed, Feb 26, 2025 at 02:46:53AM -0500, Jeff King wrote:

> I'd imagine that if I did a complete "rm -rf build && meson setup build"
> it would work. But it's pretty lousy to have to do that preemptively
> during a bisection.

Looks like this runs even deeper. On my Linux machine:

  $ git checkout origin/next
  $ meson setup build
  [...ok...]
  $ meson compile -C build
  [...ok...]

  $ git checkout origin/master
  $ meson compile -C build
  [...]
  ../meson.build:215:35: ERROR: The `!=` operator of array does not accept objects of type str ()
  FAILED: build.ninja
  /usr/bin/meson --internal regenerate /home/peff/compile/git .
  ninja: error: rebuilding 'build.ninja': subcommand failed

  $ rm -rf build
  $ meson setup build
  $ meson compile -C build
  [...ok again...]

I'm hoping you can tell me I'm holding it wrong, and there's some way to
do an incremental build when crossing these sorts of boundaries (without
blowing away all of the build products).

I do at least use ccache which makes the "rm -rf" case a little less
painful.

-Peff

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Patrick Steinhardt]

On Wed, Feb 26, 2025 at 03:20:02AM -0500, Jeff King wrote:
> On Wed, Feb 26, 2025 at 02:46:53AM -0500, Jeff King wrote:
> 
> > I'd imagine that if I did a complete "rm -rf build && meson setup build"
> > it would work. But it's pretty lousy to have to do that preemptively
> > during a bisection.
> 
> Looks like this runs even deeper. On my Linux machine:
> 
>   $ git checkout origin/next
>   $ meson setup build
>   [...ok...]
>   $ meson compile -C build
>   [...ok...]
> 
>   $ git checkout origin/master
>   $ meson compile -C build
>   [...]
>   ../meson.build:215:35: ERROR: The `!=` operator of array does not accept objects of type str ()
>   FAILED: build.ninja
>   /usr/bin/meson --internal regenerate /home/peff/compile/git .
>   ninja: error: rebuilding 'build.ninja': subcommand failed
> 
>   $ rm -rf build
>   $ meson setup build
>   $ meson compile -C build
>   [...ok again...]
> 
> I'm hoping you can tell me I'm holding it wrong, and there's some way to
> do an incremental build when crossing these sorts of boundaries (without
> blowing away all of the build products).
> 
> I do at least use ccache which makes the "rm -rf" case a little less
> painful.

You're not holding it wrong. The problem is that this patch series
introduces a backwards-incompatible change to the "sane_tools_path"
option as it changes it from a string to an array, which makes Meson
hiccup. Once Meson is not considered to be experimental anymore we
should refrain from changing option types going forward, but until then
I consider it to be okayish to break things like this.

From my point of view we can treat Meson as stable as soon as Git v2.49
is out. So from thereon, I will make sure to not include backwards
incompatible changes anymore.

Patrick

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Jeff King]

On Wed, Feb 26, 2025 at 10:15:50AM +0100, Patrick Steinhardt wrote:

> > I'm hoping you can tell me I'm holding it wrong, and there's some way to
> > do an incremental build when crossing these sorts of boundaries (without
> > blowing away all of the build products).
> > 
> > I do at least use ccache which makes the "rm -rf" case a little less
> > painful.
> 
> You're not holding it wrong. The problem is that this patch series
> introduces a backwards-incompatible change to the "sane_tools_path"
> option as it changes it from a string to an array, which makes Meson
> hiccup. Once Meson is not considered to be experimental anymore we
> should refrain from changing option types going forward, but until then
> I consider it to be okayish to break things like this.
> 
> From my point of view we can treat Meson as stable as soon as Git v2.49
> is out. So from thereon, I will make sure to not include backwards
> incompatible changes anymore.

Thanks. It's comforting to hear that this is something that's under our
control.

-Peff

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Okay, the issue was in 163a3e97ac1 (meson: simplify use of the
> common-main library, 2025-01-30). In that commit we stopped using the
> common-main dependency for a couple of binaries, including
> git-remote-https and others. What I didn't realize though is that this
> also made us not use the linker arguments anymore, but those are
> required on Windows when linking an executable.
>
> I've kicked off CI jobs on GitHub to verify that the revised version of
> this patch series works alright. Once those tests pass I'll send another
> version of this patch series.

Thanks.  What is a bit more worrying is that you earlier said your
other CI didn't notice anything funny.  Is the reason for the
non-breakage also known (like: the two CI jobs build things
differently)?

Thanks.

Re: win+Meson test failures due to ps/build-meson-fixes-0130 topic?

[Patrick Steinhardt]

On Wed, Feb 26, 2025 at 07:55:09AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Okay, the issue was in 163a3e97ac1 (meson: simplify use of the
> > common-main library, 2025-01-30). In that commit we stopped using the
> > common-main dependency for a couple of binaries, including
> > git-remote-https and others. What I didn't realize though is that this
> > also made us not use the linker arguments anymore, but those are
> > required on Windows when linking an executable.
> >
> > I've kicked off CI jobs on GitHub to verify that the revised version of
> > this patch series works alright. Once those tests pass I'll send another
> > version of this patch series.
> 
> Thanks.  What is a bit more worrying is that you earlier said your
> other CI didn't notice anything funny.  Is the reason for the
> non-breakage also known (like: the two CI jobs build things
> differently)?

Oh, yeah, I forgot to mention that detail: the jobs failed, but the
failure wasn't reported (see e.g. [1]). It's a bug in the CI setup
itself. I'll send a patch series tomorrow to fix that.

Patrick

[1]: https://gitlab.com/gitlab-org/git/-/jobs/8996075448