From 08353ebbab2dfdee50a6daa616ec8b6483cb07c8 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Wed, 11 Aug 2010 10:38:04 +0200 Subject: [PATCH 1/5] Turn unpack_trees_options.msgs into an array + enum The list of error messages was introduced as a structure, but an array indexed over an enum is more flexible, since it allows one to store a type of error message (index in the array) in a variable. This change needs to rename would_lose_untracked -> would_lose_untracked_file to avoid a clash with the function would_lose_untracked in merge-recursive.c. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- builtin/merge.c | 2 +- merge-recursive.c | 29 +++++++++++------------------ merge-recursive.h | 4 ++-- unpack-trees.c | 42 +++++++++++++++++++++--------------------- unpack-trees.h | 19 ++++++++++--------- 6 files changed, 46 insertions(+), 52 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 1994be92c..7d1706e0f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -373,7 +373,7 @@ static int merge_working_tree(struct checkout_opts *opts, topts.src_index = &the_index; topts.dst_index = &the_index; - topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches."; + topts.msgs[ERROR_NOT_UPTODATE_FILE] = "You have local changes to '%s'; cannot switch branches."; refresh_cache(REFRESH_QUIET); diff --git a/builtin/merge.c b/builtin/merge.c index 37ce4f589..115a28854 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.msgs = get_porcelain_error_msgs(); + set_porcelain_error_msgs(opts.msgs); trees[nr_trees] = parse_tree_indirect(head); if (!trees[nr_trees++]) diff --git a/merge-recursive.c b/merge-recursive.c index fb6aa4a55..d3bd96389 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -185,7 +185,7 @@ static int git_merge_trees(int index_only, opts.fn = threeway_merge; opts.src_index = &the_index; opts.dst_index = &the_index; - opts.msgs = get_porcelain_error_msgs(); + set_porcelain_error_msgs(opts.msgs); init_tree_desc_from_tree(t+0, common); init_tree_desc_from_tree(t+1, head); @@ -1178,26 +1178,19 @@ static int process_entry(struct merge_options *o, return clean_merge; } -struct unpack_trees_error_msgs get_porcelain_error_msgs(void) +void set_porcelain_error_msgs(const char **msgs) { - struct unpack_trees_error_msgs msgs = { - /* would_overwrite */ - "Your local changes to '%s' would be overwritten by merge. Aborting.", - /* not_uptodate_file */ - "Your local changes to '%s' would be overwritten by merge. Aborting.", - /* not_uptodate_dir */ - "Updating '%s' would lose untracked files in it. Aborting.", - /* would_lose_untracked */ - "Untracked working tree file '%s' would be %s by merge. Aborting", - /* bind_overlap -- will not happen here */ - NULL, - }; - if (advice_commit_before_merge) { - msgs.would_overwrite = msgs.not_uptodate_file = + if (advice_commit_before_merge) + msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] = "Your local changes to '%s' would be overwritten by merge. Aborting.\n" "Please, commit your changes or stash them before you can merge."; - } - return msgs; + else + msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] = + "Your local changes to '%s' would be overwritten by merge. Aborting."; + msgs[ERROR_NOT_UPTODATE_DIR] = + "Updating '%s' would lose untracked files in it. Aborting."; + msgs[ERROR_WOULD_LOSE_UNTRACKED] = + "Untracked working tree file '%s' would be %s by merge. Aborting"; } int merge_trees(struct merge_options *o, diff --git a/merge-recursive.h b/merge-recursive.h index b831293b3..8412db87e 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -23,8 +23,8 @@ struct merge_options { struct string_list current_directory_set; }; -/* Return a list of user-friendly error messages to be used by merge */ -struct unpack_trees_error_msgs get_porcelain_error_msgs(void); +/* Sets the list of user-friendly error messages to be used by merge */ +void set_porcelain_error_msgs(const char **msgs); /* merge_trees() but with recursive ancestor consolidation */ int merge_recursive(struct merge_options *o, diff --git a/unpack-trees.c b/unpack-trees.c index 8cf0da317..304e59a5b 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -13,37 +13,37 @@ * Error messages expected by scripts out of plumbing commands such as * read-tree. Non-scripted Porcelain is not required to use these messages * and in fact are encouraged to reword them to better suit their particular - * situation better. See how "git checkout" replaces not_uptodate_file to + * situation better. See how "git checkout" replaces ERROR_NOT_UPTODATE_FILE to * explain why it does not allow switching between branches when you have * local changes, for example. */ -static struct unpack_trees_error_msgs unpack_plumbing_errors = { - /* would_overwrite */ +const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = { + /* ERROR_WOULD_OVERWRITE */ "Entry '%s' would be overwritten by merge. Cannot merge.", - /* not_uptodate_file */ + /* ERROR_NOT_UPTODATE_FILE */ "Entry '%s' not uptodate. Cannot merge.", - /* not_uptodate_dir */ + /* ERROR_NOT_UPTODATE_DIR */ "Updating '%s' would lose untracked files in it", - /* would_lose_untracked */ + /* ERROR_WOULD_LOSE_UNTRACKED */ "Untracked working tree file '%s' would be %s by merge.", - /* bind_overlap */ + /* ERROR_BIND_OVERLAP */ "Entry '%s' overlaps with '%s'. Cannot bind.", - /* sparse_not_uptodate_file */ + /* ERROR_SPARSE_NOT_UPTODATE_FILE */ "Entry '%s' not uptodate. Cannot update sparse checkout.", - /* would_lose_orphaned */ + /* ERROR_WOULD_LOSE_ORPHANED */ "Working tree file '%s' would be %s by sparse checkout update.", }; -#define ERRORMSG(o,fld) \ - ( ((o) && (o)->msgs.fld) \ - ? ((o)->msgs.fld) \ - : (unpack_plumbing_errors.fld) ) +#define ERRORMSG(o,type) \ + ( ((o) && (o)->msgs[(type)]) \ + ? ((o)->msgs[(type)]) \ + : (unpack_plumbing_errors[(type)]) ) static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce, unsigned int set, unsigned int clear) @@ -838,7 +838,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o) { - return error(ERRORMSG(o, would_overwrite), ce->name); + return error(ERRORMSG(o, ERROR_WOULD_OVERWRITE), ce->name); } static int same(struct cache_entry *a, struct cache_entry *b) @@ -893,13 +893,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, ERRORMSG(o, ERROR_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, ERRORMSG(o, ERROR_SPARSE_NOT_UPTODATE_FILE)); } static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o) @@ -986,7 +986,7 @@ 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); + error(ERRORMSG(o, ERROR_NOT_UPTODATE_DIR), ce->name); free(pathbuf); return cnt; } @@ -1068,7 +1068,7 @@ 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); + error(ERRORMSG(o, ERROR_WOULD_LOSE_UNTRACKED), ce->name, action); } return 0; } @@ -1077,13 +1077,13 @@ static int verify_absent(struct cache_entry *ce, const char *action, { if (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o)) return 0; - return verify_absent_1(ce, action, o, ERRORMSG(o, would_lose_untracked)); + return verify_absent_1(ce, action, o, ERRORMSG(o, ERROR_WOULD_LOSE_UNTRACKED)); } static int verify_absent_sparse(struct cache_entry *ce, const char *action, struct unpack_trees_options *o) { - return verify_absent_1(ce, action, o, ERRORMSG(o, would_lose_orphaned)); + return verify_absent_1(ce, action, o, ERRORMSG(o, ERROR_WOULD_LOSE_ORPHANED)); } static int merged_entry(struct cache_entry *merge, struct cache_entry *old, @@ -1412,7 +1412,7 @@ int bind_merge(struct cache_entry **src, o->merge_size); if (a && old) return o->gently ? -1 : - error(ERRORMSG(o, bind_overlap), a->name, old->name); + error(ERRORMSG(o, ERROR_BIND_OVERLAP), a->name, old->name); if (!a) return keep_entry(old, o); else diff --git a/unpack-trees.h b/unpack-trees.h index ef70eab39..09e22522f 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -9,14 +9,15 @@ struct exclude_list; typedef int (*merge_fn_t)(struct cache_entry **src, struct unpack_trees_options *options); -struct unpack_trees_error_msgs { - const char *would_overwrite; - const char *not_uptodate_file; - const char *not_uptodate_dir; - const char *would_lose_untracked; - const char *bind_overlap; - const char *sparse_not_uptodate_file; - const char *would_lose_orphaned; +enum unpack_trees_error_types { + ERROR_WOULD_OVERWRITE = 0, + ERROR_NOT_UPTODATE_FILE, + ERROR_NOT_UPTODATE_DIR, + ERROR_WOULD_LOSE_UNTRACKED, + ERROR_BIND_OVERLAP, + ERROR_SPARSE_NOT_UPTODATE_FILE, + ERROR_WOULD_LOSE_ORPHANED, + NB_UNPACK_TREES_ERROR_TYPES }; struct unpack_trees_options { @@ -38,7 +39,7 @@ struct unpack_trees_options { int cache_bottom; struct dir_struct *dir; merge_fn_t fn; - struct unpack_trees_error_msgs msgs; + const char *msgs[NB_UNPACK_TREES_ERROR_TYPES]; int head_idx; int merge_size; From 23cbf11b5c08c6d0ab0fd0d6f2dc5a32ca31c745 Mon Sep 17 00:00:00 2001 From: Diane Gasselin Date: Wed, 11 Aug 2010 10:38:05 +0200 Subject: [PATCH 2/5] merge-recursive: porcelain messages for checkout A porcelain message was first added in checkout.c in the commit 8ccba008 (Junio C Hamano, Sat May 17 21:03:49 2008, unpack-trees: allow Porcelain to give different error messages) to give better feedback in the case of merge errors. This patch adapts the porcelain messages for the case of checkout instead. This way, when having a checkout error, "merge" no longer appears in the error message. While we're there, we add an advice in the case of would_lose_untracked_file. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- builtin/merge.c | 2 +- merge-recursive.c | 31 ++++++++++++++++++++++--------- merge-recursive.h | 7 +++++-- unpack-trees.c | 5 ++--- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 7d1706e0f..22bf47cb6 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -373,7 +373,7 @@ static int merge_working_tree(struct checkout_opts *opts, topts.src_index = &the_index; topts.dst_index = &the_index; - topts.msgs[ERROR_NOT_UPTODATE_FILE] = "You have local changes to '%s'; cannot switch branches."; + set_porcelain_error_msgs(topts.msgs, "checkout"); refresh_cache(REFRESH_QUIET); diff --git a/builtin/merge.c b/builtin/merge.c index 115a28854..de5a0f629 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; - set_porcelain_error_msgs(opts.msgs); + set_porcelain_error_msgs(opts.msgs, "merge"); trees[nr_trees] = parse_tree_indirect(head); if (!trees[nr_trees++]) diff --git a/merge-recursive.c b/merge-recursive.c index d3bd96389..b1e526b0d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -185,7 +185,7 @@ static int git_merge_trees(int index_only, opts.fn = threeway_merge; opts.src_index = &the_index; opts.dst_index = &the_index; - set_porcelain_error_msgs(opts.msgs); + set_porcelain_error_msgs(opts.msgs, "merge"); init_tree_desc_from_tree(t+0, common); init_tree_desc_from_tree(t+1, head); @@ -1178,19 +1178,32 @@ static int process_entry(struct merge_options *o, return clean_merge; } -void set_porcelain_error_msgs(const char **msgs) +void set_porcelain_error_msgs(const char **msgs, const char *cmd) { + const char *msg; + char *tmp; + const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches"; if (advice_commit_before_merge) - msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] = - "Your local changes to '%s' would be overwritten by merge. Aborting.\n" - "Please, commit your changes or stash them before you can merge."; + msg = "Your local changes to '%%s' would be overwritten by %s. Aborting.\n" + "Please, commit your changes or stash them before you can %s."; else - msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] = - "Your local changes to '%s' would be overwritten by merge. Aborting."; + msg = "Your local changes to '%%s' would be overwritten by %s. Aborting."; + tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 3); + sprintf(tmp, msg, cmd, cmd2); + msgs[ERROR_WOULD_OVERWRITE] = tmp; + msgs[ERROR_NOT_UPTODATE_FILE] = tmp; + msgs[ERROR_NOT_UPTODATE_DIR] = "Updating '%s' would lose untracked files in it. Aborting."; - msgs[ERROR_WOULD_LOSE_UNTRACKED] = - "Untracked working tree file '%s' would be %s by merge. Aborting"; + + if (advice_commit_before_merge) + msg = "Untracked working tree file '%%s' would be %%s by %s. Aborting" + "Please move or remove them before you can %s."; + else + msg = "Untracked working tree file '%%s' would be %%s by %s. Aborting"; + tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 3); + sprintf(tmp, msg, cmd, cmd2); + msgs[ERROR_WOULD_LOSE_UNTRACKED] = tmp; } int merge_trees(struct merge_options *o, diff --git a/merge-recursive.h b/merge-recursive.h index 8412db87e..08f985036 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -23,8 +23,11 @@ struct merge_options { struct string_list current_directory_set; }; -/* Sets the list of user-friendly error messages to be used by merge */ -void set_porcelain_error_msgs(const char **msgs); +/* + * Sets the list of user-friendly error messages to be used by the + * command "cmd" (either merge or checkout) + */ +void set_porcelain_error_msgs(const char **msgs, const char *cmd); /* merge_trees() but with recursive ancestor consolidation */ int merge_recursive(struct merge_options *o, diff --git a/unpack-trees.c b/unpack-trees.c index 304e59a5b..e3258317d 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -13,9 +13,8 @@ * Error messages expected by scripts out of plumbing commands such as * read-tree. Non-scripted Porcelain is not required to use these messages * and in fact are encouraged to reword them to better suit their particular - * situation better. See how "git checkout" replaces ERROR_NOT_UPTODATE_FILE to - * explain why it does not allow switching between branches when you have - * local changes, for example. + * situation better. See how "git checkout" and "git merge" replaces + * them using set_porcelain_error_msgs(), for example. */ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = { /* ERROR_WOULD_OVERWRITE */ From 08402b0409bc501deb97cf4388a78ee9f87092c6 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Wed, 11 Aug 2010 10:38:06 +0200 Subject: [PATCH 3/5] merge-recursive: distinguish "removed" and "overwritten" messages To limit the number of possible error messages, the error messages for the case would_lose_untracked_file and would_lose_orphaned in unpack_trees_options.msgs were handled with a single string, parameterized by an action string ("overwritten" or "removed"). Instead, we consider them as two different cases, with unparameterized string. This will make it easier to make separate lists sorted by error types later. Only the bind_overlap case still takes two %s parameters, but that's unavoidable. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- merge-recursive.c | 13 ++++++---- unpack-trees.c | 64 +++++++++++++++++++++++++++++------------------ unpack-trees.h | 6 +++-- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index b1e526b0d..697d948e8 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1197,13 +1197,16 @@ void set_porcelain_error_msgs(const char **msgs, const char *cmd) "Updating '%s' would lose untracked files in it. Aborting."; if (advice_commit_before_merge) - msg = "Untracked working tree file '%%s' would be %%s by %s. Aborting" + msg = "Untracked working tree file '%%s' would be %s by %s. Aborting" "Please move or remove them before you can %s."; else - msg = "Untracked working tree file '%%s' would be %%s by %s. Aborting"; - tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 3); - sprintf(tmp, msg, cmd, cmd2); - msgs[ERROR_WOULD_LOSE_UNTRACKED] = tmp; + msg = "Untracked working tree file '%%s' would be %s by %s. Aborting"; + tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("removed") + strlen(cmd2) - 4); + sprintf(tmp, msg, "removed", cmd, cmd2); + msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp; + tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("overwritten") + strlen(cmd2) - 4); + sprintf(tmp, msg, "overwritten", cmd, cmd2); + msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp; } int merge_trees(struct merge_options *o, diff --git a/unpack-trees.c b/unpack-trees.c index e3258317d..342f5ec22 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -26,8 +26,11 @@ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = { /* ERROR_NOT_UPTODATE_DIR */ "Updating '%s' would lose untracked files in it", - /* ERROR_WOULD_LOSE_UNTRACKED */ - "Untracked working tree file '%s' would be %s by merge.", + /* ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN */ + "Untracked working tree file '%s' would be overwritten by merge.", + + /* ERROR_WOULD_LOSE_UNTRACKED_REMOVED */ + "Untracked working tree file '%s' would be removed by merge.", /* ERROR_BIND_OVERLAP */ "Entry '%s' overlaps with '%s'. Cannot bind.", @@ -35,8 +38,11 @@ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = { /* ERROR_SPARSE_NOT_UPTODATE_FILE */ "Entry '%s' not uptodate. Cannot update sparse checkout.", - /* ERROR_WOULD_LOSE_ORPHANED */ - "Working tree file '%s' would be %s by sparse checkout update.", + /* ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN */ + "Working tree file '%s' would be overwritten by sparse checkout update.", + + /* ERROR_WOULD_LOSE_ORPHANED_REMOVED */ + "Working tree file '%s' would be removed by sparse checkout update.", }; #define ERRORMSG(o,type) \ @@ -131,7 +137,7 @@ static int check_updates(struct unpack_trees_options *o) } static int verify_uptodate_sparse(struct cache_entry *ce, struct unpack_trees_options *o); -static int verify_absent_sparse(struct cache_entry *ce, const char *action, struct unpack_trees_options *o); +static int verify_absent_sparse(struct cache_entry *ce, enum unpack_trees_error_types, struct unpack_trees_options *o); static int will_have_skip_worktree(const struct cache_entry *ce, struct unpack_trees_options *o) { @@ -174,7 +180,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt ce->ce_flags |= CE_WT_REMOVE; } if (was_skip_worktree && !ce_skip_worktree(ce)) { - if (verify_absent_sparse(ce, "overwritten", o)) + if (verify_absent_sparse(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) return -1; ce->ce_flags |= CE_UPDATE; } @@ -859,7 +865,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_types error_type) { struct stat st; @@ -884,7 +890,7 @@ static int verify_uptodate_1(struct cache_entry *ce, if (errno == ENOENT) return 0; return o->gently ? -1 : - error(error_msg, ce->name); + error(ERRORMSG(o, error_type), ce->name); } static int verify_uptodate(struct cache_entry *ce, @@ -892,13 +898,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, ERROR_NOT_UPTODATE_FILE)); + return verify_uptodate_1(ce, o, ERROR_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, ERROR_SPARSE_NOT_UPTODATE_FILE)); + return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE); } static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o) @@ -914,13 +920,15 @@ static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_optio * Currently, git does not checkout subprojects during a superproject * checkout, so it is not going to overwrite anything. */ -static int verify_clean_submodule(struct cache_entry *ce, const char *action, +static int verify_clean_submodule(struct cache_entry *ce, + enum unpack_trees_error_types error_type, struct unpack_trees_options *o) { return 0; } -static int verify_clean_subdirectory(struct cache_entry *ce, const char *action, +static int verify_clean_subdirectory(struct cache_entry *ce, + enum unpack_trees_error_types error_type, struct unpack_trees_options *o) { /* @@ -941,7 +949,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action, */ if (!hashcmp(sha1, ce->sha1)) return 0; - return verify_clean_submodule(ce, action, o); + return verify_clean_submodule(ce, error_type, o); } /* @@ -1010,9 +1018,9 @@ static int icase_exists(struct unpack_trees_options *o, struct cache_entry *dst, * We do not want to remove or overwrite a working tree file that * is not tracked, unless it is ignored. */ -static int verify_absent_1(struct cache_entry *ce, const char *action, - struct unpack_trees_options *o, - const char *error_msg) +static int verify_absent_1(struct cache_entry *ce, + enum unpack_trees_error_types error_type, + struct unpack_trees_options *o) { struct stat st; @@ -1050,7 +1058,7 @@ static int verify_absent_1(struct cache_entry *ce, const char *action, * files that are in "foo/" we would lose * them. */ - if (verify_clean_subdirectory(ce, action, o) < 0) + if (verify_clean_subdirectory(ce, error_type, o) < 0) return -1; return 0; } @@ -1067,22 +1075,28 @@ static int verify_absent_1(struct cache_entry *ce, const char *action, } return o->gently ? -1 : - error(ERRORMSG(o, ERROR_WOULD_LOSE_UNTRACKED), ce->name, action); + error(ERRORMSG(o, error_type), ce->name); } return 0; } -static int verify_absent(struct cache_entry *ce, const char *action, +static int verify_absent(struct cache_entry *ce, + enum unpack_trees_error_types error_type, struct unpack_trees_options *o) { if (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o)) return 0; - return verify_absent_1(ce, action, o, ERRORMSG(o, ERROR_WOULD_LOSE_UNTRACKED)); + return verify_absent_1(ce, error_type, o); } -static int verify_absent_sparse(struct cache_entry *ce, const char *action, +static int verify_absent_sparse(struct cache_entry *ce, + enum unpack_trees_error_types error_type, struct unpack_trees_options *o) { - return verify_absent_1(ce, action, o, ERRORMSG(o, ERROR_WOULD_LOSE_ORPHANED)); + enum unpack_trees_error_types orphaned_error = error_type; + if (orphaned_error == ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN) + orphaned_error = ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN; + + return verify_absent_1(ce, orphaned_error, o); } static int merged_entry(struct cache_entry *merge, struct cache_entry *old, @@ -1091,7 +1105,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old, int update = CE_UPDATE; if (!old) { - if (verify_absent(merge, "overwritten", o)) + if (verify_absent(merge, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) return -1; invalidate_ce_path(merge, o); } else if (!(old->ce_flags & CE_CONFLICTED)) { @@ -1129,7 +1143,7 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old, { /* Did it exist in the index? */ if (!old) { - if (verify_absent(ce, "removed", o)) + if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o)) return -1; return 0; } @@ -1278,7 +1292,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o) if (index) return deleted_entry(index, index, o); if (ce && !head_deleted) { - if (verify_absent(ce, "removed", o)) + if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o)) return -1; } return 0; diff --git a/unpack-trees.h b/unpack-trees.h index 09e22522f..8be6b3cca 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -13,10 +13,12 @@ enum unpack_trees_error_types { ERROR_WOULD_OVERWRITE = 0, ERROR_NOT_UPTODATE_FILE, ERROR_NOT_UPTODATE_DIR, - ERROR_WOULD_LOSE_UNTRACKED, + ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, + ERROR_WOULD_LOSE_UNTRACKED_REMOVED, ERROR_BIND_OVERLAP, ERROR_SPARSE_NOT_UPTODATE_FILE, - ERROR_WOULD_LOSE_ORPHANED, + ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN, + ERROR_WOULD_LOSE_ORPHANED_REMOVED, NB_UNPACK_TREES_ERROR_TYPES }; From e6c111b4c092c0dd24c541b9721f5bc04641dcb0 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Wed, 11 Aug 2010 10:38:07 +0200 Subject: [PATCH 4/5] unpack_trees: group error messages by type 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. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- Documentation/technical/api-tree-walking.txt | 2 + builtin/checkout.c | 1 + builtin/merge.c | 1 + merge-recursive.c | 25 +++++-- t/t3030-merge-recursive.sh | 2 +- t/t3400-rebase.sh | 3 +- t/t3404-rebase-interactive.sh | 3 +- tree-walk.c | 11 ++- tree-walk.h | 1 + unpack-trees.c | 78 +++++++++++++++++++- unpack-trees.h | 13 +++- 11 files changed, 123 insertions(+), 17 deletions(-) diff --git a/Documentation/technical/api-tree-walking.txt b/Documentation/technical/api-tree-walking.txt index 55b728632..14af37c3f 100644 --- a/Documentation/technical/api-tree-walking.txt +++ b/Documentation/technical/api-tree-walking.txt @@ -42,6 +42,8 @@ information. * `data` can be anything the `fn` callback would want to use. +* `show_all_errors` tells whether to stop at the first error or not. + Initializing ------------ diff --git a/builtin/checkout.c b/builtin/checkout.c index 22bf47cb6..894bb84db 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 de5a0f629..1bcf7fd9d 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -704,6 +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; set_porcelain_error_msgs(opts.msgs, "merge"); trees[nr_trees] = parse_tree_indirect(head); diff --git a/merge-recursive.c b/merge-recursive.c index 697d948e8..10392d966 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1184,29 +1184,42 @@ void set_porcelain_error_msgs(const char **msgs, const char *cmd) char *tmp; const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches"; if (advice_commit_before_merge) - msg = "Your local changes to '%%s' would be overwritten by %s. Aborting.\n" + msg = "Your local changes to the following files would be overwritten by %s:\n%%s" "Please, commit your changes or stash them before you can %s."; else - msg = "Your local changes to '%%s' would be overwritten by %s. Aborting."; - tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 3); + msg = "Your local changes to the following files would be overwritten by %s:\n%%s"; + tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2); sprintf(tmp, msg, cmd, cmd2); msgs[ERROR_WOULD_OVERWRITE] = tmp; msgs[ERROR_NOT_UPTODATE_FILE] = tmp; msgs[ERROR_NOT_UPTODATE_DIR] = - "Updating '%s' would lose untracked files in it. Aborting."; + "Updating the following directories would lose untracked files in it:\n%s"; if (advice_commit_before_merge) - msg = "Untracked working tree file '%%s' would be %s by %s. Aborting" + msg = "The following untracked working tree files would be %s by %s:\n%%s" "Please move or remove them before you can %s."; else - msg = "Untracked working tree file '%%s' would be %s by %s. Aborting"; + msg = "The following untracked working tree files would be %s by %s:\n%%s"; tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("removed") + strlen(cmd2) - 4); sprintf(tmp, msg, "removed", cmd, cmd2); msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp; tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("overwritten") + strlen(cmd2) - 4); sprintf(tmp, msg, "overwritten", cmd, cmd2); msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp; + + /* + * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we + * cannot easily display it as a list. + */ + msgs[ERROR_BIND_OVERLAP] = "Entry '%s' overlaps with '%s'. Cannot bind."; + + msgs[ERROR_SPARSE_NOT_UPTODATE_FILE] = + "Cannot update sparse checkout: the following entries are not up-to-date:\n%s"; + msgs[ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN] = + "The following Working tree files would be overwritten by sparse checkout update:\n%s"; + msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] = + "The following Working tree files would be removed by sparse checkout update:\n%s"; } int merge_trees(struct merge_options *o, diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index d54154453..efe2900a3 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -294,7 +294,7 @@ test_expect_success 'fail if the index has unresolved entries' ' grep "You have not concluded your merge" out && rm -f .git/MERGE_HEAD && test_must_fail git merge "$c5" 2> out && - grep "Your local changes to .* would be overwritten by merge." out + grep "Your local changes to the following files would be overwritten by merge:" out ' test_expect_success 'merge-recursive remove conflict' ' diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index d98c7b557..45ef28281 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -129,7 +129,8 @@ test_expect_success 'rebase a single mode change' ' test_expect_success 'Show verbose error when HEAD could not be detached' ' : > B && test_must_fail git rebase topic 2> output.err > output.out && - grep "Untracked working tree file .B. would be overwritten" output.err + grep "The following untracked working tree files would be overwritten by checkout:" output.err && + grep B output.err ' rm -f B diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 47ca88fc5..0eb21cbde 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -150,8 +150,9 @@ test_expect_success 'abort with error when new base cannot be checked out' ' git rm --cached file1 && git commit -m "remove file in base" && test_must_fail git rebase -i master > output 2>&1 && - grep "Untracked working tree file .file1. would be overwritten" \ + grep "The following untracked working tree files would be overwritten by checkout:" \ output && + grep "file1" output && ! test -d .git/rebase-merge && git reset --hard HEAD^ ' diff --git a/tree-walk.c b/tree-walk.c index 67a9a0c5a..a9bbf4e23 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 (!info->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/tree-walk.h b/tree-walk.h index 42110a465..88ea7e929 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -45,6 +45,7 @@ struct traverse_info { unsigned long conflicts; traverse_callback_t fn; void *data; + int show_all_errors; }; int get_tree_entry(const unsigned char *, const char *, unsigned char *, unsigned *); diff --git a/unpack-trees.c b/unpack-trees.c index 342f5ec22..7b10f92fa 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -64,6 +64,73 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce, add_index_entry(&o->result, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE); } +/* + * add error messages on path + * corresponding to the type with the message + * indicating if it should be display in porcelain or not + */ +static int add_rejected_path(struct unpack_trees_options *o, + enum unpack_trees_error_types e, + const char *path) +{ + struct rejected_paths_list *newentry; + int porcelain = o && (o)->msgs[e]; + /* + * simply display the given error message if in plumbing mode + */ + if (!porcelain) + o->show_all_errors = 0; + if (!o->show_all_errors) + return error(ERRORMSG(o, e), path); + + /* + * Otherwise, insert in a list for future display by + * display_error_msgs() + */ + newentry = xmalloc(sizeof(struct rejected_paths_list)); + newentry->path = (char *)path; + newentry->next = o->unpack_rejects[e]; + o->unpack_rejects[e] = newentry; + return -1; +} + +/* + * free all the structures allocated for the error + */ +static void free_rejected_paths(struct unpack_trees_options *o, + enum unpack_trees_error_types e) +{ + while (o->unpack_rejects[e]) { + struct rejected_paths_list *del = o->unpack_rejects[e]; + o->unpack_rejects[e] = o->unpack_rejects[e]->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 e; + int something_displayed = 0; + for (e = 0; e < NB_UNPACK_TREES_ERROR_TYPES; e++) { + if (o->unpack_rejects[e]) { + struct rejected_paths_list *rp; + struct strbuf path = STRBUF_INIT; + something_displayed = 1; + for (rp = o->unpack_rejects[e]; rp; rp = rp->next) + strbuf_addf(&path, "\t%s\n", rp->path); + error(ERRORMSG(o, e), path.buf); + strbuf_release(&path); + free_rejected_paths(o, e); + } + } + if (something_displayed) + printf("Aborting\n"); +} + /* * Unlink the last component and schedule the leading directories for * removal, such that empty directories get removed. @@ -755,6 +822,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options setup_traverse_info(&info, prefix); info.fn = unpack_callback; info.data = o; + info.show_all_errors = o->show_all_errors; if (o->prefix) { /* @@ -834,6 +902,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options 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; @@ -843,7 +913,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o) { - return error(ERRORMSG(o, ERROR_WOULD_OVERWRITE), ce->name); + return add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce->name); } static int same(struct cache_entry *a, struct cache_entry *b) @@ -890,7 +960,7 @@ static int verify_uptodate_1(struct cache_entry *ce, if (errno == ENOENT) return 0; return o->gently ? -1 : - error(ERRORMSG(o, error_type), ce->name); + add_rejected_path(o, error_type, ce->name); } static int verify_uptodate(struct cache_entry *ce, @@ -993,7 +1063,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, i = read_directory(&d, pathbuf, namelen+1, NULL); if (i) return o->gently ? -1 : - error(ERRORMSG(o, ERROR_NOT_UPTODATE_DIR), ce->name); + add_rejected_path(o, ERROR_NOT_UPTODATE_DIR, ce->name); free(pathbuf); return cnt; } @@ -1075,7 +1145,7 @@ static int verify_absent_1(struct cache_entry *ce, } return o->gently ? -1 : - error(ERRORMSG(o, error_type), ce->name); + add_rejected_path(o, error_type, ce->name); } return 0; } diff --git a/unpack-trees.h b/unpack-trees.h index 8be6b3cca..6e049b099 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -22,6 +22,11 @@ enum unpack_trees_error_types { NB_UNPACK_TREES_ERROR_TYPES }; +struct rejected_paths_list { + char *path; + struct rejected_paths_list *next; +}; + struct unpack_trees_options { unsigned int reset, merge, @@ -36,12 +41,18 @@ 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; merge_fn_t fn; const char *msgs[NB_UNPACK_TREES_ERROR_TYPES]; + /* + * Store error messages in an array, each case + * corresponding to a error message type + */ + struct rejected_paths_list *unpack_rejects[NB_UNPACK_TREES_ERROR_TYPES]; int head_idx; int merge_size; From e935e62a0fd26ff1f29a98a4ca8fa693f42357a2 Mon Sep 17 00:00:00 2001 From: Diane Gasselin Date: Wed, 11 Aug 2010 10:38:08 +0200 Subject: [PATCH 5/5] t7609: test merge and checkout error messages Test porcelain and plumbing error messages for different types of errors of merge and checkout. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- t/t7609-merge-co-error-msgs.sh | 125 +++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100755 t/t7609-merge-co-error-msgs.sh diff --git a/t/t7609-merge-co-error-msgs.sh b/t/t7609-merge-co-error-msgs.sh new file mode 100755 index 000000000..1a109b41d --- /dev/null +++ b/t/t7609-merge-co-error-msgs.sh @@ -0,0 +1,125 @@ +#!/bin/sh + +test_description='unpack-trees error messages' + +. ./test-lib.sh + + +test_expect_success 'setup' ' + echo one >one && + git add one && + git commit -a -m First && + + git checkout -b branch && + echo two >two && + echo three >three && + echo four >four && + echo five >five && + git add two three four five && + git commit -m Second && + + git checkout master && + echo other >two && + echo other >three && + echo other >four && + echo other >five +' + +cat >expect <<\EOF +error: The following untracked working tree files would be overwritten by merge: + two + three + four + five +Please move or remove them before you can merge. +EOF + +test_expect_success 'untracked files overwritten by merge' ' + test_must_fail git merge branch 2>out && + test_cmp out expect +' + +cat >expect <<\EOF +error: Your local changes to the following files would be overwritten by merge: + two + three + four +Please, commit your changes or stash them before you can merge. +error: The following untracked working tree files would be overwritten by merge: + five +Please move or remove them before you can merge. +EOF + +test_expect_success 'untracked files or local changes ovewritten by merge' ' + git add two && + git add three && + git add four && + test_must_fail git merge branch 2>out && + test_cmp out expect +' + +cat >expect <<\EOF +error: Your local changes to the following files would be overwritten by checkout: + rep/two + rep/one +Please, commit your changes or stash them before you can switch branches. +EOF + +test_expect_success 'cannot switch branches because of local changes' ' + git add five && + mkdir rep && + echo one >rep/one && + echo two >rep/two && + git add rep/one rep/two && + git commit -m Fourth && + git checkout master && + echo uno >rep/one && + echo dos >rep/two && + test_must_fail git checkout branch 2>out && + test_cmp out expect +' + +cat >expect <<\EOF +error: Your local changes to the following files would be overwritten by checkout: + rep/two + rep/one +Please, commit your changes or stash them before you can switch branches. +EOF + +test_expect_success 'not uptodate file porcelain checkout error' ' + git add rep/one rep/two && + test_must_fail git checkout branch 2>out && + test_cmp out expect +' + +cat >expect <<\EOF +error: Updating the following directories would lose untracked files in it: + rep2 + rep + +EOF + +test_expect_success 'not_uptodate_dir porcelain checkout error' ' + git init uptodate && + cd uptodate && + mkdir rep && + mkdir rep2 && + touch rep/foo && + touch rep2/foo && + git add rep/foo rep2/foo && + git commit -m init && + git checkout -b branch && + git rm rep -r && + git rm rep2 -r && + >rep && + >rep2 && + git add rep rep2&& + git commit -m "added test as a file" && + git checkout master && + >rep/untracked-file && + >rep2/untracked-file && + test_must_fail git checkout branch 2>out && + test_cmp out ../expect +' + +test_done