merge-ort: convert more error() cases to path_msg()

merge_submodule() stores errors using path_msg(), whereas other call
sites make use of the error() function.  This is inconsistent, and
moving towards path_msg() seems more friendly for libification efforts
since it will allow the caller to determine whether the error messages
need to be printed.

Note that this deferred handling of error messages changes the error
message in a recursive merge from
  error: failed to execute internal merge
to
  From inner merge:  error: failed to execute internal merge
which provides a little more information about the error which may be
useful.  Since the recursive merge strategy still only shows the older
error, we had to adjust the new testcase introduced a few commits ago to
just search for the older message somewhere in the output.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/merge-ort.c b/merge-ort.c
index b337e4d74efdca233893324940d3c17c7e5420e0..8dfe80f100914f572b270ec4c5280fd5ea64ae3a 100644 (file)
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -558,6 +558,10 @@ enum conflict_and_info_types {
         * Keep this group _last_ other than NB_TOTAL_TYPES
         */
        ERROR_SUBMODULE_CORRUPT,
+       ERROR_THREEWAY_CONTENT_MERGE_FAILED,
+       ERROR_OBJECT_WRITE_FAILED,
+       ERROR_OBJECT_READ_FAILED,
+       ERROR_OBJECT_NOT_A_BLOB,
 
        /* Keep this entry _last_ in the list */
        NB_TOTAL_TYPES,
@@ -615,6 +619,14 @@ static const char *type_short_descriptions[] = {
        /* Something is seriously wrong; cannot even perform merge */
        [ERROR_SUBMODULE_CORRUPT] =
                "ERROR (submodule corrupt)",
+       [ERROR_THREEWAY_CONTENT_MERGE_FAILED] =
+               "ERROR (three-way content merge failed)",
+       [ERROR_OBJECT_WRITE_FAILED] =
+               "ERROR (object write failed)",
+       [ERROR_OBJECT_READ_FAILED] =
+               "ERROR (object read failed)",
+       [ERROR_OBJECT_NOT_A_BLOB] =
+               "ERROR (object is not a blob)",
 };
 
 struct logical_conflict_info {
@@ -2190,15 +2202,24 @@ static int handle_content_merge(struct merge_options *opt,
                                          pathnames, extra_marker_size,
                                          &result_buf);
 
-               if ((merge_status < 0) || !result_buf.ptr)
-                       ret = error(_("failed to execute internal merge"));
+               if ((merge_status < 0) || !result_buf.ptr) {
+                       path_msg(opt, ERROR_THREEWAY_CONTENT_MERGE_FAILED, 0,
+                                pathnames[0], pathnames[1], pathnames[2], NULL,
+                                _("error: failed to execute internal merge for %s"),
+                                path);
+                       ret = -1;
+               }
 
                if (!ret &&
                    write_object_file(result_buf.ptr, result_buf.size,
-                                     OBJ_BLOB, &result->oid))
-                       ret = error(_("unable to add %s to database"), path);
-
+                                     OBJ_BLOB, &result->oid)) {
+                       path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
+                                pathnames[0], pathnames[1], pathnames[2], NULL,
+                                _("error: unable to add %s to database"), path);
+                       ret = -1;
+               }
                free(result_buf.ptr);
+
                if (ret)
                        return -1;
                if (merge_status > 0)
@@ -3577,18 +3598,26 @@ static int sort_dirs_next_to_their_children(const char *one, const char *two)
                return c1 - c2;
 }
 
-static int read_oid_strbuf(const struct object_id *oid,
-                          struct strbuf *dst)
+static int read_oid_strbuf(struct merge_options *opt,
+                          const struct object_id *oid,
+                          struct strbuf *dst,
+                          const char *path)
 {
        void *buf;
        enum object_type type;
        unsigned long size;
        buf = repo_read_object_file(the_repository, oid, &type, &size);
-       if (!buf)
-               return error(_("cannot read object %s"), oid_to_hex(oid));
+       if (!buf) {
+               path_msg(opt, ERROR_OBJECT_READ_FAILED, 0,
+                        path, NULL, NULL, NULL,
+                        _("error: cannot read object %s"), oid_to_hex(oid));
+               return -1;
+       }
        if (type != OBJ_BLOB) {
                free(buf);
-               return error(_("object %s is not a blob"), oid_to_hex(oid));
+               path_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,
+                        path, NULL, NULL, NULL,
+                        _("error: object %s is not a blob"), oid_to_hex(oid));
        }
        strbuf_attach(dst, buf, size, size + 1);
        return 0;
@@ -3612,8 +3641,8 @@ static int blob_unchanged(struct merge_options *opt,
        if (oideq(&base->oid, &side->oid))
                return 1;
 
-       if (read_oid_strbuf(&base->oid, &basebuf) ||
-           read_oid_strbuf(&side->oid, &sidebuf))
+       if (read_oid_strbuf(opt, &base->oid, &basebuf, path) ||
+           read_oid_strbuf(opt, &side->oid, &sidebuf, path))
                goto error_return;
        /*
         * Note: binary | is used so that both renormalizations are

diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
index b6db5c2cc3664f4dfba91bbe43e072718f2069b8..9bf952493471e570644d30e7f21fbc901a33f5f6 100755 (executable)
--- a/t/t6406-merge-attr.sh
+++ b/t/t6406-merge-attr.sh
@@ -295,7 +295,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal o
        >./please-abort &&
        echo "* merge=custom" >.gitattributes &&
        test_expect_code 2 git merge recursive-a 2>err &&
-       grep "^error: failed to execute internal merge" err &&
+       grep "error: failed to execute internal merge" err &&
        git ls-files -u >output &&
        git diff --name-only HEAD >>output &&
        test_must_be_empty output