threads / discuss / 22955

git-rebase -i prunes commits with empty commit-message

Subject: git-rebase -i prunes commits with empty commit-message

## tl;dr

6 messages between Mar 8, 2010 and Mar 11, 2010.

replies: 5people: 3as markdown or json

Erik Faye-Lund· Mar 8, 2010, 20:07 UTC · lore

I'm in the process of converting an SVN repo to Git, and in the process I found one quite disturbing feature of git-rebase--interactive.sh: It discards commits with empty commit messages!

Here's a recepie for reproducing the issue: --->8--- git init git commit -m "dummy" --allow-empty git commit -m "dummy" --allow-empty git commit -m "dummy" --allow-empty git filter-branch -f --msg-filter 'sed -e "s/dummy//"' git rebase -i HEAD~2 --->8---

The editor window will show "noop", and exiting the editor goes ahead and deletes all but the initial commit.

This gets even weirder if it's a mixture of empty and non-empty commits; the commit-identifiers gets appended, together with an angle-bracket ('>'), to the previous line.

I'm guessing that this is unintended behavior. This was observed on git 1.7.0.1.

-- 
Erik "kusma" Faye-Lund

Michael Haggerty· Mar 10, 2010, 13:13 UTC · re: Erik Faye-Lund · lore

Re: git-rebase -i prunes commits with empty commit-message

Erik Faye-Lund wrote:

Show 14 quoted lines

> I'm in the process of converting an SVN repo to Git, and in the
> process I found one quite disturbing feature of
> git-rebase--interactive.sh: It discards commits with empty commit
> messages!
> 
> Here's a recepie for reproducing the issue:
> --->8---
> git init
> git commit -m "dummy" --allow-empty
> git commit -m "dummy" --allow-empty
> git commit -m "dummy" --allow-empty
> git filter-branch -f --msg-filter 'sed -e "s/dummy//"'
> git rebase -i HEAD~2
> --->8---

Does git really claim to handle commits with empty commit messages? That you have to use git-filter-branch to create the test case suggests that the answer is "no", but I don't know. (git-commit, for example, refuses to create a commit with an empty message.)

If indeed git requires commit messages to be non-empty, then the fault here seemingly lies with git-filter-branch for allowing commit messages to be completely deleted.

Michael

Erik Faye-Lund· Mar 10, 2010, 13:34 UTC · re: Michael Haggerty · lore

Re: git-rebase -i prunes commits with empty commit-message

On Wed, Mar 10, 2010 at 2:13 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 20 quoted lines

> Erik Faye-Lund wrote:
>> I'm in the process of converting an SVN repo to Git, and in the
>> process I found one quite disturbing feature of
>> git-rebase--interactive.sh: It discards commits with empty commit
>> messages!
>>
>> Here's a recepie for reproducing the issue:
>> --->8---
>> git init
>> git commit -m "dummy" --allow-empty
>> git commit -m "dummy" --allow-empty
>> git commit -m "dummy" --allow-empty
>> git filter-branch -f --msg-filter 'sed -e "s/dummy//"'
>> git rebase -i HEAD~2
>> --->8---
>
> Does git really claim to handle commits with empty commit messages?
> That you have to use git-filter-branch to create the test case suggests
> that the answer is "no", but I don't know.  (git-commit, for example,
> refuses to create a commit with an empty message.)

If git didn't, I'd expect it to be impossible to create them. This was
just a minimal example on how to reproduce it, and it's a silly
use-case. I think my original way of getting into the state was a tad
more legitimate:
1) I had an SVN-repo with empty commit-messages
2) I imported that SVN-repo into git through git-svn, and git-svn
appended it's meta-data to the otherwise empty commit-message.
3) I used git-filter-branch to remove the git-svn metadata (as the
git-filter-branch man-page suggest)

I don't know what happens when git-svn encounter empty commit-messages with the --no-metadata option enabled. In my case, the decision to fully migrate the repo to Git (instead of just using git-svn as a nicer svn-frontend) came long after 1) and 2) were done.

> If indeed git requires commit messages to be non-empty, then the fault
> here seemingly lies with git-filter-branch for allowing commit messages
> to be completely deleted.

If there is some decided-upon restriction that commit-messages cannot be empty, then I agree with you.

From experiments, it seems that git-commit does not seem to allow empty commit-messages. But I can't find this documented. I'm not sure what git-commit-tree allows, but I think it should deny creating commits with empty messages (possibly unless some option is given) if there should be a restriction, because most scripts use this to generate commits AFAIK.

But to be honest, it seems to me like in this precise instance it's probably better to just fix git-rebase--interactive.sh. There's no good reason for it to barf on the commits -- especially since noon-interactive rebase handles them just fine. Unless someone screams out loud, I might take a stab at it when I get time.

-- 
Erik "kusma" Faye-Lund

Erik Faye-Lund· Mar 11, 2010, 13:14 UTC · re: Erik Faye-Lund · lore

Re: git-rebase -i prunes commits with empty commit-message

On Wed, Mar 10, 2010 at 2:34 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:

Show 7 quoted lines

>
> But to be honest, it seems to me like in this precise instance it's
> probably better to just fix git-rebase--interactive.sh. There's no
> good reason for it to barf on the commits -- especially since
> noon-interactive rebase handles them just fine. Unless someone screams
> out loud, I might take a stab at it when I get time.
>

I think I've found the culprit: git-rev-list doesn't append a newline-separator after commits with empty messages. git-rebase--interactive.sh basically eats git rev-list's output line by line, prepending "pick ".

This seems to have been introduced in 55246aa "Don't use "<unknown>" for placeholders and suppress printing of empty user formats." by Michal Vitecek. It seems he intended to fix a rev-list with --pretty=format:"" or something like that, but I can't get custom formats to work at all with rev-list, even if the documentation says it should.

Anyway, the following patch seems to fix the problem for me, but I'm not very confident that it doesn't break whatever Michal was trying to address.

--->8---

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 5679170..b13e1ba 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -134,10 +134,8 @@ static void show_commit(struct commit *commit, void *data)
                                if (graph_show_remainder(revs->graph))
                                        putchar('\n');
                        }
-               } else {
-                       if (buf.len)
-                               printf("%s%c", buf.buf, info->hdr_termination);
-               }
+               } else
+                       printf("%s%c", buf.buf, info->hdr_termination);
                strbuf_release(&buf);
        } else {
                if (graph_show_remainder(revs->graph))
--->8---

-- 
Erik "kusma" Faye-Lund

Erik Faye-Lund· Mar 11, 2010, 13:46 UTC · re: Erik Faye-Lund · lore

Re: git-rebase -i prunes commits with empty commit-message

On Thu, Mar 11, 2010 at 2:14 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:

> Anyway, the following patch seems to fix the problem for me, but I'm
> not very confident that it doesn't break whatever Michal was trying to
> address.
>

...or even the test-suite. I should have tested that before posting this, sorry for the noise. Back to the drawing-board.

-- 
Erik "kusma" Faye-Lund

Junio C Hamano· Mar 10, 2010, 19:53 UTC · re: Michael Haggerty · lore

Re: git-rebase -i prunes commits with empty commit-message

Michael Haggerty <mhagger@alum.mit.edu> writes:

> Does git really claim to handle commits with empty commit messages?

Yes, I would say it is a bug in rebase-i if it refuses to deal with histories with such commits. Warning and giving the user a chance to add message _might_ be good, but allowing no way out would be inexcusable. Even adding a fake "(original commit had no message)" as the message and continuing would be better than stopping or dropping commits.

The Porcelain "git commit" by default complains if you do not give any message, because it usually is an end-user mistake to directly use "git commit" and not writing anything about the resulting commit. However,

 (1) The plumbing allows it because such a policy to forbid commits
     without comments does not belong there, e.g. this would succeed:

     $ git commit-tree HEAD^{tree} </dev/null

 (2) Even the Porcelain allows it if you ask nicely:

     $ git commit --cleanup=verbatim -m ''

These are primarily so that you can deal with histories created by other people's tools (e.g. foreign SCM, third party Porcelains, etc.). You do not have much control over histories created by them, and our commit creating commands need to be usable as an ingredient for you to write conversion tools (like filter-branch or rebase-i).