Hi,
On a note unrelated to Jonathan's review, I noticed some weird
behavior while playing with my new series: when picking a commit
that's already picked, all the code until run_git_commit runs just
fine. Then 'git commit' fails because it's an empty commit.
Unfortunately, run_git_commit returns a positive return status from
do_pick_commit which is against the convention, leading the
cherry-picking machinery to think that a conflict was encountered.
This is highly confusing from the end-user's perspective. The
operation has suddenly stopped without an error, and '--continue'
seems to continue the operation. What is going on, right?
I was hesitant to send out the patch to make run_git_commit return an
error, because it breaks some tests in t3505-cherry-pick-empty.sh.
However, the problem is getting on my nerves now, and I believe that
the patch does the Right Thing (TM), even if it means that we have to
change the tests in t3505-cherry-pick-empty.sh. Thoughts?
-- 8< --
Subject: [PATCH] revert: Classify failure to run 'git commit' as an error
Since a93d2a (revert: Propagate errors upwards from do_pick_commit,
2011-05-20), do_pick_commit differentiates between conflicts and
errors using the signed-ness of its return status. Unfortunately, it
returns the return status from 'git commit' when it fails to run it,
breaking this convention. Change this so that any non-zero return
status from run_git_commit is classified as an error.
Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
---
builtin/revert.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8b452e8..fcd4f3a 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -603,7 +603,9 @@ static int do_pick_commit(struct commit *commit,
struct replay_opts *opts)
rerere(opts->allow_rerere_auto);
} else {
if (!opts->no_commit)
- res = run_git_commit(defmsg, opts);
+ if (run_git_commit(defmsg, opts))
+ return error(_("Failed to run 'git
commit': %s"),
+ sha1_to_hex(commit->object.sha1));
}
free_message(&msg);
--
1.7.6.351.gb35ac.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html