On Fri, Aug 2, 2024 at 2:41 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> "Kyle Lippincott via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > From: Kyle Lippincott <spectral@xxxxxxxxxx>
> >
> > The `grep` statement in this test looks for `d0.*<string>`, attempting
> > to filter to only show lines that had tabular output where the 2nd
> > column had `d0` and the final column had a substring of
> > [`git -c `]`fetch.negotiationAlgorithm`. These lines also have
> > `child_start` in the 4th column, but this isn't part of the condition.
> >
> > A subsequent line will have `d1` in the 2nd column, `start` in the 4th
> > column, and `/path/to/git/git -c fetch.negotiationAlgorihm` in the final
> > column. If `/path/to/git/git` contains the substring `d0`, then this
> > line is included by `grep` as well as the desired line, leading to an
> > effective doubling of the number of lines, and test failures.
> >
> > Tighten the grep expression to require `d0` to be surrounded by spaces,
> > and to have the `child_start` label.
>
> OK.
>
> I think I actually misinterpreted what you meant with these changes.
> It is not what the patterns are picking. It is some _other_ trace
> entry we do not necessarily care about, like label:do_write_index
> that has the path to the .git/index.lock file, that can accidentally
> contain d0, that can be picked up with a pattern that is too loose.
> So it really didn't have to clarify what it is looking for, as it
> would not help seeing what false positives the patterns are designed
> to avoid matching. Sorry about that.
I would have included examples, but they're quite long (>>>80 chars),
so seemed very out of place in both commit description and in the
codebase. With line wrapping, it wasn't very readable either. At the
risk of this also getting line-wrapped into unreadability:
test_line_count: line count for fetches != 1
23:59:48.794453 run-command.c:733 | d0 | main
| child_start | | 0.027328 | | |
..........[ch1] class:? argv:[git -c fetch.negotiationAlgorithm=noop
fetch origin --no-tags --no-write-fetch-head --recurse-submodules=no
--filter=blob:none --stdin]
23:59:48.798901 common-main.c:58 | d1 | main
| start | | 0.000852 | | |
/usr/local/google/home/spectral/src/oss/d0/git/git -c
fetch.negotiationAlgorithm=noop fetch origin --no-tags
--no-write-fetch-head --recurse-submodules=no --filter=blob:none
--stdin
where each line in the `fetches` file starts with `23:59:48` here.
It's 9 columns, separated by `|` characters, and the line we don't
want is the second one; the regex `d0.*fetch.negotiationAlgorithm`
includes it because of the `d0` in the path.
I considered using `awk -F"|" "\$2~/d0/ &&
\$9~/fetch\\.negotiationAlgorithm/{ print }" trace.output >fetches`,
but it was longer, possibly less clear, and less specific (since it
didn't include the $4~/child_start/ condition)
>
> Will queue.
>
>
