From 780308d060b2756ed52913ad2c1328c0d7049c05 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:06 +0000 Subject: t3404: use test_cmp_rev 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 Signed-off-by: Junio C Hamano --- 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 bf0dc756d2..99ecd2579f 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -186,7 +186,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' ' @@ -195,16 +195,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 ' ' 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' ' @@ -213,12 +213,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' ' @@ -278,7 +278,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 ' @@ -321,7 +321,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 ' @@ -338,7 +338,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' ' @@ -397,9 +397,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 && @@ -431,7 +431,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 ' @@ -738,7 +738,7 @@ test_expect_success 'do "noop" when there is nothing to cherry-pick' ' --author="Somebody else " && 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 ' @@ -797,7 +797,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' ' @@ -820,7 +820,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" && @@ -884,7 +884,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 ' -- cgit v1.2.3 From 5b7a64df40bec755b0d9ef477764e45ba72abf7b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 16:06:07 +0000 Subject: cherry-pick: add test for `--skip` advice in `git commit` 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 Signed-off-by: Junio C Hamano --- 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 ' -- cgit v1.2.3 From f028d661c74f29de0e26494e6c9fbcf186b0d4ae Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:08 +0000 Subject: cherry-pick: check commit error messages 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 Signed-off-by: Junio C Hamano --- 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 && -- cgit v1.2.3 From 21b11c6d1de3840dcaf3fa82dae14ca3924f689f Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 6 Dec 2019 16:06:09 +0000 Subject: sequencer: write CHERRY_PICK_HEAD for reword and edit `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 Signed-off-by: Junio C Hamano --- 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 d18aba62f5..d66856818a 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" && ( -- cgit v1.2.3 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 +++++++++++-------------- wt-status.h | 9 ++++++++- 2 files changed, 19 insertions(+), 15 deletions(-) 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); diff --git a/wt-status.h b/wt-status.h index 71c3f25f43..e38e0942dc 100644 --- a/wt-status.h +++ b/wt-status.h @@ -38,9 +38,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; -- 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(-) 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(-) 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