From 2161da103950704a92993702696c9b573dcbccae Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 19 Feb 2011 23:12:26 -0500 Subject: [PATCH 1/4] t3507: introduce pristine-detach helper All the tests in t3507 (cherry-pick with conflicts) begin with the same checkout + read-tree + clean incantation to ensure a predictable starting point. Factor out a function for that so the interesting part of the tests is easier to read. The "update-index --refresh" and "diff-index --exit-code HEAD" are not necessary as the point of this testsuite is not about testing "read-tree --reset". Improved-by: Junio C Hamano Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3507-cherry-pick-conflict.sh | 76 ++++++++++----------------------- 1 file changed, 22 insertions(+), 54 deletions(-) diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 607bf25d8f..2313d79bd4 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -11,6 +11,18 @@ test_description='test cherry-pick and revert with conflicts . ./test-lib.sh +test_cmp_rev () { + git rev-parse --verify "$1" >expect.rev && + git rev-parse --verify "$2" >actual.rev && + test_cmp expect.rev actual.rev +} + +pristine_detach () { + git checkout -f "$1^0" && + git read-tree -u --reset HEAD && + git clean -d -f -f -q -x +} + test_expect_success setup ' echo unrelated >unrelated && @@ -23,13 +35,7 @@ test_expect_success setup ' ' test_expect_success 'failed cherry-pick does not advance HEAD' ' - - git checkout -f initial^0 && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && - - git update-index --refresh && - git diff-index --exit-code HEAD && + pristine_detach initial && head=$(git rev-parse HEAD) && test_must_fail git cherry-pick picked && @@ -39,12 +45,7 @@ test_expect_success 'failed cherry-pick does not advance HEAD' ' ' test_expect_success 'advice from failed cherry-pick' " - git checkout -f initial^0 && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && - - git update-index --refresh && - git diff-index --exit-code HEAD && + pristine_detach initial && picked=\$(git rev-parse --short picked) && cat <<-EOF >expected && @@ -59,13 +60,7 @@ test_expect_success 'advice from failed cherry-pick' " " test_expect_success 'failed cherry-pick produces dirty index' ' - - git checkout -f initial^0 && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && - - git update-index --refresh && - git diff-index --exit-code HEAD && + pristine_detach initial && test_must_fail git cherry-pick picked && @@ -74,9 +69,7 @@ test_expect_success 'failed cherry-pick produces dirty index' ' ' test_expect_success 'failed cherry-pick registers participants in index' ' - - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && + pristine_detach initial && { git checkout base -- foo && git ls-files --stage foo && @@ -90,10 +83,7 @@ test_expect_success 'failed cherry-pick registers participants in index' ' 2 s/ 0 / 2 / 3 s/ 0 / 3 / " < stages > expected && - git checkout -f initial^0 && - - git update-index --refresh && - git diff-index --exit-code HEAD && + git read-tree -u --reset HEAD && test_must_fail git cherry-pick picked && git ls-files --stage --unmerged > actual && @@ -102,10 +92,7 @@ test_expect_success 'failed cherry-pick registers participants in index' ' ' test_expect_success 'failed cherry-pick describes conflict in work tree' ' - - git checkout -f initial^0 && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && + pristine_detach initial && cat <<-EOF > expected && <<<<<<< HEAD a @@ -114,9 +101,6 @@ test_expect_success 'failed cherry-pick describes conflict in work tree' ' >>>>>>> objid picked EOF - git update-index --refresh && - git diff-index --exit-code HEAD && - test_must_fail git cherry-pick picked && sed "s/[a-f0-9]*\.\.\./objid/" foo > actual && @@ -124,11 +108,8 @@ test_expect_success 'failed cherry-pick describes conflict in work tree' ' ' test_expect_success 'diff3 -m style' ' - + pristine_detach initial && git config merge.conflictstyle diff3 && - git checkout -f initial^0 && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && cat <<-EOF > expected && <<<<<<< HEAD a @@ -139,9 +120,6 @@ test_expect_success 'diff3 -m style' ' >>>>>>> objid picked EOF - git update-index --refresh && - git diff-index --exit-code HEAD && - test_must_fail git cherry-pick picked && sed "s/[a-f0-9]*\.\.\./objid/" foo > actual && @@ -149,10 +127,8 @@ test_expect_success 'diff3 -m style' ' ' test_expect_success 'revert also handles conflicts sanely' ' - git config --unset merge.conflictstyle && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && + pristine_detach initial && cat <<-EOF > expected && <<<<<<< HEAD a @@ -173,10 +149,7 @@ test_expect_success 'revert also handles conflicts sanely' ' 2 s/ 0 / 2 / 3 s/ 0 / 3 / " < stages > expected-stages && - git checkout -f initial^0 && - - git update-index --refresh && - git diff-index --exit-code HEAD && + git read-tree -u --reset HEAD && head=$(git rev-parse HEAD) && test_must_fail git revert picked && @@ -192,10 +165,8 @@ test_expect_success 'revert also handles conflicts sanely' ' ' test_expect_success 'revert conflict, diff3 -m style' ' + pristine_detach initial && git config merge.conflictstyle diff3 && - git checkout -f initial^0 && - git read-tree -u --reset HEAD && - git clean -d -f -f -q -x && cat <<-EOF > expected && <<<<<<< HEAD a @@ -206,9 +177,6 @@ test_expect_success 'revert conflict, diff3 -m style' ' >>>>>>> parent of objid picked EOF - git update-index --refresh && - git diff-index --exit-code HEAD && - test_must_fail git revert picked && sed "s/[a-f0-9]*\.\.\./objid/" foo > actual && From d7e5c0cbfb0421d8a609f1125267dbad73069410 Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Sat, 19 Feb 2011 23:12:27 -0500 Subject: [PATCH 2/4] Introduce CHERRY_PICK_HEAD When a cherry-pick conflicts git advises: $ git commit -c to preserve the original commit message and authorship. Instead, let's record the original commit id in CHERRY_PICK_HEAD and advise: $ git commit -c CHERRY_PICK_HEAD A later patch teaches git to handle the '-c CHERRY_PICK_HEAD' part. Note that we record CHERRY_PICK_HEAD even in the case where there are no conflicts so that we may use it to communicate authorship to commit; this will then allow us to remove set_author_ident_env from revert.c. However, we do not record CHERRY_PICK_HEAD when --no-commit is used, as presumably the user intends to further edit the commit and possibly even cherry-pick additional commits on top. Tests and documentation contributed by Jonathan Nieder. Reviewed-by: Jonathan Nieder Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano --- Documentation/git-cherry-pick.txt | 19 ++++++++ Documentation/revisions.txt | 5 +- branch.c | 1 + builtin/commit.c | 1 + builtin/merge.c | 7 +++ builtin/revert.c | 27 ++++++++++- t/t3507-cherry-pick-conflict.sh | 76 ++++++++++++++++++++++++++++++- 7 files changed, 132 insertions(+), 4 deletions(-) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index 73008705eb..cd848664ce 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -16,6 +16,25 @@ Given one or more existing commits, apply the change each one introduces, recording a new commit for each. This requires your working tree to be clean (no modifications from the HEAD commit). +When it is not obvious how to apply a change, the following +happens: + +1. The current branch and `HEAD` pointer stay at the last commit + successfully made. +2. The `CHERRY_PICK_HEAD` ref is set to point at the commit that + introduced the change that is difficult to apply. +3. Paths in which the change applied cleanly are updated both + in the index file and in your working tree. +4. For conflicting paths, the index file records up to three + versions, as described in the "TRUE MERGE" section of + linkgit:git-merge[1]. The working tree files will include + a description of the conflict bracketed by the usual + conflict markers `<<<<<<<` and `>>>>>>>`. +5. No other modifications are made. + +See linkgit:git-merge[1] for some hints on resolving such +conflicts. + OPTIONS ------- ...:: diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 9e92734bc1..04fceee253 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -25,7 +25,8 @@ blobs contained in a commit. first match in the following rules: . if `$GIT_DIR/` exists, that is what you mean (this is usually - useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD` and `MERGE_HEAD`); + useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD`, `MERGE_HEAD` + and `CHERRY_PICK_HEAD`); . otherwise, `refs/` if exists; @@ -46,6 +47,8 @@ you can change the tip of the branch back to the state before you ran them easily. MERGE_HEAD records the commit(s) you are merging into your branch when you run 'git merge'. +CHERRY_PICK_HEAD records the commit you are cherry-picking +when you run 'git cherry-pick'. + Note that any of the `refs/*` cases above may come either from the `$GIT_DIR/refs` directory or from the `$GIT_DIR/packed-refs` file. diff --git a/branch.c b/branch.c index 93dc866f8c..dc23e95435 100644 --- a/branch.c +++ b/branch.c @@ -217,6 +217,7 @@ void create_branch(const char *head, void remove_branch_state(void) { + unlink(git_path("CHERRY_PICK_HEAD")); unlink(git_path("MERGE_HEAD")); unlink(git_path("MERGE_RR")); unlink(git_path("MERGE_MSG")); diff --git a/builtin/commit.c b/builtin/commit.c index 03cff5af63..0def5401ab 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1424,6 +1424,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) die("cannot update HEAD ref"); } + unlink(git_path("CHERRY_PICK_HEAD")); unlink(git_path("MERGE_HEAD")); unlink(git_path("MERGE_MSG")); unlink(git_path("MERGE_MODE")); diff --git a/builtin/merge.c b/builtin/merge.c index 42fff387e6..5ba2efc214 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -969,6 +969,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix) else die("You have not concluded your merge (MERGE_HEAD exists)."); } + if (file_exists(git_path("CHERRY_PICK_HEAD"))) { + if (advice_resolve_conflict) + die("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n" + "Please, commit your changes before you can merge."); + else + die("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists)."); + } resolve_undo_clear(); if (verbosity < 0) diff --git a/builtin/revert.c b/builtin/revert.c index bb6e9e83b7..e716a1b000 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -231,6 +231,22 @@ static void set_author_ident_env(const char *message) sha1_to_hex(commit->object.sha1)); } +static void write_cherry_pick_head(void) +{ + int fd; + struct strbuf buf = STRBUF_INIT; + + strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1)); + + fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666); + if (fd < 0) + die_errno("Could not open '%s' for writing", + git_path("CHERRY_PICK_HEAD")); + if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd)) + die_errno("Could not write to '%s'", git_path("CHERRY_PICK_HEAD")); + strbuf_release(&buf); +} + static void advise(const char *advice, ...) { va_list params; @@ -246,6 +262,12 @@ static void print_advice(void) if (msg) { fprintf(stderr, "%s\n", msg); + /* + * A conflict has occured but the porcelain + * (typically rebase --interactive) wants to take care + * of the commit itself so remove CHERRY_PICK_HEAD + */ + unlink(git_path("CHERRY_PICK_HEAD")); return; } @@ -253,8 +275,7 @@ static void print_advice(void) advise("with 'git add ' or 'git rm '"); if (action == CHERRY_PICK) - advise("and commit the result with 'git commit -c %s'", - find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV)); + advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'"); } static void write_message(struct strbuf *msgbuf, const char *filename) @@ -489,6 +510,8 @@ static int do_pick_commit(void) strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); strbuf_addstr(&msgbuf, ")\n"); } + if (!no_commit) + write_cherry_pick_head(); } if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) { diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 2313d79bd4..e0bf3563ac 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -52,13 +52,87 @@ test_expect_success 'advice from failed cherry-pick' " error: could not apply \$picked... picked hint: after resolving the conflicts, mark the corrected paths hint: with 'git add ' or 'git rm ' - hint: and commit the result with 'git commit -c \$picked' + hint: and commit the result with 'git commit -c CHERRY_PICK_HEAD' EOF test_must_fail git cherry-pick picked 2>actual && test_cmp expected actual " +test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' ' + pristine_detach initial && + test_must_fail git cherry-pick picked && + test_cmp_rev picked CHERRY_PICK_HEAD +' + +test_expect_success 'successful cherry-pick does not set CHERRY_PICK_HEAD' ' + pristine_detach initial && + git cherry-pick base && + test_must_fail git rev-parse --verify CHERRY_PICK_HEAD +' + +test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' ' + pristine_detach initial && + git cherry-pick --no-commit base && + test_must_fail git rev-parse --verify CHERRY_PICK_HEAD +' + +test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' ' + pristine_detach initial && + ( + GIT_CHERRY_PICK_HELP="and then do something else" && + export GIT_CHERRY_PICK_HELP && + test_must_fail git cherry-pick picked + ) && + test_must_fail git rev-parse --verify CHERRY_PICK_HEAD +' + +test_expect_success 'git reset clears CHERRY_PICK_HEAD' ' + pristine_detach initial && + + test_must_fail git cherry-pick picked && + git reset && + + test_must_fail git rev-parse --verify CHERRY_PICK_HEAD +' + +test_expect_success 'failed commit does not clear CHERRY_PICK_HEAD' ' + pristine_detach initial && + + test_must_fail git cherry-pick picked && + test_must_fail git commit && + + test_cmp_rev picked CHERRY_PICK_HEAD +' + +test_expect_success 'cancelled commit does not clear CHERRY_PICK_HEAD' ' + pristine_detach initial && + + test_must_fail git cherry-pick picked && + echo resolved >foo && + git add foo && + git update-index --refresh -q && + test_must_fail git diff-index --exit-code HEAD && + ( + GIT_EDITOR=false && + export GIT_EDITOR && + test_must_fail git commit + ) && + + test_cmp_rev picked CHERRY_PICK_HEAD +' + +test_expect_success 'successful commit clears CHERRY_PICK_HEAD' ' + pristine_detach initial && + + test_must_fail git cherry-pick picked && + echo resolved >foo && + git add foo && + git commit && + + test_must_fail git rev-parse --verify CHERRY_PICK_HEAD +' + test_expect_success 'failed cherry-pick produces dirty index' ' pristine_detach initial && From 5b2af8cac9883ab7e666d4aa30ea7fd3d1f1d4de Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Sat, 19 Feb 2011 23:12:28 -0500 Subject: [PATCH 3/4] bash: teach __git_ps1 about CHERRY_PICK_HEAD Make the git prompt (when enabled) show a CHERRY-PICKING indicator when we are in the middle of a conflicted cherry-pick, analogous to the existing MERGING and BISECTING flags. Signed-off-by: Jay Soffian Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 893b7716ca..0b0b913d28 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -246,6 +246,8 @@ __git_ps1 () fi elif [ -f "$g/MERGE_HEAD" ]; then r="|MERGING" + elif [ -f "$g/CHERRY_PICK_HEAD" ]; then + r="|CHERRY-PICKING" elif [ -f "$g/BISECT_LOG" ]; then r="|BISECTING" fi From 37f7a8579363a98efc48dfb6964a519034fc9acc Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Sat, 19 Feb 2011 23:12:29 -0500 Subject: [PATCH 4/4] Teach commit about CHERRY_PICK_HEAD Previously the user was advised to use commit -c CHERRY_PICK_HEAD after a conflicting cherry-pick. While this would preserve the original commit's authorship, it would sadly discard cherry-pick's carefully crafted MERGE_MSG (which contains the list of conflicts as well as the original commit-id in the case of cherry-pick -x). On the other hand, if a bare 'commit' were performed, it would preserve the MERGE_MSG while resetting the authorship. In other words, there was no way to simultaneously take the authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG. This change fixes that situation. A bare 'commit' will now take the authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG. If the user wishes to reset authorship, that must now be done explicitly via --reset-author. A side-benefit of passing commit authorship along this way is that we can eliminate redundant authorship parsing code from revert.c. (Also removed an unused include from revert.c) Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano --- Documentation/git-commit.txt | 7 +- builtin/commit.c | 155 +++++++++++++++++++++++--------- builtin/revert.c | 56 +----------- t/t3507-cherry-pick-conflict.sh | 2 +- t/t7509-commit.sh | 29 ++++++ wt-status.c | 4 +- wt-status.h | 9 +- 7 files changed, 158 insertions(+), 104 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index b586c0f442..fd6a1f7e25 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -84,9 +84,10 @@ OPTIONS linkgit:git-rebase[1] for details. --reset-author:: - When used with -C/-c/--amend options, declare that the - authorship of the resulting commit now belongs of the committer. - This also renews the author timestamp. + When used with -C/-c/--amend options, or when committing after a + a conflicting cherry-pick, declare that the authorship of the + resulting commit now belongs of the committer. This also renews + the author timestamp. --short:: When doing a dry-run, give the output in the short-format. See diff --git a/builtin/commit.c b/builtin/commit.c index 0def5401ab..f963cfbd44 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -54,9 +54,17 @@ static const char empty_amend_advice[] = "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_cherry_pick_advice[] = +"The previous cherry-pick is now empty, possibly due to conflict resolution.\n" +"If you wish to commit it anyway, use:\n" +"\n" +" git commit --allow-empty\n" +"\n" +"Otherwise, please use 'git reset'\n"; + static unsigned char head_sha1[20]; -static char *use_message_buffer; +static const char *use_message_buffer; static const char commit_editmsg[] = "COMMIT_EDITMSG"; static struct lock_file index_lock; /* real index */ static struct lock_file false_lock; /* used only for partial commits */ @@ -68,6 +76,11 @@ static enum { static const char *logfile, *force_author; static const char *template_file; +/* + * The _message variables are commit names from which to take + * the commit message and/or authorship. + */ +static const char *author_message, *author_message_buffer; static char *edit_message, *use_message; static char *fixup_message, *squash_message; static int all, edit_flag, also, interactive, only, amend, signoff; @@ -88,7 +101,8 @@ static enum { } cleanup_mode; static char *cleanup_arg; -static int use_editor = 1, initial_commit, in_merge, include_status = 1; +static enum commit_whence whence; +static int use_editor = 1, initial_commit, include_status = 1; static int show_ignored_in_status; static const char *only_include_assumed; static struct strbuf message; @@ -163,6 +177,36 @@ static struct option builtin_commit_options[] = { OPT_END() }; +static void determine_whence(struct wt_status *s) +{ + if (file_exists(git_path("MERGE_HEAD"))) + whence = FROM_MERGE; + else if (file_exists(git_path("CHERRY_PICK_HEAD"))) + whence = FROM_CHERRY_PICK; + else + whence = FROM_COMMIT; + if (s) + s->whence = whence; +} + +static const char *whence_s(void) +{ + char *s = ""; + + switch (whence) { + case FROM_COMMIT: + break; + case FROM_MERGE: + s = "merge"; + break; + case FROM_CHERRY_PICK: + s = "cherry-pick"; + break; + } + + return s; +} + static void rollback_index_files(void) { switch (commit_style) { @@ -378,8 +422,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int */ commit_style = COMMIT_PARTIAL; - if (in_merge) - die("cannot do a partial commit during a merge."); + if (whence != FROM_COMMIT) + die("cannot do a partial commit during a %s.", whence_s()); memset(&partial, 0, sizeof(partial)); partial.strdup_strings = 1; @@ -469,18 +513,18 @@ static void determine_author_info(struct strbuf *author_ident) email = getenv("GIT_AUTHOR_EMAIL"); date = getenv("GIT_AUTHOR_DATE"); - if (use_message && !renew_authorship) { + if (author_message) { const char *a, *lb, *rb, *eol; - a = strstr(use_message_buffer, "\nauthor "); + a = strstr(author_message_buffer, "\nauthor "); if (!a) - die("invalid commit: %s", use_message); + die("invalid commit: %s", author_message); lb = strchrnul(a + strlen("\nauthor "), '<'); rb = strchrnul(lb, '>'); eol = strchrnul(rb, '\n'); if (!*lb || !*rb || !*eol) - die("invalid commit: %s", use_message); + die("invalid commit: %s", author_message); if (lb == a + strlen("\nauthor ")) /* \nauthor */ @@ -641,11 +685,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix, } /* - * This final case does not modify the template message, - * it just sets the argument to the prepare-commit-msg hook. + * The remaining cases don't modify the template message, but + * just set the argument(s) to the prepare-commit-msg hook. */ - else if (in_merge) + else if (whence == FROM_MERGE) hook_arg1 = "merge"; + else if (whence == FROM_CHERRY_PICK) { + hook_arg1 = "commit"; + hook_arg2 = "CHERRY_PICK_HEAD"; + } if (squash_message) { /* @@ -694,16 +742,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strbuf_addstr(&committer_ident, git_committer_info(0)); if (use_editor && include_status) { char *ai_tmp, *ci_tmp; - if (in_merge) + if (whence != FROM_COMMIT) fprintf(fp, "#\n" - "# It looks like you may be committing a MERGE.\n" + "# It looks like you may be committing a %s.\n" "# If this is not correct, please remove the file\n" "# %s\n" "# and try again.\n" "#\n", - git_path("MERGE_HEAD")); - + whence_s(), + git_path(whence == FROM_MERGE + ? "MERGE_HEAD" + : "CHERRY_PICK_HEAD")); fprintf(fp, "\n" "# Please enter the commit message for your changes."); @@ -766,11 +816,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix, fclose(fp); - if (!commitable && !in_merge && !allow_empty && + /* + * Reject an attempt to record a non-merge empty commit without + * explicit --allow-empty. In the cherry-pick case, it may be + * empty due to conflict resolution, which the user should okay. + */ + if (!commitable && whence != FROM_MERGE && !allow_empty && !(amend && is_a_merge(head_sha1))) { run_status(stdout, index_file, prefix, 0, s); if (amend) fputs(empty_amend_advice, stderr); + else if (whence == FROM_CHERRY_PICK) + fputs(empty_cherry_pick_advice, stderr); return 0; } @@ -898,6 +955,28 @@ static void handle_untracked_files_arg(struct wt_status *s) die("Invalid untracked files mode '%s'", untracked_files_arg); } +static const char *read_commit_message(const char *name) +{ + const char *out_enc, *out; + struct commit *commit; + + commit = lookup_commit_reference_by_name(name); + if (!commit) + die("could not lookup commit %s", name); + out_enc = get_commit_output_encoding(); + out = logmsg_reencode(commit, out_enc); + + /* + * If we failed to reencode the buffer, just copy it + * byte for byte so the user can try to fix it up. + * This also handles the case where input and output + * encodings are identical. + */ + if (out == NULL) + out = xstrdup(commit->buffer); + return out; +} + static int parse_and_validate_options(int argc, const char *argv[], const char * const usage[], const char *prefix, @@ -927,8 +1006,8 @@ static int parse_and_validate_options(int argc, const char *argv[], /* Sanity check options */ if (amend && initial_commit) die("You have nothing to amend."); - if (amend && in_merge) - die("You are in the middle of a merge -- cannot amend."); + if (amend && whence != FROM_COMMIT) + die("You are in the middle of a %s -- cannot amend.", whence_s()); if (fixup_message && squash_message) die("Options --squash and --fixup cannot be used together"); if (use_message) @@ -947,26 +1026,18 @@ 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 && renew_authorship) + if (!use_message && whence != FROM_CHERRY_PICK && renew_authorship) die("--reset-author can be used only with -C, -c or --amend."); if (use_message) { - const char *out_enc; - struct commit *commit; - - commit = lookup_commit_reference_by_name(use_message); - if (!commit) - die("could not lookup commit %s", use_message); - out_enc = get_commit_output_encoding(); - use_message_buffer = logmsg_reencode(commit, out_enc); - - /* - * If we failed to reencode the buffer, just copy it - * byte for byte so the user can try to fix it up. - * This also handles the case where input and output - * encodings are identical. - */ - if (use_message_buffer == NULL) - use_message_buffer = xstrdup(commit->buffer); + use_message_buffer = read_commit_message(use_message); + if (!renew_authorship) { + author_message = use_message; + author_message_buffer = use_message_buffer; + } + } + if (whence == FROM_CHERRY_PICK && !renew_authorship) { + author_message = "CHERRY_PICK_HEAD"; + author_message_buffer = read_commit_message(author_message); } if (!!also + !!only + !!all + !!interactive > 1) @@ -1117,7 +1188,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) wt_status_prepare(&s); gitmodules_config(); git_config(git_status_config, &s); - in_merge = file_exists(git_path("MERGE_HEAD")); + determine_whence(&s); argc = parse_options(argc, argv, prefix, builtin_status_options, builtin_status_usage, 0); @@ -1140,7 +1211,6 @@ int cmd_status(int argc, const char **argv, const char *prefix) } s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0; - s.in_merge = in_merge; s.ignore_submodule_arg = ignore_submodule_arg; wt_status_collect(&s); @@ -1302,8 +1372,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) wt_status_prepare(&s); git_config(git_commit_config, &s); - in_merge = file_exists(git_path("MERGE_HEAD")); - s.in_merge = in_merge; + determine_whence(&s); if (s.use_color == -1) s.use_color = git_use_color_default; @@ -1340,7 +1409,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) for (c = commit->parents; c; c = c->next) pptr = &commit_list_insert(c->item, pptr)->next; - } else if (in_merge) { + } else if (whence == FROM_MERGE) { struct strbuf m = STRBUF_INIT; FILE *fp; @@ -1369,7 +1438,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix) parents = reduce_heads(parents); } else { if (!reflog_msg) - reflog_msg = "commit"; + reflog_msg = (whence == FROM_CHERRY_PICK) + ? "commit (cherry-pick)" + : "commit"; pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next; } diff --git a/builtin/revert.c b/builtin/revert.c index e716a1b000..b3a3201c44 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -3,7 +3,6 @@ #include "object.h" #include "commit.h" #include "tag.h" -#include "wt-status.h" #include "run-command.h" #include "exec_cmd.h" #include "utf8.h" @@ -181,56 +180,6 @@ static void add_message_to_msg(struct strbuf *msgbuf, const char *message) strbuf_addstr(msgbuf, p); } -static void set_author_ident_env(const char *message) -{ - const char *p = message; - if (!p) - die ("Could not read commit message of %s", - sha1_to_hex(commit->object.sha1)); - while (*p && *p != '\n') { - const char *eol; - - for (eol = p; *eol && *eol != '\n'; eol++) - ; /* do nothing */ - if (!prefixcmp(p, "author ")) { - char *line, *pend, *email, *timestamp; - - p += 7; - line = xmemdupz(p, eol - p); - email = strchr(line, '<'); - if (!email) - die ("Could not extract author email from %s", - sha1_to_hex(commit->object.sha1)); - if (email == line) - pend = line; - else - for (pend = email; pend != line + 1 && - isspace(pend[-1]); pend--); - ; /* do nothing */ - *pend = '\0'; - email++; - timestamp = strchr(email, '>'); - if (!timestamp) - die ("Could not extract author time from %s", - sha1_to_hex(commit->object.sha1)); - *timestamp = '\0'; - for (timestamp++; *timestamp && isspace(*timestamp); - timestamp++) - ; /* do nothing */ - setenv("GIT_AUTHOR_NAME", line, 1); - setenv("GIT_AUTHOR_EMAIL", email, 1); - setenv("GIT_AUTHOR_DATE", timestamp, 1); - free(line); - return; - } - p = eol; - if (*p == '\n') - p++; - } - die ("No author information found in %s", - sha1_to_hex(commit->object.sha1)); -} - static void write_cherry_pick_head(void) { int fd; @@ -273,9 +222,7 @@ static void print_advice(void) advise("after resolving the conflicts, mark the corrected paths"); advise("with 'git add ' or 'git rm '"); - - if (action == CHERRY_PICK) - advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'"); + advise("and commit the result with 'git commit'"); } static void write_message(struct strbuf *msgbuf, const char *filename) @@ -503,7 +450,6 @@ static int do_pick_commit(void) base_label = msg.parent_label; next = commit; next_label = msg.label; - set_author_ident_env(msg.message); add_message_to_msg(&msgbuf, msg.message); if (no_replay) { strbuf_addstr(&msgbuf, "(cherry picked from commit "); diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index e0bf3563ac..95741801b0 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -52,7 +52,7 @@ test_expect_success 'advice from failed cherry-pick' " error: could not apply \$picked... picked hint: after resolving the conflicts, mark the corrected paths hint: with 'git add ' or 'git rm ' - hint: and commit the result with 'git commit -c CHERRY_PICK_HEAD' + hint: and commit the result with 'git commit' EOF test_must_fail git cherry-pick picked 2>actual && diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh index 77b6920029..b61fd3c3c4 100755 --- a/t/t7509-commit.sh +++ b/t/t7509-commit.sh @@ -157,4 +157,33 @@ test_expect_success '--reset-author should be rejected without -c/-C/--amend' ' test_must_fail git commit -a --reset-author -m done ' +test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' ' + echo "cherry-pick 1a" >>foo && + test_tick && + git commit -am "cherry-pick 1" --author="Cherry " && + git tag cherry-pick-head && + git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD && + echo "This is a MERGE_MSG" >.git/MERGE_MSG && + echo "cherry-pick 1b" >>foo && + test_tick && + git commit -a && + author_header cherry-pick-head >expect && + author_header HEAD >actual && + test_cmp expect actual && + + echo "This is a MERGE_MSG" >expect && + message_body HEAD >actual && + test_cmp expect actual +' + +test_expect_success '--reset-author with CHERRY_PICK_HEAD' ' + git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD && + echo "cherry-pick 2" >>foo && + test_tick && + git commit -am "cherry-pick 2" --reset-author && + echo "author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect && + author_header HEAD >actual && + test_cmp expect actual +' + test_done diff --git a/wt-status.c b/wt-status.c index 123582b6cb..fbaaf54dad 100644 --- a/wt-status.c +++ b/wt-status.c @@ -60,7 +60,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s) color_fprintf_ln(s->fp, c, "# Unmerged paths:"); if (!advice_status_hints) return; - if (s->in_merge) + if (s->whence != FROM_COMMIT) ; else if (!s->is_initial) color_fprintf_ln(s->fp, c, "# (use \"git reset %s ...\" to unstage)", s->reference); @@ -77,7 +77,7 @@ static void wt_status_print_cached_header(struct wt_status *s) color_fprintf_ln(s->fp, c, "# Changes to be committed:"); if (!advice_status_hints) return; - if (s->in_merge) + if (s->whence != FROM_COMMIT) ; /* NEEDSWORK: use "git reset --unresolve"??? */ else if (!s->is_initial) color_fprintf_ln(s->fp, c, "# (use \"git reset %s ...\" to unstage)", s->reference); diff --git a/wt-status.h b/wt-status.h index 20b17cf439..cec482a56e 100644 --- a/wt-status.h +++ b/wt-status.h @@ -24,6 +24,13 @@ enum untracked_status_type { SHOW_ALL_UNTRACKED_FILES }; +/* from where does this commit originate */ +enum commit_whence { + FROM_COMMIT, /* normal */ + FROM_MERGE, /* commit came from merge */ + FROM_CHERRY_PICK /* commit came from cherry-pick */ +}; + struct wt_status_change_data { int worktree_status; int index_status; @@ -40,7 +47,7 @@ struct wt_status { const char **pathspec; int verbose; int amend; - int in_merge; + enum commit_whence whence; int nowarn; int use_color; int relative_paths;