From f19b9165351a4058832bb43560178474c7501925 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 19 Jun 2024 03:00:19 +0000 Subject: [PATCH] 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 Signed-off-by: Junio C Hamano --- merge-ort.c | 53 +++++++++++++++++++++++++++++++++---------- t/t6406-merge-attr.sh | 2 +- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index b337e4d74efdca..8dfe80f100914f 100644 --- 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 b6db5c2cc3664f..9bf952493471e5 100755 --- 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