[PATCH 7/7] merge-ort: convert more error() cases to path_msg()

From

Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>

Date

Jun 13, 2024, 20:25 UTC

Message-ID

<32ae44b6260eea8dad7a428ed92e25b3891ddd9c.1718310307.git.gitgitgadget@gmail.com>

In-Reply-To

<pull.1748.git.1718310307.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

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>
---
 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 5410dec2b4f..493aa55aa80 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_CONFLICT_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 b6db5c2cc36..9bf95249347 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

-- 
gitgitgadget