[PATCH] Allow format-patch to create patches for merges

[PATCH] Allow format-patch to create patches for merges

[Nathan W. Panike]

The behavior for git format-patch is to ignore merge commits, producing an
empty patch.  The code does not allow the user to change this behavior. This
patch changes that behavior by allowing the user to specify -c or -m at the
command line to produce a patch for a merge commit.
---
Hi:

I am sure there are good reasons for the current behavior of format-patch, but
it seems to me that if the user explicitly wants to produce a patch for a merge
commit, he should be allowed to do so.  If merge_commit represents a merge,
then this patch allows the user to issue the command

git format-patch -m -1 $merge_commit 

or 

git format-patch -c -1 $merge_commit

and actually produce a patch.  The current behavior is that neither command
will produce a patch.  With or without the patch applied, the command

git format-patch -1 $merge_commit

does not produce a patch when merge_commit is a merge.  Thus the patch does not
change the default behavior of ignoring merges, at least by the limited testing
I have done.  

Thanks for your consideration.

Nathan Panike

 builtin-log.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 2ae39af..ea4729d 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -994,10 +994,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 
-		/* ignore merges */
-		if (commit->parents && commit->parents->next)
-			continue;
-
 		if (ignore_if_in_upstream &&
 				has_commit_patch_id(commit, &ids))
 			continue;
-- 
1.6.1.1.GIT

Re: [PATCH] Allow format-patch to create patches for merges

[Johannes Schindelin]

Hi,

On Mon, 26 Jan 2009, Nathan W. Panike wrote:

> The behavior for git format-patch is to ignore merge commits, producing 
> an empty patch.  The code does not allow the user to change this 
> behavior. This patch changes that behavior by allowing the user to 
> specify -c or -m at the command line to produce a patch for a merge 
> commit.

Your patch is almost perfect, except that you

- lack an explanation when this makes sense (format-patch is commonly used 
  for mail-based patch queues, and only -m 1 would make sense there, and 
  only if you run format-patch with --first-parent),

- did not add your Sign-off :-)

Ciao,
Dscho

Re: [PATCH] Allow format-patch to create patches for merges

[Nathan W. Panike]

On Mon, Jan 26, 2009 at 9:36 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 26 Jan 2009, Nathan W. Panike wrote:
>
>> The behavior for git format-patch is to ignore merge commits, producing
>> an empty patch.  The code does not allow the user to change this
>> behavior. This patch changes that behavior by allowing the user to
>> specify -c or -m at the command line to produce a patch for a merge
>> commit.
>
> Your patch is almost perfect, except that you
>
> - lack an explanation when this makes sense (format-patch is commonly used
>  for mail-based patch queues, and only -m 1 would make sense there, and
>  only if you run format-patch with --first-parent),
>
I think I have an unusual workflow where my patch makes sense,
although it probably does not for the vast majority of git users.  I
regularly use 3 machines: S, L, and H.  I keep my work synchronized by
using git.  Normally, I fetch from S to L or to H, depending on which
machine I am working on at the moment.  I also push from L or H to S.
I sporadically lose connectivity to S, so I have a hook in the repo on
S to send a backup email to me on mail server M, which has a more
reliable connection.  This email also serves as a  reminder when I
have moved from one machine to another with a degree of latency; and I
can use the mail queue on M to recreate most of my state, if I cannot
fetch from S.  In this workflow, I would really like git to create a
patch, even in the merge case, and I think I want to see that it was a
merge.

What I do not want to see is an empty patch when a non-trivial change
has occurred, which is the way it works now.

Also, I think I must be issuing the wrong command, as when I do

git format-patch --first-parent --stdout -1 $merge_commit

there is no data, with or without my patch.

> - did not add your Sign-off :-)

Oops.  Thanks for the catch.

>
> Ciao,
> Dscho
>
>

Re: [PATCH] Allow format-patch to create patches for merges

[Jeff King]

On Mon, Jan 26, 2009 at 10:27:18AM -0600, Nathan W. Panike wrote:

> I think I have an unusual workflow where my patch makes sense,
> although it probably does not for the vast majority of git users.  I
> regularly use 3 machines: S, L, and H.  I keep my work synchronized by
> using git.  Normally, I fetch from S to L or to H, depending on which
> machine I am working on at the moment.  I also push from L or H to S.
> I sporadically lose connectivity to S, so I have a hook in the repo on
> S to send a backup email to me on mail server M, which has a more
> reliable connection.  This email also serves as a  reminder when I

Have you considered sending a bundle instead of a patch in the backup
email? That is the more exact equivalent of a push (i.e., it preserves
your actual commits, sha1 and all).

-Peff

Re: [PATCH] Allow format-patch to create patches for merges

[Nathan W. Panike]

I have not used the bundle stuff, but yes, it seems to be a better fit
for what I am trying to do.

Thanks,

Nathan Panike

On Mon, Jan 26, 2009 at 2:45 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Jan 26, 2009 at 10:27:18AM -0600, Nathan W. Panike wrote:
>
>> I think I have an unusual workflow where my patch makes sense,
>> although it probably does not for the vast majority of git users.  I
>> regularly use 3 machines: S, L, and H.  I keep my work synchronized by
>> using git.  Normally, I fetch from S to L or to H, depending on which
>> machine I am working on at the moment.  I also push from L or H to S.
>> I sporadically lose connectivity to S, so I have a hook in the repo on
>> S to send a backup email to me on mail server M, which has a more
>> reliable connection.  This email also serves as a  reminder when I
>
> Have you considered sending a bundle instead of a patch in the backup
> email? That is the more exact equivalent of a push (i.e., it preserves
> your actual commits, sha1 and all).
>
> -Peff
>

Re: [PATCH] Allow format-patch to create patches for merges

[Junio C Hamano]

[-- Attachment #1: Type: text/plain, Size: 2771 bytes --]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> - lack an explanation when this makes sense (format-patch is commonly used 
>   for mail-based patch queues, and only -m 1 would make sense there, and 
>   only if you run format-patch with --first-parent),

You do not necessarily want --first-parent.

Suppose you have this topology

         B---D---E
        /   /
   M---A---C

where M is 'master', and E is 'mine'.

The format-patch command ignores merges by default because you can get
diff for A-M, B-A, C-A, E-D and serialize the resulting history without
it, and this is often sufficient.  When D merges with a conflict, or D is
an evil merge, however, you will not be able to reproduce how D looked
like on the receiving end.  Your --first-parent would instead format the
log message for A B D E with patch text of A-M, B-A, D-B and E-D.

But as a recipient of such a patch series, I'd much prefer to see the
patch text and the log message of B and C themselves.  I'd either apply
them on top of A serially, or apply them on top of A to recreate the
forked history the sender had and merge, to arrive at the state the sender
had at D either way, resolving a potential conflict (either when applying
C on top of B, or when merging between B and C), and apply E on top.

Getting a first parent diff that says "I merged random stuff here at D and
here is the difference D-B", is much less useful and throws us back to
dark ages of CVS/SVN merges, especially because C could be a long
multi-patch sequence.

I am not happy about Nathan's output.  I think "-m" output is a wrong
thing to use in that it just lets D-B and D-C patches in the same output
file, without marking that it is something you should not be applying as
part of the series blindly.  The patch is "If you reproduced my B and C
with the patches so far, here is a hint to help you recreate the merged
state D", and care must be taken to make sure both the tool and the user
notice the situation.  "git am", after you have applied B and then C, will
notice that the patches for D does not apply anyway, but the message
should tell the recipient that it is _expected_ not to apply to avoid
confusion.  One possible solution might be to always show --cc patch in
such a case, which (1) won't apply with patch nor git-am, and (2) will be
clear it is not a patch by having more than two @@ signs on each hunk
header.

If you really want to generate a patch for a merge commit (e.g. D in the
above picture), what you may want is "here is a fix-up you need to apply
on top of the result of naturally merging B and C to arrive at D".  It
could be empty if the merge is conflict-free and there is no evil amend.

Here is a food-for-thought sample history for interested parties to
experiment on.


[-- Attachment #2: a bundle out of a sample repo --]
[-- Type: application/octet-stream, Size: 2036 bytes --]