git format-patch on empty commit

Hi all,

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:

From 08cfdb2994554d834b89309ca96d9bf513e26a90 Mon Sep 17 00:00:00 2001

From: pedrorijo91 <redacted>
Date: Fri, 8 Jan 2016 12:44:57 +0000
Subject: dummy commit

diff --git a/lol.txt b/lol.txt
new file mode 100644
index 0000000..f944b38
--- /dev/null
+++ b/lol.txt

@@ -0,0 +1 @@
+:)
--

2.5.4 (Apple Git-61)



then the second case shouldn't generate something like this instead ?

From 2d486f25c48780e2e132047e681929fcccb7e60c Mon Sep 17 00:00:00 2001

From: pedrorijo91 <redacted>
Date: Fri Jan 8 12:43:55 2016 +0000
Subject: Some commit message


2.5.4 (Apple Git-61)

Thanks,
Pedro Rijo

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