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>maint
							parent
							
								
									902f235378
								
							
						
					
					
						commit
						d38a30df7d
					
				|  | @ -130,6 +130,10 @@ advice.*:: | |||
| 		Advice shown when linkgit:git-merge[1] refuses to | ||||
| 		merge to avoid overwritting local changes. | ||||
| 		Default: true. | ||||
| 	resolveConflict:: | ||||
| 		Advices shown by various commands when conflicts | ||||
| 		prevent the operation from being performed. | ||||
| 		Default: true. | ||||
| -- | ||||
|  | ||||
| core.fileMode:: | ||||
|  |  | |||
							
								
								
									
										16
									
								
								advice.c
								
								
								
								
							
							
						
						
									
										16
									
								
								advice.c
								
								
								
								
							|  | @ -3,6 +3,7 @@ | |||
| int advice_push_nonfastforward = 1; | ||||
| int advice_status_hints = 1; | ||||
| int advice_commit_before_merge = 1; | ||||
| int advice_resolve_conflict = 1; | ||||
|  | ||||
| static struct { | ||||
| 	const char *name; | ||||
|  | @ -11,6 +12,7 @@ static struct { | |||
| 	{ "pushnonfastforward", &advice_push_nonfastforward }, | ||||
| 	{ "statushints", &advice_status_hints }, | ||||
| 	{ "commitbeforemerge", &advice_commit_before_merge }, | ||||
| 	{ "resolveconflict", &advice_resolve_conflict }, | ||||
| }; | ||||
|  | ||||
| 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) | |||
|  | ||||
| 	return 0; | ||||
| } | ||||
|  | ||||
| void NORETURN die_resolve_conflict(const char *me) | ||||
| { | ||||
| 	if (advice_resolve_conflict) | ||||
| 		/* | ||||
| 		 * Message used both when 'git commit' fails and when | ||||
| 		 * other commands doing a merge do. | ||||
| 		 */ | ||||
| 		die("'%s' is not possible because you have unmerged files.\n" | ||||
| 		    "Please, fix them up in the work tree, and then use 'git add/rm <file>' as\n" | ||||
| 		    "appropriate to mark resolution and make a commit, or use 'git commit -a'.", me); | ||||
| 	else | ||||
| 		die("'%s' is not possible because you have unmerged files.", me); | ||||
| } | ||||
|  |  | |||
							
								
								
									
										5
									
								
								advice.h
								
								
								
								
							
							
						
						
									
										5
									
								
								advice.h
								
								
								
								
							|  | @ -1,10 +1,15 @@ | |||
| #ifndef ADVICE_H | ||||
| #define ADVICE_H | ||||
|  | ||||
| #include "git-compat-util.h" | ||||
|  | ||||
| extern int advice_push_nonfastforward; | ||||
| extern int advice_status_hints; | ||||
| extern int advice_commit_before_merge; | ||||
| extern int advice_resolve_conflict; | ||||
|  | ||||
| int git_default_advice_config(const char *var, const char *value); | ||||
|  | ||||
| extern void NORETURN die_resolve_conflict(const char *me); | ||||
|  | ||||
| #endif /* ADVICE_H */ | ||||
|  |  | |||
|  | @ -219,6 +219,16 @@ static void create_base_index(void) | |||
| 		exit(128); /* We've already reported the error, finish dying */ | ||||
| } | ||||
|  | ||||
| static void refresh_cache_or_die(int refresh_flags) | ||||
| { | ||||
| 	/* | ||||
| 	 * refresh_flags contains REFRESH_QUIET, so the only errors | ||||
| 	 * are for unmerged entries. | ||||
| 	 */ | ||||
| 	if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) | ||||
| 		die_resolve_conflict("commit"); | ||||
| } | ||||
|  | ||||
| static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status) | ||||
| { | ||||
| 	int fd; | ||||
|  | @ -258,7 +268,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int | |||
| 	if (all || (also && pathspec && *pathspec)) { | ||||
| 		int fd = hold_locked_index(&index_lock, 1); | ||||
| 		add_files_to_cache(also ? prefix : NULL, pathspec, 0); | ||||
| 		refresh_cache(refresh_flags); | ||||
| 		refresh_cache_or_die(refresh_flags); | ||||
| 		if (write_cache(fd, active_cache, active_nr) || | ||||
| 		    close_lock_file(&index_lock)) | ||||
| 			die("unable to write new_index file"); | ||||
|  | @ -277,7 +287,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int | |||
| 	 */ | ||||
| 	if (!pathspec || !*pathspec) { | ||||
| 		fd = hold_locked_index(&index_lock, 1); | ||||
| 		refresh_cache(refresh_flags); | ||||
| 		refresh_cache_or_die(refresh_flags); | ||||
| 		if (write_cache(fd, active_cache, active_nr) || | ||||
| 		    commit_locked_index(&index_lock)) | ||||
| 			die("unable to write new_index file"); | ||||
|  |  | |||
|  | @ -847,11 +847,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix) | |||
| 	const char *best_strategy = NULL, *wt_strategy = NULL; | ||||
| 	struct commit_list **remotes = &remoteheads; | ||||
|  | ||||
| 	if (file_exists(git_path("MERGE_HEAD"))) | ||||
| 		die("You have not concluded your merge. (MERGE_HEAD exists)"); | ||||
| 	if (read_cache_unmerged()) | ||||
| 		die("You are in the middle of a conflicted merge." | ||||
| 				" (index unmerged)"); | ||||
| 	if (read_cache_unmerged()) { | ||||
| 		die_resolve_conflict("merge"); | ||||
| 	} | ||||
| 	if (file_exists(git_path("MERGE_HEAD"))) { | ||||
| 		/* | ||||
| 		 * There is no unmerged entry, don't advise 'git | ||||
| 		 * add/rm <file>', just 'git commit'. | ||||
| 		 */ | ||||
| 		if (advice_resolve_conflict) | ||||
| 			die("You have not concluded your merge (MERGE_HEAD exists).\n" | ||||
| 			    "Please, commit your changes before you can merge."); | ||||
| 		else | ||||
| 			die("You have not concluded your merge (MERGE_HEAD exists)."); | ||||
| 	} | ||||
|  | ||||
| 	/* | ||||
| 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a | ||||
|  |  | |||
|  | @ -233,6 +233,19 @@ static struct tree *empty_tree(void) | |||
| 	return tree; | ||||
| } | ||||
|  | ||||
| static NORETURN void die_dirty_index(const char *me) | ||||
| { | ||||
| 	if (read_cache_unmerged()) { | ||||
| 		die_resolve_conflict(me); | ||||
| 	} else { | ||||
| 		if (advice_commit_before_merge) | ||||
| 			die("Your local changes would be overwritten by %s.\n" | ||||
| 			    "Please, commit your changes or stash them to proceed.", me); | ||||
| 		else | ||||
| 			die("Your local changes would be overwritten by %s.\n", me); | ||||
| 	} | ||||
| } | ||||
|  | ||||
| static int revert_or_cherry_pick(int argc, const char **argv) | ||||
| { | ||||
| 	unsigned char head[20]; | ||||
|  | @ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) | |||
| 		if (get_sha1("HEAD", head)) | ||||
| 			die ("You do not have a valid HEAD"); | ||||
| 		if (index_differs_from("HEAD", 0)) | ||||
| 			die ("Dirty index: cannot %s", me); | ||||
| 			die_dirty_index(me); | ||||
| 	} | ||||
| 	discard_cache(); | ||||
|  | ||||
|  |  | |||
							
								
								
									
										25
									
								
								git-pull.sh
								
								
								
								
							
							
						
						
									
										25
									
								
								git-pull.sh
								
								
								
								
							|  | @ -13,8 +13,29 @@ set_reflog_action "pull $*" | |||
| require_work_tree | ||||
| cd_to_toplevel | ||||
|  | ||||
| test -z "$(git ls-files -u)" || | ||||
| 	die "You are in the middle of a conflicted merge." | ||||
|  | ||||
| die_conflict () { | ||||
|     git diff-index --cached --name-status -r --ignore-submodules HEAD -- | ||||
|     if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then | ||||
| 	die "Pull 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, or use 'git commit -a'." | ||||
|     else | ||||
| 	die "Pull is not possible because you have unmerged files." | ||||
|     fi | ||||
| } | ||||
|  | ||||
| die_merge () { | ||||
|     if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then | ||||
| 	die "You have not concluded your merge (MERGE_HEAD exists). | ||||
| Please, commit your changes before you can merge." | ||||
|     else | ||||
| 	die "You have not concluded your merge (MERGE_HEAD exists)." | ||||
|     fi | ||||
| } | ||||
|  | ||||
| test -z "$(git ls-files -u)" || die_conflict | ||||
| test -f "$GIT_DIR/MERGE_HEAD" && die_merge | ||||
|  | ||||
| strategy_args= diffstat= no_commit= squash= no_ff= ff_only= | ||||
| log_arg= verbosity= | ||||
|  |  | |||
|  | @ -276,11 +276,13 @@ test_expect_success 'fail if the index has unresolved entries' ' | |||
|  | ||||
| 	test_must_fail git merge "$c5" && | ||||
| 	test_must_fail git merge "$c5" 2> out && | ||||
| 	grep "not possible because you have unmerged files" out && | ||||
| 	git add -u && | ||||
| 	test_must_fail git merge "$c5" 2> out && | ||||
| 	grep "You have not concluded your merge" out && | ||||
| 	rm -f .git/MERGE_HEAD && | ||||
| 	test_must_fail git merge "$c5" 2> out && | ||||
| 	grep "You are in the middle of a conflicted merge" out | ||||
|  | ||||
| 	grep "Your local changes to .* would be overwritten by merge." out | ||||
| ' | ||||
|  | ||||
| test_expect_success 'merge-recursive remove conflict' ' | ||||
|  |  | |||
|  | @ -66,7 +66,7 @@ test_expect_success 'revert forbidden on dirty working tree' ' | |||
| 	echo content >extra_file && | ||||
| 	git add extra_file && | ||||
| 	test_must_fail git revert HEAD 2>errors && | ||||
| 	grep "Dirty index" errors | ||||
| 	grep "Your local changes would be overwritten by " errors | ||||
|  | ||||
| ' | ||||
|  | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Matthieu Moy
						Matthieu Moy