merge-ort: upon merge abort, only show messages causing the abort

When something goes wrong enough that we need to abort early and not even attempt merging the remaining files, it probably does not make sense to report conflicts messages for the subset of files we processed before hitting the fatal error. Instead, only show the messages associated with paths where we hit the fatal error. Also, print these messages to stderr rather than stdout. 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 14949d91b60176fa01850c2a3454cd72eb9bc5d6
1 file changed +53 -25
merge-ort.c
+53 -25
@@ -543,10 +543,24 @@ enum conflict_and_info_types {
543 CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE,
544 CONFLICT_SUBMODULE_MAY_HAVE_REWINDS,
545 CONFLICT_SUBMODULE_NULL_MERGE_BASE,
546 - CONFLICT_SUBMODULE_CORRUPT,
546 +
547 + /* INSERT NEW ENTRIES HERE */
548 +
549 + /*
550 + * Keep this entry after all regular conflict and info types; only
551 + * errors (failures causing immediate abort of the merge) should
552 + * come after this.
553 + */
554 + NB_REGULAR_CONFLICT_TYPES,
555 +
556 + /*
557 + * Something is seriously wrong; cannot even perform merge;
558 + * Keep this group _last_ other than NB_TOTAL_TYPES
559 + */
560 + ERROR_SUBMODULE_CORRUPT,
561
562 /* Keep this entry _last_ in the list */
549 - NB_CONFLICT_TYPES,
563 + NB_TOTAL_TYPES,
564 };
565
566 /*
@@ -597,8 +611,10 @@ static const char *type_short_descriptions[] = {
611 "CONFLICT (submodule may have rewinds)",
612 [CONFLICT_SUBMODULE_NULL_MERGE_BASE] =
613 "CONFLICT (submodule lacks merge base)",
600 - [CONFLICT_SUBMODULE_CORRUPT] =
601 - "CONFLICT (submodule corrupt)"
614 +
615 + /* Something is seriously wrong; cannot even perform merge */
616 + [ERROR_SUBMODULE_CORRUPT] =
617 + "ERROR (submodule corrupt)",
618 };
619
620 struct logical_conflict_info {
@@ -762,7 +778,8 @@ static void path_msg(struct merge_options *opt,
778
779 /* Sanity checks */
780 assert(omittable_hint ==
765 - !starts_with(type_short_descriptions[type], "CONFLICT") ||
781 + (!starts_with(type_short_descriptions[type], "CONFLICT") &&
782 + !starts_with(type_short_descriptions[type], "ERROR")) ||
783 type == CONFLICT_DIR_RENAME_SUGGESTED);
784 if (opt->record_conflict_msgs_as_headers && omittable_hint)
785 return; /* Do not record mere hints in headers */
@@ -1817,9 +1834,9 @@ static int merge_submodule(struct merge_options *opt,
1834 /* check whether both changes are forward */
1835 ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_a);
1836 if (ret2 < 0) {
1820 - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1837 + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0,
1838 path, NULL, NULL, NULL,
1822 - _("Failed to merge submodule %s "
1839 + _("error: failed to merge submodule %s "
1840 "(repository corrupt)"),
1841 path);
1842 ret = -1;
@@ -1828,9 +1845,9 @@ static int merge_submodule(struct merge_options *opt,
1845 if (ret2 > 0)
1846 ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_b);
1847 if (ret2 < 0) {
1831 - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1848 + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0,
1849 path, NULL, NULL, NULL,
1833 - _("Failed to merge submodule %s "
1850 + _("error: failed to merge submodule %s "
1851 "(repository corrupt)"),
1852 path);
1853 ret = -1;
@@ -1848,9 +1865,9 @@ static int merge_submodule(struct merge_options *opt,
1865 /* Case #1: a is contained in b or vice versa */
1866 ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b);
1867 if (ret2 < 0) {
1851 - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1868 + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0,
1869 path, NULL, NULL, NULL,
1853 - _("Failed to merge submodule %s "
1870 + _("error: failed to merge submodule %s "
1871 "(repository corrupt)"),
1872 path);
1873 ret = -1;
@@ -1867,9 +1884,9 @@ static int merge_submodule(struct merge_options *opt,
1884 }
1885 ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a);
1886 if (ret2 < 0) {
1870 - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1887 + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0,
1888 path, NULL, NULL, NULL,
1872 - _("Failed to merge submodule %s "
1889 + _("error: failed to merge submodule %s "
1890 "(repository corrupt)"),
1891 path);
1892 ret = -1;
@@ -1901,9 +1918,9 @@ static int merge_submodule(struct merge_options *opt,
1918 &merges);
1919 switch (parent_count) {
1920 case -1:
1904 - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1921 + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0,
1922 path, NULL, NULL, NULL,
1906 - _("Failed to merge submodule %s "
1923 + _("error: failed to merge submodule %s "
1924 "(repository corrupt)"),
1925 path);
1926 ret = -1;
@@ -4646,6 +4663,7 @@ void merge_display_update_messages(struct merge_options *opt,
4663 struct hashmap_iter iter;
4664 struct strmap_entry *e;
4665 struct string_list olist = STRING_LIST_INIT_NODUP;
4666 + FILE *o = stdout;
4667
4668 if (opt->record_conflict_msgs_as_headers)
4669 BUG("Either display conflict messages or record them as headers, not both");
@@ -4662,6 +4680,10 @@ void merge_display_update_messages(struct merge_options *opt,
4680 }
4681 string_list_sort(&olist);
4682
4683 + /* Print to stderr if we hit errors rather than just conflicts */
4684 + if (result->clean < 0)
4685 + o = stderr;
4686 +
4687 /* Iterate over the items, printing them */
4688 for (int path_nr = 0; path_nr < olist.nr; ++path_nr) {
4689 struct string_list *conflicts = olist.items[path_nr].util;
@@ -4669,25 +4691,31 @@ void merge_display_update_messages(struct merge_options *opt,
4691 struct logical_conflict_info *info =
4692 conflicts->items[i].util;
4693
4694 + /* On failure, ignore regular conflict types */
4695 + if (result->clean < 0 &&
4696 + info->type < NB_REGULAR_CONFLICT_TYPES)
4697 + continue;
4698 +
4699 if (detailed) {
4673 - printf("%lu", (unsigned long)info->paths.nr);
4674 - putchar('\0');
4700 + fprintf(o, "%lu", (unsigned long)info->paths.nr);
4701 + fputc('\0', o);
4702 for (int n = 0; n < info->paths.nr; n++) {
4676 - fputs(info->paths.v[n], stdout);
4677 - putchar('\0');
4703 + fputs(info->paths.v[n], o);
4704 + fputc('\0', o);
4705 }
4679 - fputs(type_short_descriptions[info->type],
4680 - stdout);
4681 - putchar('\0');
4706 + fputs(type_short_descriptions[info->type], o);
4707 + fputc('\0', o);
4708 }
4683 - puts(conflicts->items[i].string);
4709 + fputs(conflicts->items[i].string, o);
4710 + fputc('\n', o);
4711 if (detailed)
4685 - putchar('\0');
4712 + fputc('\0', o);
4713 }
4714 }
4715 string_list_clear(&olist, 0);
4716
4690 - print_submodule_conflict_suggestion(&opti->conflicted_submodules);
4717 + if (result->clean >= 0)
4718 + print_submodule_conflict_suggestion(&opti->conflicted_submodules);
4719
4720 /* Also include needed rename limit adjustment now */
4721 diff_warn_rename_limit("merge.renamelimit",