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>

Elijah Newren committed Jun 19, 2024 at 03:00 UTC f19b9165351a4058832bb43560178474c7501925
2 files changed +42 -13
merge-ort.c
+41 -12
@@ -558,6 +558,10 @@ enum conflict_and_info_types {
558 * Keep this group _last_ other than NB_TOTAL_TYPES
559 */
560 ERROR_SUBMODULE_CORRUPT,
561 + ERROR_THREEWAY_CONTENT_MERGE_FAILED,
562 + ERROR_OBJECT_WRITE_FAILED,
563 + ERROR_OBJECT_READ_FAILED,
564 + ERROR_OBJECT_NOT_A_BLOB,
565
566 /* Keep this entry _last_ in the list */
567 NB_TOTAL_TYPES,
@@ -615,6 +619,14 @@ static const char *type_short_descriptions[] = {
619 /* Something is seriously wrong; cannot even perform merge */
620 [ERROR_SUBMODULE_CORRUPT] =
621 "ERROR (submodule corrupt)",
622 + [ERROR_THREEWAY_CONTENT_MERGE_FAILED] =
623 + "ERROR (three-way content merge failed)",
624 + [ERROR_OBJECT_WRITE_FAILED] =
625 + "ERROR (object write failed)",
626 + [ERROR_OBJECT_READ_FAILED] =
627 + "ERROR (object read failed)",
628 + [ERROR_OBJECT_NOT_A_BLOB] =
629 + "ERROR (object is not a blob)",
630 };
631
632 struct logical_conflict_info {
@@ -2190,15 +2202,24 @@ static int handle_content_merge(struct merge_options *opt,
2202 pathnames, extra_marker_size,
2203 &result_buf);
2204
2193 - if ((merge_status < 0) || !result_buf.ptr)
2194 - ret = error(_("failed to execute internal merge"));
2205 + if ((merge_status < 0) || !result_buf.ptr) {
2206 + path_msg(opt, ERROR_THREEWAY_CONTENT_MERGE_FAILED, 0,
2207 + pathnames[0], pathnames[1], pathnames[2], NULL,
2208 + _("error: failed to execute internal merge for %s"),
2209 + path);
2210 + ret = -1;
2211 + }
2212
2213 if (!ret &&
2214 write_object_file(result_buf.ptr, result_buf.size,
2198 - OBJ_BLOB, &result->oid))
2199 - ret = error(_("unable to add %s to database"), path);
2200 -
2215 + OBJ_BLOB, &result->oid)) {
2216 + path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
2217 + pathnames[0], pathnames[1], pathnames[2], NULL,
2218 + _("error: unable to add %s to database"), path);
2219 + ret = -1;
2220 + }
2221 free(result_buf.ptr);
2222 +
2223 if (ret)
2224 return -1;
2225 if (merge_status > 0)
@@ -3577,18 +3598,26 @@ static int sort_dirs_next_to_their_children(const char *one, const char *two)
3598 return c1 - c2;
3599 }
3600
3580 -static int read_oid_strbuf(const struct object_id *oid,
3581 - struct strbuf *dst)
3601 +static int read_oid_strbuf(struct merge_options *opt,
3602 + const struct object_id *oid,
3603 + struct strbuf *dst,
3604 + const char *path)
3605 {
3606 void *buf;
3607 enum object_type type;
3608 unsigned long size;
3609 buf = repo_read_object_file(the_repository, oid, &type, &size);
3587 - if (!buf)
3588 - return error(_("cannot read object %s"), oid_to_hex(oid));
3610 + if (!buf) {
3611 + path_msg(opt, ERROR_OBJECT_READ_FAILED, 0,
3612 + path, NULL, NULL, NULL,
3613 + _("error: cannot read object %s"), oid_to_hex(oid));
3614 + return -1;
3615 + }
3616 if (type != OBJ_BLOB) {
3617 free(buf);
3591 - return error(_("object %s is not a blob"), oid_to_hex(oid));
3618 + path_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,
3619 + path, NULL, NULL, NULL,
3620 + _("error: object %s is not a blob"), oid_to_hex(oid));
3621 }
3622 strbuf_attach(dst, buf, size, size + 1);
3623 return 0;
@@ -3612,8 +3641,8 @@ static int blob_unchanged(struct merge_options *opt,
3641 if (oideq(&base->oid, &side->oid))
3642 return 1;
3643
3615 - if (read_oid_strbuf(&base->oid, &basebuf) ||
3616 - read_oid_strbuf(&side->oid, &sidebuf))
3644 + if (read_oid_strbuf(opt, &base->oid, &basebuf, path) ||
3645 + read_oid_strbuf(opt, &side->oid, &sidebuf, path))
3646 goto error_return;
3647 /*
3648 * Note: binary | is used so that both renormalizations are
t/t6406-merge-attr.sh
+1 -1
@@ -295,7 +295,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal o
295 >./please-abort &&
296 echo "* merge=custom" >.gitattributes &&
297 test_expect_code 2 git merge recursive-a 2>err &&
298 - grep "^error: failed to execute internal merge" err &&
298 + grep "error: failed to execute internal merge" err &&
299 git ls-files -u >output &&
300 git diff --name-only HEAD >>output &&
301 test_must_be_empty output