{"thread":{"id":"53179","subject":"New git-rebase backend: no way to drop already-empty commits","startedAt":"2020-04-07T16:30:25Z","lastAt":"2020-04-07T21:55:30Z","messageCount":10,"participants":["Sami Boukortt","Elijah Newren","Junio C Hamano","Bryan Turner"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"394972","messageId":"CAAb+AL8+c6knrQnoi7YOjyM+Wc8-rws-JXVufBc=PciAvPPFJQ@mail.gmail.com","threadId":"53179","inReplyTo":null,"subject":"New git-rebase backend: no way to drop already-empty commits","fromName":"Sami Boukortt","fromEmail":"sami@boukortt.com","sentAt":"2020-04-07T16:30:10Z","receivedAt":"2020-04-07T16:30:25Z","isPatch":false,"sender":{"key":"sami@boukortt.com","avatar":"https://gravatar.com/avatar/92e155a3cfcf052810561e533ec02436a192a9679ec8dc42cb13141737f450fb?d=mp&s=160"},"body":"Hi,\n\nUsing git 2.26.0, I just tried using `git rebase` to strip empty\ncommits from a branch, but it leaves them as-is, even with\n`--empty=drop`. With the “apply” backend, it removes them properly. Am\nI holding it wrong?\n\n`git rebase -i` also doesn’t pre-comment them like it used to.\n\nThanks.\n\nBest,\nSami\n"},{"id":"394975","messageId":"CABPp-BGFinonZJb2u_0-fX3y_UyJ1dY+O40oN0WAzZht0ddJ4w@mail.gmail.com","threadId":"53179","inReplyTo":"CAAb+AL8+c6knrQnoi7YOjyM+Wc8-rws-JXVufBc=PciAvPPFJQ@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-04-07T16:53:53Z","receivedAt":"2020-04-07T16:54:07Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Apr 7, 2020 at 9:33 AM Sami Boukortt wrote:\n>\n> Hi,\n>\n> Using git 2.26.0, I just tried using `git rebase` to strip empty\n> commits from a branch, but it leaves them as-is, even with\n> `--empty=drop`. With the “apply” backend, it removes them properly. Am\n> I holding it wrong?\n>\n> `git rebase -i` also doesn’t pre-comment them like it used to.\n\nYes, from the manpage:\n\n\"\"\"\n--empty={drop,keep,ask}::\n How to handle commits that are not empty to start and are not\n clean cherry-picks of any upstream commit, but which become\n empty after rebasing (because they contain a subset of already\n upstream changes). With drop (the default), commits that\n become empty are dropped....\n\"\"\"\n\nand\n\n\"\"\"\nEmpty commits\n~~~~~~~~~~~~~\n\nThe apply backend unfortunately drops intentionally empty commits, i.e.\ncommits that started empty, though these are rare in practice. It\nalso drops commits that become empty and has no option for controlling\nthis behavior.\n\nThe merge backend keeps intentionally empty commits. Similar to the\napply backend, by default the merge backend drops commits that become\nempty unless -i/--interactive is specified (in which case it stops and\nasks the user what to do). The merge backend also has an\n--empty={drop,keep,ask} option for changing the behavior of handling\ncommits that become empty.\n\"\"\"\n\nTo remove previously intentional commits, whether empty or not, use -i\nand remove the lines corresponding to the commits you don't want.\n\nHope that helps,\nElijah\n"},{"id":"394978","messageId":"CAAb+AL-95RKhTghJX4uLxiM3dqZW6U914XBXaLcwK+gC_FezWw@mail.gmail.com","threadId":"53179","inReplyTo":"CABPp-BGFinonZJb2u_0-fX3y_UyJ1dY+O40oN0WAzZht0ddJ4w@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Sami Boukortt","fromEmail":"sami@boukortt.com","sentAt":"2020-04-07T17:27:54Z","receivedAt":"2020-04-07T17:28:11Z","isPatch":false,"sender":{"key":"sami@boukortt.com","avatar":"https://gravatar.com/avatar/92e155a3cfcf052810561e533ec02436a192a9679ec8dc42cb13141737f450fb?d=mp&s=160"},"body":"Thank you for the very prompt response.\n\nLe mar. 7 avr. 2020 à 18:54, Elijah Newren a écrit :\n>\n> On Tue, Apr 7, 2020 at 9:33 AM Sami Boukortt wrote:\n> >\n> > Hi,\n> >\n> > Using git 2.26.0, I just tried using `git rebase` to strip empty\n> > commits from a branch, but it leaves them as-is, even with\n> > `--empty=drop`. With the “apply” backend, it removes them properly. Am\n> > I holding it wrong?\n> >\n> > `git rebase -i` also doesn’t pre-comment them like it used to.\n>\n> Yes, from the manpage:\n>\n> \"\"\"\n> […]\n> \"\"\"\n\nD’oh, not sure how I missed this. :) Thanks.\n\n> To remove previously intentional commits, whether empty or not, use -i\n> and remove the lines corresponding to the commits you don't want.\n\nSadly, that is somewhat inconvenient, as those commits are not\nactually “intentional” from my viewpoint (though I understand that git\nhas no way of knowing this), but rather created by another tool\n(git-imerge), which means that I have to check each commit\nindividually and risk mistakes. The old `rebase -i` behavior, where\nsuch commits were automatically commented out, would be an acceptable\ncompromise, or even a comment added at the end of the commit line (so\nthat they are still kept if the editor is closed without changing the\nrebase list). If there are plans to eventually remove the “apply”\nbackend, could that workaround be considered?\n\nAlternatively, I could also use `git filter-branch` (with\n`--prune-empty`), but apparently, its use is heavily discouraged.\n\nThanks again,\nSami\n"},{"id":"394983","messageId":"xmqqh7xvtby2.fsf@gitster.c.googlers.com","threadId":"53179","inReplyTo":"CABPp-BGFinonZJb2u_0-fX3y_UyJ1dY+O40oN0WAzZht0ddJ4w@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-07T17:58:29Z","receivedAt":"2020-04-07T17:58:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Yes, from the manpage:\n>\n> ...\n>\n> and\n>\n> \"\"\"\n> Empty commits\n> ~~~~~~~~~~~~~\n>\n> The apply backend unfortunately drops intentionally empty commits, i.e.\n> commits that started empty, though these are rare in practice. It\n> also drops commits that become empty and has no option for controlling\n> this behavior.\n\nThis is a very good illustration that shows why \"switch the default\nand retire the apply backend\" deserves to be cooked for quite a long\ntime. The 'apply' dropping empty commits may have looked like an\n'unfortunate' thing to whoever wrote the above paragraph in the\ndocumentation, but it clearly shows that person (me included) did\nnot think of the ramifications deeply enough that there may be valid\nworkflows that _depend_ on the behaviour.\n\nAs we will be dropping 'apply' that could be used as an escape\nhatch, before we do so, we should teach the other backends an\nalternate escape hatch to help those who have been depending on the\nbehaviour of 'apply' that discards the empty ones, whether they\nbecome empty, or they are empty from the beginning. I think the\n\"has contents originally but becomes empty\" side is already taken\ncare of, so we'd need to make sure it is easy to optionally discard\nthe ones that are originally empty.\n\nThanks.\n"},{"id":"394985","messageId":"CABPp-BE1mDQYcj6nRfS4osn8-E_WdV-Nenn9dQSA+JCr+pjKLQ@mail.gmail.com","threadId":"53179","inReplyTo":"CAAb+AL-95RKhTghJX4uLxiM3dqZW6U914XBXaLcwK+gC_FezWw@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-04-07T18:03:22Z","receivedAt":"2020-04-07T18:03:36Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Apr 7, 2020 at 10:28 AM Sami Boukortt wrote:\n>\n> Thank you for the very prompt response.\n>\n> Le mar. 7 avr. 2020 à 18:54, Elijah Newren a écrit :\n> >\n> > On Tue, Apr 7, 2020 at 9:33 AM Sami Boukortt wrote:\n> > >\n> > > Hi,\n> > >\n> > > Using git 2.26.0, I just tried using `git rebase` to strip empty\n> > > commits from a branch, but it leaves them as-is, even with\n> > > `--empty=drop`. With the “apply” backend, it removes them properly. Am\n> > > I holding it wrong?\n> > >\n> > > `git rebase -i` also doesn’t pre-comment them like it used to.\n> >\n> > Yes, from the manpage:\n> >\n> > \"\"\"\n> > […]\n> > \"\"\"\n>\n> D’oh, not sure how I missed this. :) Thanks.\n>\n> > To remove previously intentional commits, whether empty or not, use -i\n> > and remove the lines corresponding to the commits you don't want.\n>\n> Sadly, that is somewhat inconvenient, as those commits are not\n> actually “intentional” from my viewpoint (though I understand that git\n> has no way of knowing this), but rather created by another tool\n> (git-imerge), which means that I have to check each commit\n\ngit-imerge creates non-merge commits? Is this in the case when it is\nacting like rebase? If so, is this possibly a bug in git-imerge (in\nthat it doesn't drop commits which become empty)?\n\n> individually and risk mistakes. The old `rebase -i` behavior, where\n> such commits were automatically commented out, would be an acceptable\n> compromise, or even a comment added at the end of the commit line (so\n> that they are still kept if the editor is closed without changing the\n> rebase list). If there are plans to eventually remove the “apply”\n> backend, could that workaround be considered?\n\nAutomatically commenting them out is bad; that causes frustration for\npeople having to uncomment all the commits they intended to add.\n\nBut we could add some kind of option.\n\n> Alternatively, I could also use `git filter-branch` (with\n> `--prune-empty`), but apparently, its use is heavily discouraged.\n\nYou could use\n git filter-repo --prune-empty always\n"},{"id":"394988","messageId":"CAAb+AL9KaFbOk2eMPbn0zCKC4KfX-chx4O3ndLz=zpdgpq_MVQ@mail.gmail.com","threadId":"53179","inReplyTo":"CABPp-BE1mDQYcj6nRfS4osn8-E_WdV-Nenn9dQSA+JCr+pjKLQ@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Sami Boukortt","fromEmail":"sami@boukortt.com","sentAt":"2020-04-07T18:16:47Z","receivedAt":"2020-04-07T18:17:00Z","isPatch":false,"sender":{"key":"sami@boukortt.com","avatar":"https://gravatar.com/avatar/92e155a3cfcf052810561e533ec02436a192a9679ec8dc42cb13141737f450fb?d=mp&s=160"},"body":"Le mar. 7 avr. 2020 à 20:03, Elijah Newren a écrit :\n>\n> On Tue, Apr 7, 2020 at 10:28 AM Sami Boukortt wrote:\n> >\n> > […]\n> >\n> > Sadly, that is somewhat inconvenient, as those commits are not\n> > actually “intentional” from my viewpoint (though I understand that git\n> > has no way of knowing this), but rather created by another tool\n> > (git-imerge), which means that I have to check each commit\n>\n> git-imerge creates non-merge commits? Is this in the case when it is\n> acting like rebase? If so, is this possibly a bug in git-imerge (in\n> that it doesn't drop commits which become empty)?\n\nIt is indeed with `git imerge rebase`. I don’t know enough about\ngit-imerge’s internals to know how easy that would be to fix, but it\ndoes seem as though that would be the ideal approach.\n\n> > individually and risk mistakes. The old `rebase -i` behavior, where\n> > such commits were automatically commented out, would be an acceptable\n> > compromise, or even a comment added at the end of the commit line (so\n> > that they are still kept if the editor is closed without changing the\n> > rebase list). If there are plans to eventually remove the “apply”\n> > backend, could that workaround be considered?\n>\n> Automatically commenting them out is bad; that causes frustration for\n> people having to uncomment all the commits they intended to add.\n>\n> But we could add some kind of option.\n\nInstead of automatically commenting them out, how about automatically\nannotating them while leaving them in the rebase list, like so:\n\n pick 8441f42 Commit A\n pick e3fcaf8 Commit B # empty\n pick af34c53 Commit C\n\n> > Alternatively, I could also use `git filter-branch` (with\n> > `--prune-empty`), but apparently, its use is heavily discouraged.\n>\n> You could use\n> git filter-repo --prune-empty always\n\nThat does seem like it would work, but wouldn’t it process the entire\nrepository (as opposed to filter-branch which can take a list of\nrevisions)?\n"},{"id":"394991","messageId":"CABPp-BEVaA8fnsOF9LX3x12LgYp2fF5aT_Wb3oAOtgOk6XxwMw@mail.gmail.com","threadId":"53179","inReplyTo":"CAAb+AL9KaFbOk2eMPbn0zCKC4KfX-chx4O3ndLz=zpdgpq_MVQ@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-04-07T18:29:20Z","receivedAt":"2020-04-07T18:29:34Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Apr 7, 2020 at 11:16 AM Sami Boukortt wrote:\n>\n> Le mar. 7 avr. 2020 à 20:03, Elijah Newren a écrit :\n> >\n> > On Tue, Apr 7, 2020 at 10:28 AM Sami Boukortt wrote:\n> > >\n> > > […]\n> > >\n> > > Sadly, that is somewhat inconvenient, as those commits are not\n> > > actually “intentional” from my viewpoint (though I understand that git\n> > > has no way of knowing this), but rather created by another tool\n> > > (git-imerge), which means that I have to check each commit\n> >\n> > git-imerge creates non-merge commits? Is this in the case when it is\n> > acting like rebase? If so, is this possibly a bug in git-imerge (in\n> > that it doesn't drop commits which become empty)?\n>\n> It is indeed with `git imerge rebase`. I don’t know enough about\n> git-imerge’s internals to know how easy that would be to fix, but it\n> does seem as though that would be the ideal approach.\n\nI don't either; maybe Michael Haggerty (cc'ed) can chime in on this\nside of things.\n\n> > > individually and risk mistakes. The old `rebase -i` behavior, where\n> > > such commits were automatically commented out, would be an acceptable\n> > > compromise, or even a comment added at the end of the commit line (so\n> > > that they are still kept if the editor is closed without changing the\n> > > rebase list). If there are plans to eventually remove the “apply”\n> > > backend, could that workaround be considered?\n> >\n> > Automatically commenting them out is bad; that causes frustration for\n> > people having to uncomment all the commits they intended to add.\n> >\n> > But we could add some kind of option.\n>\n> Instead of automatically commenting them out, how about automatically\n> annotating them while leaving them in the rebase list, like so:\n>\n> pick 8441f42 Commit A\n> pick e3fcaf8 Commit B # empty\n> pick af34c53 Commit C\n\nThat seems reasonable. Of course, that would make it specific to -i;\nI'm curious if that's good enough or if there are other cases out\nthere that need more. We could at least start with this, though.\n\n> > > Alternatively, I could also use `git filter-branch` (with\n> > > `--prune-empty`), but apparently, its use is heavily discouraged.\n> >\n> > You could use\n> > git filter-repo --prune-empty always\n>\n> That does seem like it would work, but wouldn’t it process the entire\n> repository (as opposed to filter-branch which can take a list of\n> revisions)?\n\nBy default, yes it processes the entire repository. You can pass\nrevisions to filter-repo with the --refs option.\n"},{"id":"394999","messageId":"CAGyf7-GNHPB_9yDQhwzt5VjEe3-SOPM-DAntTojo+zmLoCzVzA@mail.gmail.com","threadId":"53179","inReplyTo":"xmqqh7xvtby2.fsf@gitster.c.googlers.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2020-04-07T19:43:45Z","receivedAt":"2020-04-07T19:43:59Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Tue, Apr 7, 2020 at 10:58 AM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > Yes, from the manpage:\n> >\n> > ...\n> >\n> > and\n> >\n> > \"\"\"\n> > Empty commits\n> > ~~~~~~~~~~~~~\n> >\n> > The apply backend unfortunately drops intentionally empty commits, i.e.\n> > commits that started empty, though these are rare in practice. It\n> > also drops commits that become empty and has no option for controlling\n> > this behavior.\n>\n> This is a very good illustration that shows why \"switch the default\n> and retire the apply backend\" deserves to be cooked for quite a long\n> time. The 'apply' dropping empty commits may have looked like an\n> 'unfortunate' thing to whoever wrote the above paragraph in the\n> documentation, but it clearly shows that person (me included) did\n> not think of the ramifications deeply enough that there may be valid\n> workflows that _depend_ on the behaviour.\n>\n> As we will be dropping 'apply' that could be used as an escape\n> hatch, before we do so, we should teach the other backends an\n> alternate escape hatch to help those who have been depending on the\n> behaviour of 'apply' that discards the empty ones, whether they\n> become empty, or they are empty from the beginning. I think the\n> \"has contents originally but becomes empty\" side is already taken\n> care of, so we'd need to make sure it is easy to optionally discard\n> the ones that are originally empty.\n\nI wonder if the existing --empty could be extended, such that \"drop\"\nwas treated as a (deprecated?) synonym for \"drop-new\" (a new entry in\nthe list), with a new \"drop-all\". That way end users could pass\n--empty=drop-all to get the old \"apply\" behavior with \"merge\".\nSomething like \"--empty={drop-all,drop-new[,drop],keep,ask}::\". A name\nlike \"drop-all\" seems more obvious/intuitive than simply \"all\" or\n\"always\".\n\nJust a thought from someone else who was also bitten by this behavior change.\n\n>\n> Thanks.\n"},{"id":"395000","messageId":"CABPp-BF8eokQTVwgo80ffq3tn=NA=mPf7oymce5P4itDyZBiMg@mail.gmail.com","threadId":"53179","inReplyTo":"xmqqh7xvtby2.fsf@gitster.c.googlers.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-04-07T19:45:55Z","receivedAt":"2020-04-07T19:46:11Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Apr 7, 2020 at 10:58 AM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > Yes, from the manpage:\n> >\n> > ...\n> >\n> > and\n> >\n> > \"\"\"\n> > Empty commits\n> > ~~~~~~~~~~~~~\n> >\n> > The apply backend unfortunately drops intentionally empty commits, i.e.\n> > commits that started empty, though these are rare in practice. It\n> > also drops commits that become empty and has no option for controlling\n> > this behavior.\n>\n> This is a very good illustration that shows why \"switch the default\n> and retire the apply backend\" deserves to be cooked for quite a long\n> time.\n\nYep. I suspect it may be a long time before we have --whitespace=fix\nimplemented anyway (because I'm not sure there are folks who want to\ndig into xdiff), but even if it was implemented soon, retiring a\nbackend that has been the default for many, many years is the kind of\nthing that should wait a while.\n\n> The 'apply' dropping empty commits may have looked like an\n> 'unfortunate' thing to whoever wrote the above paragraph in the\n> documentation, but it clearly shows that person (me included) did\n> not think of the ramifications deeply enough that there may be valid\n> workflows that _depend_ on the behaviour.\n\nGuilty as charged. I did try to highlight the empty handling for\nreviewers when I posted the backend switch series (see e.g.\nhttps://lore.kernel.org/git/pull.679.git.git.1576861788.gitgitgadget@gmail.com/\nand\nhttps://lore.kernel.org/git/pull.679.v3.git.git.1577217299.gitgitgadget@gmail.com/\nand some of the emails between Phillip and I directly about that topic\nwhile discussing the series), but it's understandable that this could\nbe overlooked by not just me but reviewers as well -- the workaround\nof running an interactive rebase and remove the lines they don't want\nprobably seemed really simple and clear.\n\n> As we will be dropping 'apply' that could be used as an escape\n> hatch, before we do so, we should teach the other backends an\n> alternate escape hatch to help those who have been depending on the\n> behaviour of 'apply' that discards the empty ones, whether they\n> become empty, or they are empty from the beginning. I think the\n> \"has contents originally but becomes empty\" side is already taken\n> care of, so we'd need to make sure it is easy to optionally discard\n> the ones that are originally empty.\n>\n> Thanks.\n\nI will take a look into it, using (or at least starting with) the\nsuggestion Sami provided.\n"},{"id":"395011","messageId":"xmqqd08jrmeu.fsf@gitster.c.googlers.com","threadId":"53179","inReplyTo":"CABPp-BF8eokQTVwgo80ffq3tn=NA=mPf7oymce5P4itDyZBiMg@mail.gmail.com","subject":"Re: New git-rebase backend: no way to drop already-empty commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-07T21:55:21Z","receivedAt":"2020-04-07T21:55:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> As we will be dropping 'apply' that could be used as an escape\n>> hatch, before we do so, we should teach the other backends an\n>> alternate escape hatch to help those who have been depending on the\n>> behaviour of 'apply' that discards the empty ones, whether they\n>> become empty, or they are empty from the beginning. I think the\n>> \"has contents originally but becomes empty\" side is already taken\n>> care of, so we'd need to make sure it is easy to optionally discard\n>> the ones that are originally empty.\n>>\n>> Thanks.\n>\n> I will take a look into it, using (or at least starting with) the\n> suggestion Sami provided.\n\nThanks.\n"}]}