diff --git a/merge-ort.c b/merge-ort.c index ffbdb8fc8e..e9d01ac7f7 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -545,17 +545,35 @@ enum conflict_and_info_types { CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE, CONFLICT_SUBMODULE_MAY_HAVE_REWINDS, CONFLICT_SUBMODULE_NULL_MERGE_BASE, - CONFLICT_SUBMODULE_CORRUPT, + + /* INSERT NEW ENTRIES HERE */ + + /* + * Keep this entry after all regular conflict and info types; only + * errors (failures causing immediate abort of the merge) should + * come after this. + */ + NB_REGULAR_CONFLICT_TYPES, + + /* + * Something is seriously wrong; cannot even perform merge; + * 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_CONFLICT_TYPES, + NB_TOTAL_TYPES, }; /* * Short description of conflict type, relied upon by external tools. * * We can add more entries, but DO NOT change any of these strings. Also, - * Order MUST match conflict_info_and_types. + * please ensure the order matches what is used in conflict_info_and_types. */ static const char *type_short_descriptions[] = { /*** "Simple" conflicts and informational messages ***/ @@ -599,8 +617,18 @@ static const char *type_short_descriptions[] = { "CONFLICT (submodule may have rewinds)", [CONFLICT_SUBMODULE_NULL_MERGE_BASE] = "CONFLICT (submodule lacks merge base)", - [CONFLICT_SUBMODULE_CORRUPT] = - "CONFLICT (submodule corrupt)" + + /* 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 { @@ -764,7 +792,8 @@ static void path_msg(struct merge_options *opt, /* Sanity checks */ assert(omittable_hint == - !starts_with(type_short_descriptions[type], "CONFLICT") || + (!starts_with(type_short_descriptions[type], "CONFLICT") && + !starts_with(type_short_descriptions[type], "ERROR")) || type == CONFLICT_DIR_RENAME_SUGGESTED); if (opt->record_conflict_msgs_as_headers && omittable_hint) return; /* Do not record mere hints in headers */ @@ -1819,9 +1848,9 @@ static int merge_submodule(struct merge_options *opt, /* check whether both changes are forward */ ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_a); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1830,9 +1859,9 @@ static int merge_submodule(struct merge_options *opt, if (ret2 > 0) ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_b); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1850,9 +1879,9 @@ static int merge_submodule(struct merge_options *opt, /* Case #1: a is contained in b or vice versa */ ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1869,9 +1898,9 @@ static int merge_submodule(struct merge_options *opt, } ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1903,9 +1932,9 @@ static int merge_submodule(struct merge_options *opt, &merges); switch (parent_count) { case -1: - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -2111,7 +2140,7 @@ static int handle_content_merge(struct merge_options *opt, * merges, which happens for example with rename/rename(2to1) and * rename/add conflicts. */ - unsigned clean = 1; + int clean = 1; /* * handle_content_merge() needs both files to be of the same type, i.e. @@ -2175,18 +2204,28 @@ 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; - clean &= (merge_status == 0); + if (merge_status > 0) + clean = 0; path_msg(opt, INFO_AUTO_MERGING, 1, path, NULL, NULL, NULL, _("Auto-merging %s"), path); } else if (S_ISGITLINK(a->mode)) { @@ -2194,6 +2233,8 @@ static int handle_content_merge(struct merge_options *opt, clean = merge_submodule(opt, pathnames[0], two_way ? null_oid() : &o->oid, &a->oid, &b->oid, &result->oid); + if (clean < 0) + return -1; if (opt->priv->call_depth && two_way && !clean) { result->mode = o->mode; oidcpy(&result->oid, &o->oid); @@ -3559,18 +3600,27 @@ 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)); + return -1; } strbuf_attach(dst, buf, size, size + 1); return 0; @@ -3594,8 +3644,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 @@ -4645,6 +4695,7 @@ void merge_display_update_messages(struct merge_options *opt, struct hashmap_iter iter; struct strmap_entry *e; struct string_list olist = STRING_LIST_INIT_NODUP; + FILE *o = stdout; if (opt->record_conflict_msgs_as_headers) BUG("Either display conflict messages or record them as headers, not both"); @@ -4661,6 +4712,10 @@ void merge_display_update_messages(struct merge_options *opt, } string_list_sort(&olist); + /* Print to stderr if we hit errors rather than just conflicts */ + if (result->clean < 0) + o = stderr; + /* Iterate over the items, printing them */ for (int path_nr = 0; path_nr < olist.nr; ++path_nr) { struct string_list *conflicts = olist.items[path_nr].util; @@ -4668,25 +4723,31 @@ void merge_display_update_messages(struct merge_options *opt, struct logical_conflict_info *info = conflicts->items[i].util; + /* On failure, ignore regular conflict types */ + if (result->clean < 0 && + info->type < NB_REGULAR_CONFLICT_TYPES) + continue; + if (detailed) { - printf("%lu", (unsigned long)info->paths.nr); - putchar('\0'); + fprintf(o, "%lu", (unsigned long)info->paths.nr); + fputc('\0', o); for (int n = 0; n < info->paths.nr; n++) { - fputs(info->paths.v[n], stdout); - putchar('\0'); + fputs(info->paths.v[n], o); + fputc('\0', o); } - fputs(type_short_descriptions[info->type], - stdout); - putchar('\0'); + fputs(type_short_descriptions[info->type], o); + fputc('\0', o); } - puts(conflicts->items[i].string); + fputs(conflicts->items[i].string, o); + fputc('\n', o); if (detailed) - putchar('\0'); + fputc('\0', o); } } string_list_clear(&olist, 0); - print_submodule_conflict_suggestion(&opti->conflicted_submodules); + if (result->clean >= 0) + print_submodule_conflict_suggestion(&opti->conflicted_submodules); /* Also include needed rename limit adjustment now */ diff_warn_rename_limit("merge.renamelimit", @@ -5002,6 +5063,26 @@ static void merge_check_renames_reusable(struct merge_result *result, /*** Function Grouping: merge_incore_*() and their internal variants ***/ +static void move_opt_priv_to_result_priv(struct merge_options *opt, + struct merge_result *result) +{ + /* + * opt->priv and result->priv are a bit weird. opt->priv contains + * information that we can re-use in subsequent merge operations to + * enable our cached renames optimization. The best way to provide + * that to subsequent merges is putting it in result->priv. + * However, putting it directly there would mean retrofitting lots + * of functions in this file to also take a merge_result pointer, + * which is ugly and annoying. So, we just make sure at the end of + * the merge (the outer merge if there are internal recursive ones) + * to move it. + */ + assert(opt->priv && !result->priv); + result->priv = opt->priv; + result->_properly_initialized = RESULT_INITIALIZED; + opt->priv = NULL; +} + /* * Originally from merge_trees_internal(); heavily adapted, though. */ @@ -5032,6 +5113,7 @@ redo: oid_to_hex(&side1->object.oid), oid_to_hex(&side2->object.oid)); result->clean = -1; + move_opt_priv_to_result_priv(opt, result); return; } trace2_region_leave("merge", "collect_merge_info", opt->repo); @@ -5062,11 +5144,8 @@ redo: /* existence of conflicted entries implies unclean */ result->clean &= strmap_empty(&opt->priv->conflicted); } - if (!opt->priv->call_depth) { - result->priv = opt->priv; - result->_properly_initialized = RESULT_INITIALIZED; - opt->priv = NULL; - } + if (!opt->priv->call_depth || result->clean < 0) + move_opt_priv_to_result_priv(opt, result); } /* diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh index 156a1efacf..9bf9524934 100755 --- a/t/t6406-merge-attr.sh +++ b/t/t6406-merge-attr.sh @@ -185,7 +185,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal' >./please-abort && echo "* merge=custom" >.gitattributes && - test_must_fail git merge main 2>err && + test_expect_code 2 git merge main 2>err && grep "^error: failed to execute internal merge" err && git ls-files -u >output && git diff --name-only HEAD >>output && @@ -261,4 +261,44 @@ test_expect_success 'binary files with union attribute' ' grep -i "warning.*cannot merge.*HEAD vs. bin-main" output ' +test_expect_success !WINDOWS 'custom merge driver that is killed with a signal on recursive merge' ' + test_when_finished "rm -f output please-abort" && + test_when_finished "git checkout side" && + + git reset --hard anchor && + + git checkout -b base-a main^ && + echo base-a >text && + git commit -m base-a text && + + git checkout -b base-b main^ && + echo base-b >text && + git commit -m base-b text && + + git checkout -b recursive-a base-a && + test_must_fail git merge base-b && + echo recursive-a >text && + git add text && + git commit -m recursive-a && + + git checkout -b recursive-b base-b && + test_must_fail git merge base-a && + echo recursive-b >text && + git add text && + git commit -m recursive-b && + + git config --replace-all \ + merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" && + git config --replace-all \ + merge.custom.name "custom merge driver for testing" && + + >./please-abort && + echo "* merge=custom" >.gitattributes && + test_expect_code 2 git merge recursive-a 2>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 +' + test_done