* [PATCH v2 1/9] t3404: use test_cmp_rev
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 17:39 ` Junio C Hamano
2019-12-06 16:06 ` [PATCH v2 2/9] cherry-pick: add test for `--skip` advice in `git commit` Phillip Wood
` (7 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
There are a number of places where we compare two revisions with
test $(git rev-parse rev1) = $(git rev-parse rev2)
when these fail there's no indication what has gone wrong and you need
to be running with `-x` to see where the test has failed. Lets use
test_cmp_rev instead.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t/t3404-rebase-interactive.sh | 38 +++++++++++++++++------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c573c99069..d8a05fd439 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -191,7 +191,7 @@ test_expect_success 'no changes are a nop' '
git checkout branch2 &&
git rebase -i F &&
test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" &&
- test $(git rev-parse I) = $(git rev-parse HEAD)
+ test_cmp_rev I HEAD
'
test_expect_success 'test the [branch] option' '
@@ -200,16 +200,16 @@ test_expect_success 'test the [branch] option' '
git commit -m "stop here" &&
git rebase -i F branch2 &&
test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" &&
- test $(git rev-parse I) = $(git rev-parse branch2) &&
- test $(git rev-parse I) = $(git rev-parse HEAD)
+ test_cmp_rev I branch2 &&
+ test_cmp_rev I HEAD
'
test_expect_success 'test --onto <branch>' '
git checkout -b test-onto branch2 &&
git rebase -i --onto branch1 F &&
test "$(git symbolic-ref -q HEAD)" = "refs/heads/test-onto" &&
- test $(git rev-parse HEAD^) = $(git rev-parse branch1) &&
- test $(git rev-parse I) = $(git rev-parse branch2)
+ test_cmp_rev HEAD^ branch1 &&
+ test_cmp_rev I branch2
'
test_expect_success 'rebase on top of a non-conflicting commit' '
@@ -218,12 +218,12 @@ test_expect_success 'rebase on top of a non-conflicting commit' '
git rebase -i branch2 &&
test file6 = $(git diff --name-only original-branch1) &&
test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" &&
- test $(git rev-parse I) = $(git rev-parse branch2) &&
- test $(git rev-parse I) = $(git rev-parse HEAD~2)
+ test_cmp_rev I branch2 &&
+ test_cmp_rev I HEAD~2
'
test_expect_success 'reflog for the branch shows state before rebase' '
- test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1)
+ test_cmp_rev branch1@{1} original-branch1
'
test_expect_success 'reflog for the branch shows correct finish message' '
@@ -280,7 +280,7 @@ test_expect_success 'show conflicted patch' '
test_expect_success 'abort' '
git rebase --abort &&
- test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
+ test_cmp_rev new-branch1 HEAD &&
test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" &&
test_path_is_missing .git/rebase-merge
'
@@ -323,7 +323,7 @@ test_expect_success 'retain authorship w/ conflicts' '
echo resolved >conflict &&
git add conflict &&
git rebase --continue &&
- test $(git rev-parse conflict-a^0) = $(git rev-parse HEAD^) &&
+ test_cmp_rev conflict-a^0 HEAD^ &&
git show >out &&
grep AttributeMe out
'
@@ -340,7 +340,7 @@ test_expect_success 'squash' '
git rebase -i --onto master HEAD~2
) &&
test B = $(cat file7) &&
- test $(git rev-parse HEAD^) = $(git rev-parse master)
+ test_cmp_rev HEAD^ master
'
test_expect_success 'retain authorship when squashing' '
@@ -399,9 +399,9 @@ test_expect_success REBASE_P 'preserve merges with -p' '
git update-index --refresh &&
git diff-files --quiet &&
git diff-index --quiet --cached HEAD -- &&
- test $(git rev-parse HEAD~6) = $(git rev-parse branch1) &&
- test $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) &&
- test $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) &&
+ test_cmp_rev HEAD~6 branch1 &&
+ test_cmp_rev HEAD~4^2 to-be-preserved &&
+ test_cmp_rev HEAD^^2^ HEAD^^^ &&
test $(git show HEAD~5:file1) = B &&
test $(git show HEAD~3:file1) = C &&
test $(git show HEAD:file1) = E &&
@@ -433,7 +433,7 @@ test_expect_success '--continue tries to commit' '
git add file1 &&
FAKE_COMMIT_MESSAGE="chouette!" git rebase --continue
) &&
- test $(git rev-parse HEAD^) = $(git rev-parse new-branch1) &&
+ test_cmp_rev HEAD^ new-branch1 &&
git show HEAD | grep chouette
'
@@ -740,7 +740,7 @@ test_expect_success 'do "noop" when there is nothing to cherry-pick' '
--author="Somebody else <somebody@else.com>" &&
test $(git rev-parse branch3) != $(git rev-parse branch4) &&
git rebase -i branch3 &&
- test $(git rev-parse branch3) = $(git rev-parse branch4)
+ test_cmp_rev branch3 branch4
'
@@ -799,7 +799,7 @@ test_expect_success 'rebase -i continue with unstaged submodule' '
test_must_fail git rebase -i submodule-base &&
git reset &&
git rebase --continue &&
- test $(git rev-parse submodule-base) = $(git rev-parse HEAD)
+ test_cmp_rev submodule-base HEAD
'
test_expect_success 'avoid unnecessary reset' '
@@ -822,7 +822,7 @@ test_expect_success 'reword' '
git rebase -i A &&
git show HEAD | grep "E changed" &&
test $(git rev-parse master) != $(git rev-parse HEAD) &&
- test $(git rev-parse master^) = $(git rev-parse HEAD^) &&
+ test_cmp_rev master^ HEAD^ &&
FAKE_LINES="1 2 reword 3 4" FAKE_COMMIT_MESSAGE="D changed" \
git rebase -i A &&
git show HEAD^ | grep "D changed" &&
@@ -886,7 +886,7 @@ test_expect_success 'always cherry-pick with --no-ff' '
git diff HEAD~$p original-no-ff-branch~$p > out &&
test_must_be_empty out
done &&
- test $(git rev-parse HEAD~3) = $(git rev-parse original-no-ff-branch~3) &&
+ test_cmp_rev HEAD~3 original-no-ff-branch~3 &&
git diff HEAD~3 original-no-ff-branch~3 > out &&
test_must_be_empty out
'
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/9] t3404: use test_cmp_rev
2019-12-06 16:06 ` [PATCH v2 1/9] t3404: use test_cmp_rev Phillip Wood
@ 2019-12-06 17:39 ` Junio C Hamano
0 siblings, 0 replies; 31+ messages in thread
From: Junio C Hamano @ 2019-12-06 17:39 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood
Phillip Wood <phillip.wood123@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> There are a number of places where we compare two revisions with
> test $(git rev-parse rev1) = $(git rev-parse rev2)
> when these fail there's no indication what has gone wrong and you need
> to be running with `-x` to see where the test has failed. Lets use
> test_cmp_rev instead.
Makes sense. Wonder if we can have a version of coccinelle for
shell scripts ;-)
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> t/t3404-rebase-interactive.sh | 38 +++++++++++++++++------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index c573c99069..d8a05fd439 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -191,7 +191,7 @@ test_expect_success 'no changes are a nop' '
> git checkout branch2 &&
> git rebase -i F &&
> test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" &&
> - test $(git rev-parse I) = $(git rev-parse HEAD)
> + test_cmp_rev I HEAD
> '
>
> test_expect_success 'test the [branch] option' '
> @@ -200,16 +200,16 @@ test_expect_success 'test the [branch] option' '
> git commit -m "stop here" &&
> git rebase -i F branch2 &&
> test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" &&
> - test $(git rev-parse I) = $(git rev-parse branch2) &&
> - test $(git rev-parse I) = $(git rev-parse HEAD)
> + test_cmp_rev I branch2 &&
> + test_cmp_rev I HEAD
> '
>
> test_expect_success 'test --onto <branch>' '
> git checkout -b test-onto branch2 &&
> git rebase -i --onto branch1 F &&
> test "$(git symbolic-ref -q HEAD)" = "refs/heads/test-onto" &&
> - test $(git rev-parse HEAD^) = $(git rev-parse branch1) &&
> - test $(git rev-parse I) = $(git rev-parse branch2)
> + test_cmp_rev HEAD^ branch1 &&
> + test_cmp_rev I branch2
> '
>
> test_expect_success 'rebase on top of a non-conflicting commit' '
> @@ -218,12 +218,12 @@ test_expect_success 'rebase on top of a non-conflicting commit' '
> git rebase -i branch2 &&
> test file6 = $(git diff --name-only original-branch1) &&
> test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" &&
> - test $(git rev-parse I) = $(git rev-parse branch2) &&
> - test $(git rev-parse I) = $(git rev-parse HEAD~2)
> + test_cmp_rev I branch2 &&
> + test_cmp_rev I HEAD~2
> '
>
> test_expect_success 'reflog for the branch shows state before rebase' '
> - test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1)
> + test_cmp_rev branch1@{1} original-branch1
> '
>
> test_expect_success 'reflog for the branch shows correct finish message' '
> @@ -280,7 +280,7 @@ test_expect_success 'show conflicted patch' '
>
> test_expect_success 'abort' '
> git rebase --abort &&
> - test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
> + test_cmp_rev new-branch1 HEAD &&
> test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" &&
> test_path_is_missing .git/rebase-merge
> '
> @@ -323,7 +323,7 @@ test_expect_success 'retain authorship w/ conflicts' '
> echo resolved >conflict &&
> git add conflict &&
> git rebase --continue &&
> - test $(git rev-parse conflict-a^0) = $(git rev-parse HEAD^) &&
> + test_cmp_rev conflict-a^0 HEAD^ &&
> git show >out &&
> grep AttributeMe out
> '
> @@ -340,7 +340,7 @@ test_expect_success 'squash' '
> git rebase -i --onto master HEAD~2
> ) &&
> test B = $(cat file7) &&
> - test $(git rev-parse HEAD^) = $(git rev-parse master)
> + test_cmp_rev HEAD^ master
> '
>
> test_expect_success 'retain authorship when squashing' '
> @@ -399,9 +399,9 @@ test_expect_success REBASE_P 'preserve merges with -p' '
> git update-index --refresh &&
> git diff-files --quiet &&
> git diff-index --quiet --cached HEAD -- &&
> - test $(git rev-parse HEAD~6) = $(git rev-parse branch1) &&
> - test $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) &&
> - test $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) &&
> + test_cmp_rev HEAD~6 branch1 &&
> + test_cmp_rev HEAD~4^2 to-be-preserved &&
> + test_cmp_rev HEAD^^2^ HEAD^^^ &&
> test $(git show HEAD~5:file1) = B &&
> test $(git show HEAD~3:file1) = C &&
> test $(git show HEAD:file1) = E &&
> @@ -433,7 +433,7 @@ test_expect_success '--continue tries to commit' '
> git add file1 &&
> FAKE_COMMIT_MESSAGE="chouette!" git rebase --continue
> ) &&
> - test $(git rev-parse HEAD^) = $(git rev-parse new-branch1) &&
> + test_cmp_rev HEAD^ new-branch1 &&
> git show HEAD | grep chouette
> '
>
> @@ -740,7 +740,7 @@ test_expect_success 'do "noop" when there is nothing to cherry-pick' '
> --author="Somebody else <somebody@else.com>" &&
> test $(git rev-parse branch3) != $(git rev-parse branch4) &&
> git rebase -i branch3 &&
> - test $(git rev-parse branch3) = $(git rev-parse branch4)
> + test_cmp_rev branch3 branch4
>
> '
>
> @@ -799,7 +799,7 @@ test_expect_success 'rebase -i continue with unstaged submodule' '
> test_must_fail git rebase -i submodule-base &&
> git reset &&
> git rebase --continue &&
> - test $(git rev-parse submodule-base) = $(git rev-parse HEAD)
> + test_cmp_rev submodule-base HEAD
> '
>
> test_expect_success 'avoid unnecessary reset' '
> @@ -822,7 +822,7 @@ test_expect_success 'reword' '
> git rebase -i A &&
> git show HEAD | grep "E changed" &&
> test $(git rev-parse master) != $(git rev-parse HEAD) &&
> - test $(git rev-parse master^) = $(git rev-parse HEAD^) &&
> + test_cmp_rev master^ HEAD^ &&
> FAKE_LINES="1 2 reword 3 4" FAKE_COMMIT_MESSAGE="D changed" \
> git rebase -i A &&
> git show HEAD^ | grep "D changed" &&
> @@ -886,7 +886,7 @@ test_expect_success 'always cherry-pick with --no-ff' '
> git diff HEAD~$p original-no-ff-branch~$p > out &&
> test_must_be_empty out
> done &&
> - test $(git rev-parse HEAD~3) = $(git rev-parse original-no-ff-branch~3) &&
> + test_cmp_rev HEAD~3 original-no-ff-branch~3 &&
> git diff HEAD~3 original-no-ff-branch~3 > out &&
> test_must_be_empty out
> '
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 2/9] cherry-pick: add test for `--skip` advice in `git commit`
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
2019-12-06 16:06 ` [PATCH v2 1/9] t3404: use test_cmp_rev Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 16:06 ` [PATCH v2 3/9] cherry-pick: check commit error messages Phillip Wood
` (6 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02),
`git commit` learned to suggest to run `git cherry-pick --skip` when
trying to cherry-pick an empty patch, but that was never tested for.
Here is a test that verifies that a message is given to the user that
contains the correct invocation.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t3510-cherry-pick-sequence.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 793bcc7fe3..5b94fdaa67 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -123,7 +123,8 @@ test_expect_success 'revert --skip to skip commit' '
test_expect_success 'skip "empty" commit' '
pristine_detach picked &&
test_commit dummy foo d &&
- test_must_fail git cherry-pick anotherpick &&
+ test_must_fail git cherry-pick anotherpick 2>err &&
+ test_i18ngrep "git cherry-pick --skip" err &&
git cherry-pick --skip &&
test_cmp_rev dummy HEAD
'
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 3/9] cherry-pick: check commit error messages
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
2019-12-06 16:06 ` [PATCH v2 1/9] t3404: use test_cmp_rev Phillip Wood
2019-12-06 16:06 ` [PATCH v2 2/9] cherry-pick: add test for `--skip` advice in `git commit` Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 16:06 ` [PATCH v2 4/9] sequencer: write CHERRY_PICK_HEAD for reword and edit Phillip Wood
` (5 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
We disallow partial commits and amending when CHERRY_PICK_HEAD
exists. Add a couple of tests to check the error message printed in each
case before we refactor the code responsible for this.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t/t3507-cherry-pick-conflict.sh | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 9b9b4ca8d4..c9715bf674 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -161,6 +161,29 @@ test_expect_success 'successful commit clears CHERRY_PICK_HEAD' '
test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
'
+
+test_expect_success 'partial commit of cherry-pick fails' '
+ pristine_detach initial &&
+
+ test_must_fail git cherry-pick picked &&
+ echo resolved >foo &&
+ git add foo &&
+ test_must_fail git commit foo 2>err &&
+
+ test_i18ngrep "cannot do a partial commit during a cherry-pick." err
+'
+
+test_expect_success 'commit --amend of cherry-pick fails' '
+ pristine_detach initial &&
+
+ test_must_fail git cherry-pick picked &&
+ echo resolved >foo &&
+ git add foo &&
+ test_must_fail git commit --amend 2>err &&
+
+ test_i18ngrep "in the middle of a cherry-pick -- cannot amend." err
+'
+
test_expect_success 'successful final commit clears cherry-pick state' '
pristine_detach initial &&
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 4/9] sequencer: write CHERRY_PICK_HEAD for reword and edit
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
` (2 preceding siblings ...)
2019-12-06 16:06 ` [PATCH v2 3/9] cherry-pick: check commit error messages Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 16:06 ` [PATCH v2 5/9] commit: use enum value for multiple cherry-picks Phillip Wood
` (4 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
`git commit` relies on the presence of CHERRY_PICK_HEAD to show the
correct error message in the case of an empty pick. This fixes a
regression introduced by the conversion from shell to C. In the shell
version everything was a cherry-pick as far as the sequencer code was
concerned so it always wrote CHERRY_PICK_HEAD. The conversion to C
forgot to update the code that creates CHERRY_PICK_HEAD. We do not want
to create CHERRY_PICK_HEAD for fixup and squash commands as that would
prevent `git commit --amend` from running.
Note that the error message shown by `git commit` for an empty pick
during a rebase is currently wrong as it talks about running `git
cherry-pick --skip` rather than `git rebase --skip`. This will be fixed
in a future commit which is why the tests are in t3403-rebase-skip.sh.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 4 +++-
t/t3403-rebase-skip.sh | 53 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 408b7885c7..4e0370277b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1916,7 +1916,9 @@ static int do_pick_commit(struct repository *r,
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (command == TODO_PICK && !opts->no_commit && (res == 0 || res == 1) &&
+ if ((command == TODO_PICK || command == TODO_REWORD ||
+ command == TODO_EDIT) && !opts->no_commit &&
+ (res == 0 || res == 1) &&
update_ref(NULL, "CHERRY_PICK_HEAD", &commit->object.oid, NULL,
REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
res = -1;
diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index ee8a8dba52..db7e917248 100755
--- a/t/t3403-rebase-skip.sh
+++ b/t/t3403-rebase-skip.sh
@@ -29,6 +29,13 @@ test_expect_success setup '
test_tick &&
git commit -m reverted-goodbye &&
git tag reverted-goodbye &&
+ git checkout goodbye &&
+ test_tick &&
+ GIT_AUTHOR_NAME="Another Author" \
+ GIT_AUTHOR_EMAIL="another.author@example.com" \
+ git commit --amend --no-edit -m amended-goodbye &&
+ test_tick &&
+ git tag amended-goodbye &&
git checkout -f skip-reference &&
echo moo > hello &&
@@ -85,6 +92,52 @@ test_expect_success 'moved back to branch correctly' '
test_debug 'gitk --all & sleep 1'
+test_expect_success 'correct advice upon picking empty commit' '
+ test_when_finished "git rebase --abort" &&
+ test_must_fail git rebase -i --onto goodbye \
+ amended-goodbye^ amended-goodbye 2>err &&
+ test_i18ngrep "previous cherry-pick is now empty" err &&
+ test_i18ngrep "git cherry-pick --skip" err &&
+ test_must_fail git commit &&
+ test_i18ngrep "git cherry-pick --skip" err
+'
+
+test_expect_success 'correct authorship when committing empty pick' '
+ test_when_finished "git rebase --abort" &&
+ test_must_fail git rebase -i --onto goodbye \
+ amended-goodbye^ amended-goodbye &&
+ git commit --allow-empty &&
+ git log --pretty=format:"%an <%ae>%n%ad%B" -1 amended-goodbye >expect &&
+ git log --pretty=format:"%an <%ae>%n%ad%B" -1 HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'correct advice upon rewording empty commit' '
+ test_when_finished "git rebase --abort" &&
+ (
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="reword 1" git rebase -i \
+ --onto goodbye amended-goodbye^ amended-goodbye 2>err
+ ) &&
+ test_i18ngrep "previous cherry-pick is now empty" err &&
+ test_i18ngrep "git cherry-pick --skip" err &&
+ test_must_fail git commit &&
+ test_i18ngrep "git cherry-pick --skip" err
+'
+
+test_expect_success 'correct advice upon editing empty commit' '
+ test_when_finished "git rebase --abort" &&
+ (
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="edit 1" git rebase -i \
+ --onto goodbye amended-goodbye^ amended-goodbye 2>err
+ ) &&
+ test_i18ngrep "previous cherry-pick is now empty" err &&
+ test_i18ngrep "git cherry-pick --skip" err &&
+ test_must_fail git commit &&
+ test_i18ngrep "git cherry-pick --skip" err
+'
+
test_expect_success 'fixup that empties commit fails' '
test_when_finished "git rebase --abort" &&
(
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 5/9] commit: use enum value for multiple cherry-picks
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
` (3 preceding siblings ...)
2019-12-06 16:06 ` [PATCH v2 4/9] sequencer: write CHERRY_PICK_HEAD for reword and edit Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 18:13 ` Junio C Hamano
2019-12-06 16:06 ` [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer Phillip Wood
` (3 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Add FROM_CHERRY_PICK_MULTI for a sequence of cherry-picks rather than
using a separate variable.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
builtin/commit.c | 25 +++++++++++--------------
wt-status.h | 9 ++++++++-
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d85e0ad560..3b463522be 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -122,7 +122,6 @@ static enum commit_msg_cleanup_mode cleanup_mode;
static const char *cleanup_arg;
static enum commit_whence whence;
-static int sequencer_in_use;
static int use_editor = 1, include_status = 1;
static int have_option_m;
static struct strbuf message = STRBUF_INIT;
@@ -179,11 +178,9 @@ static void determine_whence(struct wt_status *s)
{
if (file_exists(git_path_merge_head(the_repository)))
whence = FROM_MERGE;
- else if (file_exists(git_path_cherry_pick_head(the_repository))) {
- whence = FROM_CHERRY_PICK;
- if (file_exists(git_path_seq_dir()))
- sequencer_in_use = 1;
- }
+ else if (file_exists(git_path_cherry_pick_head(the_repository)))
+ whence = file_exists(git_path_seq_dir()) ?
+ FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
else
whence = FROM_COMMIT;
if (s)
@@ -453,7 +450,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
if (whence != FROM_COMMIT) {
if (whence == FROM_MERGE)
die(_("cannot do a partial commit during a merge."));
- else if (whence == FROM_CHERRY_PICK)
+ else if (is_from_cherry_pick(whence))
die(_("cannot do a partial commit during a cherry-pick."));
}
@@ -771,7 +768,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/
else if (whence == FROM_MERGE)
hook_arg1 = "merge";
- else if (whence == FROM_CHERRY_PICK) {
+ else if (is_from_cherry_pick(whence)) {
hook_arg1 = "commit";
hook_arg2 = "CHERRY_PICK_HEAD";
}
@@ -948,9 +945,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
run_status(stdout, index_file, prefix, 0, s);
if (amend)
fputs(_(empty_amend_advice), stderr);
- else if (whence == FROM_CHERRY_PICK) {
+ else if (is_from_cherry_pick(whence)) {
fputs(_(empty_cherry_pick_advice), stderr);
- if (!sequencer_in_use)
+ if (whence == FROM_CHERRY_PICK_SINGLE)
fputs(_(empty_cherry_pick_advice_single), stderr);
else
fputs(_(empty_cherry_pick_advice_multi), stderr);
@@ -1143,7 +1140,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (amend && whence != FROM_COMMIT) {
if (whence == FROM_MERGE)
die(_("You are in the middle of a merge -- cannot amend."));
- else if (whence == FROM_CHERRY_PICK)
+ else if (is_from_cherry_pick(whence))
die(_("You are in the middle of a cherry-pick -- cannot amend."));
}
if (fixup_message && squash_message)
@@ -1166,7 +1163,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message = edit_message;
if (amend && !use_message && !fixup_message)
use_message = "HEAD";
- if (!use_message && whence != FROM_CHERRY_PICK && renew_authorship)
+ if (!use_message && !is_from_cherry_pick(whence) && renew_authorship)
die(_("--reset-author can be used only with -C, -c or --amend."));
if (use_message) {
use_message_buffer = read_commit_message(use_message);
@@ -1175,7 +1172,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
author_message_buffer = use_message_buffer;
}
}
- if (whence == FROM_CHERRY_PICK && !renew_authorship) {
+ if (is_from_cherry_pick(whence) && !renew_authorship) {
author_message = "CHERRY_PICK_HEAD";
author_message_buffer = read_commit_message(author_message);
}
@@ -1589,7 +1586,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
reduce_heads_replace(&parents);
} else {
if (!reflog_msg)
- reflog_msg = (whence == FROM_CHERRY_PICK)
+ reflog_msg = is_from_cherry_pick(whence)
? "commit (cherry-pick)"
: "commit";
commit_list_insert(current_head, &parents);
diff --git a/wt-status.h b/wt-status.h
index 64f1ddc9fd..0098fdb0b5 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -39,9 +39,16 @@ enum show_ignored_type {
enum commit_whence {
FROM_COMMIT, /* normal */
FROM_MERGE, /* commit came from merge */
- FROM_CHERRY_PICK /* commit came from cherry-pick */
+ FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
+ FROM_CHERRY_PICK_MULTI /* commit came from a sequence of cherry-picks */
};
+static inline int is_from_cherry_pick(enum commit_whence whence)
+{
+ return whence == FROM_CHERRY_PICK_SINGLE ||
+ whence == FROM_CHERRY_PICK_MULTI;
+}
+
struct wt_status_change_data {
int worktree_status;
int index_status;
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 5/9] commit: use enum value for multiple cherry-picks
2019-12-06 16:06 ` [PATCH v2 5/9] commit: use enum value for multiple cherry-picks Phillip Wood
@ 2019-12-06 18:13 ` Junio C Hamano
0 siblings, 0 replies; 31+ messages in thread
From: Junio C Hamano @ 2019-12-06 18:13 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood
Phillip Wood <phillip.wood123@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Add FROM_CHERRY_PICK_MULTI for a sequence of cherry-picks rather than
> using a separate variable.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> builtin/commit.c | 25 +++++++++++--------------
> wt-status.h | 9 ++++++++-
> 2 files changed, 19 insertions(+), 15 deletions(-)
Makes sense. The checks have become quite pleasant to read, thanks
to the new helper function.
> ...
> - if (whence == FROM_CHERRY_PICK && !renew_authorship) {
> + if (is_from_cherry_pick(whence) && !renew_authorship) {
> author_message = "CHERRY_PICK_HEAD";
> author_message_buffer = read_commit_message(author_message);
> }
> diff --git a/wt-status.h b/wt-status.h
> index 64f1ddc9fd..0098fdb0b5 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -39,9 +39,16 @@ enum show_ignored_type {
> enum commit_whence {
> FROM_COMMIT, /* normal */
> FROM_MERGE, /* commit came from merge */
> - FROM_CHERRY_PICK /* commit came from cherry-pick */
> + FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
> + FROM_CHERRY_PICK_MULTI /* commit came from a sequence of cherry-picks */
> };
It might be worth to end PICK_MULTI with a trailing comma to
future-proof, but not worth a reroll for this alone.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
` (4 preceding siblings ...)
2019-12-06 16:06 ` [PATCH v2 5/9] commit: use enum value for multiple cherry-picks Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 18:24 ` Junio C Hamano
2019-12-06 16:06 ` [PATCH v2 7/9] commit: give correct advice for empty commit during a rebase Phillip Wood
` (2 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Working out which command wants to create a commit requires detailed
knowledge of the sequencer internals and that knowledge is going to
increase in subsequent commits. With that in mind lets encapsulate that
knowledge in sequencer.c rather than spreading it into builtin/commit.c.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
builtin/commit.c | 5 +----
sequencer.c | 13 ++++++++++++-
sequencer.h | 3 ++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 3b463522be..d8d4c8e419 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -178,10 +178,7 @@ static void determine_whence(struct wt_status *s)
{
if (file_exists(git_path_merge_head(the_repository)))
whence = FROM_MERGE;
- else if (file_exists(git_path_cherry_pick_head(the_repository)))
- whence = file_exists(git_path_seq_dir()) ?
- FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
- else
+ else if (!sequencer_determine_whence(the_repository, &whence))
whence = FROM_COMMIT;
if (s)
s->whence = whence;
diff --git a/sequencer.c b/sequencer.c
index 4e0370277b..98e007556c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -40,7 +40,7 @@ static const char cherry_picked_prefix[] = "(cherry picked from commit ";
GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
-GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
+static GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
@@ -5256,3 +5256,14 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
return 0;
}
+
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
+{
+ if (file_exists(git_path_cherry_pick_head(r))) {
+ *whence = file_exists(git_path_seq_dir()) ?
+ FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/sequencer.h b/sequencer.h
index 56881a6baa..8d451dbfcb 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,12 +3,12 @@
#include "cache.h"
#include "strbuf.h"
+#include "wt-status.h"
struct commit;
struct repository;
const char *git_path_commit_editmsg(void);
-const char *git_path_seq_dir(void);
const char *rebase_path_todo(void);
const char *rebase_path_todo_backup(void);
@@ -202,4 +202,5 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
void sequencer_post_commit_cleanup(struct repository *r);
int sequencer_get_last_command(struct repository* r,
enum replay_action *action);
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);
#endif /* SEQUENCER_H */
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer
2019-12-06 16:06 ` [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer Phillip Wood
@ 2019-12-06 18:24 ` Junio C Hamano
2019-12-18 14:26 ` Phillip Wood
0 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2019-12-06 18:24 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood
Phillip Wood <phillip.wood123@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Working out which command wants to create a commit requires detailed
> knowledge of the sequencer internals and that knowledge is going to
> increase in subsequent commits. With that in mind lets encapsulate that
> knowledge in sequencer.c rather than spreading it into builtin/commit.c.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> builtin/commit.c | 5 +----
> sequencer.c | 13 ++++++++++++-
> sequencer.h | 3 ++-
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 3b463522be..d8d4c8e419 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -178,10 +178,7 @@ static void determine_whence(struct wt_status *s)
> {
> if (file_exists(git_path_merge_head(the_repository)))
> whence = FROM_MERGE;
> - else if (file_exists(git_path_cherry_pick_head(the_repository)))
> - whence = file_exists(git_path_seq_dir()) ?
> - FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
> - else
> + else if (!sequencer_determine_whence(the_repository, &whence))
> whence = FROM_COMMIT;
> if (s)
> s->whence = whence;
> diff --git a/sequencer.c b/sequencer.c
> index 4e0370277b..98e007556c 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -40,7 +40,7 @@ static const char cherry_picked_prefix[] = "(cherry picked from commit ";
>
> GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
>
> -GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
> +static GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
>
> static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
> static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
> @@ -5256,3 +5256,14 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
>
> return 0;
> }
> +
> +int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
> +{
> + if (file_exists(git_path_cherry_pick_head(r))) {
> + *whence = file_exists(git_path_seq_dir()) ?
> + FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
> + return 1;
> + }
> +
> + return 0;
> +}
I am not sure if this is a good move---determine_whence() that can
tell not just we are in the middle of cherry-pick (either a single
or multi) but also during a merge may be at the right abstraction
level. Why would we want to invent a separate function that says "I
dunno" during a merge, instead of moving the logic for merge to the
new helper as well? The original determine_whence that takes
wt_status and populates it still has to call the new helper either
way. Also for the matter FROM_COMMIT may also want to be part of
the helper. This all depends on the new callers you plan to invent,
of course.
Not part of this topic, but the call to file_exists() may want to
become a call to dir_exists() as git-path-seq-dir is clearly a
directory and cannot be a file, right?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer
2019-12-06 18:24 ` Junio C Hamano
@ 2019-12-18 14:26 ` Phillip Wood
0 siblings, 0 replies; 31+ messages in thread
From: Phillip Wood @ 2019-12-18 14:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood
On 06/12/2019 18:24, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> Working out which command wants to create a commit requires detailed
>> knowledge of the sequencer internals and that knowledge is going to
>> increase in subsequent commits. With that in mind lets encapsulate that
>> knowledge in sequencer.c rather than spreading it into builtin/commit.c.
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>> builtin/commit.c | 5 +----
>> sequencer.c | 13 ++++++++++++-
>> sequencer.h | 3 ++-
>> 3 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 3b463522be..d8d4c8e419 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -178,10 +178,7 @@ static void determine_whence(struct wt_status *s)
>> {
>> if (file_exists(git_path_merge_head(the_repository)))
>> whence = FROM_MERGE;
>> - else if (file_exists(git_path_cherry_pick_head(the_repository)))
>> - whence = file_exists(git_path_seq_dir()) ?
>> - FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
>> - else
>> + else if (!sequencer_determine_whence(the_repository, &whence))
>> whence = FROM_COMMIT;
>> if (s)
>> s->whence = whence;
>> diff --git a/sequencer.c b/sequencer.c
>> index 4e0370277b..98e007556c 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -40,7 +40,7 @@ static const char cherry_picked_prefix[] = "(cherry picked from commit ";
>>
>> GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
>>
>> -GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
>> +static GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
>>
>> static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
>> static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
>> @@ -5256,3 +5256,14 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
>>
>> return 0;
>> }
>> +
>> +int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
>> +{
>> + if (file_exists(git_path_cherry_pick_head(r))) {
>> + *whence = file_exists(git_path_seq_dir()) ?
>> + FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>
> I am not sure if this is a good move---determine_whence() that can
> tell not just we are in the middle of cherry-pick (either a single
> or multi) but also during a merge may be at the right abstraction
> level. Why would we want to invent a separate function that says "I
> dunno" during a merge, instead of moving the logic for merge to the
> new helper as well? The original determine_whence that takes
> wt_status and populates it still has to call the new helper either
> way. Also for the matter FROM_COMMIT may also want to be part of
> the helper. This all depends on the new callers you plan to invent,
> of course.
The idea was for determine_whence() to be able to delegate to the
sequencer to ask if it is doing something without having to expose all
the implementation details of that check in builtin/commit.c. It is
simple enough at this stage but the next patches add more complexity
which would mean exposing various sequencer state files to
builtin/commit.c. This function is only meant to be called from
determine_whence() - callers that want to know if any operations (merge,
cherry-pick etc.) are in progress should be calling that not the
function added here.
> Not part of this topic, but the call to file_exists() may want to
> become a call to dir_exists() as git-path-seq-dir is clearly a
> directory and cannot be a file, right?
Yes
Best Wishes
Phillip
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 7/9] commit: give correct advice for empty commit during a rebase
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
` (5 preceding siblings ...)
2019-12-06 16:06 ` [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-06 16:06 ` [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit Phillip Wood
2019-12-06 16:06 ` [PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts Phillip Wood
8 siblings, 0 replies; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List
Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood, Johannes Schindelin
From: Phillip Wood <phillip.wood@dunelm.org.uk>
In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02),
`git commit` learned to suggest to run `git cherry-pick --skip` when
trying to cherry-pick an empty patch.
However, it was overlooked that there are more conditions than just a
`git cherry-pick` when this advice is printed (which originally
suggested the neutral `git reset`): the same can happen during a rebase.
Let's suggest the correct command, even during a rebase.
While at it, we adjust more places in `builtin/commit.c` that
incorrectly assumed that the presence of a `CHERRY_PICK_HEAD` meant that
surely this must be a `cherry-pick` in progress.
Note: we take pains to handle the situation when a user runs a `git
cherry-pick` _during_ a rebase. This is quite valid (e.g. in an `exec`
line in an interactive rebase). On the other hand, it is not possible to
run a rebase during a cherry-pick, meaning: if both `rebase-merge/` and
`sequencer/` exist or CHERRY_PICK_HEAD and REBASE_HEAD point to the same
commit , we still want to advise to use `git cherry-pick --skip`.
Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
builtin/commit.c | 24 ++++++++++++++++-----
sequencer.c | 39 ++++++++++++++++++++++++++++-------
t/t3403-rebase-skip.sh | 36 +++++++++++++++++++++++++++-----
t/t3404-rebase-interactive.sh | 26 +++++++++++++++++++++++
wt-status.h | 8 ++++++-
5 files changed, 114 insertions(+), 19 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d8d4c8e419..c3b6b8a6bd 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -59,6 +59,9 @@ N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\
" git commit --allow-empty\n"
"\n");
+static const char empty_rebase_pick_advice[] =
+N_("Otherwise, please use 'git rebase --skip'\n");
+
static const char empty_cherry_pick_advice_single[] =
N_("Otherwise, please use 'git cherry-pick --skip'\n");
@@ -449,6 +452,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
die(_("cannot do a partial commit during a merge."));
else if (is_from_cherry_pick(whence))
die(_("cannot do a partial commit during a cherry-pick."));
+ else if (is_from_rebase(whence))
+ die(_("cannot do a partial commit during a rebase."));
}
if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
@@ -765,7 +770,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/
else if (whence == FROM_MERGE)
hook_arg1 = "merge";
- else if (is_from_cherry_pick(whence)) {
+ else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) {
hook_arg1 = "commit";
hook_arg2 = "CHERRY_PICK_HEAD";
}
@@ -942,12 +947,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
run_status(stdout, index_file, prefix, 0, s);
if (amend)
fputs(_(empty_amend_advice), stderr);
- else if (is_from_cherry_pick(whence)) {
+ else if (is_from_cherry_pick(whence) ||
+ whence == FROM_REBASE_PICK) {
fputs(_(empty_cherry_pick_advice), stderr);
if (whence == FROM_CHERRY_PICK_SINGLE)
fputs(_(empty_cherry_pick_advice_single), stderr);
- else
+ else if (whence == FROM_CHERRY_PICK_MULTI)
fputs(_(empty_cherry_pick_advice_multi), stderr);
+ else
+ fputs(_(empty_rebase_pick_advice), stderr);
}
return 0;
}
@@ -1139,6 +1147,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
die(_("You are in the middle of a merge -- cannot amend."));
else if (is_from_cherry_pick(whence))
die(_("You are in the middle of a cherry-pick -- cannot amend."));
+ else if (whence == FROM_REBASE_PICK)
+ die(_("You are in the middle of a rebase -- cannot amend."));
}
if (fixup_message && squash_message)
die(_("Options --squash and --fixup cannot be used together"));
@@ -1160,7 +1170,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message = edit_message;
if (amend && !use_message && !fixup_message)
use_message = "HEAD";
- if (!use_message && !is_from_cherry_pick(whence) && renew_authorship)
+ if (!use_message && !is_from_cherry_pick(whence) &&
+ !is_from_rebase(whence) && renew_authorship)
die(_("--reset-author can be used only with -C, -c or --amend."));
if (use_message) {
use_message_buffer = read_commit_message(use_message);
@@ -1169,7 +1180,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
author_message_buffer = use_message_buffer;
}
}
- if (is_from_cherry_pick(whence) && !renew_authorship) {
+ if ((is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) &&
+ !renew_authorship) {
author_message = "CHERRY_PICK_HEAD";
author_message_buffer = read_commit_message(author_message);
}
@@ -1585,6 +1597,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!reflog_msg)
reflog_msg = is_from_cherry_pick(whence)
? "commit (cherry-pick)"
+ : is_from_rebase(whence)
+ ? "commit (rebase)"
: "commit";
commit_list_insert(current_head, &parents);
}
diff --git a/sequencer.c b/sequencer.c
index 98e007556c..6e153fea76 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1430,9 +1430,19 @@ static int try_to_commit(struct repository *r,
return res;
}
+static int write_rebase_head(struct object_id *oid)
+{
+ if (update_ref("rebase", "REBASE_HEAD", oid,
+ NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
+ return error(_("could not update %s"), "REBASE_HEAD");
+
+ return 0;
+}
+
static int do_commit(struct repository *r,
const char *msg_file, const char *author,
- struct replay_opts *opts, unsigned int flags)
+ struct replay_opts *opts, unsigned int flags,
+ struct object_id *oid)
{
int res = 1;
@@ -1457,8 +1467,12 @@ static int do_commit(struct repository *r,
return res;
}
}
- if (res == 1)
+ if (res == 1) {
+ if (is_rebase_i(opts) && oid)
+ if (write_rebase_head(oid))
+ return -1;
return run_git_commit(r, msg_file, opts, flags);
+ }
return res;
}
@@ -1945,7 +1959,8 @@ static int do_pick_commit(struct repository *r,
flags |= ALLOW_EMPTY;
if (!opts->no_commit) {
if (author || command == TODO_REVERT || (flags & AMEND_MSG))
- res = do_commit(r, msg_file, author, opts, flags);
+ res = do_commit(r, msg_file, author, opts, flags,
+ commit? &commit->object.oid : NULL);
else
res = error(_("unable to parse commit author"));
*check_todo = !!(flags & EDIT_MSG);
@@ -2960,9 +2975,7 @@ static int make_patch(struct repository *r,
p = short_commit_name(commit);
if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
return -1;
- if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
- NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
- res |= error(_("could not update %s"), "REBASE_HEAD");
+ res |= write_rebase_head(&commit->object.oid);
strbuf_addf(&buf, "%s/patch", get_dir(opts));
memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -5260,8 +5273,18 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
{
if (file_exists(git_path_cherry_pick_head(r))) {
- *whence = file_exists(git_path_seq_dir()) ?
- FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
+ struct object_id cherry_pick_head, rebase_head;
+
+ if (file_exists(git_path_seq_dir()))
+ *whence = FROM_CHERRY_PICK_MULTI;
+ if (file_exists(rebase_path()) &&
+ !get_oid("REBASE_HEAD", &rebase_head) &&
+ !get_oid("CHERRY_PICK_HEAD", &cherry_pick_head) &&
+ oideq(&rebase_head, &cherry_pick_head))
+ *whence = FROM_REBASE_PICK;
+ else
+ *whence = FROM_CHERRY_PICK_SINGLE;
+
return 1;
}
diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index db7e917248..a927774910 100755
--- a/t/t3403-rebase-skip.sh
+++ b/t/t3403-rebase-skip.sh
@@ -97,9 +97,9 @@ test_expect_success 'correct advice upon picking empty commit' '
test_must_fail git rebase -i --onto goodbye \
amended-goodbye^ amended-goodbye 2>err &&
test_i18ngrep "previous cherry-pick is now empty" err &&
- test_i18ngrep "git cherry-pick --skip" err &&
+ test_i18ngrep "git rebase --skip" err &&
test_must_fail git commit &&
- test_i18ngrep "git cherry-pick --skip" err
+ test_i18ngrep "git rebase --skip" err
'
test_expect_success 'correct authorship when committing empty pick' '
@@ -120,9 +120,9 @@ test_expect_success 'correct advice upon rewording empty commit' '
--onto goodbye amended-goodbye^ amended-goodbye 2>err
) &&
test_i18ngrep "previous cherry-pick is now empty" err &&
- test_i18ngrep "git cherry-pick --skip" err &&
+ test_i18ngrep "git rebase --skip" err &&
test_must_fail git commit &&
- test_i18ngrep "git cherry-pick --skip" err
+ test_i18ngrep "git rebase --skip" err
'
test_expect_success 'correct advice upon editing empty commit' '
@@ -133,8 +133,34 @@ test_expect_success 'correct advice upon editing empty commit' '
--onto goodbye amended-goodbye^ amended-goodbye 2>err
) &&
test_i18ngrep "previous cherry-pick is now empty" err &&
- test_i18ngrep "git cherry-pick --skip" err &&
+ test_i18ngrep "git rebase --skip" err &&
test_must_fail git commit &&
+ test_i18ngrep "git rebase --skip" err
+'
+
+test_expect_success 'correct advice upon cherry-picking an empty commit during a rebase' '
+ test_when_finished "git rebase --abort" &&
+ (
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="1 exec_git_cherry-pick_amended-goodbye" \
+ git rebase -i goodbye^ goodbye 2>err
+ ) &&
+ test_i18ngrep "previous cherry-pick is now empty" err &&
+ test_i18ngrep "git cherry-pick --skip" err &&
+ test_must_fail git commit 2>err &&
+ test_i18ngrep "git cherry-pick --skip" err
+'
+
+test_expect_success 'correct advice upon multi cherry-pick picking an empty commit during a rebase' '
+ test_when_finished "git rebase --abort" &&
+ (
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="1 exec_git_cherry-pick_goodbye_amended-goodbye" \
+ git rebase -i goodbye^^ goodbye 2>err
+ ) &&
+ test_i18ngrep "previous cherry-pick is now empty" err &&
+ test_i18ngrep "git cherry-pick --skip" err &&
+ test_must_fail git commit 2>err &&
test_i18ngrep "git cherry-pick --skip" err
'
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d8a05fd439..5afa6f28cd 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1602,6 +1602,32 @@ test_expect_success 'post-commit hook is called' '
test_cmp expect actual
'
+test_expect_success 'correct error message for partial commit after empty pick' '
+ test_when_finished "git rebase --abort" &&
+ (
+ set_fake_editor &&
+ FAKE_LINES="2 1 1" &&
+ export FAKE_LINES &&
+ test_must_fail git rebase -i A D
+ ) &&
+ echo x >file1 &&
+ test_must_fail git commit file1 2>err &&
+ test_i18ngrep "cannot do a partial commit during a rebase." err
+'
+
+test_expect_success 'correct error message for commit --amend after empty pick' '
+ test_when_finished "git rebase --abort" &&
+ (
+ set_fake_editor &&
+ FAKE_LINES="1 1" &&
+ export FAKE_LINES &&
+ test_must_fail git rebase -i A D
+ ) &&
+ echo x>file1 &&
+ test_must_fail git commit -a --amend 2>err &&
+ test_i18ngrep "middle of a rebase -- cannot amend." err
+'
+
# This must be the last test in this file
test_expect_success '$EDITOR and friends are unchanged' '
test_editor_unchanged
diff --git a/wt-status.h b/wt-status.h
index 0098fdb0b5..5f81bf7507 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -40,7 +40,8 @@ enum commit_whence {
FROM_COMMIT, /* normal */
FROM_MERGE, /* commit came from merge */
FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
- FROM_CHERRY_PICK_MULTI /* commit came from a sequence of cherry-picks */
+ FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
+ FROM_REBASE_PICK /* commit came from a pick/reword/edit */
};
static inline int is_from_cherry_pick(enum commit_whence whence)
@@ -49,6 +50,11 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
whence == FROM_CHERRY_PICK_MULTI;
}
+static inline int is_from_rebase(enum commit_whence whence)
+{
+ return whence == FROM_REBASE_PICK;
+}
+
struct wt_status_change_data {
int worktree_status;
int index_status;
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
` (6 preceding siblings ...)
2019-12-06 16:06 ` [PATCH v2 7/9] commit: give correct advice for empty commit during a rebase Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2020-02-26 19:45 ` Elijah Newren
2019-12-06 16:06 ` [PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts Phillip Wood
8 siblings, 1 reply; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Add a specific message to advise the user what to do when a fixup or
squash command would create an empty commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Notes:
I'm slightly nervous of moving the call to determine_whence() to
finalized_deferred_config(). I think it is ok but need to do a more
thorough audit of the code to be sure.
builtin/commit.c | 32 ++++++++++++++++++++++++++++----
sequencer.c | 31 ++++++++++++++++++++++++++++++-
sequencer.h | 3 ++-
t/t3403-rebase-skip.sh | 18 ++++++++++++------
wt-status.h | 5 +++--
5 files changed, 75 insertions(+), 14 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index c3b6b8a6bd..4ae984c470 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -52,6 +52,20 @@ N_("You asked to amend the most recent commit, but doing so would make\n"
"it empty. You can repeat your command with --allow-empty, or you can\n"
"remove the commit entirely with \"git reset HEAD^\".\n");
+static const char empty_rebase_fixup_advice[] =
+N_("The fixup would make the commit empty\n"
+"If you wish to commit it anyway use:\n"
+"\n"
+" git commit --amend --allow-empty\n"
+" git rebase --continue\n"
+"\n"
+"To remove the commit entirely use:\n"
+"\n"
+" git reset HEAD^\n"
+" git rebase --continue\n"
+"\n"
+"Otherwise, please use 'git rebase --skip' to skip it\n");
+
static const char empty_cherry_pick_advice[] =
N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
"If you wish to commit it anyway, use:\n"
@@ -181,8 +195,14 @@ static void determine_whence(struct wt_status *s)
{
if (file_exists(git_path_merge_head(the_repository)))
whence = FROM_MERGE;
- else if (!sequencer_determine_whence(the_repository, &whence))
- whence = FROM_COMMIT;
+ else {
+ int res = sequencer_determine_whence(the_repository, &whence,
+ amend);
+ if (res < 0)
+ die(_("could not read sequencer state"));
+ else if (!res)
+ whence = FROM_COMMIT;
+ }
if (s)
s->whence = whence;
}
@@ -192,7 +212,6 @@ static void status_init_config(struct wt_status *s, config_fn_t fn)
wt_status_prepare(the_repository, s);
init_diff_ui_defaults();
git_config(fn, s);
- determine_whence(s);
s->hints = advice_status_hints; /* must come after git_config() */
}
@@ -943,9 +962,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/
if (!committable && whence != FROM_MERGE && !allow_empty &&
!(amend && is_a_merge(current_head))) {
+ fprintf(stderr, "\nwhence = %d\n", whence);
s->display_comment_prefix = old_display_comment_prefix;
run_status(stdout, index_file, prefix, 0, s);
- if (amend)
+ if (whence == FROM_REBASE_FIXUP)
+ fputs(_(empty_rebase_fixup_advice), stderr);
+ else if (amend)
fputs(_(empty_amend_advice), stderr);
else if (is_from_cherry_pick(whence) ||
whence == FROM_REBASE_PICK) {
@@ -1114,6 +1136,8 @@ static void finalize_deferred_config(struct wt_status *s)
if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
s->ahead_behind_flags = AHEAD_BEHIND_FULL;
+
+ determine_whence(s);
}
static int parse_and_validate_options(int argc, const char *argv[],
diff --git a/sequencer.c b/sequencer.c
index 6e153fea76..64242f4ce7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5270,7 +5270,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
return 0;
}
-int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
+ int amending)
{
if (file_exists(git_path_cherry_pick_head(r))) {
struct object_id cherry_pick_head, rebase_head;
@@ -5286,6 +5287,34 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
*whence = FROM_CHERRY_PICK_SINGLE;
return 1;
+ } else if (amending && file_exists(rebase_path_current_fixups()) &&
+ (file_exists(git_path_squash_msg(r)) ||
+ file_exists(git_path_merge_msg(r)))) {
+ /*
+ * If rebase_path_amend() exists the user is running `git
+ * commit`, if not we're committing a fixup/squash directly from
+ * the sequencer
+ */
+ if (file_exists(rebase_path_amend())) {
+ struct strbuf rev = STRBUF_INIT;
+ struct object_id to_amend, head;
+
+ if (get_oid("HEAD", &head))
+ return error(_("amending invalid head")); /* let commit deal with error */
+ if (!read_oneliner(&rev, rebase_path_amend(), 0))
+ return error(_("invalid file: '%s'"),
+ rebase_path_amend());
+ if (get_oid_hex(rev.buf, &to_amend))
+ return error(_("invalid contents: '%s'"),
+ rebase_path_amend());
+ if (oideq(&head, &to_amend)) {
+ *whence = FROM_REBASE_FIXUP;
+ return 1;
+ }
+ } else {
+ *whence = FROM_REBASE_FIXUP;
+ return 1;
+ }
}
return 0;
diff --git a/sequencer.h b/sequencer.h
index 8d451dbfcb..4e631900b4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -202,5 +202,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
void sequencer_post_commit_cleanup(struct repository *r);
int sequencer_get_last_command(struct repository* r,
enum replay_action *action);
-int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
+ int amending);
#endif /* SEQUENCER_H */
diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index a927774910..bc110b66b3 100755
--- a/t/t3403-rebase-skip.sh
+++ b/t/t3403-rebase-skip.sh
@@ -164,22 +164,28 @@ test_expect_success 'correct advice upon multi cherry-pick picking an empty comm
test_i18ngrep "git cherry-pick --skip" err
'
-test_expect_success 'fixup that empties commit fails' '
+test_expect_success 'correct advice when fixup empties commit' '
test_when_finished "git rebase --abort" &&
(
set_fake_editor &&
test_must_fail env FAKE_LINES="1 fixup 2" git rebase -i \
- goodbye^ reverted-goodbye
- )
+ goodbye^ reverted-goodbye 2>err
+ ) &&
+ test_i18ngrep "git rebase --skip" err &&
+ test_must_fail git commit --amend --no-edit 2>err &&
+ test_i18ngrep "git rebase --skip" err
'
-test_expect_success 'squash that empties commit fails' '
+test_expect_success 'correct advice when squash empties commit' '
test_when_finished "git rebase --abort" &&
(
set_fake_editor &&
test_must_fail env FAKE_LINES="1 squash 2" git rebase -i \
- goodbye^ reverted-goodbye
- )
+ goodbye^ reverted-goodbye 2>err
+ ) &&
+ test_i18ngrep "git rebase --skip" err &&
+ test_must_fail git commit --amend --no-edit 2>err &&
+ test_i18ngrep "git rebase --skip" err
'
# Must be the last test in this file
diff --git a/wt-status.h b/wt-status.h
index 5f81bf7507..a4b7fe6de9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -41,7 +41,8 @@ enum commit_whence {
FROM_MERGE, /* commit came from merge */
FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
- FROM_REBASE_PICK /* commit came from a pick/reword/edit */
+ FROM_REBASE_PICK, /* commit came from a pick/reword/edit */
+ FROM_REBASE_FIXUP /* commit came from fixup/squash */
};
static inline int is_from_cherry_pick(enum commit_whence whence)
@@ -52,7 +53,7 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
static inline int is_from_rebase(enum commit_whence whence)
{
- return whence == FROM_REBASE_PICK;
+ return whence == FROM_REBASE_PICK || whence == FROM_REBASE_FIXUP;
}
struct wt_status_change_data {
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit
2019-12-06 16:06 ` [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit Phillip Wood
@ 2020-02-26 19:45 ` Elijah Newren
0 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren @ 2020-02-26 19:45 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Junio C Hamano
On Fri, Dec 6, 2019 at 8:10 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Add a specific message to advise the user what to do when a fixup or
> squash command would create an empty commit.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>
> Notes:
> I'm slightly nervous of moving the call to determine_whence() to
> finalized_deferred_config(). I think it is ok but need to do a more
> thorough audit of the code to be sure.
Any update?
>
> builtin/commit.c | 32 ++++++++++++++++++++++++++++----
> sequencer.c | 31 ++++++++++++++++++++++++++++++-
> sequencer.h | 3 ++-
> t/t3403-rebase-skip.sh | 18 ++++++++++++------
> wt-status.h | 5 +++--
> 5 files changed, 75 insertions(+), 14 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index c3b6b8a6bd..4ae984c470 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -52,6 +52,20 @@ N_("You asked to amend the most recent commit, but doing so would make\n"
> "it empty. You can repeat your command with --allow-empty, or you can\n"
> "remove the commit entirely with \"git reset HEAD^\".\n");
>
> +static const char empty_rebase_fixup_advice[] =
> +N_("The fixup would make the commit empty\n"
> +"If you wish to commit it anyway use:\n"
> +"\n"
> +" git commit --amend --allow-empty\n"
> +" git rebase --continue\n"
> +"\n"
> +"To remove the commit entirely use:\n"
> +"\n"
> +" git reset HEAD^\n"
> +" git rebase --continue\n"
> +"\n"
> +"Otherwise, please use 'git rebase --skip' to skip it\n");
How does skipping differ from removing the commit entirely? Which one
tosses just the changes from the fixup commit, and which tosses both
the fixup and the commit it is fixing? Or do they both do the same
thing?
> static const char empty_cherry_pick_advice[] =
> N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
> "If you wish to commit it anyway, use:\n"
> @@ -181,8 +195,14 @@ static void determine_whence(struct wt_status *s)
> {
> if (file_exists(git_path_merge_head(the_repository)))
> whence = FROM_MERGE;
> - else if (!sequencer_determine_whence(the_repository, &whence))
> - whence = FROM_COMMIT;
> + else {
> + int res = sequencer_determine_whence(the_repository, &whence,
> + amend);
> + if (res < 0)
> + die(_("could not read sequencer state"));
sequencer_determine_whence() tries to determine what type of sequencer
operation is in effect, and can return a range of values in whence.
It can also fail to determine which type of sequencer operation is in
effect -- in multiple ways. And it distinguishes those with via res <
0 vs. res == 0. I guess it makes sense, but it seems a bit weird.
Not sure what to suggest. One possibly bad idea just to get the
creative juices flowing: Maybe change the call signature to
enum commit_whence sequencer_determine_whence(struct repository *r,
int amending, enum commit_whence default);
with callers passing default == FROM_COMMIT, and the function can
return FROM_COMMIT if there is no sequencer state, or FROM_ERROR
(FROM_INVALID? FROM_FAILURE?) if there is sequencer state but it
cannot be read.
(And maybe do something similar with patch 6?)
> + else if (!res)
> + whence = FROM_COMMIT;
> + }
> if (s)
> s->whence = whence;
> }
> @@ -192,7 +212,6 @@ static void status_init_config(struct wt_status *s, config_fn_t fn)
> wt_status_prepare(the_repository, s);
> init_diff_ui_defaults();
> git_config(fn, s);
> - determine_whence(s);
> s->hints = advice_status_hints; /* must come after git_config() */
> }
>
> @@ -943,9 +962,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
> */
> if (!committable && whence != FROM_MERGE && !allow_empty &&
> !(amend && is_a_merge(current_head))) {
> + fprintf(stderr, "\nwhence = %d\n", whence);
Leftover debugging?
> s->display_comment_prefix = old_display_comment_prefix;
> run_status(stdout, index_file, prefix, 0, s);
> - if (amend)
> + if (whence == FROM_REBASE_FIXUP)
> + fputs(_(empty_rebase_fixup_advice), stderr);
> + else if (amend)
> fputs(_(empty_amend_advice), stderr);
> else if (is_from_cherry_pick(whence) ||
> whence == FROM_REBASE_PICK) {
> @@ -1114,6 +1136,8 @@ static void finalize_deferred_config(struct wt_status *s)
>
> if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
> s->ahead_behind_flags = AHEAD_BEHIND_FULL;
> +
> + determine_whence(s);
> }
>
> static int parse_and_validate_options(int argc, const char *argv[],
> diff --git a/sequencer.c b/sequencer.c
> index 6e153fea76..64242f4ce7 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5270,7 +5270,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
> return 0;
> }
>
> -int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
> +int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
> + int amending)
> {
> if (file_exists(git_path_cherry_pick_head(r))) {
> struct object_id cherry_pick_head, rebase_head;
> @@ -5286,6 +5287,34 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
> *whence = FROM_CHERRY_PICK_SINGLE;
>
> return 1;
> + } else if (amending && file_exists(rebase_path_current_fixups()) &&
> + (file_exists(git_path_squash_msg(r)) ||
> + file_exists(git_path_merge_msg(r)))) {
> + /*
> + * If rebase_path_amend() exists the user is running `git
> + * commit`, if not we're committing a fixup/squash directly from
> + * the sequencer
> + */
> + if (file_exists(rebase_path_amend())) {
> + struct strbuf rev = STRBUF_INIT;
> + struct object_id to_amend, head;
> +
> + if (get_oid("HEAD", &head))
> + return error(_("amending invalid head")); /* let commit deal with error */
> + if (!read_oneliner(&rev, rebase_path_amend(), 0))
> + return error(_("invalid file: '%s'"),
> + rebase_path_amend());
> + if (get_oid_hex(rev.buf, &to_amend))
> + return error(_("invalid contents: '%s'"),
> + rebase_path_amend());
> + if (oideq(&head, &to_amend)) {
> + *whence = FROM_REBASE_FIXUP;
> + return 1;
> + }
> + } else {
> + *whence = FROM_REBASE_FIXUP;
> + return 1;
> + }
> }
>
> return 0;
> diff --git a/sequencer.h b/sequencer.h
> index 8d451dbfcb..4e631900b4 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -202,5 +202,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
> void sequencer_post_commit_cleanup(struct repository *r);
> int sequencer_get_last_command(struct repository* r,
> enum replay_action *action);
> -int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);
> +int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
> + int amending);
> #endif /* SEQUENCER_H */
> diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
> index a927774910..bc110b66b3 100755
> --- a/t/t3403-rebase-skip.sh
> +++ b/t/t3403-rebase-skip.sh
> @@ -164,22 +164,28 @@ test_expect_success 'correct advice upon multi cherry-pick picking an empty comm
> test_i18ngrep "git cherry-pick --skip" err
> '
>
> -test_expect_success 'fixup that empties commit fails' '
> +test_expect_success 'correct advice when fixup empties commit' '
> test_when_finished "git rebase --abort" &&
> (
> set_fake_editor &&
> test_must_fail env FAKE_LINES="1 fixup 2" git rebase -i \
> - goodbye^ reverted-goodbye
> - )
> + goodbye^ reverted-goodbye 2>err
> + ) &&
> + test_i18ngrep "git rebase --skip" err &&
> + test_must_fail git commit --amend --no-edit 2>err &&
> + test_i18ngrep "git rebase --skip" err
> '
>
> -test_expect_success 'squash that empties commit fails' '
> +test_expect_success 'correct advice when squash empties commit' '
> test_when_finished "git rebase --abort" &&
> (
> set_fake_editor &&
> test_must_fail env FAKE_LINES="1 squash 2" git rebase -i \
> - goodbye^ reverted-goodbye
> - )
> + goodbye^ reverted-goodbye 2>err
> + ) &&
> + test_i18ngrep "git rebase --skip" err &&
> + test_must_fail git commit --amend --no-edit 2>err &&
> + test_i18ngrep "git rebase --skip" err
> '
>
> # Must be the last test in this file
> diff --git a/wt-status.h b/wt-status.h
> index 5f81bf7507..a4b7fe6de9 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -41,7 +41,8 @@ enum commit_whence {
> FROM_MERGE, /* commit came from merge */
> FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
> FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
> - FROM_REBASE_PICK /* commit came from a pick/reword/edit */
> + FROM_REBASE_PICK, /* commit came from a pick/reword/edit */
> + FROM_REBASE_FIXUP /* commit came from fixup/squash */
> };
>
> static inline int is_from_cherry_pick(enum commit_whence whence)
> @@ -52,7 +53,7 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
>
> static inline int is_from_rebase(enum commit_whence whence)
> {
> - return whence == FROM_REBASE_PICK;
> + return whence == FROM_REBASE_PICK || whence == FROM_REBASE_FIXUP;
> }
>
> struct wt_status_change_data {
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
` (7 preceding siblings ...)
2019-12-06 16:06 ` [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit Phillip Wood
@ 2019-12-06 16:06 ` Phillip Wood
2019-12-18 14:35 ` Phillip Wood
8 siblings, 1 reply; 31+ messages in thread
From: Phillip Wood @ 2019-12-06 16:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Since the inception of CHERRY_PICK_HEAD in d7e5c0cbfb ("Introduce
CHERRY_PICK_HEAD", 2011-02-19) 'rebase -i' has removed it when there are
conflicts. The rationale for this was that the rebase wanted to handle
the conflicts itself. However sometimes (e.g. after an edit command) the
user wants to commit the conflict resolution before making some other
changes or running some tests. Without CHERRY_PICK_HEAD the authorship
information is lost when the user makes the commit. Fix this by leaving
CHERRY_PICK_HEAD when we're not amending.
Note that this changes the output of `git status`. The advice to run
`git reset` is not appropriate for rebase as we do not allow partial
commits.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Notes:
This has semantic conflicts with ra/rebase-i-more-options as it does not
respect the options passed to rebase when the user commits
I haven't checked how this affects the shell prompt in contrib yet, I
suspect it may need changing to cope the presence of CHERRY_PICK_HEAD
during a rebase.
I'd like to change the existing authorship tests to rely on the "Original
Author" changes here, but they are a web of hidden interdependencies which is
hard to untangle.
sequencer.c | 12 ++--
t/t3404-rebase-interactive.sh | 104 +++++++++++++++++++++++++---------
t/t7512-status-help.sh | 2 -
3 files changed, 85 insertions(+), 33 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 64242f4ce7..624e96c930 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -372,11 +372,15 @@ static void print_advice(struct repository *r, int show_hint,
if (msg) {
fprintf(stderr, "%s\n", msg);
/*
- * A conflict has occurred but the porcelain
- * (typically rebase --interactive) wants to take care
- * of the commit itself so remove CHERRY_PICK_HEAD
+ * A conflict has occurred but the porcelain wants to take care
+ * of the commit itself so remove CHERRY_PICK_HEAD. Note that we
+ * do not do this for interactive rebases anymore in order to
+ * preserve the author identity when the user runs 'git commit'
+ * to commit the conflict resolution rather than relying on
+ * 'rebase --continue' to do it for them.
*/
- unlink(git_path_cherry_pick_head(r));
+ if (!is_rebase_i(opts))
+ unlink(git_path_cherry_pick_head(r));
return;
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 5afa6f28cd..5cd7db18f8 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -33,31 +33,35 @@ Initial setup:
# in the expect2 file for the 'stop on conflicting pick' test.
test_expect_success 'setup' '
- test_commit A file1 &&
- test_commit B file1 &&
- test_commit C file2 &&
- test_commit D file1 &&
- test_commit E file3 &&
- git checkout -b branch1 A &&
- test_commit F file4 &&
- test_commit G file1 &&
- test_commit H file5 &&
- git checkout -b branch2 F &&
- test_commit I file6 &&
- git checkout -b conflict-branch A &&
- test_commit one conflict &&
- test_commit two conflict &&
- test_commit three conflict &&
- test_commit four conflict &&
- git checkout -b no-conflict-branch A &&
- test_commit J fileJ &&
- test_commit K fileK &&
- test_commit L fileL &&
- test_commit M fileM &&
- git checkout -b no-ff-branch A &&
- test_commit N fileN &&
- test_commit O fileO &&
- test_commit P fileP
+ (
+ GIT_AUTHOR_NAME="Original Author" &&
+ GIT_AUTHOR_EMAIL="original.author@example.com" &&
+ test_commit A file1 &&
+ test_commit B file1 &&
+ test_commit C file2 &&
+ test_commit D file1 &&
+ test_commit E file3 &&
+ git checkout -b branch1 A &&
+ test_commit F file4 &&
+ test_commit G file1 &&
+ test_commit H file5 &&
+ git checkout -b branch2 F &&
+ test_commit I file6 &&
+ git checkout -b conflict-branch A &&
+ test_commit one conflict &&
+ test_commit two conflict &&
+ test_commit three conflict &&
+ test_commit four conflict &&
+ git checkout -b no-conflict-branch A &&
+ test_commit J fileJ &&
+ test_commit K fileK &&
+ test_commit L fileL &&
+ test_commit M fileM &&
+ git checkout -b no-ff-branch A &&
+ test_commit N fileN &&
+ test_commit O fileO &&
+ test_commit P fileP
+ )
'
# "exec" commands are run with the user shell by default, but this may
@@ -252,12 +256,12 @@ test_expect_success 'stop on conflicting pick' '
-A
+G
EOF
- cat >expect2 <<-\EOF &&
+ cat >expect2 <<-EOF &&
<<<<<<< HEAD
D
=======
G
- >>>>>>> 5d18e54... G
+ >>>>>>> $(git rev-parse --short HEAD)... G
EOF
git tag new-branch1 &&
test_must_fail git rebase -i master &&
@@ -1628,6 +1632,52 @@ test_expect_success 'correct error message for commit --amend after empty pick'
test_i18ngrep "middle of a rebase -- cannot amend." err
'
+test_expect_success 'correct error message for partial commit after confilct' '
+ test_when_finished "git rebase --abort" &&
+ git checkout D &&
+ (
+ set_fake_editor &&
+ FAKE_LINES="2 3" &&
+ export FAKE_LINES &&
+ test_must_fail git rebase -i A
+ ) &&
+ echo x >file1 &&
+ echo y >file2 &&
+ git add file1 file2 &&
+ test_must_fail git commit file1 2>err &&
+ test_i18ngrep "cannot do a partial commit during a rebase." err
+'
+
+test_expect_success 'correct error message for commit --amend after conflict' '
+ test_when_finished "git rebase --abort" &&
+ git checkout D &&
+ (
+ set_fake_editor &&
+ FAKE_LINES=3 &&
+ export FAKE_LINES &&
+ test_must_fail git rebase -i A
+ ) &&
+ echo x>file1 &&
+ test_must_fail git commit -a --amend 2>err &&
+ test_i18ngrep "middle of a rebase -- cannot amend." err
+'
+
+test_expect_success 'correct authorship and message after conflict' '
+ git checkout D &&
+ (
+ set_fake_editor &&
+ FAKE_LINES=3 &&
+ export FAKE_LINES &&
+ test_must_fail git rebase -i A
+ ) &&
+ echo x >file1 &&
+ git commit -a &&
+ git log --pretty=format:"%an <%ae>%n%ad%n%B" -1 D >expect &&
+ git log --pretty=format:"%an <%ae>%n%ad%n%B" -1 HEAD >actual &&
+ test_cmp expect actual &&
+ git rebase --continue
+'
+
# This must be the last test in this file
test_expect_success '$EDITOR and friends are unchanged' '
test_editor_unchanged
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index c1eb72555d..2adceb35e2 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -148,7 +148,6 @@ You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO
(use "git rebase --abort" to check out the original branch)
Unmerged paths:
- (use "git reset HEAD <file>..." to unstage)
(use "git add <file>..." to mark resolution)
both modified: main.txt
@@ -176,7 +175,6 @@ You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO
(all conflicts fixed: run "git rebase --continue")
Changes to be committed:
- (use "git reset HEAD <file>..." to unstage)
modified: main.txt
--
2.24.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts
2019-12-06 16:06 ` [PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts Phillip Wood
@ 2019-12-18 14:35 ` Phillip Wood
0 siblings, 0 replies; 31+ messages in thread
From: Phillip Wood @ 2019-12-18 14:35 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood
On 06/12/2019 16:06, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Since the inception of CHERRY_PICK_HEAD in d7e5c0cbfb ("Introduce
> CHERRY_PICK_HEAD", 2011-02-19) 'rebase -i' has removed it when there are
> conflicts. The rationale for this was that the rebase wanted to handle
> the conflicts itself. However sometimes (e.g. after an edit command) the
> user wants to commit the conflict resolution before making some other
> changes or running some tests. Without CHERRY_PICK_HEAD the authorship
> information is lost when the user makes the commit. Fix this by leaving
> CHERRY_PICK_HEAD when we're not amending.
I'm not so sure about this approach as it wont work with 'merge'
commands when rebasing. I wonder if it would be better to add a new file
COMMIT_AUTHOR (or maybe MERGE_AUTHOR) that can be parsed by
split_ident() and sets the authorship for a commit. The file would
override $GIT_AUTHOR_NAME/EMAIL/DATE but could be overridden on the
commandline by --author/date/reset-author
Best Wishes
Phillip
> Note that this changes the output of `git status`. The advice to run
> `git reset` is not appropriate for rebase as we do not allow partial
> commits.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>
> Notes:
> This has semantic conflicts with ra/rebase-i-more-options as it does not
> respect the options passed to rebase when the user commits
>
> I haven't checked how this affects the shell prompt in contrib yet, I
> suspect it may need changing to cope the presence of CHERRY_PICK_HEAD
> during a rebase.
>
> I'd like to change the existing authorship tests to rely on the "Original
> Author" changes here, but they are a web of hidden interdependencies which is
> hard to untangle.
>
> sequencer.c | 12 ++--
> t/t3404-rebase-interactive.sh | 104 +++++++++++++++++++++++++---------
> t/t7512-status-help.sh | 2 -
> 3 files changed, 85 insertions(+), 33 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 64242f4ce7..624e96c930 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -372,11 +372,15 @@ static void print_advice(struct repository *r, int show_hint,
> if (msg) {
> fprintf(stderr, "%s\n", msg);
> /*
> - * A conflict has occurred but the porcelain
> - * (typically rebase --interactive) wants to take care
> - * of the commit itself so remove CHERRY_PICK_HEAD
> + * A conflict has occurred but the porcelain wants to take care
> + * of the commit itself so remove CHERRY_PICK_HEAD. Note that we
> + * do not do this for interactive rebases anymore in order to
> + * preserve the author identity when the user runs 'git commit'
> + * to commit the conflict resolution rather than relying on
> + * 'rebase --continue' to do it for them.
> */
> - unlink(git_path_cherry_pick_head(r));
> + if (!is_rebase_i(opts))
> + unlink(git_path_cherry_pick_head(r));
> return;
> }
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 5afa6f28cd..5cd7db18f8 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -33,31 +33,35 @@ Initial setup:
> # in the expect2 file for the 'stop on conflicting pick' test.
>
> test_expect_success 'setup' '
> - test_commit A file1 &&
> - test_commit B file1 &&
> - test_commit C file2 &&
> - test_commit D file1 &&
> - test_commit E file3 &&
> - git checkout -b branch1 A &&
> - test_commit F file4 &&
> - test_commit G file1 &&
> - test_commit H file5 &&
> - git checkout -b branch2 F &&
> - test_commit I file6 &&
> - git checkout -b conflict-branch A &&
> - test_commit one conflict &&
> - test_commit two conflict &&
> - test_commit three conflict &&
> - test_commit four conflict &&
> - git checkout -b no-conflict-branch A &&
> - test_commit J fileJ &&
> - test_commit K fileK &&
> - test_commit L fileL &&
> - test_commit M fileM &&
> - git checkout -b no-ff-branch A &&
> - test_commit N fileN &&
> - test_commit O fileO &&
> - test_commit P fileP
> + (
> + GIT_AUTHOR_NAME="Original Author" &&
> + GIT_AUTHOR_EMAIL="original.author@example.com" &&
> + test_commit A file1 &&
> + test_commit B file1 &&
> + test_commit C file2 &&
> + test_commit D file1 &&
> + test_commit E file3 &&
> + git checkout -b branch1 A &&
> + test_commit F file4 &&
> + test_commit G file1 &&
> + test_commit H file5 &&
> + git checkout -b branch2 F &&
> + test_commit I file6 &&
> + git checkout -b conflict-branch A &&
> + test_commit one conflict &&
> + test_commit two conflict &&
> + test_commit three conflict &&
> + test_commit four conflict &&
> + git checkout -b no-conflict-branch A &&
> + test_commit J fileJ &&
> + test_commit K fileK &&
> + test_commit L fileL &&
> + test_commit M fileM &&
> + git checkout -b no-ff-branch A &&
> + test_commit N fileN &&
> + test_commit O fileO &&
> + test_commit P fileP
> + )
> '
>
> # "exec" commands are run with the user shell by default, but this may
> @@ -252,12 +256,12 @@ test_expect_success 'stop on conflicting pick' '
> -A
> +G
> EOF
> - cat >expect2 <<-\EOF &&
> + cat >expect2 <<-EOF &&
> <<<<<<< HEAD
> D
> =======
> G
> - >>>>>>> 5d18e54... G
> + >>>>>>> $(git rev-parse --short HEAD)... G
> EOF
> git tag new-branch1 &&
> test_must_fail git rebase -i master &&
> @@ -1628,6 +1632,52 @@ test_expect_success 'correct error message for commit --amend after empty pick'
> test_i18ngrep "middle of a rebase -- cannot amend." err
> '
>
> +test_expect_success 'correct error message for partial commit after confilct' '
> + test_when_finished "git rebase --abort" &&
> + git checkout D &&
> + (
> + set_fake_editor &&
> + FAKE_LINES="2 3" &&
> + export FAKE_LINES &&
> + test_must_fail git rebase -i A
> + ) &&
> + echo x >file1 &&
> + echo y >file2 &&
> + git add file1 file2 &&
> + test_must_fail git commit file1 2>err &&
> + test_i18ngrep "cannot do a partial commit during a rebase." err
> +'
> +
> +test_expect_success 'correct error message for commit --amend after conflict' '
> + test_when_finished "git rebase --abort" &&
> + git checkout D &&
> + (
> + set_fake_editor &&
> + FAKE_LINES=3 &&
> + export FAKE_LINES &&
> + test_must_fail git rebase -i A
> + ) &&
> + echo x>file1 &&
> + test_must_fail git commit -a --amend 2>err &&
> + test_i18ngrep "middle of a rebase -- cannot amend." err
> +'
> +
> +test_expect_success 'correct authorship and message after conflict' '
> + git checkout D &&
> + (
> + set_fake_editor &&
> + FAKE_LINES=3 &&
> + export FAKE_LINES &&
> + test_must_fail git rebase -i A
> + ) &&
> + echo x >file1 &&
> + git commit -a &&
> + git log --pretty=format:"%an <%ae>%n%ad%n%B" -1 D >expect &&
> + git log --pretty=format:"%an <%ae>%n%ad%n%B" -1 HEAD >actual &&
> + test_cmp expect actual &&
> + git rebase --continue
> +'
> +
> # This must be the last test in this file
> test_expect_success '$EDITOR and friends are unchanged' '
> test_editor_unchanged
> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
> index c1eb72555d..2adceb35e2 100755
> --- a/t/t7512-status-help.sh
> +++ b/t/t7512-status-help.sh
> @@ -148,7 +148,6 @@ You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO
> (use "git rebase --abort" to check out the original branch)
>
> Unmerged paths:
> - (use "git reset HEAD <file>..." to unstage)
> (use "git add <file>..." to mark resolution)
>
> both modified: main.txt
> @@ -176,7 +175,6 @@ You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO
> (all conflicts fixed: run "git rebase --continue")
>
> Changes to be committed:
> - (use "git reset HEAD <file>..." to unstage)
>
> modified: main.txt
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread