[PATCH] refuse to merge during a merge

The following is an easy mistake to make for users coming from version
control systems with an "update and commit"-style workflow.

	1. git merge
	2. resolve conflicts
	3. git pull, instead of commit

This overrides MERGE_HEAD, starting a new merge with dirty index. IOW,
probably not what the user intented. Instead, refuse to merge again if a
merge is in progress.

Reported-by: Dave Olszewski <redacted>
Signed-off-by: Clemens Buchacher <redacted>
---

 builtin-merge.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 0b58e5e..74a8c8f 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c

@@ -836,7 +836,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	struct commit_list **remotes = &remoteheads;
 
 	setup_work_tree();
-	if (read_cache_unmerged())
+	if (read_cache_unmerged() || file_exists(git_path("MERGE_HEAD")))
 		die("You are in the middle of a conflicted merge.");
 
 	/*

-- 
1.6.3.1.147.g637c3

MERGE_HEAD file could also happen in case of --no-commit option. In
that case there might be no conflict and the message would look
confusing to the user. I suggest to change a message to "You are in
the middle of a uncommitted or conflicted merge." or something like
it.

Regards,
Constantine

On Thu, May 28, 2009 at 1:04 AM, Clemens Buchacher [off-list ref] wrote:

+       if (read_cache_unmerged() || file_exists(git_path("MERGE_HEAD")))
               die("You are in the middle of a conflicted merge.");

Could the error message also give possible solutions?   "Commit the
current merge first with 'git commit', or discard the current merge
attempt with 'git reset --hard'" or something.  Or at least a pointer
to where to read for more info.

John

On Thu, May 28, 2009 at 05:12:40PM +0100, John Tapsell wrote:

quoted

+       if (read_cache_unmerged() || file_exists(git_path("MERGE_HEAD")))
               die("You are in the middle of a conflicted merge.");

Could the error message also give possible solutions?   "Commit the
current merge first with 'git commit', or discard the current merge
attempt with 'git reset --hard'" or something.  Or at least a pointer
to where to read for more info.

How about this.

fatal: You are in the middle of a [conflicted] merge. To complete the merge
[resolve conflicts and] commit the changes. To abort, use "git reset HEAD".

The part about resolving changes is only displayed if there are unmerged
entries. I intentionally left out --hard, because it potentially removes
changes unrelated to the merge (if the work tree was dirty prior to the
merge). The user will find out how to reset the work tree by reading the
docs.

Clemens

Clemens Buchacher [off-list ref] writes:

On Thu, May 28, 2009 at 05:12:40PM +0100, John Tapsell wrote:

quoted

quoted

+       if (read_cache_unmerged() || file_exists(git_path("MERGE_HEAD")))
               die("You are in the middle of a conflicted merge.");

Could the error message also give possible solutions?   "Commit the
current merge first with 'git commit', or discard the current merge
attempt with 'git reset --hard'" or something.  Or at least a pointer
to where to read for more info.

How about this.

fatal: You are in the middle of a [conflicted] merge. To complete the merge
[resolve conflicts and] commit the changes. To abort, use "git reset HEAD".

The part about resolving changes is only displayed if there are unmerged
entries. I intentionally left out --hard, because it potentially removes
changes unrelated to the merge (if the work tree was dirty prior to the
merge). The user will find out how to reset the work tree by reading the
docs.

Why not advertise new "git reset --merge HEAD" then?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Jakub Narebski wrote:

Clemens Buchacher [off-list ref] writes:

quoted

fatal: You are in the middle of a [conflicted] merge. To complete the merge
[resolve conflicts and] commit the changes. To abort, use "git reset HEAD".

The part about resolving changes is only displayed if there are unmerged
entries. I intentionally left out --hard, because it potentially removes
changes unrelated to the merge (if the work tree was dirty prior to the
merge). The user will find out how to reset the work tree by reading the
docs.

Why not advertise new "git reset --merge HEAD" then?

That doesn't deal with conflicts at all.  It fills the rather
different case where you did a clean merge with some uncommitted
changes in the worktree, but then want to discard the merge again
without losing the uncommitted changes.  In absence of the changes,
you would just use --hard, but here you want to move the branch tip
while merging them over, similar to what 'git checkout -m' does for
moving HEAD.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

The following is an easy mistake to make for users coming from version
control systems with an "update and commit"-style workflow.

        1. git pull
        2. resolve conflicts
        3. git pull

Step 3 overrides MERGE_HEAD, starting a new merge with dirty index.
IOW, probably not what the user intented. Instead, refuse to merge
again if a merge is in progress and present the user with his options.

"git reset --hard" is not suggested, because it potentially removes
changes unrelated to the merge (if the work tree was dirty prior to
the merge).

Reported-by: Dave Olszewski <redacted>
Signed-off-by: Clemens Buchacher <redacted>
---

Ok, since I'm not seeing any more objections. Here's the code.

Clemens

 builtin-merge.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 0b58e5e..8169ded 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c

@@ -834,10 +834,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	struct commit_list *common = NULL;
 	const char *best_strategy = NULL, *wt_strategy = NULL;
 	struct commit_list **remotes = &remoteheads;
+	int unmerged;
 
 	setup_work_tree();
-	if (read_cache_unmerged())
-		die("You are in the middle of a conflicted merge.");
+	unmerged = read_cache_unmerged();
+	if (unmerged || file_exists(git_path("MERGE_HEAD")))
+		die("You are in the middle of a %smerge. To complete "
+			"the merge %scommit the changes. To abort, "
+			"use \"git reset HEAD\".",
+			unmerged ? "conflicted " : "",
+			unmerged ? "resolve conflicts and " : "");
 
 	/*
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a

-- 
1.6.3.1.147.g637c3

"git reset --hard" is not suggested, because it potentially removes
changes unrelated to the merge (if the work tree was dirty prior to
the merge).

Sorry could you just clarify...  if the user does "git reset HEAD"
will that sometimes always or never fail?

If it sometimes or always fails, then doesn't it seem kinda confusing
if the user is told to run that command, but then when they do they
get an error?

John

On Sun, May 31, 2009 at 12:05:31PM +0100, John Tapsell wrote:

quoted

"git reset --hard" is not suggested, because it potentially removes
changes unrelated to the merge (if the work tree was dirty prior to
the merge).

Sorry could you just clarify...  if the user does "git reset HEAD"
will that sometimes always or never fail?

It will never fail. It aborts the merge as suggested. But "git reset --hard
HEAD" would also reset the work tree, so that "any changes to tracked files
in the working tree since <commit> are lost." This is generally desireable,
since an incomplete merge also leaves the auto-merged files in the work
tree.

But if the user does not already know that, it's better to leave the user
wondering how to clean a dirty work tree, than to suggest a potentially
harmful operation.

Clemens