Re: [PATCH 3/5] revert: Allow mixed pick and revert instructions
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Aug 11, 2011, 06:52 UTC
- Message-ID
- <CALkWK0kxi+-UEG9g_=BC8vK9DGA85+OKJSrXWH=3XsxpLg-E8g@mail.gmail.com>
- In-Reply-To
- <20110810151527.GC31315@elie.gateway.2wire.net>
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@gmail.com> --- 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