Skip to content

Commit

Permalink
Be more user-friendly when refusing to do something because of conflict.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Matthieu Moy authored and Junio C Hamano committed Jan 12, 2010
1 parent 902f235 commit d38a30d
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 13 deletions.
4 changes: 4 additions & 0 deletions Documentation/config.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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::
Expand Down
16 changes: 16 additions & 0 deletions advice.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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 changes: 5 additions & 0 deletions advice.h
Original file line number Diff line number Diff line change
@@ -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 */
14 changes: 12 additions & 2 deletions builtin-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down
19 changes: 14 additions & 5 deletions builtin-merge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 14 additions & 1 deletion builtin-revert.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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();

Expand Down
25 changes: 23 additions & 2 deletions git-pull.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
6 changes: 4 additions & 2 deletions t/t3030-merge-recursive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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' '
Expand Down
2 changes: 1 addition & 1 deletion t/t3501-revert-cherry-pick.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
'

Expand Down

0 comments on commit d38a30d

Please sign in to comment.