Subject: Re: [PATCH v2 3/3] t6421: fix test to work when repo dir contains d0

From: Junio C Hamano <[email-masked]>
Date: Fri, 02 Aug 2024 17:27:32 -0700

Kyle Lippincott <[email-masked]> writes:

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

Absolutely.  It turned out not to be so useful to show the shape of
potential matches, like this one:

 ... run-command.c:733            | d0 | main      | child_start  |...

To explain why spaces around " d0 " matters, the readers need to
understand that other trace entries that are irrelevant for our
purpose, like this one

    ... | label:do_write_index /path/to/t/trash directory.../.git/index.lock

we want reject, and for that we want the pattern to be specific
enough by looking for " do " that is followed by " child_start ".
Otherwise the leading paths that can contain anything won't easily
match, and the original of looking for just "d0" was way too error
prone.  But it is hard to leave a concise hint for that there.

So, again, sorry about the bad suggestion.

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

Yeah, using the syntactic clue -F"|" would also be a way to convey
the intention (i.e. "we are dealing with tabular output and we
expect nth column to be X"), but what you have is probably good
enough---it certainly is simpler to read and understand.  I briefly
considered that looking for "| d0 |" (i.e. explicitly mentioning the
column separator in the pattern) would make it even more obvious
what we are looking for, but having to worry about quoting "|" in
regexp would negate the benefit of obviousness out of the approach
to use "grep".

Thanks.