* New git-rebase backend: no way to drop already-empty commits @ 2020-04-07 16:30 Sami Boukortt 2020-04-07 16:53 ` Elijah Newren 0 siblings, 1 reply; 10+ messages in thread From: Sami Boukortt @ 2020-04-07 16:30 UTC (permalink / raw) To: git Hi, Using git 2.26.0, I just tried using `git rebase` to strip empty commits from a branch, but it leaves them as-is, even with `--empty=drop`. With the “apply” backend, it removes them properly. Am I holding it wrong? `git rebase -i` also doesn’t pre-comment them like it used to. Thanks. Best, Sami ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 16:30 New git-rebase backend: no way to drop already-empty commits Sami Boukortt @ 2020-04-07 16:53 ` Elijah Newren 2020-04-07 17:27 ` Sami Boukortt 2020-04-07 17:58 ` Junio C Hamano 0 siblings, 2 replies; 10+ messages in thread From: Elijah Newren @ 2020-04-07 16:53 UTC (permalink / raw) To: Sami Boukortt; +Cc: Git Mailing List On Tue, Apr 7, 2020 at 9:33 AM Sami Boukortt <sami@boukortt.com> wrote: > > Hi, > > Using git 2.26.0, I just tried using `git rebase` to strip empty > commits from a branch, but it leaves them as-is, even with > `--empty=drop`. With the “apply” backend, it removes them properly. Am > I holding it wrong? > > `git rebase -i` also doesn’t pre-comment them like it used to. Yes, from the manpage: """ --empty={drop,keep,ask}:: How to handle commits that are not empty to start and are not clean cherry-picks of any upstream commit, but which become empty after rebasing (because they contain a subset of already upstream changes). With drop (the default), commits that become empty are dropped.... """ and """ Empty commits ~~~~~~~~~~~~~ The apply backend unfortunately drops intentionally empty commits, i.e. commits that started empty, though these are rare in practice. It also drops commits that become empty and has no option for controlling this behavior. The merge backend keeps intentionally empty commits. Similar to the apply backend, by default the merge backend drops commits that become empty unless -i/--interactive is specified (in which case it stops and asks the user what to do). The merge backend also has an --empty={drop,keep,ask} option for changing the behavior of handling commits that become empty. """ To remove previously intentional commits, whether empty or not, use -i and remove the lines corresponding to the commits you don't want. Hope that helps, Elijah ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 16:53 ` Elijah Newren @ 2020-04-07 17:27 ` Sami Boukortt 2020-04-07 18:03 ` Elijah Newren 2020-04-07 17:58 ` Junio C Hamano 1 sibling, 1 reply; 10+ messages in thread From: Sami Boukortt @ 2020-04-07 17:27 UTC (permalink / raw) To: Elijah Newren; +Cc: Git Mailing List Thank you for the very prompt response. Le mar. 7 avr. 2020 à 18:54, Elijah Newren <newren@gmail.com> a écrit : > > On Tue, Apr 7, 2020 at 9:33 AM Sami Boukortt <sami@boukortt.com> wrote: > > > > Hi, > > > > Using git 2.26.0, I just tried using `git rebase` to strip empty > > commits from a branch, but it leaves them as-is, even with > > `--empty=drop`. With the “apply” backend, it removes them properly. Am > > I holding it wrong? > > > > `git rebase -i` also doesn’t pre-comment them like it used to. > > Yes, from the manpage: > > """ > […] > """ D’oh, not sure how I missed this. :) Thanks. > To remove previously intentional commits, whether empty or not, use -i > and remove the lines corresponding to the commits you don't want. Sadly, that is somewhat inconvenient, as those commits are not actually “intentional” from my viewpoint (though I understand that git has no way of knowing this), but rather created by another tool (git-imerge), which means that I have to check each commit individually and risk mistakes. The old `rebase -i` behavior, where such commits were automatically commented out, would be an acceptable compromise, or even a comment added at the end of the commit line (so that they are still kept if the editor is closed without changing the rebase list). If there are plans to eventually remove the “apply” backend, could that workaround be considered? Alternatively, I could also use `git filter-branch` (with `--prune-empty`), but apparently, its use is heavily discouraged. Thanks again, Sami ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 17:27 ` Sami Boukortt @ 2020-04-07 18:03 ` Elijah Newren 2020-04-07 18:16 ` Sami Boukortt 0 siblings, 1 reply; 10+ messages in thread From: Elijah Newren @ 2020-04-07 18:03 UTC (permalink / raw) To: Sami Boukortt; +Cc: Git Mailing List, Michael Haggerty On Tue, Apr 7, 2020 at 10:28 AM Sami Boukortt <sami@boukortt.com> wrote: > > Thank you for the very prompt response. > > Le mar. 7 avr. 2020 à 18:54, Elijah Newren <newren@gmail.com> a écrit : > > > > On Tue, Apr 7, 2020 at 9:33 AM Sami Boukortt <sami@boukortt.com> wrote: > > > > > > Hi, > > > > > > Using git 2.26.0, I just tried using `git rebase` to strip empty > > > commits from a branch, but it leaves them as-is, even with > > > `--empty=drop`. With the “apply” backend, it removes them properly. Am > > > I holding it wrong? > > > > > > `git rebase -i` also doesn’t pre-comment them like it used to. > > > > Yes, from the manpage: > > > > """ > > […] > > """ > > D’oh, not sure how I missed this. :) Thanks. > > > To remove previously intentional commits, whether empty or not, use -i > > and remove the lines corresponding to the commits you don't want. > > Sadly, that is somewhat inconvenient, as those commits are not > actually “intentional” from my viewpoint (though I understand that git > has no way of knowing this), but rather created by another tool > (git-imerge), which means that I have to check each commit git-imerge creates non-merge commits? Is this in the case when it is acting like rebase? If so, is this possibly a bug in git-imerge (in that it doesn't drop commits which become empty)? > individually and risk mistakes. The old `rebase -i` behavior, where > such commits were automatically commented out, would be an acceptable > compromise, or even a comment added at the end of the commit line (so > that they are still kept if the editor is closed without changing the > rebase list). If there are plans to eventually remove the “apply” > backend, could that workaround be considered? Automatically commenting them out is bad; that causes frustration for people having to uncomment all the commits they intended to add. But we could add some kind of option. > Alternatively, I could also use `git filter-branch` (with > `--prune-empty`), but apparently, its use is heavily discouraged. You could use git filter-repo --prune-empty always ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 18:03 ` Elijah Newren @ 2020-04-07 18:16 ` Sami Boukortt 2020-04-07 18:29 ` Elijah Newren 0 siblings, 1 reply; 10+ messages in thread From: Sami Boukortt @ 2020-04-07 18:16 UTC (permalink / raw) To: Elijah Newren; +Cc: Git Mailing List, Michael Haggerty Le mar. 7 avr. 2020 à 20:03, Elijah Newren <newren@gmail.com> a écrit : > > On Tue, Apr 7, 2020 at 10:28 AM Sami Boukortt <sami@boukortt.com> wrote: > > > > […] > > > > Sadly, that is somewhat inconvenient, as those commits are not > > actually “intentional” from my viewpoint (though I understand that git > > has no way of knowing this), but rather created by another tool > > (git-imerge), which means that I have to check each commit > > git-imerge creates non-merge commits? Is this in the case when it is > acting like rebase? If so, is this possibly a bug in git-imerge (in > that it doesn't drop commits which become empty)? It is indeed with `git imerge rebase`. I don’t know enough about git-imerge’s internals to know how easy that would be to fix, but it does seem as though that would be the ideal approach. > > individually and risk mistakes. The old `rebase -i` behavior, where > > such commits were automatically commented out, would be an acceptable > > compromise, or even a comment added at the end of the commit line (so > > that they are still kept if the editor is closed without changing the > > rebase list). If there are plans to eventually remove the “apply” > > backend, could that workaround be considered? > > Automatically commenting them out is bad; that causes frustration for > people having to uncomment all the commits they intended to add. > > But we could add some kind of option. Instead of automatically commenting them out, how about automatically annotating them while leaving them in the rebase list, like so: pick 8441f42 Commit A pick e3fcaf8 Commit B # empty pick af34c53 Commit C > > Alternatively, I could also use `git filter-branch` (with > > `--prune-empty`), but apparently, its use is heavily discouraged. > > You could use > git filter-repo --prune-empty always That does seem like it would work, but wouldn’t it process the entire repository (as opposed to filter-branch which can take a list of revisions)? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 18:16 ` Sami Boukortt @ 2020-04-07 18:29 ` Elijah Newren 0 siblings, 0 replies; 10+ messages in thread From: Elijah Newren @ 2020-04-07 18:29 UTC (permalink / raw) To: Sami Boukortt; +Cc: Git Mailing List, Michael Haggerty On Tue, Apr 7, 2020 at 11:16 AM Sami Boukortt <sami@boukortt.com> wrote: > > Le mar. 7 avr. 2020 à 20:03, Elijah Newren <newren@gmail.com> a écrit : > > > > On Tue, Apr 7, 2020 at 10:28 AM Sami Boukortt <sami@boukortt.com> wrote: > > > > > > […] > > > > > > Sadly, that is somewhat inconvenient, as those commits are not > > > actually “intentional” from my viewpoint (though I understand that git > > > has no way of knowing this), but rather created by another tool > > > (git-imerge), which means that I have to check each commit > > > > git-imerge creates non-merge commits? Is this in the case when it is > > acting like rebase? If so, is this possibly a bug in git-imerge (in > > that it doesn't drop commits which become empty)? > > It is indeed with `git imerge rebase`. I don’t know enough about > git-imerge’s internals to know how easy that would be to fix, but it > does seem as though that would be the ideal approach. I don't either; maybe Michael Haggerty (cc'ed) can chime in on this side of things. > > > individually and risk mistakes. The old `rebase -i` behavior, where > > > such commits were automatically commented out, would be an acceptable > > > compromise, or even a comment added at the end of the commit line (so > > > that they are still kept if the editor is closed without changing the > > > rebase list). If there are plans to eventually remove the “apply” > > > backend, could that workaround be considered? > > > > Automatically commenting them out is bad; that causes frustration for > > people having to uncomment all the commits they intended to add. > > > > But we could add some kind of option. > > Instead of automatically commenting them out, how about automatically > annotating them while leaving them in the rebase list, like so: > > pick 8441f42 Commit A > pick e3fcaf8 Commit B # empty > pick af34c53 Commit C That seems reasonable. Of course, that would make it specific to -i; I'm curious if that's good enough or if there are other cases out there that need more. We could at least start with this, though. > > > Alternatively, I could also use `git filter-branch` (with > > > `--prune-empty`), but apparently, its use is heavily discouraged. > > > > You could use > > git filter-repo --prune-empty always > > That does seem like it would work, but wouldn’t it process the entire > repository (as opposed to filter-branch which can take a list of > revisions)? By default, yes it processes the entire repository. You can pass revisions to filter-repo with the --refs option. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 16:53 ` Elijah Newren 2020-04-07 17:27 ` Sami Boukortt @ 2020-04-07 17:58 ` Junio C Hamano 2020-04-07 19:43 ` Bryan Turner 2020-04-07 19:45 ` Elijah Newren 1 sibling, 2 replies; 10+ messages in thread From: Junio C Hamano @ 2020-04-07 17:58 UTC (permalink / raw) To: Elijah Newren; +Cc: Sami Boukortt, Git Mailing List Elijah Newren <newren@gmail.com> writes: > Yes, from the manpage: > > ... > > and > > """ > Empty commits > ~~~~~~~~~~~~~ > > The apply backend unfortunately drops intentionally empty commits, i.e. > commits that started empty, though these are rare in practice. It > also drops commits that become empty and has no option for controlling > this behavior. This is a very good illustration that shows why "switch the default and retire the apply backend" deserves to be cooked for quite a long time. The 'apply' dropping empty commits may have looked like an 'unfortunate' thing to whoever wrote the above paragraph in the documentation, but it clearly shows that person (me included) did not think of the ramifications deeply enough that there may be valid workflows that _depend_ on the behaviour. As we will be dropping 'apply' that could be used as an escape hatch, before we do so, we should teach the other backends an alternate escape hatch to help those who have been depending on the behaviour of 'apply' that discards the empty ones, whether they become empty, or they are empty from the beginning. I think the "has contents originally but becomes empty" side is already taken care of, so we'd need to make sure it is easy to optionally discard the ones that are originally empty. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 17:58 ` Junio C Hamano @ 2020-04-07 19:43 ` Bryan Turner 2020-04-07 19:45 ` Elijah Newren 1 sibling, 0 replies; 10+ messages in thread From: Bryan Turner @ 2020-04-07 19:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Elijah Newren, Sami Boukortt, Git Mailing List On Tue, Apr 7, 2020 at 10:58 AM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > > Yes, from the manpage: > > > > ... > > > > and > > > > """ > > Empty commits > > ~~~~~~~~~~~~~ > > > > The apply backend unfortunately drops intentionally empty commits, i.e. > > commits that started empty, though these are rare in practice. It > > also drops commits that become empty and has no option for controlling > > this behavior. > > This is a very good illustration that shows why "switch the default > and retire the apply backend" deserves to be cooked for quite a long > time. The 'apply' dropping empty commits may have looked like an > 'unfortunate' thing to whoever wrote the above paragraph in the > documentation, but it clearly shows that person (me included) did > not think of the ramifications deeply enough that there may be valid > workflows that _depend_ on the behaviour. > > As we will be dropping 'apply' that could be used as an escape > hatch, before we do so, we should teach the other backends an > alternate escape hatch to help those who have been depending on the > behaviour of 'apply' that discards the empty ones, whether they > become empty, or they are empty from the beginning. I think the > "has contents originally but becomes empty" side is already taken > care of, so we'd need to make sure it is easy to optionally discard > the ones that are originally empty. I wonder if the existing --empty could be extended, such that "drop" was treated as a (deprecated?) synonym for "drop-new" (a new entry in the list), with a new "drop-all". That way end users could pass --empty=drop-all to get the old "apply" behavior with "merge". Something like "--empty={drop-all,drop-new[,drop],keep,ask}::". A name like "drop-all" seems more obvious/intuitive than simply "all" or "always". Just a thought from someone else who was also bitten by this behavior change. > > Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 17:58 ` Junio C Hamano 2020-04-07 19:43 ` Bryan Turner @ 2020-04-07 19:45 ` Elijah Newren 2020-04-07 21:55 ` Junio C Hamano 1 sibling, 1 reply; 10+ messages in thread From: Elijah Newren @ 2020-04-07 19:45 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sami Boukortt, Git Mailing List On Tue, Apr 7, 2020 at 10:58 AM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > > Yes, from the manpage: > > > > ... > > > > and > > > > """ > > Empty commits > > ~~~~~~~~~~~~~ > > > > The apply backend unfortunately drops intentionally empty commits, i.e. > > commits that started empty, though these are rare in practice. It > > also drops commits that become empty and has no option for controlling > > this behavior. > > This is a very good illustration that shows why "switch the default > and retire the apply backend" deserves to be cooked for quite a long > time. Yep. I suspect it may be a long time before we have --whitespace=fix implemented anyway (because I'm not sure there are folks who want to dig into xdiff), but even if it was implemented soon, retiring a backend that has been the default for many, many years is the kind of thing that should wait a while. > The 'apply' dropping empty commits may have looked like an > 'unfortunate' thing to whoever wrote the above paragraph in the > documentation, but it clearly shows that person (me included) did > not think of the ramifications deeply enough that there may be valid > workflows that _depend_ on the behaviour. Guilty as charged. I did try to highlight the empty handling for reviewers when I posted the backend switch series (see e.g. https://lore.kernel.org/git/pull.679.git.git.1576861788.gitgitgadget@gmail.com/ and https://lore.kernel.org/git/pull.679.v3.git.git.1577217299.gitgitgadget@gmail.com/ and some of the emails between Phillip and I directly about that topic while discussing the series), but it's understandable that this could be overlooked by not just me but reviewers as well -- the workaround of running an interactive rebase and remove the lines they don't want probably seemed really simple and clear. > As we will be dropping 'apply' that could be used as an escape > hatch, before we do so, we should teach the other backends an > alternate escape hatch to help those who have been depending on the > behaviour of 'apply' that discards the empty ones, whether they > become empty, or they are empty from the beginning. I think the > "has contents originally but becomes empty" side is already taken > care of, so we'd need to make sure it is easy to optionally discard > the ones that are originally empty. > > Thanks. I will take a look into it, using (or at least starting with) the suggestion Sami provided. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: New git-rebase backend: no way to drop already-empty commits 2020-04-07 19:45 ` Elijah Newren @ 2020-04-07 21:55 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2020-04-07 21:55 UTC (permalink / raw) To: Elijah Newren; +Cc: Sami Boukortt, Git Mailing List Elijah Newren <newren@gmail.com> writes: >> As we will be dropping 'apply' that could be used as an escape >> hatch, before we do so, we should teach the other backends an >> alternate escape hatch to help those who have been depending on the >> behaviour of 'apply' that discards the empty ones, whether they >> become empty, or they are empty from the beginning. I think the >> "has contents originally but becomes empty" side is already taken >> care of, so we'd need to make sure it is easy to optionally discard >> the ones that are originally empty. >> >> Thanks. > > I will take a look into it, using (or at least starting with) the > suggestion Sami provided. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-07 21:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-07 16:30 New git-rebase backend: no way to drop already-empty commits Sami Boukortt 2020-04-07 16:53 ` Elijah Newren 2020-04-07 17:27 ` Sami Boukortt 2020-04-07 18:03 ` Elijah Newren 2020-04-07 18:16 ` Sami Boukortt 2020-04-07 18:29 ` Elijah Newren 2020-04-07 17:58 ` Junio C Hamano 2020-04-07 19:43 ` Bryan Turner 2020-04-07 19:45 ` Elijah Newren 2020-04-07 21:55 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).