revert: Propagate errors upwards from do_pick_commit
Currently, revert_or_cherry_pick can fail in two ways. If it encounters a conflict, it returns a positive number indicating the intended exit status for the git wrapper to pass on; for all other errors, it calls die(). The latter behavior is inconsiderate towards callers, as it denies them the opportunity to recover from errors and do other things. After this patch, revert_or_cherry_pick will still return a positive return value to indicate an exit status for conflicts as before, while for some other errors, it will print an error message and return -1 instead of die()-ing. The cmd_revert and cmd_cherry_pick are adjusted to handle the fatal errors by die()-ing themselves. While the full benefits of this patch will only be seen once all the "die" calls are replaced with calls to "error", its immediate impact is to change some "fatal:" messages to say "error:" and to add a new "fatal: cherry-pick failed" message at the end when the operation fails. Inspired-by: Christian Couder <chriscool@tuxfamily.org> Mentored-by: Jonathan Nieder <jrnieder@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ramkumar Ramachandra committed
Aug 4, 2011 at 16:09 UTC
cf3e2486d698356a130ebcda9d728c53a0569813
1 file changed
+40
-46
builtin/revert.c
+40
-46
@@ -66,15 +66,6 @@ struct replay_opts {
66
67
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
68
69
-static void fatal(const char *advice, ...)
70
-{
71
- va_list params;
72
-
73
- va_start(params, advice);
74
- vreportf("fatal: ", advice, params);
75
- va_end(params);
76
-}
77
-
69
static const char *action_name(const struct replay_opts *opts)
70
{
71
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -356,25 +347,20 @@ static struct tree *empty_tree(void)
347
return tree;
348
}
349
359
-static NORETURN void die_dirty_index(struct replay_opts *opts)
350
+static int error_dirty_index(struct replay_opts *opts)
351
{
361
- if (read_cache_unmerged()) {
362
- die_resolve_conflict(action_name(opts));
363
- } else {
364
- if (advice_commit_before_merge) {
365
- if (opts->action == REVERT)
366
- die(_("Your local changes would be overwritten by revert.\n"
367
- "Please, commit your changes or stash them to proceed."));
368
- else
369
- die(_("Your local changes would be overwritten by cherry-pick.\n"
370
- "Please, commit your changes or stash them to proceed."));
371
- } else {
372
- if (opts->action == REVERT)
373
- die(_("Your local changes would be overwritten by revert.\n"));
374
- else
375
- die(_("Your local changes would be overwritten by cherry-pick.\n"));
376
- }
377
- }
352
+ if (read_cache_unmerged())
353
+ return error_resolve_conflict(action_name(opts));
354
+
355
+ /* Different translation strings for cherry-pick and revert */
356
+ if (opts->action == CHERRY_PICK)
357
+ error(_("Your local changes would be overwritten by cherry-pick."));
358
+ else
359
+ error(_("Your local changes would be overwritten by revert."));
360
+
361
+ if (advice_commit_before_merge)
362
+ advise(_("Commit your changes or stash them to proceed."));
363
+ return -1;
364
}
365
366
static int fast_forward_to(const unsigned char *to, const unsigned char *from)
@@ -492,9 +478,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
478
die (_("Your index file is unmerged."));
479
} else {
480
if (get_sha1("HEAD", head))
495
- die (_("You do not have a valid HEAD"));
481
+ return error(_("You do not have a valid HEAD"));
482
if (index_differs_from("HEAD", 0))
497
- die_dirty_index(opts);
483
+ return error_dirty_index(opts);
484
}
485
discard_cache();
486
@@ -507,20 +493,20 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
493
struct commit_list *p;
494
495
if (!opts->mainline)
510
- die(_("Commit %s is a merge but no -m option was given."),
511
- sha1_to_hex(commit->object.sha1));
496
+ return error(_("Commit %s is a merge but no -m option was given."),
497
+ sha1_to_hex(commit->object.sha1));
498
499
for (cnt = 1, p = commit->parents;
500
cnt != opts->mainline && p;
501
cnt++)
502
p = p->next;
503
if (cnt != opts->mainline || !p)
518
- die(_("Commit %s does not have parent %d"),
519
- sha1_to_hex(commit->object.sha1), opts->mainline);
504
+ return error(_("Commit %s does not have parent %d"),
505
+ sha1_to_hex(commit->object.sha1), opts->mainline);
506
parent = p->item;
507
} else if (0 < opts->mainline)
522
- die(_("Mainline was specified but commit %s is not a merge."),
523
- sha1_to_hex(commit->object.sha1));
508
+ return error(_("Mainline was specified but commit %s is not a merge."),
509
+ sha1_to_hex(commit->object.sha1));
510
else
511
parent = commit->parents->item;
512
@@ -530,12 +516,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
516
if (parent && parse_commit(parent) < 0)
517
/* TRANSLATORS: The first %s will be "revert" or
518
"cherry-pick", the second %s a SHA1 */
533
- die(_("%s: cannot parse parent commit %s"),
534
- action_name(opts), sha1_to_hex(parent->object.sha1));
519
+ return error(_("%s: cannot parse parent commit %s"),
520
+ action_name(opts), sha1_to_hex(parent->object.sha1));
521
522
if (get_message(commit, &msg) != 0)
537
- die(_("Cannot get commit message for %s"),
538
- sha1_to_hex(commit->object.sha1));
523
+ return error(_("Cannot get commit message for %s"),
524
+ sha1_to_hex(commit->object.sha1));
525
526
/*
527
* "commit" is an existing commit. We would want to apply
@@ -997,27 +983,28 @@ static int pick_revisions(struct replay_opts *opts)
983
984
walk_revs_populate_todo(&todo_list, opts);
985
if (create_seq_dir() < 0) {
1000
- fatal(_("A cherry-pick or revert is in progress."));
986
+ error(_("A cherry-pick or revert is in progress."));
987
advise(_("Use --continue to continue the operation"));
988
advise(_("or --reset to forget about it"));
1003
- exit(128);
989
+ return -1;
990
}
991
if (get_sha1("HEAD", sha1)) {
992
if (opts->action == REVERT)
1007
- die(_("Can't revert as initial commit"));
1008
- die(_("Can't cherry-pick into empty head"));
993
+ return error(_("Can't revert as initial commit"));
994
+ return error(_("Can't cherry-pick into empty head"));
995
}
996
save_head(sha1_to_hex(sha1));
997
save_opts(opts);
998
}
999
return pick_commits(todo_list, opts);
1000
error:
1015
- die(_("No %s in progress"), action_name(opts));
1001
+ return error(_("No %s in progress"), action_name(opts));
1002
}
1003
1004
int cmd_revert(int argc, const char **argv, const char *prefix)
1005
{
1006
struct replay_opts opts;
1007
+ int res;
1008
1009
memset(&opts, 0, sizeof(opts));
1010
if (isatty(0))
@@ -1025,16 +1012,23 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
1012
opts.action = REVERT;
1013
git_config(git_default_config, NULL);
1014
parse_args(argc, argv, &opts);
1028
- return pick_revisions(&opts);
1015
+ res = pick_revisions(&opts);
1016
+ if (res < 0)
1017
+ die(_("revert failed"));
1018
+ return res;
1019
}
1020
1021
int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
1022
{
1023
struct replay_opts opts;
1024
+ int res;
1025
1026
memset(&opts, 0, sizeof(opts));
1027
opts.action = CHERRY_PICK;
1028
git_config(git_default_config, NULL);
1029
parse_args(argc, argv, &opts);
1039
- return pick_revisions(&opts);
1030
+ res = pick_revisions(&opts);
1031
+ if (res < 0)
1032
+ die(_("cherry-pick failed"));
1033
+ return res;
1034
}