{"thread":{"id":"46654","subject":"[BUG] rebase -i with empty commits + exec","startedAt":"2017-08-22T23:08:55Z","lastAt":"2017-08-24T13:35:37Z","messageCount":9,"participants":["Stefan Beller","Stephan Beyer","Johannes Schindelin","Junio C Hamano","Philip Oakley","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"326989","messageId":"CAGZ79kYinci-OWXV2VfScLPcUDCHyhSb=7TxTWUWHPnKV5PuDA@mail.gmail.com","threadId":"46654","inReplyTo":null,"subject":"[BUG] rebase -i with empty commits + exec","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-22T23:08:46Z","receivedAt":"2017-08-22T23:08:55Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently I am working on a longer series, for which I decided\nto keep track of progress in an empty commit. This empty commit\nis in the middle of the series (to divide the commits into two sets,\nthe foundation that I consider stable and the later parts that are not\nas stable for my development, they contain things that may be useful)\n\nThen I invoked \"git rebase -i -x make\" to see\nin which shape the series is.\n\nThe editor opened proposing the following instruction sheet,\nwhich in my opinion is buggy:\n\n pick 1234 some commit\n exec make\n pick 2345 another commit\n exec make\n pick 3456 third commit\n # pick 4567 empty commit\n exec make\n pick 5678 yet another commit\n exec make\n\nI think the lines of the empty commit and the following exec should\nbe swapped, because that exec should work on the third commit.\nMaybe we'd want to see another commented exec:\n\n pick 1234 some commit\n exec make\n pick 2345 another commit\n exec make\n pick 3456 third commit\n exec make\n # pick 4567 empty commit\n # exec make <- unsure about this line\n pick 5678 yet another commit\n exec make\n\nThoughts?\n"},{"id":"327013","messageId":"36e9c381-81b0-ae71-153a-0bcabc59856c@gmx.net","threadId":"46654","inReplyTo":"CAGZ79kYinci-OWXV2VfScLPcUDCHyhSb=7TxTWUWHPnKV5PuDA@mail.gmail.com","subject":"[BUG] rebase -i with only empty commits","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2017-08-23T09:08:36Z","receivedAt":"2017-08-23T09:08:44Z","isPatch":false,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nOn 08/23/2017 01:08 AM, Stefan Beller wrote:\n> The editor opened proposing the following instruction sheet,\n> which in my opinion is buggy:\n> \n> pick 1234 some commit\n> exec make\n> pick 2345 another commit\n> exec make\n> pick 3456 third commit\n> # pick 4567 empty commit\n> exec make\n> pick 5678 yet another commit\n> exec make\n\nThis reminds me of another bug I stumbled over recently regarding empty\ncommits.\n\nDo this:\n\t# repo preparation:\n\tgit init\n\t:> file1\n\tgit add file1\n\tgit commit -m \"add file1\"\n\t:> file2\n\tgit add file2\n\tgit commit -m \"add file2\"\n\n\t# the bug:\n\tgit checkout -b to-be-rebased master^\n\tgit commit --allow-empty -m \"empty commit\"\n\tgit rebase -i master\n\nIt says \"Nothing to do\".\nUnsurprisingly, the problem persists when you apply other empty commits:\n\n\tgit commit --allow-empty -m \"another empty commit\"\n\tgit rebase -i master\n\nAdding a \"real\" commit solves the problem:\n\n\t:>file3\n\tgit add file3\n\tgit commit -m \"add file3\"\n\nAdding further empty commits is no problem:\n\n\tgit commit --allow-empty -m \"yet another empty commit\"\n\nSo the problem seems to be that rebase -i (like rebase without -i)\nconsiders \"empty commits\" as commits to be ignored. However, when using\nrebase -i one expects that you can include the empty commit...\n\nAlso, the behavior is odd. When I only have empty commits, a \"git rebase\nmaster\" works as expected like a \"git reset --hard master\" but \"git\nrebase -i\" does nothing.\n\nThe expected behavior would be that the editor shows up with a\ngit-rebase-todo like:\n\t# pick 3d0f6c49 empty commit\n\t# pick bbbc5941 another empty commit\n\tnoop\n\nThanks\nStephan\n"},{"id":"327042","messageId":"alpine.DEB.2.21.1.1708231639470.19382@virtualbox","threadId":"46654","inReplyTo":"36e9c381-81b0-ae71-153a-0bcabc59856c@gmx.net","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-08-23T14:40:17Z","receivedAt":"2017-08-23T14:40:49Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 23 Aug 2017, Stephan Beyer wrote:\n\n> On 08/23/2017 01:08 AM, Stefan Beller wrote:\n> > The editor opened proposing the following instruction sheet,\n> > which in my opinion is buggy:\n> > \n> > pick 1234 some commit\n> > exec make\n> > pick 2345 another commit\n> > exec make\n> > pick 3456 third commit\n> > # pick 4567 empty commit\n> > exec make\n> > pick 5678 yet another commit\n> > exec make\n> \n> This reminds me of another bug I stumbled over recently regarding empty\n> commits.\n> \n> Do this:\n> \t# repo preparation:\n> \tgit init\n> \t:> file1\n> \tgit add file1\n> \tgit commit -m \"add file1\"\n> \t:> file2\n> \tgit add file2\n> \tgit commit -m \"add file2\"\n> \n> \t# the bug:\n> \tgit checkout -b to-be-rebased master^\n> \tgit commit --allow-empty -m \"empty commit\"\n> \tgit rebase -i master\n> \n> It says \"Nothing to do\".\n> Unsurprisingly, the problem persists when you apply other empty commits:\n> \n> \tgit commit --allow-empty -m \"another empty commit\"\n> \tgit rebase -i master\n> \n> Adding a \"real\" commit solves the problem:\n> \n> \t:>file3\n> \tgit add file3\n> \tgit commit -m \"add file3\"\n> \n> Adding further empty commits is no problem:\n> \n> \tgit commit --allow-empty -m \"yet another empty commit\"\n> \n> So the problem seems to be that rebase -i (like rebase without -i)\n> considers \"empty commits\" as commits to be ignored. However, when using\n> rebase -i one expects that you can include the empty commit...\n> \n> Also, the behavior is odd. When I only have empty commits, a \"git rebase\n> master\" works as expected like a \"git reset --hard master\" but \"git\n> rebase -i\" does nothing.\n> \n> The expected behavior would be that the editor shows up with a\n> git-rebase-todo like:\n> \t# pick 3d0f6c49 empty commit\n> \t# pick bbbc5941 another empty commit\n> \tnoop\n\nThese days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\nyou want to do that, too?\n\nCiao,\nDscho\n"},{"id":"327043","messageId":"ece1018b-98b1-e9b7-546a-d7c68dacfbb7@gmx.net","threadId":"46654","inReplyTo":"alpine.DEB.2.21.1.1708231639470.19382@virtualbox","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2017-08-23T15:19:36Z","receivedAt":"2017-08-23T15:19:46Z","isPatch":false,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nOn 08/23/2017 04:40 PM, Johannes Schindelin wrote:\n> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\n> you want to do that, too?\n\nThat's a very valuable hint, thank you very much!\n\nBest\nStephan\n"},{"id":"327052","messageId":"CAGZ79ka-SS8ZyZkp6d=0iNtTtQgh2cAhzJSt4m22F88vb1rbhw@mail.gmail.com","threadId":"46654","inReplyTo":"ece1018b-98b1-e9b7-546a-d7c68dacfbb7@gmx.net","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-23T17:29:09Z","receivedAt":"2017-08-23T17:29:15Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 23, 2017 at 8:19 AM, Stephan Beyer wrote:\n> Hi,\n>\n> On 08/23/2017 04:40 PM, Johannes Schindelin wrote:\n>> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\n>> you want to do that, too?\n>\n> That's a very valuable hint, thank you very much!\n\nWhile -k side steps the original problem, it seems like it would\nhave helped me, too.\n\nIs there any value in discussing turning it on by default?\n"},{"id":"327058","messageId":"565221dd-b66f-8b88-1bdd-8d0d55c4dc96@gmx.net","threadId":"46654","inReplyTo":"CAGZ79ka-SS8ZyZkp6d=0iNtTtQgh2cAhzJSt4m22F88vb1rbhw@mail.gmail.com","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2017-08-23T18:06:15Z","receivedAt":"2017-08-23T18:06:22Z","isPatch":false,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"On 08/23/2017 07:29 PM, Stefan Beller wrote:\n> On Wed, Aug 23, 2017 at 8:19 AM, Stephan Beyer wrote:\n>> On 08/23/2017 04:40 PM, Johannes Schindelin wrote:\n>>> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\n>>> you want to do that, too?\n>>\n>> That's a very valuable hint, thank you very much!\n> \n> While -k side steps the original problem, it seems like it would\n> have helped me, too.\n> \n> Is there any value in discussing turning it on by default?\n\nI also wondered why empty commits are \"discriminated\" in such a way.\nI first thought that if you rebase branch A onto B but branch A and B\ncontain commits with the same changes, then these commits would become\nnew empty commits instead of simply being ignored. But I just checked\nthis theory and it is now falsified :)\n\nIt seems empty commits occur *only* if the user wants them to occur\n(--allow-empty). If they occur unintentionally (for example, by\nimporting some SVN), one can eliminate them using filter-branch or\nrebase (by commenting out these picks).\nSo it is still unclear to me, why empty commits are handled in such a\nspecial way.\n\nBest\nStephan\n\nPS: Although -k helps, the original behavior of rebase -i is still a bug.\n"},{"id":"327064","messageId":"xmqqo9r6yw7i.fsf@gitster.mtv.corp.google.com","threadId":"46654","inReplyTo":"565221dd-b66f-8b88-1bdd-8d0d55c4dc96@gmx.net","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-23T18:29:05Z","receivedAt":"2017-08-23T18:29:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephan Beyer writes:\n\n> On 08/23/2017 07:29 PM, Stefan Beller wrote:\n>> On Wed, Aug 23, 2017 at 8:19 AM, Stephan Beyer wrote:\n>>> On 08/23/2017 04:40 PM, Johannes Schindelin wrote:\n>>>> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\n>>>> you want to do that, too?\n>>>\n>>> That's a very valuable hint, thank you very much!\n>> \n>> While -k side steps the original problem, it seems like it would\n>> have helped me, too.\n>> \n>> Is there any value in discussing turning it on by default?\n\nAs we often see on this list, people tend to be blinded by their own\nneeds and fail to strike the right balance when considering the pros\nand cons of a change that may help their immediate desire, I think\nthere is a value in discussing it, even if the conclusion turns out\nthat it is not a good idea to change the default. Rather, make that\n\"even if\" \"especially if\"---we would reach a reasonable balance\nbetween pros and cons and won't have to repeat the discussion.\n\n> I also wondered why empty commits are \"discriminated\" in such a way.\n> I first thought that if you rebase branch A onto B but branch A and B\n> contain commits with the same changes, then these commits would become\n> new empty commits instead of simply being ignored. But I just checked\n> this theory and it is now falsified :)\n\nIf you add an identical patch on each of your two toy example\nbranches and try to rebase one over the other, an early logic that\nculls the set of commits to replay based on patch ID will remove\nthem so you would not even see \"it has become empty\".\n\nI think you can redo your example by having two consecutive commits\non one branch, and then make one commit on the other branch whose\neffect is those two commits' effect combined. Then rebase the\nlatter on top of the former. \"cherry-pick A..B\" seems to be aware\nof this more interesting case and allows you to control the\nbehaviour by having --keep-redundant-commits and --allow-empty as\ntwo separate options, but it seems there is no similar provision in\n\"rebase\".\n"},{"id":"327101","messageId":"D0634C4264E4400492C9424041D7AF02@PhilipOakley","threadId":"46654","inReplyTo":"alpine.DEB.2.21.1.1708231639470.19382@virtualbox","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2017-08-23T22:42:55Z","receivedAt":"2017-08-23T22:43:02Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" \n\n\n>> So the problem seems to be that rebase -i (like rebase without -i)\n>> considers \"empty commits\" as commits to be ignored. However, when using\n>> rebase -i one expects that you can include the empty commit...\n>>\n>> Also, the behavior is odd. When I only have empty commits, a \"git rebase\n>> master\" works as expected like a \"git reset --hard master\" but \"git\n>> rebase -i\" does nothing.\n>>\n>> The expected behavior would be that the editor shows up with a\n>> git-rebase-todo like:\n>> # pick 3d0f6c49 empty commit\n>> # pick bbbc5941 another empty commit\n>> noop\n>\n> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\n> you want to do that, too?\n>\n> Ciao,\n> Dscho\n\nIs the -k option actually documented? I couldn't see it in the man pages. \nI'm guessing it's the same as `--keep-empty`.\n--\nPhilip \n\n"},{"id":"327125","messageId":"374688cd-1d62-ef4f-a989-346ad1953ae8@talktalk.net","threadId":"46654","inReplyTo":"alpine.DEB.2.21.1.1708231639470.19382@virtualbox","subject":"Re: [BUG] rebase -i with only empty commits","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-08-24T13:35:29Z","receivedAt":"2017-08-24T13:35:37Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 23/08/17 15:40, Johannes Schindelin wrote:\n> \n> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe\n> you want to do that, too?\n> \n> Ciao,\n> Dscho\n> \n\nThis is slightly off topic but when I was preparing the patches for [1]\nI noticed a couple of potential bugs with rebase --keep-empty that I\nhaven't got around to doing anything about.\n\n1 - If 'rebase --keep-empty' stops for a conflict resolution then it\ncannot resume. This is because it uses cherry-pick rather than\nformat-patch/am and does not create $GIT_DIR/rebase-apply so there is no\nsaved rebase state for continue to use. In any case the --continue code\ndoes not have the cherry-pick special case for --keep-empty that the\nstartup code does. I think this could be fixed by using an implicit\ninteractive rebase.\n\n2 - The opt-spec allows '--no-keep-empty' but as far as I could see that\noption is never checked for in the rebase code.\n\nBest Wishes\n\nPhillip\n\n\n[1]\nhttps://public-inbox.org/git/20170726102720.15274-1-phillip.wood@talktalk.net/\n"}]}