git format-patch on empty commit

git format-patch on empty commit

[pedro rijo]

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 <pedrorijo91@gmail.com>
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 <pedrorijo91@gmail.com>
Date: Fri Jan 8 12:43:55 2016 +0000
Subject: Some commit message


2.5.4 (Apple Git-61)

Thanks,
Pedro Rijo

Re: git format-patch on empty commit

[Jeff King]

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

Re: git format-patch on empty commit

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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.

The patch based workflow support is geared towards helping the
recipient of the patches a lot more than the contributors, and to
prevent mistakes while applying the patches, "am" would stop when it
sees such an empty e-mail as you saw (in the later part of message I
am not quoting).  After all, a "format-patch" output that does not
have any patch would be indistinguishable from discussion e-mail
messages and the recipient would not want to end up with no-op
commits that record such messages.

So I think skipping no-op commit from the output was done pretty
much deliberately and it is definitely not a bug.  I however do not
think it is incorrect to say that it is a lack of feature that
nobody so far found necessary or beneficial.

I would not refuse to consider adding a new option to "format-patch"
to emit such a no-op message, and add a "having no patch is OK, just
record a no-op commit" option to "am", though.  But I do not see a
clear benefit from such change--it sounds more like a set of
"because we could" not "because we need to" changes to me.

Thanks.

Re: git format-patch on empty commit

[Jeff King]

On Mon, Jan 11, 2016 at 01:04:42PM -0800, Junio C Hamano wrote:

> So I think skipping no-op commit from the output was done pretty
> much deliberately and it is definitely not a bug.  I however do not
> think it is incorrect to say that it is a lack of feature that
> nobody so far found necessary or beneficial.
> 
> I would not refuse to consider adding a new option to "format-patch"
> to emit such a no-op message, and add a "having no patch is OK, just
> record a no-op commit" option to "am", though.  But I do not see a
> clear benefit from such change--it sounds more like a set of
> "because we could" not "because we need to" changes to me.

I think we are more or less on the same page. To me it is less "because
we could" but that some people seem to find empty commits useful in
their workflow (which is why we have "commit --allow-empty"), and if you
use such a workflow, it is probably reasonable for "format-patch | am"
or "rebase" to be trusted to preserve the history (at least with an
option).

I do not use such workflows myself[1], so I will leave open the question
of whether they have a benefit. :)

-Peff

[1] Though I will confess to finding "commit --allow-empty" quite useful
    when debugging.

Re: git format-patch on empty commit

[pedro rijo]

To be honest I've been playing a bit with some git features that I've
never explored, and was trying to create a fairly simple 'email CLI'
upon git send-email and other git commands (just because git can) just
for the fun.

Just like you said, there's seems not be a 'clear benefit from such
change--it sounds more like a set of
"because we could" not "because we need to" changes'. I was more
curious to understand this git behavior than asking to fix the
bug/implement the feature, so please feel free not to implement this.

Thanks,
Pedro Rijo

2016-01-11 21:04 GMT+00:00 Junio C Hamano <gitster@pobox.com>:
> Jeff King <peff@peff.net> writes:
>
>> 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.
>
> The patch based workflow support is geared towards helping the
> recipient of the patches a lot more than the contributors, and to
> prevent mistakes while applying the patches, "am" would stop when it
> sees such an empty e-mail as you saw (in the later part of message I
> am not quoting).  After all, a "format-patch" output that does not
> have any patch would be indistinguishable from discussion e-mail
> messages and the recipient would not want to end up with no-op
> commits that record such messages.
>
> So I think skipping no-op commit from the output was done pretty
> much deliberately and it is definitely not a bug.  I however do not
> think it is incorrect to say that it is a lack of feature that
> nobody so far found necessary or beneficial.
>
> I would not refuse to consider adding a new option to "format-patch"
> to emit such a no-op message, and add a "having no patch is OK, just
> record a no-op commit" option to "am", though.  But I do not see a
> clear benefit from such change--it sounds more like a set of
> "because we could" not "because we need to" changes to me.
>
> Thanks.
>



-- 
Obrigado,

Pedro Rijo