sequencer: fix empty commit check when amending
This fixes a regression introduced in 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24). When amending a commit try_to_commit() was using the wrong parent when checking if the commit would be empty. When amending we need to check against HEAD^ not HEAD. t3403 may not seem like the natural home for the new tests but a further patch series will improve the advice printed by `git commit`. That series will mutate these tests to check that the advice includes suggesting `rebase --skip` to skip the fixup that would empty the commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Nov 22, 2019 at 19:43 UTC
2d05ef2778b99e05e6a8a40c5b8d59a3c4ab1274
2 files changed
+53
-5
sequencer.c
+21
-5
@@ -1351,11 +1351,27 @@ static int try_to_commit(struct repository *r,
1351
goto out;
1352
}
1353
1354
- if (!(flags & ALLOW_EMPTY) && oideq(current_head ?
1355
- get_commit_tree_oid(current_head) :
1356
- the_hash_algo->empty_tree, &tree)) {
1357
- res = 1; /* run 'git commit' to display error message */
1358
- goto out;
1354
+ if (!(flags & ALLOW_EMPTY)) {
1355
+ struct commit *first_parent = current_head;
1356
+
1357
+ if (flags & AMEND_MSG) {
1358
+ if (current_head->parents) {
1359
+ first_parent = current_head->parents->item;
1360
+ if (repo_parse_commit(r, first_parent)) {
1361
+ res = error(_("could not parse HEAD commit"));
1362
+ goto out;
1363
+ }
1364
+ } else {
1365
+ first_parent = NULL;
1366
+ }
1367
+ }
1368
+ if (oideq(first_parent
1369
+ ? get_commit_tree_oid(first_parent)
1370
+ : the_hash_algo->empty_tree,
1371
+ &tree)) {
1372
+ res = 1; /* run 'git commit' to display error message */
1373
+ goto out;
1374
+ }
1375
}
1376
1377
if (find_hook("prepare-commit-msg")) {
t/t3403-rebase-skip.sh
+32
@@ -7,6 +7,8 @@ test_description='git rebase --merge --skip tests'
7
8
. ./test-lib.sh
9
10
+. "$TEST_DIRECTORY"/lib-rebase.sh
11
+
12
# we assume the default git am -3 --skip strategy is tested independently
13
# and always works :)
14
@@ -20,6 +22,13 @@ test_expect_success setup '
22
git commit -a -m "hello world" &&
23
echo goodbye >> hello &&
24
git commit -a -m "goodbye" &&
25
+ git tag goodbye &&
26
+
27
+ git checkout --detach &&
28
+ git checkout HEAD^ . &&
29
+ test_tick &&
30
+ git commit -m reverted-goodbye &&
31
+ git tag reverted-goodbye &&
32
33
git checkout -f skip-reference &&
34
echo moo > hello &&
@@ -76,4 +85,27 @@ test_expect_success 'moved back to branch correctly' '
85
86
test_debug 'gitk --all & sleep 1'
87
88
+test_expect_success 'fixup that empties commit fails' '
89
+ test_when_finished "git rebase --abort" &&
90
+ (
91
+ set_fake_editor &&
92
+ test_must_fail env FAKE_LINES="1 fixup 2" git rebase -i \
93
+ goodbye^ reverted-goodbye
94
+ )
95
+'
96
+
97
+test_expect_success 'squash that empties commit fails' '
98
+ test_when_finished "git rebase --abort" &&
99
+ (
100
+ set_fake_editor &&
101
+ test_must_fail env FAKE_LINES="1 squash 2" git rebase -i \
102
+ goodbye^ reverted-goodbye
103
+ )
104
+'
105
+
106
+# Must be the last test in this file
107
+test_expect_success '$EDITOR and friends are unchanged' '
108
+ test_editor_unchanged
109
+'
110
+
111
test_done