commit: give correct advice for empty commit during a rebase
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Dec 6, 2019 at 16:06 UTC
430b75f7209c554d88e3554eb54cebf4ba1e4608
5 files changed
+114
-19
builtin/commit.c
+19
-5
@@ -59,6 +59,9 @@ N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\
59
" git commit --allow-empty\n"
60
"\n");
61
62
+static const char empty_rebase_pick_advice[] =
63
+N_("Otherwise, please use 'git rebase --skip'\n");
64
+
65
static const char empty_cherry_pick_advice_single[] =
66
N_("Otherwise, please use 'git cherry-pick --skip'\n");
67
@@ -449,6 +452,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
452
die(_("cannot do a partial commit during a merge."));
453
else if (is_from_cherry_pick(whence))
454
die(_("cannot do a partial commit during a cherry-pick."));
455
+ else if (is_from_rebase(whence))
456
+ die(_("cannot do a partial commit during a rebase."));
457
}
458
459
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,
770
*/
771
else if (whence == FROM_MERGE)
772
hook_arg1 = "merge";
768
- else if (is_from_cherry_pick(whence)) {
773
+ else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) {
774
hook_arg1 = "commit";
775
hook_arg2 = "CHERRY_PICK_HEAD";
776
}
@@ -942,12 +947,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
947
run_status(stdout, index_file, prefix, 0, s);
948
if (amend)
949
fputs(_(empty_amend_advice), stderr);
945
- else if (is_from_cherry_pick(whence)) {
950
+ else if (is_from_cherry_pick(whence) ||
951
+ whence == FROM_REBASE_PICK) {
952
fputs(_(empty_cherry_pick_advice), stderr);
953
if (whence == FROM_CHERRY_PICK_SINGLE)
954
fputs(_(empty_cherry_pick_advice_single), stderr);
949
- else
955
+ else if (whence == FROM_CHERRY_PICK_MULTI)
956
fputs(_(empty_cherry_pick_advice_multi), stderr);
957
+ else
958
+ fputs(_(empty_rebase_pick_advice), stderr);
959
}
960
return 0;
961
}
@@ -1152,6 +1160,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
1160
die(_("You are in the middle of a merge -- cannot amend."));
1161
else if (is_from_cherry_pick(whence))
1162
die(_("You are in the middle of a cherry-pick -- cannot amend."));
1163
+ else if (whence == FROM_REBASE_PICK)
1164
+ die(_("You are in the middle of a rebase -- cannot amend."));
1165
}
1166
if (fixup_message && squash_message)
1167
die(_("Options --squash and --fixup cannot be used together"));
@@ -1173,7 +1183,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
1183
use_message = edit_message;
1184
if (amend && !use_message && !fixup_message)
1185
use_message = "HEAD";
1176
- if (!use_message && !is_from_cherry_pick(whence) && renew_authorship)
1186
+ if (!use_message && !is_from_cherry_pick(whence) &&
1187
+ !is_from_rebase(whence) && renew_authorship)
1188
die(_("--reset-author can be used only with -C, -c or --amend."));
1189
if (use_message) {
1190
use_message_buffer = read_commit_message(use_message);
@@ -1182,7 +1193,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
1193
author_message_buffer = use_message_buffer;
1194
}
1195
}
1185
- if (is_from_cherry_pick(whence) && !renew_authorship) {
1196
+ if ((is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) &&
1197
+ !renew_authorship) {
1198
author_message = "CHERRY_PICK_HEAD";
1199
author_message_buffer = read_commit_message(author_message);
1200
}
@@ -1602,6 +1614,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
1614
if (!reflog_msg)
1615
reflog_msg = is_from_cherry_pick(whence)
1616
? "commit (cherry-pick)"
1617
+ : is_from_rebase(whence)
1618
+ ? "commit (rebase)"
1619
: "commit";
1620
commit_list_insert(current_head, &parents);
1621
}
sequencer.c
+31
-8
@@ -1429,9 +1429,19 @@ out:
1429
return res;
1430
}
1431
1432
+static int write_rebase_head(struct object_id *oid)
1433
+{
1434
+ if (update_ref("rebase", "REBASE_HEAD", oid,
1435
+ NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
1436
+ return error(_("could not update %s"), "REBASE_HEAD");
1437
+
1438
+ return 0;
1439
+}
1440
+
1441
static int do_commit(struct repository *r,
1442
const char *msg_file, const char *author,
1434
- struct replay_opts *opts, unsigned int flags)
1443
+ struct replay_opts *opts, unsigned int flags,
1444
+ struct object_id *oid)
1445
{
1446
int res = 1;
1447
@@ -1456,8 +1466,12 @@ static int do_commit(struct repository *r,
1466
return res;
1467
}
1468
}
1459
- if (res == 1)
1469
+ if (res == 1) {
1470
+ if (is_rebase_i(opts) && oid)
1471
+ if (write_rebase_head(oid))
1472
+ return -1;
1473
return run_git_commit(r, msg_file, opts, flags);
1474
+ }
1475
1476
return res;
1477
}
@@ -1945,7 +1959,8 @@ static int do_pick_commit(struct repository *r,
1959
flags |= ALLOW_EMPTY;
1960
if (!opts->no_commit) {
1961
if (author || command == TODO_REVERT || (flags & AMEND_MSG))
1948
- res = do_commit(r, msg_file, author, opts, flags);
1962
+ res = do_commit(r, msg_file, author, opts, flags,
1963
+ commit? &commit->object.oid : NULL);
1964
else
1965
res = error(_("unable to parse commit author"));
1966
*check_todo = !!(flags & EDIT_MSG);
@@ -2964,9 +2979,7 @@ static int make_patch(struct repository *r,
2979
p = short_commit_name(commit);
2980
if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
2981
return -1;
2967
- if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
2968
- NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
2969
- res |= error(_("could not update %s"), "REBASE_HEAD");
2982
+ res |= write_rebase_head(&commit->object.oid);
2983
2984
strbuf_addf(&buf, "%s/patch", get_dir(opts));
2985
memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -5316,8 +5329,18 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
5329
int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
5330
{
5331
if (file_exists(git_path_cherry_pick_head(r))) {
5319
- *whence = file_exists(git_path_seq_dir()) ?
5320
- FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
5332
+ struct object_id cherry_pick_head, rebase_head;
5333
+
5334
+ if (file_exists(git_path_seq_dir()))
5335
+ *whence = FROM_CHERRY_PICK_MULTI;
5336
+ if (file_exists(rebase_path()) &&
5337
+ !get_oid("REBASE_HEAD", &rebase_head) &&
5338
+ !get_oid("CHERRY_PICK_HEAD", &cherry_pick_head) &&
5339
+ oideq(&rebase_head, &cherry_pick_head))
5340
+ *whence = FROM_REBASE_PICK;
5341
+ else
5342
+ *whence = FROM_CHERRY_PICK_SINGLE;
5343
+
5344
return 1;
5345
}
5346
t/t3403-rebase-skip.sh
+31
-5
@@ -97,9 +97,9 @@ test_expect_success 'correct advice upon picking empty commit' '
97
test_must_fail git rebase -i --onto goodbye \
98
amended-goodbye^ amended-goodbye 2>err &&
99
test_i18ngrep "previous cherry-pick is now empty" err &&
100
- test_i18ngrep "git cherry-pick --skip" err &&
100
+ test_i18ngrep "git rebase --skip" err &&
101
test_must_fail git commit &&
102
- test_i18ngrep "git cherry-pick --skip" err
102
+ test_i18ngrep "git rebase --skip" err
103
'
104
105
test_expect_success 'correct authorship when committing empty pick' '
@@ -120,9 +120,9 @@ test_expect_success 'correct advice upon rewording empty commit' '
120
--onto goodbye amended-goodbye^ amended-goodbye 2>err
121
) &&
122
test_i18ngrep "previous cherry-pick is now empty" err &&
123
- test_i18ngrep "git cherry-pick --skip" err &&
123
+ test_i18ngrep "git rebase --skip" err &&
124
test_must_fail git commit &&
125
- test_i18ngrep "git cherry-pick --skip" err
125
+ test_i18ngrep "git rebase --skip" err
126
'
127
128
test_expect_success 'correct advice upon editing empty commit' '
@@ -133,8 +133,34 @@ test_expect_success 'correct advice upon editing empty commit' '
133
--onto goodbye amended-goodbye^ amended-goodbye 2>err
134
) &&
135
test_i18ngrep "previous cherry-pick is now empty" err &&
136
- test_i18ngrep "git cherry-pick --skip" err &&
136
+ test_i18ngrep "git rebase --skip" err &&
137
test_must_fail git commit &&
138
+ test_i18ngrep "git rebase --skip" err
139
+'
140
+
141
+test_expect_success 'correct advice upon cherry-picking an empty commit during a rebase' '
142
+ test_when_finished "git rebase --abort" &&
143
+ (
144
+ set_fake_editor &&
145
+ test_must_fail env FAKE_LINES="1 exec_git_cherry-pick_amended-goodbye" \
146
+ git rebase -i goodbye^ goodbye 2>err
147
+ ) &&
148
+ test_i18ngrep "previous cherry-pick is now empty" err &&
149
+ test_i18ngrep "git cherry-pick --skip" err &&
150
+ test_must_fail git commit 2>err &&
151
+ test_i18ngrep "git cherry-pick --skip" err
152
+'
153
+
154
+test_expect_success 'correct advice upon multi cherry-pick picking an empty commit during a rebase' '
155
+ test_when_finished "git rebase --abort" &&
156
+ (
157
+ set_fake_editor &&
158
+ test_must_fail env FAKE_LINES="1 exec_git_cherry-pick_goodbye_amended-goodbye" \
159
+ git rebase -i goodbye^^ goodbye 2>err
160
+ ) &&
161
+ test_i18ngrep "previous cherry-pick is now empty" err &&
162
+ test_i18ngrep "git cherry-pick --skip" err &&
163
+ test_must_fail git commit 2>err &&
164
test_i18ngrep "git cherry-pick --skip" err
165
'
166
t/t3404-rebase-interactive.sh
+26
@@ -1599,6 +1599,32 @@ test_expect_success 'post-commit hook is called' '
1599
test_cmp expect actual
1600
'
1601
1602
+test_expect_success 'correct error message for partial commit after empty pick' '
1603
+ test_when_finished "git rebase --abort" &&
1604
+ (
1605
+ set_fake_editor &&
1606
+ FAKE_LINES="2 1 1" &&
1607
+ export FAKE_LINES &&
1608
+ test_must_fail git rebase -i A D
1609
+ ) &&
1610
+ echo x >file1 &&
1611
+ test_must_fail git commit file1 2>err &&
1612
+ test_i18ngrep "cannot do a partial commit during a rebase." err
1613
+'
1614
+
1615
+test_expect_success 'correct error message for commit --amend after empty pick' '
1616
+ test_when_finished "git rebase --abort" &&
1617
+ (
1618
+ set_fake_editor &&
1619
+ FAKE_LINES="1 1" &&
1620
+ export FAKE_LINES &&
1621
+ test_must_fail git rebase -i A D
1622
+ ) &&
1623
+ echo x>file1 &&
1624
+ test_must_fail git commit -a --amend 2>err &&
1625
+ test_i18ngrep "middle of a rebase -- cannot amend." err
1626
+'
1627
+
1628
# This must be the last test in this file
1629
test_expect_success '$EDITOR and friends are unchanged' '
1630
test_editor_unchanged
wt-status.h
+7
-1
@@ -39,7 +39,8 @@ enum commit_whence {
39
FROM_COMMIT, /* normal */
40
FROM_MERGE, /* commit came from merge */
41
FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
42
- FROM_CHERRY_PICK_MULTI /* commit came from a sequence of cherry-picks */
42
+ FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
43
+ FROM_REBASE_PICK /* commit came from a pick/reword/edit */
44
};
45
46
static inline int is_from_cherry_pick(enum commit_whence whence)
@@ -48,6 +49,11 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
49
whence == FROM_CHERRY_PICK_MULTI;
50
}
51
52
+static inline int is_from_rebase(enum commit_whence whence)
53
+{
54
+ return whence == FROM_REBASE_PICK;
55
+}
56
+
57
struct wt_status_change_data {
58
int worktree_status;
59
int index_status;