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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 29, 2024, 20:12 UTC
Message-ID: <xmqqwml4gdol.fsf@gitster.g>
In-Reply-To: <ZqeGBJrqIrU2A4jl@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

Show 18 quoted lines

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