All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Triplett <josh@joshtriplett.org>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>,
	Kevin Willford <kewillf@microsoft.com>,
	Xiaolong Ye <xiaolong.ye@intel.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [RFC/PATCH v2 0/3] patch-id for merges
Date: Wed, 7 Sep 2016 15:51:04 -0700	[thread overview]
Message-ID: <20160907225104.f5wi2yo4d2f26tti@x> (raw)
In-Reply-To: <20160907220101.hwwutkiagfottbdd@sigill.intra.peff.net>

On Wed, Sep 07, 2016 at 06:01:01PM -0400, Jeff King wrote:
> Here's a re-roll of the series I posted at:
> 
>   http://public-inbox.org/git/20160907075346.z6wtmqnfc6bsunjb@sigill.intra.peff.net/
> 
> Basically, it drops the time for "format-patch --cherry-pick" on a
> particular case from 3 minutes down to 3 seconds, by avoiding diffs
> on merge commits. Compared to v1, it fixes the totally-broken handling
> of commit_patch_id() pointed out by Johannes.
> 
> We can drop the diffs on the merge commits because they're quite broken,
> as discussed in the commit message of patch 3 (they don't take into
> account any parent except the first). So what do we do when somebody
> asks for the patch-id of a merge commit?
> 
> This is still marked RFC, because there are really two approaches here,
> and I'm not sure which one is better for "format-patch --base". I'd like
> to get input from Xiaolong Ye (who worked on --base), and Josh Triplett
> (who has proposed some patches in that area, and is presumably using
> them).

Thanks.

I'd love to see a more resilient patch-id mechanism, to make it easier
to match up patches between branches.  I don't think it makes sense to
talk about the patch-id of a merge commit (though it might make sense
for a merge which makes additional changes not present in any of the
parents).  Even if someone wants to match up merge commits with merge
commits, I don't think that should happen via patch-id; I think that
should happen in terms of "what patches does this merge introduce",
without constructing a merge-patch-id via a Merkle tree of commit
patch-ids.

So, I think this patch series makes sense (modulo the comments about the
commit message in patch 3).  We already don't respect merge commits when
doing format-patch; this seems consistent with that.  If we ever make it
possible for format-patch to handle merge commits, then we should also
allow it to have merge commits as prerequisites.

- Josh Triplett

  parent reply	other threads:[~2016-09-07 22:51 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-07  7:53 [RFC/PATCH 0/2] more patch-id speedups Jeff King
2016-09-07  7:54 ` [PATCH 1/2] patch-ids: turn off rename detection Jeff King
2016-09-07 12:53   ` Johannes Schindelin
2016-09-07  7:54 ` [PATCH 2/2] patch-ids: skip merge commits Jeff King
2016-09-07 12:52   ` Johannes Schindelin
2016-09-07 18:46     ` Jeff King
2016-09-07 22:08       ` Jeff King
2016-09-07 13:06 ` [RFC/PATCH 0/2] more patch-id speedups Johannes Schindelin
2016-09-07 18:49   ` Jeff King
2016-09-07 22:01 ` [RFC/PATCH v2 0/3] patch-id for merges Jeff King
2016-09-07 22:02   ` [PATCH 1/3] patch-ids: turn off rename detection Jeff King
2016-09-07 22:12     ` Jacob Keller
2016-09-07 22:04   ` [PATCH 2/3] diff_flush_patch_id: stop returning error result Jeff King
2016-09-08  0:51     ` Ramsay Jones
2016-09-08  7:20       ` Jeff King
2016-09-09 10:28     ` Johannes Schindelin
2016-09-09 10:40       ` Jeff King
2016-09-09 12:58         ` Johannes Schindelin
2016-09-09 19:37           ` Jeff King
2016-09-07 22:04   ` [PATCH 3/3] patch-ids: use commit sha1 as patch-id for merge commits Jeff King
2016-09-07 22:28     ` Jacob Keller
2016-09-07 22:38       ` Jeff King
2016-09-07 22:51   ` Josh Triplett [this message]
2016-09-08  7:30     ` [RFC/PATCH v2 0/3] patch-id for merges Jeff King
2016-09-09 20:34   ` [PATCH v3 0/2] " Jeff King
2016-09-09 20:34     ` [PATCH v3 1/2] patch-ids: turn off rename detection Jeff King
2016-09-09 20:34     ` [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null" Jeff King
2016-09-09 20:37       ` Jeff King
2016-09-09 21:13       ` Junio C Hamano
2016-09-09 21:01     ` [PATCH v3 0/2] patch-id for merges Junio C Hamano
2016-09-12 15:59       ` Jeff King
2016-09-12 17:18         ` Junio C Hamano
2016-09-12 17:56           ` Jeff King
2016-09-12 20:44             ` Junio C Hamano
2016-09-25 18:25     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160907225104.f5wi2yo4d2f26tti@x \
    --to=josh@joshtriplett.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=kewillf@microsoft.com \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    --cc=xiaolong.ye@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.