# format-patch on permission change gives empty patch 9 messages from 2010-10-07 to 2010-10-07. Participants: David Miller, Junio C Hamano, Sverre Rabbelier, Jakub Narebski, Nicolas Pitre. Thread: https://gitlist.dev/t/25373 ## David Miller, 2010-10-07 00:37 Subject: format-patch on permission change gives empty patch Message-ID: <20101006.173714.245380201.davem@davemloft.net> URL: https://gitlist.dev/e/20101006.173714.245380201.davem%40davemloft.net ``` When I ask git to format-patch a commit that is just a file permission change, it ends up generating an empty file, not even the commit message is included. davem@sunset:~/src/GIT/net-2.6$ git show commit a5dbc62ed61bf4cc57e22b78e5794880f1c74b90 Author: Joe Perches Date: Wed Oct 6 17:32:49 2010 -0700 Documentation/networking/ixgbevf.txt: Change file permissions to 644 Signed-off-by: Joe Perches Signed-off-by: David S. Miller You have new mail in /var/mail/davem davem@sunset:~/src/GIT/net-2.6$ git format-patch HEAD^ 0001-Documentation-networking-ixgbevf.txt-Change-file-per.patch davem@sunset:~/src/GIT/net-2.6$ ls -l 0001-Documentation-networking-ixgbevf.txt-Change-file-per.patch -rw-r--r-- 1 davem davem 0 Oct 6 17:36 0001-Documentation-networking-ixgbevf.txt-Change-file-per.patch davem@sunset:~/src/GIT/net-2.6$ git version git version 1.7.3.1 ``` ## David Miller, 2010-10-07 00:40 Subject: Re: format-patch on permission change gives empty patch Message-ID: <20101006.174008.70175671.davem@davemloft.net> URL: https://gitlist.dev/e/20101006.174008.70175671.davem%40davemloft.net In-Reply-To: <20101006.173714.245380201.davem@davemloft.net> ``` From: David Miller Date: Wed, 06 Oct 2010 17:37:14 -0700 (PDT) > > When I ask git to format-patch a commit that is just a file > permission change, it ends up generating an empty file, not > even the commit message is included. Ok it turns out that the commit in question was a NOP since the file permissions didn't change. But even if the patch is truly empty, format-patch should still give me the commit message shouldn't it? ``` ## Junio C Hamano, 2010-10-07 04:13 Subject: Re: format-patch on permission change gives empty patch Message-ID: <7vtykyy70w.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtykyy70w.fsf%40alter.siamese.dyndns.org In-Reply-To: <20101006.174008.70175671.davem@davemloft.net> ``` David Miller writes: > From: David Miller > Date: Wed, 06 Oct 2010 17:37:14 -0700 (PDT) > >> >> When I ask git to format-patch a commit that is just a file >> permission change, it ends up generating an empty file, not >> even the commit message is included. > > Ok it turns out that the commit in question was a NOP since the file > permissions didn't change. > > But even if the patch is truly empty, format-patch should still give > me the commit message shouldn't it? Probably; we have strongly encouraged people not to commit no-op, so I guess nobody stumbled upon this corner case. Perhaps something like this? builtin/log.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 22d1290..6baba7d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1057,6 +1057,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.verbose_header = 1; rev.diff = 1; rev.no_merges = 1; + rev.always_show_header = 1; DIFF_OPT_SET(&rev.diffopt, RECURSIVE); rev.subject_prefix = fmt_patch_subject_prefix; memset(&s_r_opt, 0, sizeof(s_r_opt)); ``` ## Junio C Hamano, 2010-10-07 04:40 Subject: Re: format-patch on permission change gives empty patch Message-ID: <7vocb6y5sq.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vocb6y5sq.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vtykyy70w.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > David Miller writes: > >> From: David Miller >> Date: Wed, 06 Oct 2010 17:37:14 -0700 (PDT) >> >>> >>> When I ask git to format-patch a commit that is just a file >>> permission change, it ends up generating an empty file, not >>> even the commit message is included. >> >> Ok it turns out that the commit in question was a NOP since the file >> permissions didn't change. >> >> But even if the patch is truly empty, format-patch should still give >> me the commit message shouldn't it? > > Probably; we have strongly encouraged people not to commit no-op, so I > guess nobody stumbled upon this corner case. > > Perhaps something like this? Actually, I have a feeling that this is not merely a corner case we didn't care about. A half-good news is that format-patch already takes --always command line option to generate a message out of an empty commit, but because it cannot be applied with "am", it is rather pointless. BUT. The weatherbaloon patch is probably a bad idea. "git rebase", especially when rebasing a side branch imported from some foreign SCM, would rather badly break with this patch, because its "format-patch | am" pipeline depends on format-patch to skip a no-op commit. Otherwise, "am" will complain about a patchless message. So in a sense, the current behaviour is internally consistent and deliberately so. I have a mixed feeling about where to go next. (1) Treat "rebase" as a way to reproduce a reasonable history; the current behaviour to drop empty commits is consistent with this view, as a history with an empty commit is _not_ entirely reasonable. (2) Treat "rebase" as a way to reproduce history faithfully, even an unreasonable one. We could teach "--allow-empty" to "am", and rewrite the pipeline as "format-patch --always | am --allow-empty" to implement it. I think I would eventually end up doing the latter, but not tonight. ``` ## Sverre Rabbelier, 2010-10-07 08:15 Subject: Re: format-patch on permission change gives empty patch Message-ID: URL: https://gitlist.dev/e/AANLkTimA4jrDXmfJp2em34fj6b1sj9kdEyZ_xPTzqcEN%40mail.gmail.com In-Reply-To: <7vocb6y5sq.fsf@alter.siamese.dyndns.org> ``` Heya, On Thu, Oct 7, 2010 at 06:40, Junio C Hamano wrote: >  (2) Treat "rebase" as a way to reproduce history faithfully, even an >     unreasonable one.  We could teach "--allow-empty" to "am", and >     rewrite the pipeline as "format-patch --always | am --allow-empty" to >     implement it. Wouldn't we then have to keep adding options for other corner cases? Perhaps a '--plumbing' flag that makes format-patch behave sanely (e.g., also do stuff like, turn off color and whatnot) and that makes 'git am' accept everything it knows how to understand? -- Cheers, Sverre Rabbelier ``` ## Jakub Narebski, 2010-10-07 10:05 Subject: Re: format-patch on permission change gives empty patch Message-ID: URL: https://gitlist.dev/e/m3eic2fher.fsf%40localhost.localdomain In-Reply-To: ``` Sverre Rabbelier writes: > On Thu, Oct 7, 2010 at 06:40, Junio C Hamano wrote: > >  (2) Treat "rebase" as a way to reproduce history faithfully, even an > >     unreasonable one.  We could teach "--allow-empty" to "am", and > >     rewrite the pipeline as "format-patch --always | am --allow-empty" to > >     implement it. > > Wouldn't we then have to keep adding options for other corner cases? > Perhaps a '--plumbing' flag that makes format-patch behave sanely > (e.g., also do stuff like, turn off color and whatnot) and that makes > 'git am' accept everything it knows how to understand? Hmmm... doesn't rebase pipeline use "format-patch --rebasing" (where '--rebasing' is internal option deliberately left undocumented)? -- Jakub Narebski Poland ShadeHawk on #git ``` ## Sverre Rabbelier, 2010-10-07 10:12 Subject: Re: format-patch on permission change gives empty patch Message-ID: URL: https://gitlist.dev/e/AANLkTik%3D3bT3h3tCsm3nAdi%3DVbwJsY0%2B33HPpF0_m36X%40mail.gmail.com In-Reply-To: ``` Heya, On Thu, Oct 7, 2010 at 12:05, Jakub Narebski wrote: > Hmmm... doesn't rebase pipeline use "format-patch --rebasing" (where > '--rebasing' is internal option deliberately left undocumented)? If so, that's exactly what I mean :) -- Cheers, Sverre Rabbelier ``` ## Nicolas Pitre, 2010-10-07 19:06 Subject: Re: format-patch on permission change gives empty patch Message-ID: URL: https://gitlist.dev/e/alpine.LFD.2.00.1010071503210.3107%40xanadu.home In-Reply-To: <7vocb6y5sq.fsf@alter.siamese.dyndns.org> ``` On Wed, 6 Oct 2010, Junio C Hamano wrote: > I have a mixed feeling about where to go next. > > (1) Treat "rebase" as a way to reproduce a reasonable history; the > current behaviour to drop empty commits is consistent with this view, > as a history with an empty commit is _not_ entirely reasonable. But a file mode change isn't exactly an empty commit, no? Nicolas ``` ## Junio C Hamano, 2010-10-07 20:29 Subject: Re: format-patch on permission change gives empty patch Message-ID: <7vmxqpwxue.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmxqpwxue.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Nicolas Pitre writes: > On Wed, 6 Oct 2010, Junio C Hamano wrote: > >> I have a mixed feeling about where to go next. >> >> (1) Treat "rebase" as a way to reproduce a reasonable history; the >> current behaviour to drop empty commits is consistent with this view, >> as a history with an empty commit is _not_ entirely reasonable. > > But a file mode change isn't exactly an empty commit, no? The second message from DaveM: Message-ID: <20101006.174008.70175671.davem@davemloft.net> Ok it turns out that the commit in question was a NOP since the file permissions didn't change. ```