* [BUG] rebase -i with empty commits + exec
@ 2017-08-22 23:08 Stefan Beller
2017-08-23 9:08 ` [BUG] rebase -i with only empty commits Stephan Beyer
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Beller @ 2017-08-22 23:08 UTC (permalink / raw)
To: git
Currently I am working on a longer series, for which I decided
to keep track of progress in an empty commit. This empty commit
is in the middle of the series (to divide the commits into two sets,
the foundation that I consider stable and the later parts that are not
as stable for my development, they contain things that may be useful)
Then I invoked "git rebase -i <base> -x make" to see
in which shape the series is.
The editor opened proposing the following instruction sheet,
which in my opinion is buggy:
pick 1234 some commit
exec make
pick 2345 another commit
exec make
pick 3456 third commit
# pick 4567 empty commit
exec make
pick 5678 yet another commit
exec make
I think the lines of the empty commit and the following exec should
be swapped, because that exec should work on the third commit.
Maybe we'd want to see another commented exec:
pick 1234 some commit
exec make
pick 2345 another commit
exec make
pick 3456 third commit
exec make
# pick 4567 empty commit
# exec make <- unsure about this line
pick 5678 yet another commit
exec make
Thoughts?
^ permalink raw reply [flat|nested] 9+ messages in thread* [BUG] rebase -i with only empty commits 2017-08-22 23:08 [BUG] rebase -i with empty commits + exec Stefan Beller @ 2017-08-23 9:08 ` Stephan Beyer 2017-08-23 14:40 ` Johannes Schindelin 0 siblings, 1 reply; 9+ messages in thread From: Stephan Beyer @ 2017-08-23 9:08 UTC (permalink / raw) To: Git Hi, On 08/23/2017 01:08 AM, Stefan Beller wrote: > The editor opened proposing the following instruction sheet, > which in my opinion is buggy: > > pick 1234 some commit > exec make > pick 2345 another commit > exec make > pick 3456 third commit > # pick 4567 empty commit > exec make > pick 5678 yet another commit > exec make This reminds me of another bug I stumbled over recently regarding empty commits. Do this: # repo preparation: git init :> file1 git add file1 git commit -m "add file1" :> file2 git add file2 git commit -m "add file2" # the bug: git checkout -b to-be-rebased master^ git commit --allow-empty -m "empty commit" git rebase -i master It says "Nothing to do". Unsurprisingly, the problem persists when you apply other empty commits: git commit --allow-empty -m "another empty commit" git rebase -i master Adding a "real" commit solves the problem: :>file3 git add file3 git commit -m "add file3" Adding further empty commits is no problem: git commit --allow-empty -m "yet another empty commit" So the problem seems to be that rebase -i (like rebase without -i) considers "empty commits" as commits to be ignored. However, when using rebase -i one expects that you can include the empty commit... Also, the behavior is odd. When I only have empty commits, a "git rebase master" works as expected like a "git reset --hard master" but "git rebase -i" does nothing. The expected behavior would be that the editor shows up with a git-rebase-todo like: # pick 3d0f6c49 empty commit # pick bbbc5941 another empty commit noop Thanks Stephan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 9:08 ` [BUG] rebase -i with only empty commits Stephan Beyer @ 2017-08-23 14:40 ` Johannes Schindelin 2017-08-23 15:19 ` Stephan Beyer ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Johannes Schindelin @ 2017-08-23 14:40 UTC (permalink / raw) To: Stephan Beyer; +Cc: Git Hi, On Wed, 23 Aug 2017, Stephan Beyer wrote: > On 08/23/2017 01:08 AM, Stefan Beller wrote: > > The editor opened proposing the following instruction sheet, > > which in my opinion is buggy: > > > > pick 1234 some commit > > exec make > > pick 2345 another commit > > exec make > > pick 3456 third commit > > # pick 4567 empty commit > > exec make > > pick 5678 yet another commit > > exec make > > This reminds me of another bug I stumbled over recently regarding empty > commits. > > Do this: > # repo preparation: > git init > :> file1 > git add file1 > git commit -m "add file1" > :> file2 > git add file2 > git commit -m "add file2" > > # the bug: > git checkout -b to-be-rebased master^ > git commit --allow-empty -m "empty commit" > git rebase -i master > > It says "Nothing to do". > Unsurprisingly, the problem persists when you apply other empty commits: > > git commit --allow-empty -m "another empty commit" > git rebase -i master > > Adding a "real" commit solves the problem: > > :>file3 > git add file3 > git commit -m "add file3" > > Adding further empty commits is no problem: > > git commit --allow-empty -m "yet another empty commit" > > So the problem seems to be that rebase -i (like rebase without -i) > considers "empty commits" as commits to be ignored. However, when using > rebase -i one expects that you can include the empty commit... > > Also, the behavior is odd. When I only have empty commits, a "git rebase > master" works as expected like a "git reset --hard master" but "git > rebase -i" does nothing. > > The expected behavior would be that the editor shows up with a > git-rebase-todo like: > # pick 3d0f6c49 empty commit > # pick bbbc5941 another empty commit > noop These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe you want to do that, too? Ciao, Dscho ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 14:40 ` Johannes Schindelin @ 2017-08-23 15:19 ` Stephan Beyer 2017-08-23 17:29 ` Stefan Beller 2017-08-23 22:42 ` Philip Oakley 2017-08-24 13:35 ` Phillip Wood 2 siblings, 1 reply; 9+ messages in thread From: Stephan Beyer @ 2017-08-23 15:19 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Git Hi, On 08/23/2017 04:40 PM, Johannes Schindelin wrote: > These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe > you want to do that, too? That's a very valuable hint, thank you very much! Best Stephan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 15:19 ` Stephan Beyer @ 2017-08-23 17:29 ` Stefan Beller 2017-08-23 18:06 ` Stephan Beyer 0 siblings, 1 reply; 9+ messages in thread From: Stefan Beller @ 2017-08-23 17:29 UTC (permalink / raw) To: Stephan Beyer; +Cc: Johannes Schindelin, Git On Wed, Aug 23, 2017 at 8:19 AM, Stephan Beyer <s-beyer@gmx.net> wrote: > Hi, > > On 08/23/2017 04:40 PM, Johannes Schindelin wrote: >> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe >> you want to do that, too? > > That's a very valuable hint, thank you very much! While -k side steps the original problem, it seems like it would have helped me, too. Is there any value in discussing turning it on by default? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 17:29 ` Stefan Beller @ 2017-08-23 18:06 ` Stephan Beyer 2017-08-23 18:29 ` Junio C Hamano 0 siblings, 1 reply; 9+ messages in thread From: Stephan Beyer @ 2017-08-23 18:06 UTC (permalink / raw) To: Stefan Beller; +Cc: Johannes Schindelin, Git On 08/23/2017 07:29 PM, Stefan Beller wrote: > On Wed, Aug 23, 2017 at 8:19 AM, Stephan Beyer <s-beyer@gmx.net> wrote: >> On 08/23/2017 04:40 PM, Johannes Schindelin wrote: >>> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe >>> you want to do that, too? >> >> That's a very valuable hint, thank you very much! > > While -k side steps the original problem, it seems like it would > have helped me, too. > > Is there any value in discussing turning it on by default? I also wondered why empty commits are "discriminated" in such a way. I first thought that if you rebase branch A onto B but branch A and B contain commits with the same changes, then these commits would become new empty commits instead of simply being ignored. But I just checked this theory and it is now falsified :) It seems empty commits occur *only* if the user wants them to occur (--allow-empty). If they occur unintentionally (for example, by importing some SVN), one can eliminate them using filter-branch or rebase (by commenting out these picks). So it is still unclear to me, why empty commits are handled in such a special way. Best Stephan PS: Although -k helps, the original behavior of rebase -i is still a bug. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 18:06 ` Stephan Beyer @ 2017-08-23 18:29 ` Junio C Hamano 0 siblings, 0 replies; 9+ messages in thread From: Junio C Hamano @ 2017-08-23 18:29 UTC (permalink / raw) To: Stephan Beyer; +Cc: Stefan Beller, Johannes Schindelin, Git Stephan Beyer <s-beyer@gmx.net> writes: > On 08/23/2017 07:29 PM, Stefan Beller wrote: >> On Wed, Aug 23, 2017 at 8:19 AM, Stephan Beyer <s-beyer@gmx.net> wrote: >>> On 08/23/2017 04:40 PM, Johannes Schindelin wrote: >>>> These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe >>>> you want to do that, too? >>> >>> That's a very valuable hint, thank you very much! >> >> While -k side steps the original problem, it seems like it would >> have helped me, too. >> >> Is there any value in discussing turning it on by default? As we often see on this list, people tend to be blinded by their own needs and fail to strike the right balance when considering the pros and cons of a change that may help their immediate desire, I think there is a value in discussing it, even if the conclusion turns out that it is not a good idea to change the default. Rather, make that "even if" "especially if"---we would reach a reasonable balance between pros and cons and won't have to repeat the discussion. > I also wondered why empty commits are "discriminated" in such a way. > I first thought that if you rebase branch A onto B but branch A and B > contain commits with the same changes, then these commits would become > new empty commits instead of simply being ignored. But I just checked > this theory and it is now falsified :) If you add an identical patch on each of your two toy example branches and try to rebase one over the other, an early logic that culls the set of commits to replay based on patch ID will remove them so you would not even see "it has become empty". I think you can redo your example by having two consecutive commits on one branch, and then make one commit on the other branch whose effect is those two commits' effect combined. Then rebase the latter on top of the former. "cherry-pick A..B" seems to be aware of this more interesting case and allows you to control the behaviour by having --keep-redundant-commits and --allow-empty as two separate options, but it seems there is no similar provision in "rebase". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 14:40 ` Johannes Schindelin 2017-08-23 15:19 ` Stephan Beyer @ 2017-08-23 22:42 ` Philip Oakley 2017-08-24 13:35 ` Phillip Wood 2 siblings, 0 replies; 9+ messages in thread From: Philip Oakley @ 2017-08-23 22:42 UTC (permalink / raw) To: Johannes Schindelin, Stephan Beyer; +Cc: Git From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de> <snip> >> So the problem seems to be that rebase -i (like rebase without -i) >> considers "empty commits" as commits to be ignored. However, when using >> rebase -i one expects that you can include the empty commit... >> >> Also, the behavior is odd. When I only have empty commits, a "git rebase >> master" works as expected like a "git reset --hard master" but "git >> rebase -i" does nothing. >> >> The expected behavior would be that the editor shows up with a >> git-rebase-todo like: >> # pick 3d0f6c49 empty commit >> # pick bbbc5941 another empty commit >> noop > > These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe > you want to do that, too? > > Ciao, > Dscho Is the -k option actually documented? I couldn't see it in the man pages. I'm guessing it's the same as `--keep-empty`. -- Philip ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [BUG] rebase -i with only empty commits 2017-08-23 14:40 ` Johannes Schindelin 2017-08-23 15:19 ` Stephan Beyer 2017-08-23 22:42 ` Philip Oakley @ 2017-08-24 13:35 ` Phillip Wood 2 siblings, 0 replies; 9+ messages in thread From: Phillip Wood @ 2017-08-24 13:35 UTC (permalink / raw) To: Johannes Schindelin, Stephan Beyer; +Cc: Git, Junio C Hamano On 23/08/17 15:40, Johannes Schindelin wrote: > > These days, I reflexively type `rebase -ki` instead of `rebase -i`. Maybe > you want to do that, too? > > Ciao, > Dscho > This is slightly off topic but when I was preparing the patches for [1] I noticed a couple of potential bugs with rebase --keep-empty that I haven't got around to doing anything about. 1 - If 'rebase --keep-empty' stops for a conflict resolution then it cannot resume. This is because it uses cherry-pick rather than format-patch/am and does not create $GIT_DIR/rebase-apply so there is no saved rebase state for continue to use. In any case the --continue code does not have the cherry-pick special case for --keep-empty that the startup code does. I think this could be fixed by using an implicit interactive rebase. 2 - The opt-spec allows '--no-keep-empty' but as far as I could see that option is never checked for in the rebase code. Best Wishes Phillip [1] https://public-inbox.org/git/20170726102720.15274-1-phillip.wood@talktalk.net/ ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-08-24 13:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-22 23:08 [BUG] rebase -i with empty commits + exec Stefan Beller 2017-08-23 9:08 ` [BUG] rebase -i with only empty commits Stephan Beyer 2017-08-23 14:40 ` Johannes Schindelin 2017-08-23 15:19 ` Stephan Beyer 2017-08-23 17:29 ` Stefan Beller 2017-08-23 18:06 ` Stephan Beyer 2017-08-23 18:29 ` Junio C Hamano 2017-08-23 22:42 ` Philip Oakley 2017-08-24 13:35 ` Phillip Wood
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.