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

[PATCH 2/5 v2] unpack_trees: group errors by type

From
DGDiane Gasselin <diane.gasselin@ensimag.imag.fr>
Date
Jun 15, 2010, 12:22 UTC
Message-ID
<1276604576-28092-3-git-send-email-diane.gasselin@ensimag.imag.fr>
In-Reply-To
<1276604576-28092-2-git-send-email-diane.gasselin@ensimag.imag.fr>
When an error is encountered, it calls add_rejected_file() which either
- directly displays the error message and stops if in plumbing mode
  (i.e. if show_all_errors is not initialized at 1)
- or stores it so that it will be displayed at the end with display_error_msgs(),

Storing the files by error type permits to have a list of files for which there is the same error instead of having a serie of almost identical errors.

As each bind_overlap error combines a file and an old file, a list cannot be done, therefore, theses errors are not stored but directly displayed.

Update t3030 to expect failure for a test based on an error message.
Signed-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>
Signed-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>
Signed-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>
---
 builtin/checkout.c         |    1 +
 builtin/merge.c            |    2 +-
 t/t3030-merge-recursive.sh |    2 +-
 tree-walk.c                |   11 +++-
 unpack-trees.c             |  119 +++++++++++++++++++++++++++++++++++++++++---
 unpack-trees.h             |   31 +++++++++++-
 6 files changed, 152 insertions(+), 14 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6f34566..23eae56 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -392,6 +392,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.dir = xcalloc(1, sizeof(*topts.dir));
 		topts.dir->flags |= DIR_SHOW_IGNORED;
 		topts.dir->exclude_per_dir = ".gitignore";
+		topts.show_all_errors = 1;
 		tree = parse_tree_indirect(old->commit ?
 					   old->commit->object.sha1 :
 					   (unsigned char *)EMPTY_TREE_SHA1_BIN);
diff --git a/builtin/merge.c b/builtin/merge.c
index 501177f..2c2f904 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -704,7 +704,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
 	opts.verbose_update = 1;
 	opts.merge = 1;
 	opts.fn = twoway_merge;
-
+	opts.show_all_errors = 1;
 	opts.msgs = get_porcelain_error_msgs("merge");
 
 	trees[nr_trees] = parse_tree_indirect(head);
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 9929f82..7ef8dd4 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -269,7 +269,7 @@ test_expect_success 'merge-recursive result' '
 
 '
 
-test_expect_success 'fail if the index has unresolved entries' '
+test_expect_failure 'fail if the index has unresolved entries' '
 
 	rm -fr [abcd] &&
 	git checkout -f "$c1" &&
diff --git a/tree-walk.c b/tree-walk.c
index 67a9a0c..b8b7f00 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "tree-walk.h"
+#include "unpack-trees.h"
 #include "tree.h"
 
 static const char *get_mode(const char *str, unsigned int *modep)
@@ -310,6 +311,7 @@ static void free_extended_entry(struct tree_desc_x *t)
 int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 {
 	int ret = 0;
+	int error = 0;
 	struct name_entry *entry = xmalloc(n*sizeof(*entry));
 	int i;
 	struct tree_desc_x *tx = xcalloc(n, sizeof(*tx));
@@ -377,8 +379,11 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		if (!mask)
 			break;
 		ret = info->fn(n, mask, dirmask, entry, info);
-		if (ret < 0)
-			break;
+		if (ret < 0) {
+			error = ret;
+			if (!((struct unpack_trees_options*)(info->data))->show_all_errors)
+				break;
+		}
 		mask &= ret;
 		ret = 0;
 		for (i = 0; i < n; i++)
@@ -389,7 +394,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 	for (i = 0; i < n; i++)
 		free_extended_entry(tx + i);
 	free(tx);
-	return ret;
+	return error;
 }
 
 static int find_tree_entry(struct tree_desc *t, const char *name, unsigned char *result, unsigned *mode)
diff --git a/unpack-trees.c b/unpack-trees.c
index c29a9e0..78ecdc9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -60,6 +60,92 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
 }
 
 /*
+ * add error messages on path <path> and action <action>
+ * corresponding to the type <e> with the message <msg>
+ * indicating if it should be display in porcelain or not
+ */
+static int add_rejected_path(struct unpack_trees_options *o,
+			     enum unpack_trees_error e,
+			     const char *path,
+			     const char *action,
+			     int porcelain,
+			     const char *msg)
+{
+	struct rejected_paths_list *newentry;
+	struct rejected_paths **rp;
+	/*
+	 * simply display the given error message if in plumbing mode
+	 */
+	if (!porcelain)
+		o->show_all_errors = 0;
+	if (!o->show_all_errors)
+		return error(msg, path, action);
+	/*
+	 * if there is a porcelain error message defined,
+	 * the error is stored in order to be nicely displayed later
+	 */
+	if (e == would_lose_untracked_overwritten && !strcmp(action, "removed"))
+		e = would_lose_untracked_removed;
+
+	rp = &o->unpack_rejects[e];
+
+	if (!o->unpack_rejects[e]) {
+		*rp = malloc(sizeof(struct rejected_paths));
+		(*rp)->list = NULL;
+	}
+	newentry = malloc(sizeof(struct rejected_paths_list));
+	newentry->path = (char *)path;
+	newentry->next = (*rp)->list;
+	(*rp)->list = newentry;
+	(*rp)->msg = msg;
+	(*rp)->action = (char *)action;
+	return -1;
+}
+
+/*
+ * free all the structures allocated for the error <e>
+ */
+static void free_rejected_paths(struct unpack_trees_options *o,
+				enum unpack_trees_error e)
+{
+	while (o->unpack_rejects[e]->list) {
+		struct rejected_paths_list *del = o->unpack_rejects[e]->list;
+		o->unpack_rejects[e]->list = o->unpack_rejects[e]->list->next;
+		free(del);
+	}
+	free(o->unpack_rejects[e]);
+}
+
+/*
+ * display all the error messages stored in a nice way
+ */
+static void display_error_msgs(struct unpack_trees_options *o)
+{
+	int i;
+	int something_is_displayed = 0;
+	for (i = 0; i < NB_UNPACK_TREES_ERROR; i++) {
+		if (o->unpack_rejects[i] && o->unpack_rejects[i]->list) {
+			struct rejected_paths *rp = o->unpack_rejects[i];
+			struct rejected_paths_list *f = rp->list;
+			char *action = rp->action;
+			struct strbuf path = STRBUF_INIT;
+			something_is_displayed  = 1;
+			for (f = rp->list; f; f = f->next)
+				strbuf_addf(&path, "\t%s\n", f->path);
+			if (i == would_lose_untracked_overwritten ||
+			    i == would_lose_untracked_removed)
+				error(rp->msg, action, path.buf);
+			else
+				error(rp->msg, path.buf, action);
+			strbuf_release(&path);
+			free_rejected_paths(o, i);
+		}
+	}
+	if (something_is_displayed)
+		printf("Aborting\n");
+}
+
+/*
  * Unlink the last component and schedule the leading directories for
  * removal, such that empty directories get removed.
  */
@@ -819,6 +905,8 @@ done:
 	return ret;
 
 return_failed:
+	if (o->show_all_errors)
+		display_error_msgs(o);
 	mark_all_ce_unused(o->src_index);
 	ret = unpack_failed(o, NULL);
 	goto done;
@@ -828,7 +916,9 @@ return_failed:
 
 static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o)
 {
-	return error(ERRORMSG(o, would_overwrite), ce->name);
+	return add_rejected_path(o, would_overwrite, ce->name, NULL,
+				 (o && (o)->msgs.would_overwrite),
+				 ERRORMSG(o, would_overwrite));
 }
 
 static int same(struct cache_entry *a, struct cache_entry *b)
@@ -850,7 +940,7 @@ static int same(struct cache_entry *a, struct cache_entry *b)
  */
 static int verify_uptodate_1(struct cache_entry *ce,
 				   struct unpack_trees_options *o,
-				   const char *error_msg)
+				   enum unpack_trees_error error)
 {
 	struct stat st;
 
@@ -874,8 +964,16 @@ static int verify_uptodate_1(struct cache_entry *ce,
 	}
 	if (errno == ENOENT)
 		return 0;
-	return o->gently ? -1 :
-		error(error_msg, ce->name);
+	if (error == sparse_not_uptodate_file)
+		return o->gently ? -1 :
+			add_rejected_path(o, sparse_not_uptodate_file, ce->name, NULL,
+					  (o && (o)->msgs.sparse_not_uptodate_file),
+					  ERRORMSG(o, sparse_not_uptodate_file));
+	else
+		return o->gently ? -1 :
+			add_rejected_path(o, not_uptodate_file, ce->name, NULL,
+					  (o && (o)->msgs.not_uptodate_file),
+					  ERRORMSG(o, not_uptodate_file));
 }
 
 static int verify_uptodate(struct cache_entry *ce,
@@ -883,13 +981,13 @@ static int verify_uptodate(struct cache_entry *ce,
 {
 	if (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))
 		return 0;
-	return verify_uptodate_1(ce, o, ERRORMSG(o, not_uptodate_file));
+	return verify_uptodate_1(ce, o, not_uptodate_file);
 }
 
 static int verify_uptodate_sparse(struct cache_entry *ce,
 				  struct unpack_trees_options *o)
 {
-	return verify_uptodate_1(ce, o, ERRORMSG(o, sparse_not_uptodate_file));
+	return verify_uptodate_1(ce, o, sparse_not_uptodate_file);
 }
 
 static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o)
@@ -976,7 +1074,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
 	i = read_directory(&d, pathbuf, namelen+1, NULL);
 	if (i)
 		return o->gently ? -1 :
-			error(ERRORMSG(o, not_uptodate_dir), ce->name);
+			add_rejected_path(o, not_uptodate_dir, ce->name, NULL,
+					  (o && (o)->msgs.not_uptodate_dir),
+					  ERRORMSG(o, not_uptodate_dir));
 	free(pathbuf);
 	return cnt;
 }
@@ -1058,7 +1158,10 @@ static int verify_absent_1(struct cache_entry *ce, const char *action,
 		}
 
 		return o->gently ? -1 :
-			error(ERRORMSG(o, would_lose_untracked), ce->name, action);
+			add_rejected_path(o, would_lose_untracked_overwritten,
+					  ce->name, action,
+					  (o && (o)->msgs.would_lose_untracked),
+					  ERRORMSG(o, would_lose_untracked));
 	}
 	return 0;
 }
diff --git a/unpack-trees.h b/unpack-trees.h
index ef70eab..1f8e71e 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -2,6 +2,7 @@
 #define UNPACK_TREES_H
 
 #define MAX_UNPACK_TREES 8
+#define NB_UNPACK_TREES_ERROR 6
 
 struct unpack_trees_options;
 struct exclude_list;
@@ -19,6 +20,27 @@ struct unpack_trees_error_msgs {
 	const char *would_lose_orphaned;
 };
 
+struct rejected_paths_list {
+	char *path;
+	struct rejected_paths_list *next;
+};
+
+struct rejected_paths {
+	char *action;
+	const char *msg;
+	struct rejected_paths_list *list;
+};
+
+
+enum unpack_trees_error{
+	would_overwrite,
+	not_uptodate_file,
+	not_uptodate_dir,
+	would_lose_untracked_overwritten,
+	would_lose_untracked_removed,
+	sparse_not_uptodate_file
+};
+
 struct unpack_trees_options {
 	unsigned int reset,
 		     merge,
@@ -33,7 +55,8 @@ struct unpack_trees_options {
 		     diff_index_cached,
 		     debug_unpack,
 		     skip_sparse_checkout,
-		     gently;
+		     gently,
+		     show_all_errors;
 	const char *prefix;
 	int cache_bottom;
 	struct dir_struct *dir;
@@ -51,6 +74,12 @@ struct unpack_trees_options {
 	struct index_state result;
 
 	struct exclude_list *el; /* for internal use */
+
+	/*
+	 * Store error messages in an array, each case
+	 * corresponding to a error message type
+	 */
+	struct rejected_paths *unpack_rejects[NB_UNPACK_TREES_ERROR];
 };
 
 extern int unpack_trees(unsigned n, struct tree_desc *t,
-- 
1.6.6.7.ga5fe3
Previous: Diane GasselinNext: Diane Gasselin
Message 3 of 13 in “unpack_trees: nicer error messages”
  1. 0/5 unpack_trees: nicer error messagesDiane Gasselin, Jun 15, 2010
  2. 1/5 merge-recursive: porcelain messages for checkoutDiane Gasselin, Jun 15, 2010
  3. 2/5 unpack_trees: group errors by typeDiane Gasselin, Jun 15, 2010
  4. 3/5 unpack_trees_options: update porcelain messagesDiane Gasselin, Jun 15, 2010
  5. 4/5 tests: update porcelain expected messageDiane Gasselin, Jun 15, 2010
  6. 5/5 t7609: test merge and checkout error messagesDiane Gasselin, Jun 15, 2010
  7. Matthieu MoyJun 15, 2010
  8. Matthieu MoyJun 15, 2010
  9. Matthieu MoyJun 15, 2010
  10. Diane GasselinJun 15, 2010
  11. Matthieu MoyJun 15, 2010
  12. Diane GasselinJun 15, 2010
  13. Matthieu MoyJun 15, 2010

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.