Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 28, 2014, 22:26 UTC
- Message-ID
- <xmqqppl67ydr.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <xmqqzjka7z26.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 53 quoted lines
>> @@ -99,6 +116,18 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st
>> if (!memcmp(line, "@@ -", 4)) {
>> /* Parse next hunk, but ignore line numbers. */
>> scan_hunk_header(line, &before, &after);
>> + if (stable) {
>> + if (hunks) {
>> + flush_one_hunk(result, &ctx);
>> + memcpy(&ctx, &header_ctx,
>> + sizeof ctx);
>> + } else {
>> + /* Save ctx for next hunk. */
>> + memcpy(&header_ctx, &ctx,
>> + sizeof ctx);
>> + }
>> + }
>> + hunks++;
>> continue;
>> }
>>
>> @@ -107,7 +136,10 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st
>> break;
>>
>> /* Else we're parsing another header. */
>> + if (stable && hunks)
>> + flush_one_hunk(result, &ctx);
>> before = after = -1;
>> + hunks = 0;
>> }
>>
>> /* If we get here, we're inside a hunk. */
>> @@ -119,39 +151,46 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st
>> /* Compute the sha without whitespace */
>> len = remove_space(line);
>> patchlen += len;
>> - git_SHA1_Update(ctx, line, len);
>> + git_SHA1_Update(&ctx, line, len);
>> }
>>
>> if (!found_next)
>> hashclr(next_sha1);
>>
>> + flush_one_hunk(result, &ctx);
>
> What I read from these changes is that you do not do anything
> special about the per-file header, so two no overlapping patches
> with a single hunk each that touches the same path concatenated
> together would not result in the same patch-id as a single-patch
> that has the same two hunks. Which would break your earlier 'Yes,
> reordering only the hunks will not make sense, but before each hunk
> you could insert the same "diff --git a/... b/..." to make them a
> concatenation of patches that touch the same file', I would think.
>
> Is that what we want to happen? Or is my reading mistaken?Heh, I was blind---copying into header_ctx and then restoring that in preparation for the next round is exactly for duplicating the per-file header sum to each and every hunk, so you are already doing that.