Skip to content

Commit d38a30d

Browse files
moygitster
moy
authored and
gitster
committed
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>
1 parent 902f235 commit d38a30d

File tree

9 files changed

+93
-13
lines changed

9 files changed

+93
-13
lines changed

‎Documentation/config.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ advice.*::
130130
Advice shown when linkgit:git-merge[1] refuses to
131131
merge to avoid overwritting local changes.
132132
Default: true.
133+
resolveConflict::
134+
Advices shown by various commands when conflicts
135+
prevent the operation from being performed.
136+
Default: true.
133137
--
134138

135139
core.fileMode::

‎advice.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
int advice_push_nonfastforward = 1;
44
int advice_status_hints = 1;
55
int advice_commit_before_merge = 1;
6+
int advice_resolve_conflict = 1;
67

78
static struct {
89
const char *name;
@@ -11,6 +12,7 @@ static struct {
1112
{ "pushnonfastforward", &advice_push_nonfastforward },
1213
{ "statushints", &advice_status_hints },
1314
{ "commitbeforemerge", &advice_commit_before_merge },
15+
{ "resolveconflict", &advice_resolve_conflict },
1416
};
1517

1618
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)
2729

2830
return 0;
2931
}
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‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
#ifndef ADVICE_H
22
#define ADVICE_H
33

4+
#include "git-compat-util.h"
5+
46
extern int advice_push_nonfastforward;
57
extern int advice_status_hints;
68
extern int advice_commit_before_merge;
9+
extern int advice_resolve_conflict;
710

811
int git_default_advice_config(const char *var, const char *value);
912

13+
extern void NORETURN die_resolve_conflict(const char *me);
14+
1015
#endif /* ADVICE_H */

‎builtin-commit.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,16 @@ static void create_base_index(void)
219219
exit(128); /* We've already reported the error, finish dying */
220220
}
221221

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+
222232
static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)
223233
{
224234
int fd;
@@ -258,7 +268,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
258268
if (all || (also && pathspec && *pathspec)) {
259269
int fd = hold_locked_index(&index_lock, 1);
260270
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
261-
refresh_cache(refresh_flags);
271+
refresh_cache_or_die(refresh_flags);
262272
if (write_cache(fd, active_cache, active_nr) ||
263273
close_lock_file(&index_lock))
264274
die("unable to write new_index file");
@@ -277,7 +287,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
277287
*/
278288
if (!pathspec || !*pathspec) {
279289
fd = hold_locked_index(&index_lock, 1);
280-
refresh_cache(refresh_flags);
290+
refresh_cache_or_die(refresh_flags);
281291
if (write_cache(fd, active_cache, active_nr) ||
282292
commit_locked_index(&index_lock))
283293
die("unable to write new_index file");

‎builtin-merge.c‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -847,11 +847,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
847847
const char *best_strategy = NULL, *wt_strategy = NULL;
848848
struct commit_list **remotes = &remoteheads;
849849

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+
}
855864

856865
/*
857866
* Check if we are _not_ on a detached HEAD, i.e. if there is a

‎builtin-revert.c‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)
233233
return tree;
234234
}
235235

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+
236249
static int revert_or_cherry_pick(int argc, const char **argv)
237250
{
238251
unsigned char head[20];
@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
269282
if (get_sha1("HEAD", head))
270283
die ("You do not have a valid HEAD");
271284
if (index_differs_from("HEAD", 0))
272-
die ("Dirty index: cannot %s", me);
285+
die_dirty_index(me);
273286
}
274287
discard_cache();
275288

‎git-pull.sh‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,29 @@ set_reflog_action "pull $*"
1313
require_work_tree
1414
cd_to_toplevel
1515

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
1839

1940
strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
2041
log_arg= verbosity=

‎t/t3030-merge-recursive.sh‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,11 +276,13 @@ test_expect_success 'fail if the index has unresolved entries' '
276276
277277
test_must_fail git merge "$c5" &&
278278
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 &&
279282
grep "You have not concluded your merge" out &&
280283
rm -f .git/MERGE_HEAD &&
281284
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
284286
'
285287

286288
test_expect_success 'merge-recursive remove conflict' '

‎t/t3501-revert-cherry-pick.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ test_expect_success 'revert forbidden on dirty working tree' '
6666
echo content >extra_file &&
6767
git add extra_file &&
6868
test_must_fail git revert HEAD 2>errors &&
69-
grep "Dirty index" errors
69+
grep "Your local changes would be overwritten by " errors
7070
7171
'
7272

0 commit comments

Comments
 (0)