Re: git format-patch on empty commit

[Jeff King <peff@peff.net>]

On Mon, Jan 11, 2016 at 02:19:52PM +0000, pedro rijo wrote:

> Couldn't find any explanation on git docs on this issue:
> 
> If I create a dummy commit, with some dummy diff, I get a normal patch
> when I run
> 
> $ git format-patch -1 -o outgoing/ -p -k
> 
> but if the last commit is an empty commit, generated by
> 
> $ git commit --allow-empty "Some commit message"
> 
> then the output of the format patch will be an empty patch. If the
> first case produces something like this:

I'm not sure if this is a bug or not.

In the beginning, git's revision-traversal machinery generally does not
show commits which have no diff. Over the years, commands like "git log"
learned to set the "always_show_header" option to show even empty
commits. But format-patch never did.

It's pretty easy to do:

diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..f132ce7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1283,6 +1283,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.commit_format = CMIT_FMT_EMAIL;
 	rev.verbose_header = 1;
 	rev.diff = 1;
+	rev.always_show_header = 1;
 	rev.max_parents = 1;
 	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 	rev.subject_prefix = fmt_patch_subject_prefix;

So on the one hand, it is probably better not to silently omit commits,
and it would make format-patch consistent with log and other commands.
And what is produced now is somewhat nonsensical. For "format-patch
--stdout", omitting the header is OK; we just skip the commit entirely.
But for writing patches to individual files, you end up with an empty
file!

On the other hand, the point of format-patch is generally to feed the
output to "git am". And I don't think "am" handles such empty patches
very well (it complains with "Patch is empty. Was it split wrong?", and
there's no way to say "no, really, just apply the empty commit").

So even if you generated such patches, I'm not sure what you would do
with them.

I'd be more sympathetic to a patch like the one above if "am" also
learned about empty commits (even if it required the user to say "empty
commits are OK, as we already do for "commit" and some other commands).
Note that the patch above also seems to trigger a test failure in t3421
(which is a rebase test, and rebase builds on "format-patch | am"). That
would need to be investigated, too. I'm not sure if there's a subtle
case I'm missing, or if the test script is simply a little too strict.

-Peff