git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 6/7] merge-ort: upon merge abort, only show messages causing the abort

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Jun 19, 2024, 03:00 UTC
Message-ID
<6756956d0c7e3672a1a3b362b31b9d7e29bc5b9f.1718766019.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1748.v2.git.1718766019.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>

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>
---
 merge-ort.c | 78 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 25 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index d0b13463283..b337e4d74ef 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -543,10 +543,24 @@ 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,
 
 	/* Keep this entry _last_ in the list */
-	NB_CONFLICT_TYPES,
+	NB_TOTAL_TYPES,
 };
 
 /*
@@ -597,8 +611,10 @@ 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)",
 };
 
 struct logical_conflict_info {
@@ -762,7 +778,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 */
@@ -1817,9 +1834,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;
@@ -1828,9 +1845,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;
@@ -1848,9 +1865,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;
@@ -1867,9 +1884,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;
@@ -1901,9 +1918,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;
@@ -4646,6 +4663,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");
@@ -4662,6 +4680,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;
@@ -4669,25 +4691,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",

-- 
gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 27 of 34 in “Fix and improve some error codepaths in merge-ort”
  1. 0/7 Fix and improve some error codepaths in merge-ortElijah Newren via GitGitGadget, Jun 13, 2024
  2. 1/7 merge-ort: extract handling of priv member into reusable functionElijah Newren via GitGitGadget, Jun 13, 2024
  3. Junio C HamanoJun 13, 2024
  4. Taylor BlauJun 13, 2024
  5. 2/7 merge-ort: maintain expected invariant for priv memberElijah Newren via GitGitGadget, Jun 13, 2024
  6. Taylor BlauJun 13, 2024
  7. Elijah NewrenJun 19, 2024
  8. 3/7 merge-ort: fix type of local 'clean' var in handle_content_merge()Elijah Newren via GitGitGadget, Jun 13, 2024
  9. 4/7 merge-ort: clearer propagation of failure-to-function from merge_submoduleElijah Newren via GitGitGadget, Jun 13, 2024
  10. 5/7 merge-ort: loosen commented requirementsElijah Newren via GitGitGadget, Jun 13, 2024
  11. 6/7 merge-ort: upon merge abort, only show messages causing the abortElijah Newren via GitGitGadget, Jun 13, 2024
  12. Eric SunshineJun 14, 2024
  13. Elijah NewrenJun 19, 2024
  14. 7/7 merge-ort: convert more error() cases to path_msg()Elijah Newren via GitGitGadget, Jun 13, 2024
  15. Taylor BlauJun 13, 2024
  16. 0/7 Fix and improve some error codepaths in merge-ortElijah Newren via GitGitGadget, Jun 19, 2024
  17. 1/7 merge-ort: extract handling of priv member into reusable functionElijah Newren via GitGitGadget, Jun 19, 2024
  18. 2/7 merge-ort: maintain expected invariant for priv memberElijah Newren via GitGitGadget, Jun 19, 2024
  19. Derrick StoleeJun 28, 2024
  20. 3/7 merge-ort: fix type of local 'clean' var in handle_content_merge()Elijah Newren via GitGitGadget, Jun 19, 2024
  21. Derrick StoleeJun 28, 2024
  22. 4/7 merge-ort: clearer propagation of failure-to-function from merge_submoduleElijah Newren via GitGitGadget, Jun 19, 2024
  23. Derrick StoleeJun 28, 2024
  24. Elijah NewrenJun 28, 2024
  25. Derrick StoleeJun 28, 2024
  26. 5/7 merge-ort: loosen commented requirementsElijah Newren via GitGitGadget, Jun 19, 2024
  27. 6/7 merge-ort: upon merge abort, only show messages causing the abortElijah Newren via GitGitGadget, Jun 19, 2024
  28. 7/7 merge-ort: convert more error() cases to path_msg()Elijah Newren via GitGitGadget, Jun 19, 2024
  29. Jeff KingJul 2, 2024
  30. Elijah NewrenJul 3, 2024
  31. Junio C HamanoJul 3, 2024
  32. Jeff KingJul 6, 2024
  33. Derrick StoleeJun 28, 2024
  34. Junio C HamanoJun 28, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.