Re: [PATCH] mergetool: Provide an empty file when no base exists

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] mergetool: Provide an empty file when no base exists

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:50

David Aguilar [off-list ref] writes:

quoted

     local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
     remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"

Sorry to be ping-pong-ing like this, but wouldn't we have a similar issue
when LOCAL or REMOTE does not exist (e.g. "they modified, we removed")?

Yes.  I'll have a [PATCH v3] soon.

It almost makes me wonder if the obviously simplest change to make
checkout_staged_file create an empty file when the asked-for stage does
not exist, i.e.

	checkout_stage ()
        {
		if test -n "$1"
                then
			... whatever checkout_staged_file() does ...
		else
			>"$4"
                fi
        }

	checkout_stage $base_mode 1 "$MERGED" "$BASE"
	checkout_stage $local_mode 2 "$MERGED" "$LOCAL"
	checkout_stage $remote_mode 3 "$MERGED" "$REMOTE"

Or even:

	checkout_stage ()
	{
		tmpfile=$(...)
                if test $? -eq 0 -a ...
                then
                	mv -- "$(git rev-parse ...)"
		else
			>"$3"
		fi
        }

	checkout_stage 1 "$MERGED" "$BASE"
	checkout_stage 2 "$MERGED" "$LOCAL"
	checkout_stage 3 "$MERGED" "$REMOTE"

as the current checkout_staged_file() does not seem to check errors in any
meaningful way.

[PATCH v3] mergetool: Provide an empty file when needed

From: David Aguilar <hidden>
Date: 2016-06-15 22:52:50

Some merge tools cannot cope when $LOCAL, $BASE, or $REMOTE
are missing.  $BASE can be missing when two branches
independently add the same filename.  $LOCAL and $REMOTE
can be missing when a delete/modify conflict occurs.

Provide an empty file to make these tools happy.

Reported-by: Jason Wenger <redacted>
Signed-off-by: David Aguilar <redacted>
---
We now create the empty file in checkout_staged_file()
as part of the error checking section.

 git-mergetool.sh     |    8 +++++---
 t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..24bedc5 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh

@@ -185,6 +185,8 @@ checkout_staged_file () {
 
     if test $? -eq 0 -a -n "$tmpfile" ; then
 	mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
+    else
+	>"$3"
     fi
 }

@@ -224,9 +226,9 @@ merge_file () {
     mv -- "$MERGED" "$BACKUP"
     cp -- "$BACKUP" "$MERGED"
 
-    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
-    local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
-    remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
+    checkout_staged_file 1 "$MERGED" "$BASE"
+    checkout_staged_file 2 "$MERGED" "$LOCAL"
+    checkout_staged_file 3 "$MERGED" "$REMOTE"
 
     if test -z "$local_mode" -o -z "$remote_mode"; then
 	echo "Deleted merge conflict for '$MERGED':"

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 4aab2a7..2272743 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh

@@ -39,6 +39,7 @@ test_expect_success 'setup' '
     echo branch1 change >file1 &&
     echo branch1 newfile >file2 &&
     echo branch1 spaced >"spaced name" &&
+    echo branch1 both added > both &&
     echo branch1 change file11 >file11 &&
     echo branch1 change file13 >file13 &&
     echo branch1 sub >subdir/file3 &&

@@ -50,6 +51,7 @@ test_expect_success 'setup' '
 	git checkout -b submod-branch1
     ) &&
     git add file1 "spaced name" file11 file13 file2 subdir/file3 submod &&
+    git add both &&
     git rm file12 &&
     git commit -m "branch1 changes" &&

@@ -58,6 +60,7 @@ test_expect_success 'setup' '
     echo master updated >file1 &&
     echo master new >file2 &&
     echo master updated spaced >"spaced name" &&
+    echo master both added > both &&
     echo master updated file12 >file12 &&
     echo master updated file14 >file14 &&
     echo master new sub >subdir/file3 &&

@@ -69,18 +72,22 @@ test_expect_success 'setup' '
 	git checkout -b submod-master
     ) &&
     git add file1 "spaced name" file12 file14 file2 subdir/file3 submod &&
+    git add both &&
     git rm file11 &&
     git commit -m "master updates" &&
 
     git config merge.tool mytool &&
     git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
-    git config mergetool.mytool.trustExitCode true
+    git config mergetool.mytool.trustExitCode true &&
+    git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
+    git config mergetool.mybase.trustExitCode true
 '
 
 test_expect_success 'custom mergetool' '
     git checkout -b test1 branch1 &&
     git submodule update -N &&
     test_must_fail git merge master >/dev/null 2>&1 &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "" | git mergetool file1 file1 ) &&
     ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
     ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&

@@ -101,6 +108,7 @@ test_expect_success 'mergetool crlf' '
     ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
     ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
     ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&

@@ -131,6 +139,7 @@ test_expect_success 'mergetool on file in parent dir' '
 	cd subdir &&
 	( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
 	( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
+	( yes "" | git mergetool ../both >/dev/null 2>&1 ) &&
 	( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) &&
 	( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) &&
 	( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) &&

@@ -212,6 +221,7 @@ test_expect_success 'deleted vs modified submodule' '
     test_must_fail git merge master &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "r" | git mergetool submod ) &&
     rmdir submod && mv submod-movedaside submod &&

@@ -228,6 +238,7 @@ test_expect_success 'deleted vs modified submodule' '
     test_must_fail git merge master &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "l" | git mergetool submod ) &&
     test ! -e submod &&

@@ -241,6 +252,7 @@ test_expect_success 'deleted vs modified submodule' '
     test_must_fail git merge test6 &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "r" | git mergetool submod ) &&
     test ! -e submod &&

@@ -256,6 +268,7 @@ test_expect_success 'deleted vs modified submodule' '
     test_must_fail git merge test6 &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "l" | git mergetool submod ) &&
     test "$(cat submod/bar)" = "master submodule" &&

@@ -279,6 +292,7 @@ test_expect_success 'file vs modified submodule' '
     test_must_fail git merge master &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "r" | git mergetool submod ) &&
     rmdir submod && mv submod-movedaside submod &&

@@ -294,6 +308,7 @@ test_expect_success 'file vs modified submodule' '
     test_must_fail git merge master &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "l" | git mergetool submod ) &&
     git submodule update -N &&

@@ -309,6 +324,7 @@ test_expect_success 'file vs modified submodule' '
     test_must_fail git merge test7 &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "r" | git mergetool submod ) &&
     test -d submod.orig &&

@@ -324,6 +340,7 @@ test_expect_success 'file vs modified submodule' '
     test_must_fail git merge test7 &&
     test -n "$(git ls-files -u)" &&
     ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both>/dev/null 2>&1 ) &&
     ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
     ( yes "l" | git mergetool submod ) &&
     test "$(cat submod/bar)" = "master submodule" &&

@@ -445,4 +462,12 @@ test_expect_success 'directory vs modified submodule' '
     git submodule update -N
 '
 
+test_expect_success 'file with no base' '
+    git checkout -b test13 branch1 &&
+    test_must_fail git merge master &&
+    git mergetool --no-prompt --tool mybase -- base &&
+    test "$(cat "$MERGED")" = "" &&
+    git reset --hard master >/dev/null 2>&1
+'
+
 test_done

-- 
1.7.9.rc2.1.g36b4c

Re: [PATCH v3] mergetool: Provide an empty file when needed

From: David Aguilar <hidden>
Date: 2016-06-15 22:52:50

On Thu, Jan 19, 2012 at 11:47 PM, David Aguilar [off-list ref] wrote:

Some merge tools cannot cope when $LOCAL, $BASE, or $REMOTE
are missing.  $BASE can be missing when two branches
independently add the same filename.  $LOCAL and $REMOTE
can be missing when a delete/modify conflict occurs.

Provide an empty file to make these tools happy.

Reported-by: Jason Wenger <redacted>
Signed-off-by: David Aguilar <redacted>
---
We now create the empty file in checkout_staged_file()
as part of the error checking section.

 git-mergetool.sh     |    8 +++++---
 t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)


I certainly like this version the best.  This diff is smaller and it's
all handled in one place.  It should've been marked PATCH v4 ;-)

Thanks for the review.

quoted hunk

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..24bedc5 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh

@@ -185,6 +185,8 @@ checkout_staged_file () {

    if test $? -eq 0 -a -n "$tmpfile" ; then
       mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
+    else
+       >"$3"
    fi
 }

@@ -224,9 +226,9 @@ merge_file () {

    mv -- "$MERGED" "$BACKUP"
    cp -- "$BACKUP" "$MERGED"

-    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
-    local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
-    remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
+    checkout_staged_file 1 "$MERGED" "$BASE"
+    checkout_staged_file 2 "$MERGED" "$LOCAL"
+    checkout_staged_file 3 "$MERGED" "$REMOTE"

    if test -z "$local_mode" -o -z "$remote_mode"; then
       echo "Deleted merge conflict for '$MERGED':"

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 4aab2a7..2272743 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh

@@ -39,6 +39,7 @@ test_expect_success 'setup' '

    echo branch1 change >file1 &&
    echo branch1 newfile >file2 &&
    echo branch1 spaced >"spaced name" &&
+    echo branch1 both added > both &&
    echo branch1 change file11 >file11 &&
    echo branch1 change file13 >file13 &&
    echo branch1 sub >subdir/file3 &&

@@ -50,6 +51,7 @@ test_expect_success 'setup' '

       git checkout -b submod-branch1
    ) &&
    git add file1 "spaced name" file11 file13 file2 subdir/file3 submod &&
+    git add both &&
    git rm file12 &&
    git commit -m "branch1 changes" &&

@@ -58,6 +60,7 @@ test_expect_success 'setup' '

    echo master updated >file1 &&
    echo master new >file2 &&
    echo master updated spaced >"spaced name" &&
+    echo master both added > both &&
    echo master updated file12 >file12 &&
    echo master updated file14 >file14 &&
    echo master new sub >subdir/file3 &&

@@ -69,18 +72,22 @@ test_expect_success 'setup' '

       git checkout -b submod-master
    ) &&
    git add file1 "spaced name" file12 file14 file2 subdir/file3 submod &&
+    git add both &&
    git rm file11 &&
    git commit -m "master updates" &&

    git config merge.tool mytool &&
    git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
-    git config mergetool.mytool.trustExitCode true
+    git config mergetool.mytool.trustExitCode true &&
+    git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
+    git config mergetool.mybase.trustExitCode true
 '

 test_expect_success 'custom mergetool' '
    git checkout -b test1 branch1 &&
    git submodule update -N &&
    test_must_fail git merge master >/dev/null 2>&1 &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "" | git mergetool file1 file1 ) &&
    ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
    ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&

@@ -101,6 +108,7 @@ test_expect_success 'mergetool crlf' '

    ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
    ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
    ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&

@@ -131,6 +139,7 @@ test_expect_success 'mergetool on file in parent dir' '

       cd subdir &&
       ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
       ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
+       ( yes "" | git mergetool ../both >/dev/null 2>&1 ) &&
       ( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) &&
       ( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) &&
       ( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) &&

@@ -212,6 +221,7 @@ test_expect_success 'deleted vs modified submodule' '

    test_must_fail git merge master &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "r" | git mergetool submod ) &&
    rmdir submod && mv submod-movedaside submod &&

@@ -228,6 +238,7 @@ test_expect_success 'deleted vs modified submodule' '

    test_must_fail git merge master &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "l" | git mergetool submod ) &&
    test ! -e submod &&

@@ -241,6 +252,7 @@ test_expect_success 'deleted vs modified submodule' '

    test_must_fail git merge test6 &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "r" | git mergetool submod ) &&
    test ! -e submod &&

@@ -256,6 +268,7 @@ test_expect_success 'deleted vs modified submodule' '

    test_must_fail git merge test6 &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "l" | git mergetool submod ) &&
    test "$(cat submod/bar)" = "master submodule" &&

@@ -279,6 +292,7 @@ test_expect_success 'file vs modified submodule' '

    test_must_fail git merge master &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "r" | git mergetool submod ) &&
    rmdir submod && mv submod-movedaside submod &&

@@ -294,6 +308,7 @@ test_expect_success 'file vs modified submodule' '

    test_must_fail git merge master &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "l" | git mergetool submod ) &&
    git submodule update -N &&

@@ -309,6 +324,7 @@ test_expect_success 'file vs modified submodule' '

    test_must_fail git merge test7 &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "r" | git mergetool submod ) &&
    test -d submod.orig &&

@@ -324,6 +340,7 @@ test_expect_success 'file vs modified submodule' '

    test_must_fail git merge test7 &&
    test -n "$(git ls-files -u)" &&
    ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+    ( yes "" | git mergetool both>/dev/null 2>&1 ) &&
    ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
    ( yes "l" | git mergetool submod ) &&
    test "$(cat submod/bar)" = "master submodule" &&

@@ -445,4 +462,12 @@ test_expect_success 'directory vs modified submodule' '

    git submodule update -N
 '

+test_expect_success 'file with no base' '
+    git checkout -b test13 branch1 &&
+    test_must_fail git merge master &&
+    git mergetool --no-prompt --tool mybase -- base &&
+    test "$(cat "$MERGED")" = "" &&
+    git reset --hard master >/dev/null 2>&1
+'
+
 test_done
--
1.7.9.rc2.1.g36b4c



-- 
            David

Re: [PATCH v3] mergetool: Provide an empty file when needed

From: Jason Wenger <hidden>
Date: 2016-06-15 22:52:50

On Fri, Jan 20, 2012 at 01:53, David Aguilar [off-list ref] wrote:

On Thu, Jan 19, 2012 at 11:47 PM, David Aguilar [off-list ref] wrote:

quoted

Some merge tools cannot cope when $LOCAL, $BASE, or $REMOTE
are missing.  $BASE can be missing when two branches
independently add the same filename.  $LOCAL and $REMOTE
can be missing when a delete/modify conflict occurs.

Provide an empty file to make these tools happy.

This is cleaner, yes -- but is this extra processing on $LOCAL and
$REMOTE necessary?  Git mergetool doesn't actually call an external
mergetool during del/mod conflicts -- instead it goes into an
alternate processing and prompts the user interactively whether to
take the deleted or modified file.  Can these changes be reached?
(command line option I'm not aware of?)

--jcw

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help