t2400 on freebsd12

t2400 on freebsd12

[D. Ben Knoble]

CI for an i18n PR [1] is failing on freebsd12, but I cannot reproduce
it. Is this known (a search of archives and the master branch doesn't
reveal anything)?

Summary output:

Test Summary Report
-------------------
t2400-worktree-add.sh                            (Wstat: 256 Tests:
227 Failed: 27)
  Failed tests:  50-52, 91-93, 107-109, 123-125, 139-141
                159-161, 175-177, 191-193, 207-209
  Non-zero exit status: 1

Proximate log entry:

[16:19:43] t2400-worktree-add.sh ..............................
Dubious, test returned 1 (wstat 256, 0x100)
Failed 27/227 subtests

I can see no mention of git-bundle in these tests, so I don't think my
PR is responsible for the breakage. I don't have a system on which to
bisect, however.

-- 
D. Ben Knoble

[1]: https://github.com/gitgitgadget/git/pull/1550/

Re: t2400 on freebsd12

[D. Ben Knoble]

Bump: this bug seems to affect several GitGitGadget PRs in CI, which
also renders GGG unusable for sending mail, IIUC.


-- 
D. Ben Knoble

Re: t2400 on freebsd12

[Junio C Hamano]

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

> "D. Ben Knoble" <ben.knoble@gmail.com> wrote:
>> CI for an i18n PR [1] is failing on freebsd12, but I cannot reproduce
>> it. Is this known (a search of archives and the master branch doesn't
>> reveal anything)?
>> 
>> Summary output:
>> 
>> Test Summary Report
>> -------------------
>> t2400-worktree-add.sh                            (Wstat: 256 Tests:
>> 227 Failed: 27)
>>   Failed tests:  50-52, 91-93, 107-109, 123-125, 139-141
>>                 159-161, 175-177, 191-193, 207-209
>>   Non-zero exit status: 1
>> 
>> Proximate log entry:
>> 
>> [16:19:43] t2400-worktree-add.sh ..............................
>> Dubious, test returned 1 (wstat 256, 0x100)
>> Failed 27/227 subtests

Those listed in https://github.com/gitgitgadget/git/actions do not
seem to even run Cirrus CI.  Perhaps GitGitGadget folks wanted to
make sure that changes that may break FreeBSD will not escape to the
public list, and added it as an extra for pull requests?  The runs
shown at the end of PR in https://github.com/gitgitgadget/git/pulls
however do run Cirrus CI and some, but not all, of them do fail.

Given that the tests seem to randomly fail, I can believe if this is
due to a flakey test that needs to be fixed, but from what we can
see in the webpage of Cirrus CI, I cannot even guess what the
problem is.

I do not offhand know how well the FreeBSD port has been maintained,
or those who have (or had once in the past) stake in it are keeping
an eye on it.  Anybody?

> Bump: this bug seems to affect several GitGitGadget PRs in CI, which
> also renders GGG unusable for sending mail, IIUC.

By the way, is this really "blocking" use of GGG in any way?  I do
recall seeing messages regarding gitk from Jens Lidestrom that are
shown in https://github.com/gitgitgadget/git/pull/1551 but the CI
run report at the end of that page does have a failing CirrusCI.

Re: t2400 on freebsd12

[Eric Sunshine]

On Thu, Jul 13, 2023 at 4:31 PM Junio C Hamano <gitster@pobox.com> wrote:
> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
> > Bump: this bug seems to affect several GitGitGadget PRs in CI, which
> > also renders GGG unusable for sending mail, IIUC.
>
> By the way, is this really "blocking" use of GGG in any way?  I do
> recall seeing messages regarding gitk from Jens Lidestrom that are
> shown in https://github.com/gitgitgadget/git/pull/1551 but the CI
> run report at the end of that page does have a failing CirrusCI.

Failed CI doesn't block GGG (at least not last time I used it). You
can "/submit" the pull request to the Git mailing list even if CI
failed or is not yet finished running.

Re: t2400 on freebsd12

[D. Ben Knoble]

On Thu, Jul 13, 2023 at 4:43 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > By the way, is this really "blocking" use of GGG in any way?  I do
> > recall seeing messages regarding gitk from Jens Lidestrom that are
> > shown in https://github.com/gitgitgadget/git/pull/1551 but the CI
> > run report at the end of that page does have a failing CirrusCI.
>
> Failed CI doesn't block GGG (at least not last time I used it). You
> can "/submit" the pull request to the Git mailing list even if CI
> failed or is not yet finished running.

Hm, I thought I recalled that it did, but I will try again. Thanks for
the attention.

-- 
D. Ben Knoble

Re: t2400 on freebsd12

[Jacob Abel]

On 23/07/13 01:27PM, Junio C Hamano wrote:
> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
> 
> > "D. Ben Knoble" <ben.knoble@gmail.com> wrote:
> > [...]
> >>
> >> Summary output:
> >>
> >> Test Summary Report
> >> -------------------
> >> t2400-worktree-add.sh                            (Wstat: 256 Tests:
> >> 227 Failed: 27)
> >>   Failed tests:  50-52, 91-93, 107-109, 123-125, 139-141
> >>                 159-161, 175-177, 191-193, 207-209
> >>   Non-zero exit status: 1
> >>
> >> Proximate log entry:
> >>
> >> [16:19:43] t2400-worktree-add.sh ..............................
> >> Dubious, test returned 1 (wstat 256, 0x100)
> >> Failed 27/227 subtests
> 
> [...]
> 
> Given that the tests seem to randomly fail, I can believe if this is
> due to a flakey test that needs to be fixed, but from what we can
> see in the webpage of Cirrus CI, I cannot even guess what the
> problem is.
> 
> I do not offhand know how well the FreeBSD port has been maintained,
> or those who have (or had once in the past) stake in it are keeping
> an eye on it.  Anybody?

I wrote these tests[1]. All the tests that are failing are:

- running `git worktree add` without `--orphan` or `--quiet`.
- running in a repo with 1 local branch with a valid commit.
- running in a worktree with an invalid/unborn HEAD.

The resulting code path should be git failing with:

- a warning containing the path to the HEAD file for the worktree 
  that is in scope and the contents of that HEAD.
- a hint to try using `--orphan`.
- an error `fatal: invalid reference`.

There are other minor variations between all the tests but these are
the commonalities between the tests. No other tests stress that
particular codepath.

My guess is something in `can_use_local_ref()` (`builtin/worktree.c`) 
is triggering the crash.

> 
> [...]

1. https://lore.kernel.org/git/20230517214711.12467-1-jacobabel@nullpo.dev/

Re: t2400 on freebsd12

[Eric Sunshine]

On Fri, Jul 14, 2023 at 2:30 AM Jacob Abel <jacobabel@nullpo.dev> wrote:
> On 23/07/13 01:27PM, Junio C Hamano wrote:
> > "D. Ben Knoble" <ben.knoble@gmail.com> writes:
> > >> t2400-worktree-add.sh                            (Wstat: 256 Tests:
> > >> 227 Failed: 27)
> > >>   Failed tests:  50-52, 91-93, 107-109, 123-125, 139-141
> > >>                 159-161, 175-177, 191-193, 207-209
>>
> > I do not offhand know how well the FreeBSD port has been maintained,
> > or those who have (or had once in the past) stake in it are keeping
> > an eye on it.  Anybody?
>
> I wrote these tests[1]. All the tests that are failing are:
>
> - running `git worktree add` without `--orphan` or `--quiet`.
> - running in a repo with 1 local branch with a valid commit.
> - running in a worktree with an invalid/unborn HEAD.
>
> 1. https://lore.kernel.org/git/20230517214711.12467-1-jacobabel@nullpo.dev/

I haven't been following this thread closely, but I wonder if the
`grep` introduced by patch [3/8] of the cited patch series is
problematic:

    grep -E "fatal:( options)? .* cannot be used together" actual

since BSD lineage regexp (including macOS) historically did not
support the "?" repetition operator. Perhaps an easy fix would be to
simplify this to:

    grep "cannot be used together" actual

Re: t2400 on freebsd12

[Jacob Abel]

On 23/07/14 12:19PM, Eric Sunshine wrote:
> On Fri, Jul 14, 2023 at 2:30 AM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > On 23/07/13 01:27PM, Junio C Hamano wrote:
> > > "D. Ben Knoble" <ben.knoble@gmail.com> writes:
> > > >> t2400-worktree-add.sh                            (Wstat: 256 Tests:
> > > >> 227 Failed: 27)
> > > >>   Failed tests:  50-52, 91-93, 107-109, 123-125, 139-141
> > > >>                 159-161, 175-177, 191-193, 207-209
> >>
> > > I do not offhand know how well the FreeBSD port has been maintained,
> > > or those who have (or had once in the past) stake in it are keeping
> > > an eye on it.  Anybody?
> >
> > I wrote these tests[1]. All the tests that are failing are:
> >
> > - running `git worktree add` without `--orphan` or `--quiet`.
> > - running in a repo with 1 local branch with a valid commit.
> > - running in a worktree with an invalid/unborn HEAD.
> >
> > 1. https://lore.kernel.org/git/20230517214711.12467-1-jacobabel@nullpo.dev/
> 
> I haven't been following this thread closely, but I wonder if the
> `grep` introduced by patch [3/8] of the cited patch series is
> problematic:
> 
>     grep -E "fatal:( options)? .* cannot be used together" actual
> 
> since BSD lineage regexp (including macOS) historically did not
> support the "?" repetition operator. Perhaps an easy fix would be to
> simplify this to:
> 
>     grep "cannot be used together" actual

The only tests that use the `?` operator are tests 32-38 which use 
`test_wt_add_excl()`. Those tests all seem to be consistently passing.

I probably should have mentioned in my original post the exact lines
that were causing the error. Line numbers mentioned below are from the
current head of the master branch (830b4a04c4, 'the tenth batch')

Tests 50-52 are the `test_wt_add_orphan_hint()` tests on lines 428-430
of t2400 (the fn is defined right above them). These were introduced in 
patch 6/8 [1].

The rest of the tests correspond to the `test_dwim_orphan('warn_bad_head', ...)` 
tests on lines 1039-1041 and likewise that function is defined
directly above those lines. These were introduced in patch 8/8 [2] 
however the bulk of the test code was introduced in the previous 
patch (7/8) [3].

Of particular note out of the details I gave in my previous post is 
that these tests all cause the command to emit the bad HEAD warning. 
I bring this up because in that warning code (patch 8/8 [2]) there 
is path string manipulation and a file read (both of which could be
stepping on some platform dependent behavior).

1. https://lore.kernel.org/git/20230517214711.12467-7-jacobabel@nullpo.dev/
2. https://lore.kernel.org/git/20230517214711.12467-9-jacobabel@nullpo.dev/
3. https://lore.kernel.org/git/20230517214711.12467-8-jacobabel@nullpo.dev/

Re: t2400 on freebsd12

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> I haven't been following this thread closely, but I wonder if the
> `grep` introduced by patch [3/8] of the cited patch series is
> problematic:
>
>     grep -E "fatal:( options)? .* cannot be used together" actual
>
> since BSD lineage regexp (including macOS) historically did not
> support the "?" repetition operator. Perhaps an easy fix would be to
> simplify this to:
>
>     grep "cannot be used together" actual

We do not seem to get the same breakage on macOS CI runs (otherwise
this would have been caught much earlier).  We do have many "grep
-E" invocations to ask for ERE but not many of them uses zero-or-one
'?'  in our test suite.  But I would be somewhat surprised if
test_dir_is_empty is broken on FreeBSD and nobody has noticed it for
this long.

I got an impression from the discussion so far that this breakage is
flaky and not always reproducible.  I wonder if "stress" thing helps
the chance to reproduce for those with FreeBSD boxes?

   $ cd t && sh ./t2400-* --stress

Thanks.

Re: t2400 on freebsd12

[Jacob Abel]

On 23/07/14 12:19PM, Eric Sunshine wrote:
>
> [...]
> 
> I haven't been following this thread closely, but I wonder if the
> `grep` introduced by patch [3/8] of the cited patch series is
> problematic:
> 
>     grep -E "fatal:( options)? .* cannot be used together" actual
> 
> since BSD lineage regexp (including macOS) historically did not
> support the "?" repetition operator. Perhaps an easy fix would be to
> simplify this to:
> 
>     grep "cannot be used together" actual

Thank you for this insight. It didn't end up being exactly this issue
but it seems to be a grep issue nonetheless. I've submitted a 
patch [1] which should resolve the issue (I tested it on a freebsd12
VM locally).

The TLDR of the issue is that grep 2.5 (GNU or BSD) doesn't seem to
recognise `\s` (or its inverted counterpart) as ERE but newer GNU (and
potentially other) grep versions do.

1. https://lore.kernel.org/git/20230715025512.7574-1-jacobabel@nullpo.dev/

Re: t2400 on freebsd12

[Eric Sunshine]

On Fri, Jul 14, 2023 at 11:02 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> On 23/07/14 12:19PM, Eric Sunshine wrote:
> > I haven't been following this thread closely, but I wonder if the
> > `grep` introduced by patch [3/8] of the cited patch series is
> > problematic:
> >
> >     grep -E "fatal:( options)? .* cannot be used together" actual
>
> Thank you for this insight. It didn't end up being exactly this issue
> but it seems to be a grep issue nonetheless. I've submitted a
> patch [1] which should resolve the issue (I tested it on a freebsd12
> VM locally).
>
> The TLDR of the issue is that grep 2.5 (GNU or BSD) doesn't seem to
> recognise `\s` (or its inverted counterpart) as ERE but newer GNU (and
> potentially other) grep versions do.

That's great to hear. Thanks for digging into this and submitting a fix.

(The FreeBSD 12 VM I created with the idea of investigating this is
now unnecessary.)