From 130ab8ab9c64b59b367c08a041200b6b75758b91 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 11 Aug 2010 03:36:07 -0500 Subject: [PATCH 1/5] =?UTF-8?q?Eliminate=20=E2=80=9CFinished=20cherry-pick?= =?UTF-8?q?/revert=E2=80=9D=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When cherry-pick was written (v0.99.6~63, 2005-08-27), “git commit” was quiet, and the output from cherry-pick provided useful information about the progress of a rebase. Now next to the output from “git commit”, the cherry-pick notification is so much noise (except for the name of the picked commit). $ git cherry-pick ..topic Finished cherry-pick of 499088b. [detached HEAD 17e1ff2] Move glob module to libdpkg Author: Guillem Jover 8 files changed, 12 insertions(+), 9 deletions(-) rename {src => lib/dpkg}/glob.c (98%) rename {src => lib/dpkg}/glob.h (93%) Finished cherry-pick of ae947e1. [detached HEAD 058caa3] libdpkg: Add missing symbols to Versions script Author: Guillem Jover 1 files changed, 2 insertions(+), 0 deletions(-) $ The noise is especially troublesome when sifting through the output of a rebase or multiple cherry-pick that eventually failed. With the commit subject, it is already not hard to figure out where the commit came from. So drop the “Finished” message. Cc: Christian Couder Cc: Thomas Rast Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/howto/revert-branch-rebase.txt | 6 --- builtin/revert.c | 2 - contrib/examples/git-revert.sh | 1 - t/t3508-cherry-pick-many-commits.sh | 42 ++++++++++++++------ 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/Documentation/howto/revert-branch-rebase.txt b/Documentation/howto/revert-branch-rebase.txt index 8c32da6deb..093c656048 100644 --- a/Documentation/howto/revert-branch-rebase.txt +++ b/Documentation/howto/revert-branch-rebase.txt @@ -112,25 +112,19 @@ $ git tag pu-anchor pu $ git rebase master * Applying: Redo "revert" using three-way merge machinery. First trying simple merge strategy to cherry-pick. -Finished one cherry-pick. * Applying: Remove git-apply-patch-script. First trying simple merge strategy to cherry-pick. Simple cherry-pick fails; trying Automatic cherry-pick. Removing Documentation/git-apply-patch-script.txt Removing git-apply-patch-script -Finished one cherry-pick. * Applying: Document "git cherry-pick" and "git revert" First trying simple merge strategy to cherry-pick. -Finished one cherry-pick. * Applying: mailinfo and applymbox updates First trying simple merge strategy to cherry-pick. -Finished one cherry-pick. * Applying: Show commits in topo order and name all commits. First trying simple merge strategy to cherry-pick. -Finished one cherry-pick. * Applying: More documentation updates. First trying simple merge strategy to cherry-pick. -Finished one cherry-pick. ------------------------------------------------ The temporary tag 'pu-anchor' is me just being careful, in case 'git diff --git a/builtin/revert.c b/builtin/revert.c index e261fb2391..c3d64af02d 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -521,8 +521,6 @@ static int do_pick_commit(void) } else { if (!no_commit) res = run_git_commit(defmsg); - if (!res) - fprintf(stderr, "Finished %s.\n", mebuf.buf); } strbuf_release(&mebuf); diff --git a/contrib/examples/git-revert.sh b/contrib/examples/git-revert.sh index 49f00321b2..60a05a8b97 100755 --- a/contrib/examples/git-revert.sh +++ b/contrib/examples/git-revert.sh @@ -181,7 +181,6 @@ Conflicts: esac exit 1 } -echo >&2 "Finished one $me." # If we are cherry-pick, and if the merge did not result in # hand-editing, we will hit this commit and inherit the original diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index 0f61495b2c..8e09fd0319 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -35,36 +35,54 @@ test_expect_success setup ' ' test_expect_success 'cherry-pick first..fourth works' ' - cat <<-EOF >expected && - Finished cherry-pick of commit $(git rev-parse --short second). - Finished cherry-pick of commit $(git rev-parse --short third). - Finished cherry-pick of commit $(git rev-parse --short fourth). + cat <<-\EOF >expected && + [master OBJID] second + Author: A U Thor + 1 files changed, 1 insertions(+), 0 deletions(-) + [master OBJID] third + Author: A U Thor + 1 files changed, 1 insertions(+), 0 deletions(-) + [master OBJID] fourth + Author: A U Thor + 1 files changed, 1 insertions(+), 0 deletions(-) EOF git checkout -f master && git reset --hard first && test_tick && - git cherry-pick first..fourth 2>actual && + git cherry-pick first..fourth >actual && git diff --quiet other && git diff --quiet HEAD other && - test_cmp expected actual && + + sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" actual.fuzzy && + test_cmp expected actual.fuzzy && check_head_differs_from fourth ' test_expect_success 'cherry-pick --strategy resolve first..fourth works' ' - cat <<-EOF >expected && - Finished cherry-pick of commit $(git rev-parse --short second) with strategy resolve. - Finished cherry-pick of commit $(git rev-parse --short third) with strategy resolve. - Finished cherry-pick of commit $(git rev-parse --short fourth) with strategy resolve. + cat <<-\EOF >expected && + Trying simple merge. + [master OBJID] second + Author: A U Thor + 1 files changed, 1 insertions(+), 0 deletions(-) + Trying simple merge. + [master OBJID] third + Author: A U Thor + 1 files changed, 1 insertions(+), 0 deletions(-) + Trying simple merge. + [master OBJID] fourth + Author: A U Thor + 1 files changed, 1 insertions(+), 0 deletions(-) EOF git checkout -f master && git reset --hard first && test_tick && - git cherry-pick --strategy resolve first..fourth 2>actual && + git cherry-pick --strategy resolve first..fourth >actual && git diff --quiet other && git diff --quiet HEAD other && - test_cmp expected actual && + sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" actual.fuzzy && + test_cmp expected actual.fuzzy && check_head_differs_from fourth ' From 2a41dfb03b93c3e5b7d1deca537276aed063a044 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 11 Aug 2010 03:36:41 -0500 Subject: [PATCH 2/5] Introduce advise() to print hints Like error(), warn(), and die(), advise() prints a short message with a formulaic prefix to stderr. It is local to revert.c for now because I am not sure this is the right API (we may want to take an array of advice lines or a boolean argument for easy suppression of unwanted advice). Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/builtin/revert.c b/builtin/revert.c index c3d64af02d..74c1581fdc 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -241,6 +241,15 @@ static void set_author_ident_env(const char *message) sha1_to_hex(commit->object.sha1)); } +static void advise(const char *advice, ...) +{ + va_list params; + + va_start(params, advice); + vreportf("hint: ", advice, params); + va_end(params); +} + static char *help_msg(void) { struct strbuf helpbuf = STRBUF_INIT; From 981ff5c37ae20687c98d98c8689d5e89016026d2 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 11 Aug 2010 03:37:24 -0500 Subject: [PATCH 3/5] cherry-pick/revert: Use error() for failure message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When cherry-pick fails after picking a large series of commits, it can be hard to pick out the error message and advice. Clarify the error and prefix it with “error: ” to help. Before: Automatic cherry-pick failed. [...advice...] After: error: could not apply 7ab78c9... Do something neat. [...advice...] Noticed-by: Thomas Rast Encouraged-by: Sverre Rabbelier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 74c1581fdc..9a7483b66f 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -524,8 +524,11 @@ static int do_pick_commit(void) } if (res) { - fprintf(stderr, "Automatic %s failed.%s\n", - mebuf.buf, help_msg()); + error("could not %s %s... %s", + action == REVERT ? "revert" : "apply", + find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), + msg.subject); + fprintf(stderr, help_msg()); rerere(allow_rerere_auto); } else { if (!no_commit) From 314eeb6e483350cc7ef0bee0498ff24a12346495 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 11 Aug 2010 03:37:51 -0500 Subject: [PATCH 4/5] cherry-pick/revert: Use advise() for hints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When cherry-pick fails after picking a large series of commits, it can be hard to pick out the error message and advice. Prefix the advice with “hint: ” to help. Before: error: could not apply 7ab78c9... foo After resolving the conflicts, mark the corrected paths with 'git add ' or 'git rm ' and commit the result with: git commit -c 7ab78c9a7898b87127365478431289cb98f8d98f After: error: could not apply 7ab78c9... foo 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 7ab78c9' Noticed-by: Thomas Rast Encouraged-by: Sverre Rabbelier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 36 +++++++++++---------------------- git-rebase--interactive.sh | 6 +++--- t/t3507-cherry-pick-conflict.sh | 20 ++++++++++++++++++ 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 9a7483b66f..7f35cc6e17 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -250,27 +250,21 @@ static void advise(const char *advice, ...) va_end(params); } -static char *help_msg(void) +static void print_advice(void) { - struct strbuf helpbuf = STRBUF_INIT; char *msg = getenv("GIT_CHERRY_PICK_HELP"); - if (msg) - return msg; - - strbuf_addstr(&helpbuf, " After resolving the conflicts,\n" - "mark the corrected paths with 'git add ' or 'git rm '\n" - "and commit the result"); - - if (action == CHERRY_PICK) { - strbuf_addf(&helpbuf, " with: \n" - "\n" - " git commit -c %s\n", - sha1_to_hex(commit->object.sha1)); + if (msg) { + fprintf(stderr, "%s\n", msg); + return; } - else - strbuf_addch(&helpbuf, '.'); - return strbuf_detach(&helpbuf, NULL); + + 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 %s'", + find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV)); } static void write_message(struct strbuf *msgbuf, const char *filename) @@ -404,7 +398,6 @@ static int do_pick_commit(void) struct commit_message msg = { NULL, NULL, NULL, NULL, NULL }; char *defmsg = NULL; struct strbuf msgbuf = STRBUF_INIT; - struct strbuf mebuf = STRBUF_INIT; int res; if (no_commit) { @@ -501,9 +494,6 @@ static int do_pick_commit(void) } } - strbuf_addf(&mebuf, "%s of commit %s", me, - find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV)); - if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) { res = do_recursive_merge(base, next, base_label, next_label, head, &msgbuf); @@ -512,7 +502,6 @@ static int do_pick_commit(void) struct commit_list *common = NULL; struct commit_list *remotes = NULL; - strbuf_addf(&mebuf, " with strategy %s", strategy); write_message(&msgbuf, defmsg); commit_list_insert(base, &common); @@ -528,14 +517,13 @@ static int do_pick_commit(void) action == REVERT ? "revert" : "apply", find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), msg.subject); - fprintf(stderr, help_msg()); + print_advice(); rerere(allow_rerere_auto); } else { if (!no_commit) res = run_git_commit(defmsg); } - strbuf_release(&mebuf); free_message(&msg); free(defmsg); diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 31e68603f4..8f6876d301 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -113,9 +113,9 @@ REBASE_ROOT= AUTOSQUASH= NEVER_FF= -GIT_CHERRY_PICK_HELP=" After resolving the conflicts, -mark the corrected paths with 'git add ', and -run 'git rebase --continue'" +GIT_CHERRY_PICK_HELP="\ +hint: after resolving the conflicts, mark the corrected paths +hint: with 'git add ' and run 'git rebase --continue'" export GIT_CHERRY_PICK_HELP warn () { diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index e25cf8039a..3f29594329 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -38,6 +38,26 @@ test_expect_success 'failed cherry-pick does not advance HEAD' ' test "$head" = "$newhead" ' +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 && + + picked=$(git rev-parse --short picked) && + cat <<-EOF >expected && + 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' + EOF + test_must_fail git cherry-pick picked 2>actual && + + test_cmp expected actual +' + test_expect_success 'failed cherry-pick produces dirty index' ' git checkout -f initial^0 && From 997b688769f8d3ea539990ab53d6a44a46a2b2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 18 Aug 2010 14:36:44 +0000 Subject: [PATCH 5/5] tests: fix syntax error in "Use advise() for hints" test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the test introduced in the "Use advise() for hints" patch by Jonathan Nieder not to use '' for quotes inside '' delimited code. It ended up introducing a file called to the main git repository. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t3507-cherry-pick-conflict.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 3f29594329..607bf25d8f 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -38,7 +38,7 @@ test_expect_success 'failed cherry-pick does not advance HEAD' ' test "$head" = "$newhead" ' -test_expect_success 'advice from failed cherry-pick' ' +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 && @@ -46,17 +46,17 @@ test_expect_success 'advice from failed cherry-pick' ' git update-index --refresh && git diff-index --exit-code HEAD && - picked=$(git rev-parse --short picked) && + picked=\$(git rev-parse --short picked) && cat <<-EOF >expected && - error: could not apply $picked... picked + 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 \$picked' EOF test_must_fail git cherry-pick picked 2>actual && test_cmp expected actual -' +" test_expect_success 'failed cherry-pick produces dirty index' '