@matheusbsbs / matheus-bagliano / commits / f241ff0d0a

prepare the builtins for a libified merge_recursive()

Previously, callers of merge_trees() or merge_recursive() expected that code to die() with an error message. This used to be okay because we called those commands from scripts, and had a chance to print out a message in case the command failed fatally (read: with exit code 128). As scripting incurs its own set of problems (portability, speed, idiosyncrasies of different shells, limited data structures leading to inefficient code), we are converting more and more of these scripts into builtins, using library functions directly. We already tried to use merge_recursive() directly in the builtin git-am, for example. Unfortunately, we had to roll it back temporarily because some of the code in merge-recursive.c still deemed it okay to call die(), when the builtin am code really wanted to print out a useful advice after the merge failed fatally. In the next commits, we want to fix that. The code touched by this commit expected merge_trees() to die() with some useful message when there is an error condition, but merge_trees() is going to be improved by converting all die() calls to return error() instead (i.e. return value -1 after printing out the message as before), so that the caller can react more flexibly. This is a step to prepare for the version of merge_trees() that no longer dies, even if we just imitate the previous behavior by calling exit(128): this is what callers of e.g. `git merge` have come to expect. Note that the callers of the sequencer (revert and cherry-pick) already fail fast even for the return value -1; The only difference is that they now get a chance to say "<command> failed". A caller of merge_trees() might want handle error messages themselves (or even suppress them). As this patch is already complex enough, we leave that change for a later patch. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Jul 26, 2016 at 18:06 UTC f241ff0d0a9ddb84b2f673a1b7a92fea0d6add3a

3 files changed +9 -1

builtin/checkout.c

+3 -1

@@ -567,8 +567,10 @@ static int merge_working_tree(const struct checkout_opts *opts,

567 o.ancestor = old->name;

568 o.branch1 = new->name;

569 o.branch2 = "local";

570 - merge_trees(&o, new->commit->tree, work,

570 + ret = merge_trees(&o, new->commit->tree, work,

571 old->commit->tree, &result);

572 + if (ret < 0)

573 + exit(128);

574 ret = reset_tree(new->commit->tree, opts, 0,

575 writeout_error);

576 if (ret)

builtin/merge.c

@@ -673,6 +673,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,

673 hold_locked_index(&lock, 1);

674 clean = merge_recursive(&o, head,

675 remoteheads->item, reversed, &result);

676 + if (clean < 0)

677 + exit(128);

678 if (active_cache_changed &&

679 write_locked_index(&the_index, &lock, COMMIT_LOCK))

680 die (_("unable to write %s"), get_index_file());

sequencer.c

@@ -293,6 +293,8 @@ static int do_recursive_merge(struct commit *base, struct commit *next,

293 clean = merge_trees(&o,

294 head_tree,

295 next_tree, base_tree, &result);

296 + if (clean < 0)

297 + return clean;

298

299 if (active_cache_changed &&

300 write_locked_index(&the_index, &index_lock, COMMIT_LOCK))

@@ -559,6 +561,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)

561 if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {

562 res = do_recursive_merge(base, next, base_label, next_label,

563 head, &msgbuf, opts);

564 + if (res < 0)

565 + return res;

566 write_message(&msgbuf, git_path_merge_msg());

567 } else {

568 struct commit_list *common = NULL;