threads / patch / 23004

patchstash: dont save during a conflicted merge

Subject: [PATCH] stash: dont save during a conflicted merge

## tl;dr

3 messages between Mar 13, 2010 and Mar 16, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Dave Olszewski· Mar 13, 2010, 03:40 UTC · lore

Similar to commit c8c562a, if a user is resolving conflicts, they may think it wise to stash their current work tree and git pull to see if there are additional changes on the remote.

The stash will fail to save if the index contains unmerged entries, but if the conflicts are resolved, the stash will succeed, and both MERGE_HEAD and MERGE_MSG will be removed. This is probably a mistake, and we should warn the user and refuse to stash.

Signed-off-by: Dave Olszewski <cxreg@pobox.com>
---
 git-stash.sh     |    5 +++++
 t/t3903-stash.sh |   19 +++++++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)
Show changes to 2 files +24 −0

git-stash.sh, t/t3903-stash.sh

diff --git a/git-stash.sh b/git-stash.sh
index aa47e54..1a70f8d 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -172,6 +172,11 @@ save_stash () {
 	test -f "$GIT_DIR/logs/$ref_stash" ||
 		clear_stash || die "Cannot initialize stash"
 
+	if test -f "$GIT_DIR/MERGE_HEAD"
+	then
+		die "You have not concluded your merge. (MERGE_HEAD exists)";
+	fi
+
 	create_stash "$stash_msg"
 
 	# Make sure the reflog for stash is kept.
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 476e5ec..9915f4f 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -228,4 +228,23 @@ test_expect_success 'stash --invalid-option' '
 	test bar,bar2 = $(cat file),$(cat file2)
 '
 
+test_expect_success 'stash during merge' '
+	git branch other &&
+	git checkout master &&
+	echo conflict > conflict &&
+	git add conflict &&
+	git commit -m "conflict" &&
+	git checkout other &&
+	echo other content > conflict &&
+	git add conflict &&
+	git commit -m "other branch conflict" &&
+	git checkout master &&
+	test_must_fail git merge other &&
+	test_must_fail git stash &&
+	git add . &&
+	git status &&
+	test_must_fail git stash &&
+	git reset --hard
+'
+
 test_done

-- 
1.7.0.2.200.ga611.dirty
Junio C Hamano· Mar 15, 2010, 22:14 UTC · re: Dave Olszewski · lore

Re: [PATCH] stash: dont save during a conflicted merge

Dave Olszewski <cxreg@pobox.com> writes:
Show 8 quoted lines
> Similar to commit c8c562a, if a user is resolving conflicts, they may
> think it wise to stash their current work tree and git pull to see if
> there are additional changes on the remote.
>
> The stash will fail to save if the index contains unmerged entries, but
> if the conflicts are resolved, the stash will succeed, and both
> MERGE_HEAD and MERGE_MSG will be removed.  This is probably a mistake,
> and we should warn the user and refuse to stash.
Warning is probably Ok, but refusing with die() might be too much.

When trying a topic with more than one integration branches (think "master", "next, "pu"), and the merge is a bit too hairy that I am not very confident with the resolution, I've deliberately used stash to record a tentative conflict resolution to avoid contaminating my rerere database:

    $ git merge topic
    ... heavy conflicts, manually "resolved" to a dubious result ...
    $ git rerere clear
    $ git stash save "tentative merge of topic"
    $ git stash apply
    ... test test test ...
    $ git reset --hard
    $ git checkout another-integration-branch
    $ git stash apply
    ... test test test ...
    ... repeat the above for other integration branches ...
This is using the stash as a glorified form of
    $ git diff HEAD >./+save-tentative-merge
and then applying it to other integration branches to test out
    $ git reset --hard
    $ git checkout another-integration-branch
    $ git apply ./+save-tentative-merge

but it actually is better than diff/apply because stash application uses a real three-way merge.

So I am not entirely happy with this feature-removal.
Dave Olszewski· Mar 16, 2010, 03:05 UTC · re: Junio C Hamano · lore

Re: Re: [PATCH] stash: dont save during a conflicted merge

On Mon, 15 Mar 2010, Junio C Hamano wrote:
Show 44 quoted lines
> Dave Olszewski <cxreg@pobox.com> writes:
>
>> Similar to commit c8c562a, if a user is resolving conflicts, they may
>> think it wise to stash their current work tree and git pull to see if
>> there are additional changes on the remote.
>>
>> The stash will fail to save if the index contains unmerged entries, but
>> if the conflicts are resolved, the stash will succeed, and both
>> MERGE_HEAD and MERGE_MSG will be removed.  This is probably a mistake,
>> and we should warn the user and refuse to stash.
>
> Warning is probably Ok, but refusing with die() might be too much.
>
> When trying a topic with more than one integration branches (think
> "master", "next, "pu"), and the merge is a bit too hairy that I am not
> very confident with the resolution, I've deliberately used stash to record
> a tentative conflict resolution to avoid contaminating my rerere database:
>
>    $ git merge topic
>    ... heavy conflicts, manually "resolved" to a dubious result ...
>    $ git rerere clear
>    $ git stash save "tentative merge of topic"
>    $ git stash apply
>    ... test test test ...
>    $ git reset --hard
>    $ git checkout another-integration-branch
>    $ git stash apply
>    ... test test test ...
>    ... repeat the above for other integration branches ...
>
> This is using the stash as a glorified form of
>
>    $ git diff HEAD >./+save-tentative-merge
>
> and then applying it to other integration branches to test out
>
>    $ git reset --hard
>    $ git checkout another-integration-branch
>    $ git apply ./+save-tentative-merge
>
> but it actually is better than diff/apply because stash application uses a
> real three-way merge.
>
> So I am not entirely happy with this feature-removal.

This is an interesting use-case. If you determine that your resolution is satisfactory, how then do you complete your merge? You can't apply a stash on a dirty index, and the MERGE_* files are gone. It seems like using this workflow to "pause" and resume a merge is difficult, although it's exactly the thing that led to this patch in the first place. Maybe git-stash could hold onto those files somehow if they exist when saving?

← back to recent threads