Re: [PATCH 5/5] patch-id: tighten code to detect the patch header

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Jun 21, 2024 at 04:18:26PM -0700, Junio C Hamano wrote:
>> @@ -196,11 +211,13 @@ static void generate_id_list(unsigned flags)
>>  	struct strbuf line_buf = STRBUF_INIT;
>>  
>>  	oidclr(&oid);
>> +	flags |= GOPID_FIND_HEADER;
>>  	while (!feof(stdin)) {
>>  		patchlen = get_one_patchid(&n, &result, &line_buf, flags);
>>  		if (patchlen)
>>  			flush_current_id(&oid, &result);
>>  		oidcpy(&oid, &n);
>> +		flags &= ~GOPID_FIND_HEADER;
>>  	}
>
> I think I'm missing the obvious. But why don't we have to set
> `GOPID_FIND_HEADER` when we have flushed the current patch ID? Is this
> because we know that `get_one_patchid()` stops once it finds the next
> line starting with a commit?

Yup the original control flow is rather convoluted.  The first call
stops when it finds the header that begins the log message part and
returns, but the subsequent calls are to (1) skip the log message
and then (2) parse and hash the diff part, until it finds another
header that begins the log message part of the _next_ patch and
return.  GOPID_FIND_HEADER bit is used to tell the callee when we
haven't found the header (hence we can stop at a line whose
beginning looks like a hash) or the previous round already found the
header and we positively know we are now in the "skip the log
message" phase (hence a line whose beginning looks like a hash is
not a new header).

> Makes me wonder what happens when there is
> non-diff garbage between patches for which we are about to generate
> patch IDs.

"Skip non-diff garbage until we see a patch" is the mechanism used
to skip the log message, so it would be a reasonable thing to skip
such no-diff garbage between patches, no?