From 8d57f7574993deb906461d4267373e1c9c733053 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:10 +0000 Subject: commit: use enum value for multiple cherry-picks Add FROM_CHERRY_PICK_MULTI for a sequence of cherry-picks rather than using a separate variable. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) (limited to 'builtin/commit.c') diff --git a/builtin/commit.c b/builtin/commit.c index 294dc574cd..f5e354c9f0 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); @@ -1156,7 +1153,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) @@ -1179,7 +1176,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); @@ -1188,7 +1185,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); } @@ -1606,7 +1603,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); -- cgit v1.2.3 From 901ba7b1efe8ba9464aac528ecd46e8dd4f01003 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:11 +0000 Subject: commit: encapsulate determine_whence() for sequencer 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 Signed-off-by: Junio C Hamano --- builtin/commit.c | 5 +---- sequencer.c | 13 ++++++++++++- sequencer.h | 3 ++- 3 files changed, 15 insertions(+), 6 deletions(-) (limited to 'builtin/commit.c') diff --git a/builtin/commit.c b/builtin/commit.c index f5e354c9f0..8a7c839d57 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 d66856818a..dcc2063d33 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") @@ -5312,3 +5312,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 9f9ae291e3..e56e29ceea 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); @@ -208,4 +208,5 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, void sequencer_post_commit_cleanup(struct repository *r, int verbose); 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 */ -- cgit v1.2.3 From 430b75f7209c554d88e3554eb54cebf4ba1e4608 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:12 +0000 Subject: 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 Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- 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(-) (limited to 'builtin/commit.c') diff --git a/builtin/commit.c b/builtin/commit.c index 8a7c839d57..efc201d581 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; } @@ -1152,6 +1160,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")); @@ -1173,7 +1183,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); @@ -1182,7 +1193,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); } @@ -1602,6 +1614,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 dcc2063d33..47e36b6a3c 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1429,9 +1429,19 @@ out: 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; @@ -1456,8 +1466,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); @@ -2964,9 +2979,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)); @@ -5316,8 +5329,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 99ecd2579f..cdbf763385 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1599,6 +1599,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 e38e0942dc..73ab5d4da1 100644 --- a/wt-status.h +++ b/wt-status.h @@ -39,7 +39,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) @@ -48,6 +49,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; -- cgit v1.2.3