{"thread":{"id":"36892","subject":"Confusing error message in rebase when commit becomes empty","startedAt":"2014-06-11T12:49:04Z","lastAt":"2014-06-13T07:31:13Z","messageCount":7,"participants":["Peter Krefting","Phil Hord","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"243905","messageId":"alpine.DEB.2.00.1406111321090.6307@ds9.cixit.se","threadId":"36892","inReplyTo":null,"subject":"Confusing error message in rebase when commit becomes empty","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2014-06-11T12:49:04Z","receivedAt":"2014-06-11T12:49:04Z","isPatch":false,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Hi!\n\nI am rebasing a branch to combine a couple of commits. One is a revert \nof a previous commit. Since there are commits in-between, I do \n\"squash\" to make sure I get everything, and then add the actual change \non top of that. The problem is that rebase stops with a confusing \nerror message (from commit, presumably):\n\n   $ git rebase --interactive\n   [...]\n   You asked to amend the most recent commit, but doing so would make\n   it empty. You can repeat your command with --allow-empty, or you can\n   remove the commit entirely with \"git reset HEAD^\".\n   rebase in progress; onto 342b22f\n   You are currently rebasing branch 'mybranch' on '342b22f'.\n\n   No changes\n\n   Could not apply 4682a1f20f6ac29546536921bc6ea0386441e23e... Revert \"something\"\n\nOK, so I should retry the command with --allow-empty, then:\n\n   $ git rebase --interactive --allow-empty\n   error: unknown option `allow-empty'\n\nNope, that's not quite right.\n\nRunning \"git rebase --continue\" does work as expected, but perhaps it \njust shouldn't stop in this case?\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"243912","messageId":"CABURp0rC3yktDiRVQwo9E+x5_wKLnLVRx87ej+m+TyUdBkMpOA@mail.gmail.com","threadId":"36892","inReplyTo":"alpine.DEB.2.00.1406111321090.6307@ds9.cixit.se","subject":"Re: Confusing error message in rebase when commit becomes empty","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2014-06-11T15:44:17Z","receivedAt":"2014-06-11T15:44:17Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Wed, Jun 11, 2014 at 8:49 AM, Peter Krefting <peter@softwolves.pp.se> wrote:\n> I am rebasing a branch to combine a couple of commits. One is a revert of a previous commit. Since there are commits in-between, I do \"squash\" to make sure I get everything, and then add the actual change on top of that. The problem is that rebase stops with a confusing error message (from commit, presumably):\n>\n>   $ git rebase --interactive\n>   [...]\n>   You asked to amend the most recent commit, but doing so would make\n>   it empty. You can repeat your command with --allow-empty, or you can\n>   remove the commit entirely with \"git reset HEAD^\".\n>   rebase in progress; onto 342b22f\n>   You are currently rebasing branch 'mybranch' on '342b22f'.\n>\n>   No changes\n>\n>   Could not apply 4682a1f20f6ac29546536921bc6ea0386441e23e... Revert \"something\"\n>\n> OK, so I should retry the command with --allow-empty, then:\n>\n>   $ git rebase --interactive --allow-empty\n>   error: unknown option `allow-empty'\n>\n> Nope, that's not quite right.\n\n\nThe correct switch for rebase is --keep-empty, but it is too late to\nchoose it once the interactive rebase is underway.  I think the\ncorrect advice might be something like this:\n\n  You asked to squash this commit and its parent, but doing so would make\n  it empty. You can drop this empty commit with \"git reset HEAD^\" , or you can\n  keep it with \"git commit --amend --allow-empty\".\n\nBut I have not tested this.\n\n\n> Running \"git rebase --continue\" does work as expected, but perhaps it just shouldn't stop in this case?\n\n\nWhat does it mean when you say it worked as expected?  Did it leave\nthe empty commit, omit the empty commit, or leave some un-squashed\ncommit?  It's not clear to me what --continue _should_ do in this\ncase, but it does seem like the two options here should be\n\n 1. keep the empty commit\n 2. drop the empty commit\n\nI would expect \"git rebase --skip\" to drop the empty commit, but maybe\nit will \"skip\" the squash instead.  I don't know.  Better advice here\nis certainly needed.\n"},{"id":"243914","messageId":"alpine.DEB.2.00.1406111854180.21431@ds9.cixit.se","threadId":"36892","inReplyTo":"CABURp0rC3yktDiRVQwo9E+x5_wKLnLVRx87ej+m+TyUdBkMpOA@mail.gmail.com","subject":"Re: Confusing error message in rebase when commit becomes empty","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2014-06-11T17:57:10Z","receivedAt":"2014-06-11T17:57:10Z","isPatch":false,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Phil Hord:\n\n> What does it mean when you say it worked as expected?  Did it leave\n> the empty commit, omit the empty commit, or leave some un-squashed\n> commit?\n\nActually, it did not work as expected I noted afterward, it just \ndropped the reversion commit, and did not squash the next commit into \nit as I had asked, so from three commits, \"change\", \"revert\", \n\"new-change\" I had two, \"change\", \"new-change\" with the end result \nbeing the same (i.e., instead of squashing all three into one \n\"new-change\", I had \"change\" and \"revert\" + \"new-change\").\n\n> It's not clear to me what --continue _should_ do in this \n> case, but it does seem like the two options here should be\n\nI sort of expect a squashed commit of \"change\" + \"revert\" to be an \nempty commit, and of \"change\" + \"revert\" + \"new-change\" to be a commit \nof \"new-change\".\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"243922","messageId":"CABURp0q1c+75VzMfttDWVXhTFma3g=Tj938J_9i9xhG1yuMpdQ@mail.gmail.com","threadId":"36892","inReplyTo":"alpine.DEB.2.00.1406111854180.21431@ds9.cixit.se","subject":"Re: Confusing error message in rebase when commit becomes empty","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2014-06-11T19:04:16Z","receivedAt":"2014-06-11T19:04:16Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Wed, Jun 11, 2014 at 1:57 PM, Peter Krefting <peter@softwolves.pp.se> wrote:\n> Phil Hord:\n>\n>\n>> What does it mean when you say it worked as expected?  Did it leave\n>> the empty commit, omit the empty commit, or leave some un-squashed\n>> commit?\n>\n>\n> Actually, it did not work as expected I noted afterward, it just dropped the\n> reversion commit, and did not squash the next commit into it as I had asked,\n> so from three commits, \"change\", \"revert\", \"new-change\" I had two, \"change\",\n> \"new-change\" with the end result being the same (i.e., instead of squashing\n> all three into one \"new-change\", I had \"change\" and \"revert\" +\n> \"new-change\").\n\nDid you have a series of three commits being squashed in your to-do\nlist?  I mean, did you have a list like this:\n\n   pick ...  do foo\n   squash ...  revert \"do foo\"\n   squash ...  What I really meant to do.\n\nI suppose the rebase stopped after the first squash failed due to the\nemptiness of the proposed result.  Then rebase --continue proceeded,\nhaving decided that you were finished with the 'revert' commit.  Then\n... I would expect the next commit would actually be squashed, but I\ncan only speculate at the reasons it might have decided not to after\nyour continue.\n\nThis actually sounds like another case of a bug I reported a few weeks\nago[1] and which Fabian Ruch was kindly investigating[2] and trying to\nfix.  I don't think his fix would have helped in this case, but I do\nthink it is worthy of consideration for that same patch series.\n\n>> It's not clear to me what --continue _should_ do in this case, but it does\n>> seem like the two options here should be\n>\n> I sort of expect a squashed commit of \"change\" + \"revert\" to be an empty\n> commit, and of \"change\" + \"revert\" + \"new-change\" to be a commit of\n> \"new-change\".\n\nYes, but empty commits are discouraged on some projects.  If you want\nyour \"change + revert = empty\" commit to appear after the squash, I\nwould expect you would want to use --keep-empty on your inital rebase\ncommand.  But I'm not sure that will do what you expected either; it\nmay only keep previously-empty commits during the rebase.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/245688\n[2] http://www.mail-archive.com/git%40vger.kernel.org/msg51703.html\n"},{"id":"244061","messageId":"alpine.DEB.2.00.1406130823251.27436@ds9.cixit.se","threadId":"36892","inReplyTo":"CABURp0q1c+75VzMfttDWVXhTFma3g=Tj938J_9i9xhG1yuMpdQ@mail.gmail.com","subject":"Re: Confusing error message in rebase when commit becomes empty","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2014-06-13T07:25:43Z","receivedAt":"2014-06-13T07:25:43Z","isPatch":false,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Phil Hord:\n\n> Did you have a series of three commits being squashed in your to-do\n> list?  I mean, did you have a list like this:\n>\n>   pick ...  do foo\n>   squash ...  revert \"do foo\"\n>   squash ...  What I really meant to do.\n\nYes, that is exactly what I had. Plus an extra commit that I moved to \nthe end, which was originally placed between the \"do foo\" and \"revert \ndo foo\" commits (which is why I wasn't 110% sure the combination of \nthe two would produce an empty commit).\n\n> Yes, but empty commits are discouraged on some projects.  If you \n> want your \"change + revert = empty\" commit to appear after the \n> squash, I would expect you would want to use --keep-empty on your \n> inital rebase command.  But I'm not sure that will do what you \n> expected either; it may only keep previously-empty commits during \n> the rebase.\n\nThe thing is that I wasn't expecting it to come out empty, as I had \nanother commit to squash into it. That the interim throw-away squashed \ncommit was empty should have been an internal matter to rebase, IMHO.\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"244062","messageId":"20140613072749.GD7908@sigill.intra.peff.net","threadId":"36892","inReplyTo":"alpine.DEB.2.00.1406111321090.6307@ds9.cixit.se","subject":"Re: Confusing error message in rebase when commit becomes empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-13T07:27:50Z","receivedAt":"2014-06-13T07:27:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 11, 2014 at 01:49:04PM +0100, Peter Krefting wrote:\n\n> Hi!\n> \n> I am rebasing a branch to combine a couple of commits. One is a revert of a\n> previous commit. Since there are commits in-between, I do \"squash\" to make\n> sure I get everything, and then add the actual change on top of that. The\n> problem is that rebase stops with a confusing error message (from commit,\n> presumably):\n> \n>   $ git rebase --interactive\n>   [...]\n>   You asked to amend the most recent commit, but doing so would make\n>   it empty. You can repeat your command with --allow-empty, or you can\n>   remove the commit entirely with \"git reset HEAD^\".\n>   rebase in progress; onto 342b22f\n>   You are currently rebasing branch 'mybranch' on '342b22f'.\n> \n>   No changes\n> \n>   Could not apply 4682a1f20f6ac29546536921bc6ea0386441e23e... Revert \"something\"\n> \n> OK, so I should retry the command with --allow-empty, then:\n> \n>   $ git rebase --interactive --allow-empty\n>   error: unknown option `allow-empty'\n> \n> Nope, that's not quite right.\n\nYeah, that message comes from \"commit --amend\", which is called by\nrebase to handle the squash. The \"repeat your command\" part is\nconfusing. The right thing to do here is:\n\n  git commit --amend --allow-empty\n\nif you want to have an empty commit, or:\n\n  git reset HEAD^\n\nif you want to have nothing.\n\nOf course the first one would never occur to you, because it is not\n\"your command\" in the first place. :)\n\nWe could change it to say \"use git commit --amend --allow-empty\", though\nthat is slightly incomplete for other cases (e.g., you might have\nactually said \"git commit --amend -a\", and the right advice is to\ninclude that \"-a\".\n\nCommit understands a \"whence\" flag that could let it customize the\nmessage for the case of rebase. But I think you would have to teach\ndetermine_whence to figure out that we are in a rebase.\n\n> Running \"git rebase --continue\" does work as expected, but perhaps it just\n> shouldn't stop in this case?\n\nAs you noticed later in the thread, doing \"--continue\" omits the revert.\nThat's because it is telling rebase \"OK, I've fixed this up, we can keep\ngoing\". But of course it wasn't fixed.\n"},{"id":"244063","messageId":"20140613073113.GE7908@sigill.intra.peff.net","threadId":"36892","inReplyTo":"alpine.DEB.2.00.1406130823251.27436@ds9.cixit.se","subject":"Re: Confusing error message in rebase when commit becomes empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-13T07:31:13Z","receivedAt":"2014-06-13T07:31:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 13, 2014 at 08:25:43AM +0100, Peter Krefting wrote:\n\n> >Yes, but empty commits are discouraged on some projects.  If you want your\n> >\"change + revert = empty\" commit to appear after the squash, I would\n> >expect you would want to use --keep-empty on your inital rebase command.\n> >But I'm not sure that will do what you expected either; it may only keep\n> >previously-empty commits during the rebase.\n> \n> The thing is that I wasn't expecting it to come out empty, as I had another\n> commit to squash into it. That the interim throw-away squashed commit was\n> empty should have been an internal matter to rebase, IMHO.\n\nThat's a good point that I neglected in my other response. Maybe the\nright solution is for \"rebase --interactive\" to always pass\n\"--allow-empty\" when doing a squash. And then either:\n\n  1. Always keep such empty commits. A user who is surprised by them\n     being empty can then revisit them. Or drop them by doing another\n     rebase without --keep-empty.\n\n  2. Notice ourselves that the end-result of the whole squash is an\n     empty commit, and stop to let the user deal with it.\n\n-Peff\n"}]}