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>

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 27c1a05246c21278a730698bd7de85eccc96d721..07dea3bc814ba26edd13a0e3ae73b3298958c3e9 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -567,8 +567,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
                        o.ancestor = old->name;
                        o.branch1 = new->name;
                        o.branch2 = "local";
-                       merge_trees(&o, new->commit->tree, work,
+                       ret = merge_trees(&o, new->commit->tree, work,
                                old->commit->tree, &result);
+                       if (ret < 0)
+                               exit(128);
                        ret = reset_tree(new->commit->tree, opts, 0,
                                         writeout_error);
                        if (ret)

diff --git a/builtin/merge.c b/builtin/merge.c
index 19b3bc2f2fafe08d5ec03ab493d9d8f189db275b..148a9a51b9eb45b0b5113680aaf6a71c5db60e09 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -673,6 +673,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
                hold_locked_index(&lock, 1);
                clean = merge_recursive(&o, head,
                                remoteheads->item, reversed, &result);
+               if (clean < 0)
+                       exit(128);
                if (active_cache_changed &&
                    write_locked_index(&the_index, &lock, COMMIT_LOCK))
                        die (_("unable to write %s"), get_index_file());

diff --git a/sequencer.c b/sequencer.c
index cdfac82b117e9ff1084f3fbe36eeb85f45ce056c..286a4358520429bc8433eaf192beca9690c71ade 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -293,6 +293,8 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
        clean = merge_trees(&o,
                            head_tree,
                            next_tree, base_tree, &result);
+       if (clean < 0)
+               return clean;
 
        if (active_cache_changed &&
            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)
        if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
                res = do_recursive_merge(base, next, base_label, next_label,
                                         head, &msgbuf, opts);
+               if (res < 0)
+                       return res;
                write_message(&msgbuf, git_path_merge_msg());
        } else {
                struct commit_list *common = NULL;