Skip to content

Commit

Permalink
merge-ort: convert more error() cases to path_msg()
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
newren committed Jun 13, 2024
1 parent ef9b1a6 commit e2742ec
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 13 deletions.
53 changes: 41 additions & 12 deletions merge-ort.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion t/t6406-merge-attr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e2742ec

Please sign in to comment.