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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 3, 2024, 00:27 UTC
Message-ID: <xmqqcymq8n7v.fsf@gitster.g>
In-Reply-To: <CAO_smVh-gQWy7xUJgFjd6gUWCTV5jTYJ9E9E3rvaQg0EY-2BdQ@mail.gmail.com>

Kyle Lippincott <spectral@google.com> writes:

Show 7 quoted lines

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