@matheusbsbs / matheus-bagliano / commits / d38a30df7d
Fork 0

Be more user-friendly when refusing to do something because of conflict.

Various commands refuse to run in the presence of conflicts (commit, merge, pull, cherry-pick/revert). They all used to provide rough, and inconsistant error messages. A new variable advice.resolveconflict is introduced, and allows more verbose messages, pointing the user to the appropriate solution. For commit, the error message used to look like this: $ git commit foo.txt: needs merge foo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169) foo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030) foo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4) error: Error building trees The "need merge" line is given by refresh_cache. We add the IN_PORCELAIN option to make the output more consistant with the other porcelain commands, and catch the error in return, to stop with a clean error message. The next lines were displayed by a call to cache_tree_update(), which is not reached anymore if we noticed the conflict. The new output looks like: U foo.txt fatal: 'commit' is not possible because you have unmerged files. Please, fix them up in the work tree, and then use 'git add/rm <file>' as appropriate to mark resolution and make a commit, or use 'git commit -a'. Pull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD exists instead of waiting for merge to complain. The behavior of merge and the test-case are slightly modified to reflect the usual flow: start with conflicts, fix them, and afterwards get rid of MERGE_HEAD, with different error messages at each stage. Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matthieu Moy committed Jan 12, 2010 at 10:54 UTC d38a30df7dd54c5c6883af1de1a03ec7d523cee5
9 files changed +93 -13
Documentation/config.txt
+4
@@ -130,6 +130,10 @@ advice.*::
130 Advice shown when linkgit:git-merge[1] refuses to
131 merge to avoid overwritting local changes.
132 Default: true.
133 + resolveConflict::
134 + Advices shown by various commands when conflicts
135 + prevent the operation from being performed.
136 + Default: true.
137 --
138
139 core.fileMode::
advice.c
+16
@@ -3,6 +3,7 @@
3 int advice_push_nonfastforward = 1;
4 int advice_status_hints = 1;
5 int advice_commit_before_merge = 1;
6 +int advice_resolve_conflict = 1;
7
8 static struct {
9 const char *name;
@@ -11,6 +12,7 @@ static struct {
12 { "pushnonfastforward", &advice_push_nonfastforward },
13 { "statushints", &advice_status_hints },
14 { "commitbeforemerge", &advice_commit_before_merge },
15 + { "resolveconflict", &advice_resolve_conflict },
16 };
17
18 int git_default_advice_config(const char *var, const char *value)
@@ -27,3 +29,17 @@ int git_default_advice_config(const char *var, const char *value)
29
30 return 0;
31 }
32 +
33 +void NORETURN die_resolve_conflict(const char *me)
34 +{
35 + if (advice_resolve_conflict)
36 + /*
37 + * Message used both when 'git commit' fails and when
38 + * other commands doing a merge do.
39 + */
40 + die("'%s' is not possible because you have unmerged files.\n"
41 + "Please, fix them up in the work tree, and then use 'git add/rm <file>' as\n"
42 + "appropriate to mark resolution and make a commit, or use 'git commit -a'.", me);
43 + else
44 + die("'%s' is not possible because you have unmerged files.", me);
45 +}
advice.h
+5
@@ -1,10 +1,15 @@
1 #ifndef ADVICE_H
2 #define ADVICE_H
3
4 +#include "git-compat-util.h"
5 +
6 extern int advice_push_nonfastforward;
7 extern int advice_status_hints;
8 extern int advice_commit_before_merge;
9 +extern int advice_resolve_conflict;
10
11 int git_default_advice_config(const char *var, const char *value);
12
13 +extern void NORETURN die_resolve_conflict(const char *me);
14 +
15 #endif /* ADVICE_H */
builtin-commit.c
+12 -2
@@ -219,6 +219,16 @@ static void create_base_index(void)
219 exit(128); /* We've already reported the error, finish dying */
220 }
221
222 +static void refresh_cache_or_die(int refresh_flags)
223 +{
224 + /*
225 + * refresh_flags contains REFRESH_QUIET, so the only errors
226 + * are for unmerged entries.
227 + */
228 + if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))
229 + die_resolve_conflict("commit");
230 +}
231 +
232 static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)
233 {
234 int fd;
@@ -258,7 +268,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
268 if (all || (also && pathspec && *pathspec)) {
269 int fd = hold_locked_index(&index_lock, 1);
270 add_files_to_cache(also ? prefix : NULL, pathspec, 0);
261 - refresh_cache(refresh_flags);
271 + refresh_cache_or_die(refresh_flags);
272 if (write_cache(fd, active_cache, active_nr) ||
273 close_lock_file(&index_lock))
274 die("unable to write new_index file");
@@ -277,7 +287,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
287 */
288 if (!pathspec || !*pathspec) {
289 fd = hold_locked_index(&index_lock, 1);
280 - refresh_cache(refresh_flags);
290 + refresh_cache_or_die(refresh_flags);
291 if (write_cache(fd, active_cache, active_nr) ||
292 commit_locked_index(&index_lock))
293 die("unable to write new_index file");
builtin-merge.c
+14 -5
@@ -847,11 +847,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
847 const char *best_strategy = NULL, *wt_strategy = NULL;
848 struct commit_list **remotes = &remoteheads;
849
850 - if (file_exists(git_path("MERGE_HEAD")))
851 - die("You have not concluded your merge. (MERGE_HEAD exists)");
852 - if (read_cache_unmerged())
853 - die("You are in the middle of a conflicted merge."
854 - " (index unmerged)");
850 + if (read_cache_unmerged()) {
851 + die_resolve_conflict("merge");
852 + }
853 + if (file_exists(git_path("MERGE_HEAD"))) {
854 + /*
855 + * There is no unmerged entry, don't advise 'git
856 + * add/rm <file>', just 'git commit'.
857 + */
858 + if (advice_resolve_conflict)
859 + die("You have not concluded your merge (MERGE_HEAD exists).\n"
860 + "Please, commit your changes before you can merge.");
861 + else
862 + die("You have not concluded your merge (MERGE_HEAD exists).");
863 + }
864
865 /*
866 * Check if we are _not_ on a detached HEAD, i.e. if there is a
builtin-revert.c
+14 -1
@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)
233 return tree;
234 }
235
236 +static NORETURN void die_dirty_index(const char *me)
237 +{
238 + if (read_cache_unmerged()) {
239 + die_resolve_conflict(me);
240 + } else {
241 + if (advice_commit_before_merge)
242 + die("Your local changes would be overwritten by %s.\n"
243 + "Please, commit your changes or stash them to proceed.", me);
244 + else
245 + die("Your local changes would be overwritten by %s.\n", me);
246 + }
247 +}
248 +
249 static int revert_or_cherry_pick(int argc, const char **argv)
250 {
251 unsigned char head[20];
@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
282 if (get_sha1("HEAD", head))
283 die ("You do not have a valid HEAD");
284 if (index_differs_from("HEAD", 0))
272 - die ("Dirty index: cannot %s", me);
285 + die_dirty_index(me);
286 }
287 discard_cache();
288
git-pull.sh
+23 -2
@@ -13,8 +13,29 @@ set_reflog_action "pull $*"
13 require_work_tree
14 cd_to_toplevel
15
16 -test -z "$(git ls-files -u)" ||
17 - die "You are in the middle of a conflicted merge."
16 +
17 +die_conflict () {
18 + git diff-index --cached --name-status -r --ignore-submodules HEAD --
19 + if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
20 + die "Pull is not possible because you have unmerged files.
21 +Please, fix them up in the work tree, and then use 'git add/rm <file>'
22 +as appropriate to mark resolution, or use 'git commit -a'."
23 + else
24 + die "Pull is not possible because you have unmerged files."
25 + fi
26 +}
27 +
28 +die_merge () {
29 + if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
30 + die "You have not concluded your merge (MERGE_HEAD exists).
31 +Please, commit your changes before you can merge."
32 + else
33 + die "You have not concluded your merge (MERGE_HEAD exists)."
34 + fi
35 +}
36 +
37 +test -z "$(git ls-files -u)" || die_conflict
38 +test -f "$GIT_DIR/MERGE_HEAD" && die_merge
39
40 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
41 log_arg= verbosity=
t/t3030-merge-recursive.sh
+4 -2
@@ -276,11 +276,13 @@ test_expect_success 'fail if the index has unresolved entries' '
276
277 test_must_fail git merge "$c5" &&
278 test_must_fail git merge "$c5" 2> out &&
279 + grep "not possible because you have unmerged files" out &&
280 + git add -u &&
281 + test_must_fail git merge "$c5" 2> out &&
282 grep "You have not concluded your merge" out &&
283 rm -f .git/MERGE_HEAD &&
284 test_must_fail git merge "$c5" 2> out &&
282 - grep "You are in the middle of a conflicted merge" out
283 -
285 + grep "Your local changes to .* would be overwritten by merge." out
286 '
287
288 test_expect_success 'merge-recursive remove conflict' '
t/t3501-revert-cherry-pick.sh
+1 -1
@@ -66,7 +66,7 @@ test_expect_success 'revert forbidden on dirty working tree' '
66 echo content >extra_file &&
67 git add extra_file &&
68 test_must_fail git revert HEAD 2>errors &&
69 - grep "Dirty index" errors
69 + grep "Your local changes would be overwritten by " errors
70
71 '
72