Re: [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null"

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 9, 2016, 21:13 UTC
Message-ID: <xmqqbmzwaia1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160909203447.nlyx6v76fpyx2vzl@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> This patch defines the patch-id of a merge commit as
> essentially "null"; it has no patch-id. As a result,
> merges cannot match patch-ids via "--cherry-pick", and
> "format-patch --base" will not list merges in its list of
> prerequisite patch ids.

At first I wondered if such a change would make all merges look the same, but the patch-ids.c comparison is not for ordering/sorting but only for equality, so as long as the comparison function knows that a comparison of anything with "null" yields "They are different", we are OK.

Show 23 quoted lines

> diff --git a/patch-ids.c b/patch-ids.c
> index 77e4663..8d06099 100644
> --- a/patch-ids.c
> +++ b/patch-ids.c
> @@ -7,18 +7,40 @@
>  int commit_patch_id(struct commit *commit, struct diff_options *options,
>  		    unsigned char *sha1, int diff_header_only)
>  {
> -	if (commit->parents)
> +	if (commit->parents) {
> +		if (commit->parents->next)
> +			return PATCH_ID_NONE;
>  		diff_tree_sha1(commit->parents->item->object.oid.hash,
>  			       commit->object.oid.hash, "", options);
> -	else
> +	} else
>  		diff_root_tree_sha1(commit->object.oid.hash, "", options);
>  	diffcore_std(options);
> -	return diff_flush_patch_id(options, sha1, diff_header_only);
> +	if (diff_flush_patch_id(options, sha1, diff_header_only))
> +		return PATCH_ID_ERROR;
> +	return PATCH_ID_OK;
> +}

Looks sensible.  Thanks.