Re: [BUG] git-rebase: reword squashes commits in case of merge-conflicts
On Sat, Jun 16, 2018 at 12:08 PM Elijah Newren wrote: > Subject: [PATCH] sequencer: do not squash 'reword' commits when we hit > conflicts > [...] > Signed-off-by: Elijah Newren > --- > diff --git a/t/t3423-rebase-reword.sh b/t/t3423-rebase-reword.sh > @@ -0,0 +1,44 @@ > +test_expect_success 'reword comes after a conflict preserves commit' ' s/comes// > + git checkout stuff^0 && > + > + set_fake_editor && > + test_must_fail env FAKE_LINES="reword 2" \ > + git rebase -i -v master && If some command fails between here and "git rebase --continue" below, then the test will exit with an in-progress (dangling) rebase, which could confuse tests added later to this script. I wonder, therefore, if it would make sense to insert the following cleanup before "git rebase -i": test_when_finished "reset_rebase" && > + git checkout --theirs file-2 && > + git add file-2 && > + FAKE_COMMIT_MESSAGE="feature_b_reworded" git rebase --continue && > + > + test "$(git log -1 --format=%B)" = "feature_b_reworded" && > + test $(git rev-list --count HEAD) = 2 > +'
Re: [BUG] git-rebase: reword squashes commits in case of merge-conflicts
On Mon, Jun 11, 2018 at 06:06:11PM +0200, ch wrote:
> During a recent rebase operation on one of my repositories a number of commits
> unexpectedly ended up getting squashed into other commits. After some
> experiments it turned out that the 'reword' instruction seems to squash the
> referenced commit into the preceding commit if there's a merge-conflict.
>
> Here's a small reproduction recipe to illustrate the behavior:
Indeed I can reproduce. It bisects back to when we switched to using the
rebase-helper builtin, so this started with git-2.13.0. I can also slim down
your reproduction testcase a bit; you don't need any additional commits after
the reword, and the only reason to have commits before it is to induce a merge
conflict by dropping them. The fact that there are lots of fixups floating
around is not necessary.
Anyway, patch below.
-- >8 --
Subject: [PATCH] sequencer: do not squash 'reword' commits when we hit
conflicts
Ever since commit 18633e1a22 ("rebase -i: use the rebase--helper builtin",
2017-02-09), when a commit marked as 'reword' in an interactive rebase
has conflicts and fails to apply, when the rebase is resumed that commit
will be squashed into its parent with its commit message taken.
The issue can be understood better by looking at commit 56dc3ab04b
("sequencer (rebase -i): implement the 'edit' command", 2017-01-02), which
introduced error_with_patch() for the edit command. For the edit command,
it needs to stop the rebase whether or not the patch applies cleanly. If
the patch does apply cleanly, then when it resumes it knows it needs to
amend all changes into the previous commit. If it does not apply cleanly,
then the changes should not be amended. Thus, it passes !res (success of
applying the 'edit' commit) to error_with_patch() for the to_amend flag.
The problematic line of code actually came from commit 04efc8b57c
("sequencer (rebase -i): implement the 'reword' command", 2017-01-02).
Note that to get to this point in the code:
* !!res (i.e. patch application failed)
* item->command < TODO_SQUASH
* item->command != TODO_EDIT
* !is_fixup(item->command) [i.e. not squash or fixup]
So that means this can only be a failed patch application that is either a
pick, revert, or reword. For any of those cases we want a new commit, so
we should not set the to_amend flag.
Reported-by: ch
Signed-off-by: Elijah Newren
---
sequencer.c | 2 +-
t/t3423-rebase-reword.sh | 44
2 files changed, 45 insertions(+), 1 deletion(-)
create mode 100755 t/t3423-rebase-reword.sh
diff --git a/sequencer.c b/sequencer.c
index cca968043e..9e6d1ee368 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3217,7 +3217,7 @@ static int pick_commits(struct todo_list *todo_list,
struct replay_opts *opts)
} else if (res && is_rebase_i(opts) && item->commit)
return res | error_with_patch(item->commit,
item->arg, item->arg_len, opts, res,
- item->command == TODO_REWORD);
+ 0);
} else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(item->arg + item->arg_len);
int saved = *end_of_arg;
diff --git a/t/t3423-rebase-reword.sh b/t/t3423-rebase-reword.sh
new file mode 100755
index 00..31c09efd22
--- /dev/null
+++ b/t/t3423-rebase-reword.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description='git rebase interactive with rewording'
+
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_expect_success 'setup' '
+ test_commit master file-1 test &&
+
+ git checkout -b stuff &&
+
+ test_commit feature_a file-2 aaa &&
+ test_commit feature_b file-2 ddd
+'
+
+test_expect_success 'reword without issues functions as intended' '
+ git checkout stuff^0 &&
+
+ set_fake_editor &&
+ FAKE_LINES="pick 1 reword 2" FAKE_COMMIT_MESSAGE="feature_b_reworded" \
+ git rebase -i -v master &&
+
+ test "$(git log -1 --format=%B)" = "feature_b_reworded" &&
+ test $(git rev-list --count HEAD) = 3
+'
+
+test_expect_success 'reword comes after a conflict preserves commit' '
+ git checkout stuff^0 &&
+
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="reword 2" \
+ git rebase -i -v master &&
+
+ git checkout --theirs file-2 &&
+ git add file-2 &&
+ FAKE_COMMIT_MESSAGE="feature_b_reworded" git rebase --continue &&
+
+ test "$(git log -1 --format=%B)" = "feature_b_reworded" &&
+ test $(git rev-list --count HEAD) = 2
+'
+
+test_done
--
2.18.0.rc2.3.g7fc0d4cb57
Re: [BUG] git-rebase: reword squashes commits in case of merge-conflicts
On 12.06.2018 12:08, Jeff King wrote: > Thanks for a thorough report. I couldn't reproduce it on v2.17.1 on > Linux, which makes me wonder if the issue is related to git-for-windows > somehow. To the best of my knowledge (and a quick scan of "git diff" > results) the code should be the same, though. I gave native Git version 2.17.1 on Ubuntu 18.04 (no WSL this time) a try and was able to reproduce the issue outlined in my first email. In case there is a misunderstanding, when I wrote On 11.06.2018 18:06, ch wrote: > After the rebase the 'stuff' branch only has a single commit even though I'd > expect there to be two according to the instructions that were passed to > git-rebase. I actually meant that there's only a single commit exclusively reachable from 'stuff' after the rebase, i.e. $ git log stuff ^master only lists a single commit while there actually should be two unless I'm missing something. Sorry for being a bit sloppy in my initial report. - ch
Re: [BUG] git-rebase: reword squashes commits in case of merge-conflicts
On Mon, Jun 11, 2018 at 06:06:11PM +0200, ch wrote:
> After the rebase the 'stuff' branch only has a single commit even though I'd
> expect there to be two according to the instructions that were passed to
> git-rebase. It works as expected if there's either no merge-conflict at the
> reword or if the conflicting commit is applied as 'pick'.
>
> I'm running git version 2.17.1.windows.2. I also tried native git version
> 2.7.4
> via WSL (running Ubuntu 16.04.4 LTS) and this version does not exhibit this
> behavior.
Thanks for a thorough report. I couldn't reproduce it on v2.17.1 on
Linux, which makes me wonder if the issue is related to git-for-windows
somehow. To the best of my knowledge (and a quick scan of "git diff"
results) the code should be the same, though.
I'm cc-ing Johannes, who is both the resident interactive-rebase expert
_and_ the Git for Windows maintainer. Copious quoting below.
-Peff
-- >8 --
> Hi all!
>
> During a recent rebase operation on one of my repositories a number of commits
> unexpectedly ended up getting squashed into other commits. After some
> experiments it turned out that the 'reword' instruction seems to squash the
> referenced commit into the preceding commit if there's a merge-conflict.
>
> Here's a small reproduction recipe to illustrate the behavior:
>
>1. Create a small test repository using the following Bash script:
>
>
> function add_file()
> {
> echo "$1" > "$2"
> git add "$2"
> git commit -m "$3"
> }
>
> git init .
>
> add_file "test" "file-1" "master"
>
> git checkout -b stuff
>
> add_file "aaa" "file-2" "feature_a"
> add_file "bbb" "file-2" "fixup! feature_a"
> add_file "ccc" "file-2" "fixup! feature_a"
>
> add_file "ddd" "file-2" "feature_b"
> add_file "eee" "file-2" "fixup! feature_b"
> add_file "fff" "file-2" "fixup! feature_b"
>
>
>2. Run
>
> $ git rebase --autosquash --interactive --onto master master stuff
>
> to interactively rebase 'stuff' onto 'master'. This should generate the
> following todo-stream:
>
>
> pick ... feature_a
> fixup ... fixup! feature_a
> fixup fixup! feature_a
> pick feature_b
> fixup ... fixup! feature_b
> fixup ... fixup! feature_b
>
>
>3. Remove the fixup line right before the second pick (i.e. 'fixup
> ')
> in order to enforce a merge-conflict later when applying commit
> .
>
>4. Replace the second pick instruction (i.e. 'pick ') with a
> reword.
>
>5. Launch the rebase operation. It should fail at the 'reword '
> instruction due to a merge-conflict.
>
>6. Resolve the conflict by taking the remote-side (i.e. 'ddd'), add the
> change to the index with git-add and run
>
> $ git rebase --continue
>
> This should spawn the commit message editor and after saving and
> quitting
> the rebase should finish cleanly.
>
> After the rebase the 'stuff' branch only has a single commit even though I'd
> expect there to be two according to the instructions that were passed to
> git-rebase. It works as expected if there's either no merge-conflict at the
> reword or if the conflicting commit is applied as 'pick'.
>
> I'm running git version 2.17.1.windows.2. I also tried native git version
> 2.7.4
> via WSL (running Ubuntu 16.04.4 LTS) and this version does not exhibit this
> behavior.
>
> - ch
