Git development
 help / color / mirror / Atom feed
* Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
From: Kirill Smelkov @ 2012-01-20  6:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kirill Smelkov, git
In-Reply-To: <7v8vl3ic6o.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 02:01:51PM -0800, Junio C Hamano wrote:
> Kirill Smelkov <kirr@navytux.spb.ru> writes:
> 
> > Starting from GNU Make 3.82 $(wildcard ...) no longer sorts the result
> > (from NEWS):
> >
> >     * WARNING: Backward-incompatibility!
> >       Wildcards were not documented as returning sorted values, but the results
> >       have been sorted up until this release..  If your makefiles require sorted
> >       results from wildcard expansions, use the $(sort ...)  function to request
> >       it explicitly.
> >
> >     http://repo.or.cz/w/make.git/commitdiff/2a59dc32aaf0681dec569f32a9d7ab88a379d34f
> >
> > so we have to sort tests list or else they are executed in seemingly
> > random order even for -j1.
> 
> I do not necessarily buy your "so we HAVE TO, OR ELSE".
> 
> Even though I can understand "We can sort the list of tests _if_ we do not
> want them executed in seemingly random order when running 'make -j1'", I
> tend to think that *if* is a big one.  Aren't these tests designed not to
> depend on each other anyway?

Yes, they don't depend on each other, but what's the point in not
sorting them? I usually watch test progress visually, and if tests are
sorted, even with make -j4 they go more or less incrementally by their t
number.

On my netbook, adding $(sort ...) adds approximately 0.008s to make
startup, so imho there is no performance penalty to adding that sort.


Thanks,
Kirill

^ permalink raw reply

* Re: Interesting behavior in git mergetool with no BASE revision
From: David Aguilar @ 2012-01-20  6:43 UTC (permalink / raw)
  To: Jason Wenger; +Cc: git
In-Reply-To: <CAM6z=4_+yC4EnL9SZFd6=Nxs89QeHevNCakVzVycGBe7G+nTKQ@mail.gmail.com>

On Wed, Jan 18, 2012 at 3:05 PM, Jason Wenger <jcwenger@gmail.com> wrote:
> Doing a git merge on 1.7.4.3, on a case where both branches have a
> file created, and the base does not.  Per git-mergetool:
>
> "the configured command line will be invoked with $BASE set to the
> name of a temporary file containing the common base for the merge, if
> available;"
>
> So testing in this case, I set my mergetool cmd as "echo $MERGED
> $LOCAL $REMOTE $BASE", and I get the following:

I have not seen this in practice.  Maybe escape the variables?

In any case, I added a test case to cover this case.  I think we
should at least provide an empty file in place of a missing $BASE.



> cio/.cproject ./cio/.cproject.LOCAL.9029.cproject
> ./cio/.cproject.REMOTE.9029.cproject
> ./cio/.cproject.BASE.9029.cproject
>
> ls -a cio shows the following files:
>
> .cproject
> .cproject.LOCAL.9325.cproject
> .cproject.BACKUP.9325.cproject
> .cproject.REMOTE.9325.cproject
>
> So the lack of base file makes sense -- There is no base to start
> from.  However, $BASE evaluates to ./cio/.cproject.BASE.9029.cproject,
> which is a nonexistent file.  This makes my actual mergetool upset to
> no end.  Intuitively from documents, I would expect $BASE to evaluate
> to an empty string in this case.
>
> Is this intended behavior?

Not really.  I'll send a patch shortly.
-- 
            David

^ permalink raw reply

* [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20  6:46 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <CAJDDKr5mUiJkNk-urNn5fP5x+gkzaTfx2y=K1S0AJZCy7Muwdg@mail.gmail.com>

Some mergetools cannot cope when $BASE is missing.
This can happen when two branches add the same file.
Provide an empty file to make these tools happy.

Reported-by: Jason Wenger <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool.sh     |    6 +++++-
 t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..8521b81 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -224,7 +224,11 @@ merge_file () {
     mv -- "$MERGED" "$BACKUP"
     cp -- "$BACKUP" "$MERGED"
 
-    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
+    if base_present; then
+	checkout_staged_file 1 "$MERGED" "$BASE"
+    else
+	touch "$BASE"
+    fi
     local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
     remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
 
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.gb566f

^ permalink raw reply related

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20  6:57 UTC (permalink / raw)
  To: gitster; +Cc: git, Jason Wenger
In-Reply-To: <1327042010-79552-1-git-send-email-davvid@gmail.com>

[apologies for forgetting to cc: you on the patch Jason]

On Thu, Jan 19, 2012 at 10:46 PM, David Aguilar <davvid@gmail.com> wrote:
> Some mergetools cannot cope when $BASE is missing.
> This can happen when two branches add the same file.

This might be better worded as "when two branches independently add
the same filename".
Feel free to squash... or I'll resend if you prefer.

Let me know if this does the trick for you, Jason.  The test case is
happy; I hope your mergetool is as well.

Cheers,
David

> Provide an empty file to make these tools happy.
>
> Reported-by: Jason Wenger <jcwenger@gmail.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>  git-mergetool.sh     |    6 +++++-
>  t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
>  2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 085e213..8521b81 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -224,7 +224,11 @@ merge_file () {
>     mv -- "$MERGED" "$BACKUP"
>     cp -- "$BACKUP" "$MERGED"
>
> -    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
> +    if base_present; then
> +       checkout_staged_file 1 "$MERGED" "$BASE"
> +    else
> +       touch "$BASE"
> +    fi
>     local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
>     remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
>
> 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.gb566f
>

^ permalink raw reply

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: Junio C Hamano @ 2012-01-20  7:00 UTC (permalink / raw)
  To: David Aguilar; +Cc: git
In-Reply-To: <1327042010-79552-1-git-send-email-davvid@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> Some mergetools cannot cope when $BASE is missing.
> This can happen when two branches add the same file.
> Provide an empty file to make these tools happy.
>
> Reported-by: Jason Wenger <jcwenger@gmail.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>  git-mergetool.sh     |    6 +++++-
>  t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
>  2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 085e213..8521b81 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -224,7 +224,11 @@ merge_file () {
>      mv -- "$MERGED" "$BACKUP"
>      cp -- "$BACKUP" "$MERGED"
>  
> -    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
> +    if base_present; then
> +	checkout_staged_file 1 "$MERGED" "$BASE"
> +    else
> +	touch "$BASE"
> +    fi

Using "touch" for things like this is a disease.

You not just want to make sure it exists, but also you want to make sure
it is empty, so it would make your intention more explicit and clear if
you wrote this as

	>"$BASE"

instead.

I also wonder if it may help mergetools if we come up with a fake base
image using the common material between the two files, in a way similar to
how git-merge-one-file.sh does it (look for "Added $4 in both, but
differently"), but obviously it would belong to a separate patch.

Thanks.

^ permalink raw reply

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20  7:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhazqhn8u.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 11:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> Some mergetools cannot cope when $BASE is missing.
>> This can happen when two branches add the same file.
>> Provide an empty file to make these tools happy.
>>
>> Reported-by: Jason Wenger <jcwenger@gmail.com>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>>  git-mergetool.sh     |    6 +++++-
>>  t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
>>  2 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-mergetool.sh b/git-mergetool.sh
>> index 085e213..8521b81 100755
>> --- a/git-mergetool.sh
>> +++ b/git-mergetool.sh
>> @@ -224,7 +224,11 @@ merge_file () {
>>      mv -- "$MERGED" "$BACKUP"
>>      cp -- "$BACKUP" "$MERGED"
>>
>> -    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
>> +    if base_present; then
>> +     checkout_staged_file 1 "$MERGED" "$BASE"
>> +    else
>> +     touch "$BASE"
>> +    fi
>
> Using "touch" for things like this is a disease.
>
> You not just want to make sure it exists, but also you want to make sure
> it is empty, so it would make your intention more explicit and clear if
> you wrote this as
>
>        >"$BASE"
>
> instead.
>
> I also wonder if it may help mergetools if we come up with a fake base
> image using the common material between the two files, in a way similar to
> how git-merge-one-file.sh does it (look for "Added $4 in both, but
> differently"), but obviously it would belong to a separate patch.
>
> Thanks.

Ah, thanks for the pointer.  I'll resend shortly.  A "fake base" would
be very helpful.
-- 
            David

^ permalink raw reply

* [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20  7:10 UTC (permalink / raw)
  To: gitster; +Cc: jcwenger, git
In-Reply-To: <7vhazqhn8u.fsf@alter.siamese.dyndns.org>

Some mergetools cannot cope when $BASE is missing.
This can happen when two branches add the same file.
Provide an empty file to make these tools happy.

Reported-by: Jason Wenger <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool.sh     |    6 +++++-
 t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..0131559 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -224,7 +224,11 @@ merge_file () {
     mv -- "$MERGED" "$BACKUP"
     cp -- "$BACKUP" "$MERGED"
 
-    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
+    if base_present; then
+	checkout_staged_file 1 "$MERGED" "$BASE"
+    else
+	:>"$BASE"
+    fi
     local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
     remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
 
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.gdcba7

^ permalink raw reply related

* Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
From: Junio C Hamano @ 2012-01-20  7:14 UTC (permalink / raw)
  To: Kirill Smelkov; +Cc: git
In-Reply-To: <20120120063450.GA15371@mini.zxlink>

Kirill Smelkov <kirr@navytux.spb.ru> writes:

>> I do not necessarily buy your "so we HAVE TO, OR ELSE".
>> 
>> Even though I can understand "We can sort the list of tests _if_ we do not
>> want them executed in seemingly random order when running 'make -j1'", I
>> tend to think that *if* is a big one.  Aren't these tests designed not to
>> depend on each other anyway?
>
> Yes, they don't depend on each other, but what's the point in not
> sorting them? I usually watch test progress visually, and if tests are
> sorted, even with make -j4 they go more or less incrementally by their t
> number.
>
> On my netbook, adding $(sort ...) adds approximately 0.008s to make
> startup, so imho there is no performance penalty to adding that sort.

Heh, who said anything about performance?

I was pointing out that your justification "we HAVE TO" was wrong.

If you are doing this for perceived prettyness and not as a fix for any
correctness issue, I want to see the patch honestly described as such;
that's all.

By the way, if I recall correctly, $(sort) in GNU make not just sorts but
as a nice side effect removes duplicates. So if we used a(n fictional)
construct in our Makefile like this:

    T = $(wildcard *.sh a.*)

that might produce duplicates (i.e. "a.sh" might appear twice), which
might leave us two identical pathnames in $T and cause us trouble.  Even
if we do not have such a use currently, rewriting $(wildcard) like your
patch does using $(sort $(wildcard ...)) may be a good way to future-proof
our Makefile, and if you justify your patch that way, it would be a
possible correctness hardening, not just cosmetics, and phrasing it with
"HAVE TO" may be justifiable.

Care to try if $(wildcard *.sh a.*) give you duplicated output with newer
GNU make? I am lazy but am a bit curious ;-)

^ permalink raw reply

* [PATCH v2] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20  7:14 UTC (permalink / raw)
  To: gitster; +Cc: jcwenger, git
In-Reply-To: <7vhazqhn8u.fsf@alter.siamese.dyndns.org>

Some mergetools cannot cope when $BASE is missing,
for example when two branches independently add the same filename.
Provide an empty file to make these tools happy.

Reported-by: Jason Wenger <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Fixed commit message too.

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

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..0131559 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -224,7 +224,11 @@ merge_file () {
     mv -- "$MERGED" "$BACKUP"
     cp -- "$BACKUP" "$MERGED"
 
-    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
+    if base_present; then
+	checkout_staged_file 1 "$MERGED" "$BASE"
+    else
+	:>"$BASE"
+    fi
     local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
     remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
 
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.gdcba7

^ permalink raw reply related

* Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
From: Kirill Smelkov @ 2012-01-20  7:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kirill Smelkov, git
In-Reply-To: <7vbopyhmlx.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 11:14:18PM -0800, Junio C Hamano wrote:
> Kirill Smelkov <kirr@navytux.spb.ru> writes:
> 
> >> I do not necessarily buy your "so we HAVE TO, OR ELSE".
> >> 
> >> Even though I can understand "We can sort the list of tests _if_ we do not
> >> want them executed in seemingly random order when running 'make -j1'", I
> >> tend to think that *if* is a big one.  Aren't these tests designed not to
> >> depend on each other anyway?
> >
> > Yes, they don't depend on each other, but what's the point in not
> > sorting them? I usually watch test progress visually, and if tests are
> > sorted, even with make -j4 they go more or less incrementally by their t
> > number.
> >
> > On my netbook, adding $(sort ...) adds approximately 0.008s to make
> > startup, so imho there is no performance penalty to adding that sort.
> 
> Heh, who said anything about performance?
> 
> I was pointing out that your justification "we HAVE TO" was wrong.
> 
> If you are doing this for perceived prettyness and not as a fix for any
> correctness issue, I want to see the patch honestly described as such;
> that's all.

I agree about rewording.


> By the way, if I recall correctly, $(sort) in GNU make not just sorts but
> as a nice side effect removes duplicates. So if we used a(n fictional)
> construct in our Makefile like this:
> 
>     T = $(wildcard *.sh a.*)
> 
> that might produce duplicates (i.e. "a.sh" might appear twice), which
> might leave us two identical pathnames in $T and cause us trouble.  Even
> if we do not have such a use currently, rewriting $(wildcard) like your
> patch does using $(sort $(wildcard ...)) may be a good way to future-proof
> our Makefile, and if you justify your patch that way, it would be a
> possible correctness hardening, not just cosmetics, and phrasing it with
> "HAVE TO" may be justifiable.
> 
> Care to try if $(wildcard *.sh a.*) give you duplicated output with newer
> GNU make? I am lazy but am a bit curious ;-)

Sure. Please give me time untill evening (GMT+0400), or maybe till the
weekend.


Kirill

^ permalink raw reply

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: Junio C Hamano @ 2012-01-20  7:22 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, jcwenger, git
In-Reply-To: <1327043453-80965-1-git-send-email-davvid@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> +    if base_present; then
> +	checkout_staged_file 1 "$MERGED" "$BASE"
> +    else
> +	:>"$BASE"

Just a style, but please write this as either one of the following:

	>"$BASE"
        : >"$BASE"

I tend to prefer the former, but if you have to write a command, we want
to see a SP before the redirection (and no SP before the redirect target).

> +    fi
>      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")?

^ permalink raw reply

* Re: difftool / mergetool waiting
From: David Aguilar @ 2012-01-20  7:25 UTC (permalink / raw)
  To: Jonathan Seng; +Cc: Andreas Schwab, git
In-Reply-To: <m239biadb5.fsf@igel.home>

On Sat, Jan 14, 2012 at 12:38 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Jonathan Seng <nekenyu@gmail.com> writes:
>
>> If wait is false, git would fire off the tool command and proceed to
>> the next then exit cleanly.
>
> That doesn't work for git mergetool, it wouldn't be able to postprocess
> the result of calling the tool command.

The difficulty on the difftool side is that the git-difftool--helper
scriptlet is being driven by git diff (as a $GIT_EXTERNAL_DIFF) and
depends on git diff to do the temp file cleanup.  Not blocking would
return control immediately to git diff.  This is unwanted because the
temp files would likely be removed before the tools have a chance to
open them.

You could write a script that simply forks off multiple difftool
commands.  The difftool stuff in git-cola does exactly that, for
example.
-- 
            David

^ permalink raw reply

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20  7:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: jcwenger, git
In-Reply-To: <7v7h0mhm8g.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 11:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> +    if base_present; then
>> +     checkout_staged_file 1 "$MERGED" "$BASE"
>> +    else
>> +     :>"$BASE"
>
> Just a style, but please write this as either one of the following:
>
>        >"$BASE"
>        : >"$BASE"
>
> I tend to prefer the former, but if you have to write a command, we want
> to see a SP before the redirection (and no SP before the redirect target).
>
>> +    fi
>>      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.
-- 
            David

^ permalink raw reply

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: Junio C Hamano @ 2012-01-20  7:37 UTC (permalink / raw)
  To: David Aguilar; +Cc: jcwenger, git
In-Reply-To: <CAJDDKr490XFqGe+y1pa7fnNhPMRE3ZR3=A+_XNUrCA1GZv+wkw@mail.gmail.com>

David Aguilar <davvid@gmail.com> writes:

>>>      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.

^ permalink raw reply

* [PATCH v3] mergetool: Provide an empty file when needed
From: David Aguilar @ 2012-01-20  7:37 UTC (permalink / raw)
  To: gitster; +Cc: jcwenger, git
In-Reply-To: <7v7h0mhm8g.fsf@alter.siamese.dyndns.org>

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 <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool.sh     |   20 +++++++++++++++++---
 t/t7610-mergetool.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..8cd70eb 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -224,9 +224,23 @@ 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"
+    if base_present; then
+	checkout_staged_file 1 "$MERGED" "$BASE"
+    else
+	>"$BASE"
+    fi
+
+    if local_present; then
+	checkout_staged_file 2 "$MERGED" "$LOCAL"
+    else
+	>"$LOCAL"
+    fi
+
+    if remote_present; then
+	checkout_staged_file 3 "$MERGED" "$REMOTE"
+    else
+	>"$REMOTE"
+    fi
 
     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.g1c18

^ permalink raw reply related

* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: Junio C Hamano @ 2012-01-20  7:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Aguilar, jcwenger, git
In-Reply-To: <7vy5t2g6za.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Yuck, sorry for sending out a edit-in-progress copy.

> 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.

It should read

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 is good enough, i.e.

^ permalink raw reply

* [PATCH v3] mergetool: Provide an empty file when needed
From: David Aguilar @ 2012-01-20  7:47 UTC (permalink / raw)
  To: gitster; +Cc: jcwenger, git
In-Reply-To: <7vy5t2g6za.fsf@alter.siamese.dyndns.org>

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 <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
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

^ permalink raw reply related

* Re: [PATCH v3] mergetool: Provide an empty file when needed
From: David Aguilar @ 2012-01-20  7:53 UTC (permalink / raw)
  To: gitster; +Cc: jcwenger, git
In-Reply-To: <1327045655-3368-1-git-send-email-davvid@gmail.com>

On Thu, Jan 19, 2012 at 11:47 PM, David Aguilar <davvid@gmail.com> 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 <jcwenger@gmail.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> 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.

> 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

^ permalink raw reply

* Re: [BUG] Git bisect not finding the right commit
From: Andreas J. Koenig @ 2012-01-20  7:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlip4je87.fsf@alter.siamese.dyndns.org>

>>>>> On Thu, 19 Jan 2012 00:20:08 -0800, Junio C Hamano <gitster@pobox.com> said:

 jch> andreas.koenig.7os6VVqR@franz.ak.mind.de (Andreas J. Koenig) writes:
 >> A v5.15.5
 >> B v5.15.5-20-gfd76d40
 >> C v5.15.5-81-gcfe287a
 >> D v5.15.5-159-ga71d67b
 >> E v5.15.4-110-g27b29ec
 >> F v5.15.4-169-g3582575
 >> 
 >> The change in perl I tried to locate was v5.15.5-13-gff0cf12, between A
 >> and B. Bisect did not find it, it returned me E instead. Here the wrong
 >> bisect output:

 jch> Just for the sake of simplicity, I'll call ff0cf12 "Q" (the Questionable
 jch> one).

 >> % git bisect start v5.15.5-159-ga71d67b v5.15.5

 jch> You start by telling Git that D is bad and A is good.

 jch> I can see that D does contain Q (i.e. "git log D..Q" gives nothing), which
 jch> you should read as "D is _contaminated_ by the breakage Q introduced", so
 jch> D is indeed bad.

 jch> On the other hand, A does _not_ contain Q (i.e. "git log A..Q" gives
 jch> output), which you should read as "A is _not_ contaminated by the breakage
 jch> Q introduced", so A is indeed good.

 jch> So far so good...

 >> Already on 'blead'
 >> Bisecting: 77 revisions left to test after this (roughly 6 steps)
 >> [cfe287a06b2ed98c25aebb477f6b400409f1fc85] Merge remote-tracking branch 'p5p/smoke-me/gsoc-pod' into blead
 >> % git describe
 >> v5.15.5-81-gcfe287a

 jch> This is your "C", and "git log C..Q" does not give anything. C is
 jch> contaminated by Q, hence it is bad.

 >> % git bisect bad
 >> Bisecting: 40 revisions left to test after this (roughly 5 steps)
 >> [baf7658bacfa659cdab08050470b20ebd5973384] Update htmlview.t for new Pod::Html
 >> % git describe
 >> v5.15.4-149-gbaf7658

 jch> Here, baf7658 does not contain Q, so you are supposed to answer it is
 jch> GOOD.

 >> % git bisect bad

 jch> But you answered that it is BAD.

 jch> Why?

The reason turned out to be that a perl module that was involved in the
testing had been upgraded in the meantime (YAML-0.77 to 0.78). So your
whole answer was correctly describing the situation. From this point
GiGo started to happen because the rest of the tests involved also used
the newer version of that module while some other tests were done with
the older version.

So thank you for clearing that up!

 jch> [...]

>>>>> On Thu, 19 Jan 2012 09:23:01 +0100, Johannes Sixt <j.sixt@viscovery.net> said:

 js> Am 1/19/2012 4:29, schrieb Andreas J. Koenig:
 >> - A -> B      ->     C - D ->
 >>          \         /
 >>           - E - F -
 >> 
 >> A v5.15.5
 >> B v5.15.5-20-gfd76d40
 >> C v5.15.5-81-gcfe287a
 >> D v5.15.5-159-ga71d67b
 >> E v5.15.4-110-g27b29ec
 >> F v5.15.4-169-g3582575

 js> I haven't looked at the actual history, but given the names of the commits
 js> as produced by git-describe, I doubt that your history graph sketched
 js> above is correct. Doesn't it look more like this:

 js>       A -- B -- C -- D --
 js>      /         /
 js>  -- X -- E -- F

 js> where X is v5.15.4?

Yes, thank you for finding that out. X is actually v5.15.4-109-g3ea0c58
and since there was a long timespan between the start of the development
of the code and the merge (May-Nov), the gitk presentation got a bit
complex to read.

 js> To find a commit between A and B, you must declare F as "good".

Correct! The reason it happened described above.

Thank you folks for taking the time and making such a careful assessment!
-- 
andreas

^ permalink raw reply

* Re: More support on branch description?
From: Nguyen Thai Ngoc Duy @ 2012-01-20  8:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v4nvrib7u.fsf@alter.siamese.dyndns.org>

On Fri, Jan 20, 2012 at 5:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
> You however are misguided to say "Showing a short summary along...".

It should have been "short status".

> The branch description support is to give users a place to record detailed
> explanation about a branch, similar in size to what you would normally
> place in a log message of a commit or a cover letter of a series.  There
> wasn't any convenient place to do so for a branch that is (1) inherently a
> moving target while it is being developed and (2) is not a good match for
> tags and notes.

I was thinking about that (and wondering if I abused
branch.*.description), maybe we can have a similar convention for
commit message, one short line, empty line, then more detail
explanation. But branch.*.description is tied to
format-patch/request-pull, maybe another config key.

> There already is a good place for a brief summary and it is called "branch
> name". Name your branches just like you name your functions.

Yes, but it's not suitable for current status of the branch.
-- 
Duy

^ permalink raw reply

* Cannot migrate from SVN to GIT -> Path problem
From: Asuka @ 2012-01-20  9:26 UTC (permalink / raw)
  To: git

Hi there

I´m trying to migrate a repository from svn to git with the following
command 

git svn clone http://host/svn/project/subproject --no-metadata --stdlayout
-A ~/Desktop/users.txt subproject. 

Unfortunately I receive no data in the new git repository except the .git
folder and I get the following warning

W: Ignoring error from SVN, path probably does not exist: (175002): RA layer
request failed:REPORT request failed on '/svn/!svn/bc/100': REPORT of
'/svn/!svn/bc/100': 200 OK (http:/host). 

What does that mean??

Then it seams to start a migration 

Checked through r100
Checked through r200
Checked through r300
Checked through r400
Checked through r500

......

But as I said, I receive no data.

Does anybody know what to do now?

Best wishes




--
View this message in context: http://git.661346.n2.nabble.com/Cannot-migrate-from-SVN-to-GIT-Path-problem-tp7206879p7206879.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined
From: Ævar Arnfjörð Bjarmason @ 2012-01-20  9:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Alex Riesen, Git Mailing List
In-Reply-To: <7vhazrk0jx.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 19:30, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> ... We have, e.g., NO_MMAP, and I can set it to request
>> that some alternative is used, even if I have a working mmap(). The option
>> name "NO_GETTEXT" is in exactly the same spirit.
>>
>>> In the current approach we take for shell scripts, we cannot have "No i18n
>>> whatsoever and messages are emit with printf and echo". We always have to
>>> go through gettext/eval_gettext even though they may be an implementation
>>> that does not do i18n at all.
>>
>> Just like we go through _() in C code, even though there may be an
>> implementation that does not do i18n at all, right?
>
> Yes, just like that. The small detail that _() can be #define'd out to
> empty while gettext/eval_gettext cannot be made to be no-impact like that
> does not really matter.
>
>> In C, it is easy, in shell code it may be more involved.
>
> Correct.

To elaborate, the C code can:

 * Use the system gettext library to get translations.

 * Use the system gettext library, but effectively be pass-through
   because the user has the C locale.

 * Use our fallback functions which in any modern compiler will be
   optimized out.

However with the shell code we can:

 1. Be using the system gettext & eval_gettext to get translations.

 2. Be using the system gettext & eval_gettext as pass-through, either
    because we don't have translations since we've installed with
    NO_GETTEXT=YesPlease, or because we're in the C locale.

 3. Haven't detected that gettext.sh etc. exists, so we have to provide
    our own fallbacks.

The proposed patch would move all users of NO_GETTEXT=YesPlease to #3,
even though on most platforms we don't need to define our own dummy
fallbacks since the system already provides them.

I don't particularly like it because I'd rather use the OS vendor's
implementation if possible, even for fallback.

However it being broken is also unacceptable, but I think the way
forward is to detect the breakage either at compile time or at
runtime, to that end Alex could you provide us with the output from
the following commands on the offending system where this is broken:

    $ type gettext.sh
    $ gettext.sh --version
    $ gettext -h
    $ gettext "some test text"
    $ . gettext.sh
    eval_gettext
    $ variable=value eval_gettext "some \$variable"

Then how the eval_gettext function is defined:

    $ type eval_gettext
    eval_gettext is a function
    eval_gettext ()
    {
        gettext "$1" | ( export PATH `envsubst --variables "$1"`;
        envsubst "$1" )
    }

And then a --version for whatever programs that function uses,
e.g. here:

    $ envsubst --version

Once we know how it breaks we can e.g. add configure tests for
checking whether we can use the system's gettext library for the
fallbacks.

Could you also run the git test suite as described in t/README? I'd
expect a lot of the i18n tests to fail, but it would be curious to see
which ones exactly.

^ permalink raw reply

* Re: [PATCH] i18n: disable i18n for shell scripts if NO_GETTEXT defined
From: Alex Riesen @ 2012-01-20 10:40 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Junio C Hamano, Johannes Sixt, Git Mailing List
In-Reply-To: <CACBZZX7iiF2um11FvD+MBz=rZb7RrHtCJp3PqexLnSp3-Cbqug@mail.gmail.com>

On Fri, Jan 20, 2012 at 10:50, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> However with the shell code we can:
>
>  1. Be using the system gettext & eval_gettext to get translations.
>
>  2. Be using the system gettext & eval_gettext as pass-through, either
>    because we don't have translations since we've installed with
>    NO_GETTEXT=YesPlease, or because we're in the C locale.
>
>  3. Haven't detected that gettext.sh etc. exists, so we have to provide
>    our own fallbacks.
>
> The proposed patch would move all users of NO_GETTEXT=YesPlease to #3,
> even though on most platforms we don't need to define our own dummy
> fallbacks since the system already provides them.
>
> I don't particularly like it because I'd rather use the OS vendor's
> implementation if possible, even for fallback.

Well, I dunno. I wouldn't trust anything to this particular "OS vendoer".

> However it being broken is also unacceptable, but I think the way
> forward is to detect the breakage either at compile time or at
> runtime, ...

Better at runtime, unless the packager explicitly stated they don't want
any of this.

> ... to that end Alex could you provide us with the output from
> the following commands on the offending system where this is broken:
>
>    $ type gettext.sh

gettext.sh is /usr/bin/gettext.sh

>    $ gettext.sh --version

/usr/bin/gettext.sh (GNU gettext-runtime) 0.18.1
Copyright (C) 2003-2007 Free Software Foundation, Inc.
License GPLv2+: GNU GPL version 2 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Bruno Haible

>    $ gettext -h

Nothing. Exit code 127.

>    $ gettext "some test text"

Nothing. Exit code 127.

>    $ . gettext.sh

Nothing. Exit code 0.

>    eval_gettext

Nothing. Exit code 127.

>    $ variable=value eval_gettext "some \$variable"

Nothing. Exit code 127.

> Then how the eval_gettext function is defined:
>
>    $ type eval_gettext

eval_gettext is a function
eval_gettext ()
{
    gettext "$1" | ( export PATH `envsubst --variables "$1"`;
    envsubst "$1" )
}

> And then a --version for whatever programs that function uses,
> e.g. here:
>
>    $ envsubst --version

Nothing. Exit code 127.

> Once we know how it breaks we can e.g. add configure tests for
> checking whether we can use the system's gettext library for the
> fallbacks.

The exit code seems to be a good enough test here, but testing some
output (or even translation) would be safer.

I believe gettext (the binary) just doesn't start at all here. Maybe
some Cygwin library wrong or missing library. Happens all the time
here, as we have different Cygwin installations depending on the
currently used toolchain. QNX Momentics, in particular. Different
versions of them, and it is too cumbersome to keep them apart.

> Could you also run the git test suite as described in t/README? I'd
> expect a lot of the i18n tests to fail, but it would be curious to see
> which ones exactly.

Yes, they do. Can't run them on this problematic system, because they
tend to crash it, if run for an undetermined while. On the other system,
which can run them, gettext works (it is an older Cygwin installation),
so almost all tests pass (some still don't, but for reasons unrelated).
Strangely enough, the problematic system can build. So I don't copy
the git binaries, they are actually built on that system.

^ permalink raw reply

* [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation
From: Ævar Arnfjörð Bjarmason @ 2012-01-20 12:49 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Alex Riesen, Johannes Sixt,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <CALxABCZWBtgX736Acoy-CCAz8RJb0EKnHf+7g72dOdVS+BOhSw@mail.gmail.com>

Even though we can load gettext.sh the gettext(1) and eval_gettext
functions it provides might be completely broken. This reportedly
happens on some Cygwin installations where we can load gettext.sh, but
gettext and eval_gettext both return exit code 127 and no output.

The reason we're trying to load gettext.sh (or the equivalent Solaris
implementation) at all is so we don't have to provide our own fallback
implementation if the OS already has one installed, but because we
didn't test whether it actually worked under GNU gettext we might end
up with broken functions.

Change the detection in git-sh-i18n so that it tests that the output
of "gettext test" produces "test", on Solaris we already test that
"gettext -h" produces "-h", so we were already guarded against the
same sort of failure there.

Reported-by: Alex Riesen <raa.lkml@gmail.com>
---
Here's a minimal patch to git-sh-i18n that should make things work on
Cygwin and any other platforms with broken gettext functions while
also using the OS-provided functions if they work.

I've added a new t0201-gettext-fallbacks-broken-gettext.sh test that
tests this. This required a small change in lib-gettext.sh so I
wouldn't load test-lib.sh twice.

Note that there's already a t0201* test in the repo. Maybe we want to
increment all the gettext test numbers by one to make room for it?

As an aside I'm really not a big fan of having hardcoded numbers in
the test files like this. We don't care about the order of execution
here.

 git-sh-i18n.sh                              |    2 +-
 t/lib-gettext.sh                            |    7 +++++-
 t/t0201-gettext-fallbacks-broken-gettext.sh |   28 +++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100755 t/t0201-gettext-fallbacks-broken-gettext.sh

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..26a57b0 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -18,7 +18,7 @@ export TEXTDOMAINDIR
 
 if test -z "$GIT_GETTEXT_POISON"
 then
-	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
+	if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1 && test "$(gettext test 2>&1)" = "test"
 	then
 		# This is GNU libintl's gettext.sh, we don't need to do anything
 		# else than setting up the environment and loading gettext.sh
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..2c5b758 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -3,7 +3,12 @@
 # Copyright (c) 2010 Ævar Arnfjörð Bjarmason
 #
 
-. ./test-lib.sh
+if test -z "$TEST_DIRECTORY"
+then
+	# In case the test loaded test-lib.sh by itself to do some tests
+	# prior to loading us.
+	. ./test-lib.sh
+fi
 
 GIT_TEXTDOMAINDIR="$GIT_BUILD_DIR/po/build/locale"
 GIT_PO_PATH="$GIT_BUILD_DIR/po"
diff --git a/t/t0201-gettext-fallbacks-broken-gettext.sh b/t/t0201-gettext-fallbacks-broken-gettext.sh
new file mode 100755
index 0000000..92b95ae
--- /dev/null
+++ b/t/t0201-gettext-fallbacks-broken-gettext.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Ævar Arnfjörð Bjarmason
+#
+
+test_description='Gettext Shell fallbacks with broken gettext'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a fake broken gettext(1)' '
+	cat >gettext <<-\EOF &&
+	#!/bin/sh
+	exit 1
+	EOF
+	chmod +x gettext &&
+    ! ./gettext
+'
+
+PATH=.:$PATH
+. "$TEST_DIRECTORY"/lib-gettext.sh
+
+test_expect_success C_LOCALE_OUTPUT '$GIT_INTERNAL_GETTEXT_SH_SCHEME" is fallthrough with broken gettext(1)' '
+    echo fallthrough >expect &&
+    echo $GIT_INTERNAL_GETTEXT_SH_SCHEME >actual &&
+    test_cmp expect actual
+'
+
+test_done
-- 
1.7.7.3

^ permalink raw reply related

* Re: [PATCH] git-add: allow --ignore-missing always, not just in dry run
From: Thomas Rast @ 2012-01-20 12:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vl3jzst.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <trast@student.ethz.ch> writes:
>
>>   $ git grep 'git[ -]add' t/ | wc -l
>>   1540
>>   $ git grep 'git[ -]update-index --add' t/ | wc -l
>>   269
>>   $ git grep 'git[ -]update-index --add' v1.6.0 t/ | wc -l
>>   251
>>   $ git grep 'git[ -]add' v1.6.0 t/ | wc -l
>>   705
>
> Stop being silly.
>
> Have you actually looked at these usage?  Some of them are genuinely
> testing if "git add" works correctly, so it is out of the scope of this
> discussion, but others that could be "git update-index" are feeding the
> paths known to the script to exist (and we want 'git add' to error out
> if that is not the case).

I'm sorry if I sound silly, that was totally not the point.  I also
admit that I did not look at the usages at all.  I merely wanted to
point out that the understanding in the git community *itself* has
evolved to use git-add instead of git update-index --add in its own
scripting.  Admittedly the statistics are even more striking than I
could possibly hope for.

So I am challenging the notion that git-add is not recommended for use
in scripts, which is how I understood your parenthetical remark

} If somebody is writing a script using "git add" (which is not recommended
} to begin with)

We're no longer following that advice ourselves, how can we expect users
to adhere to it?

> More generally, scripts in t/ directories are "scripts", but it is totally
> different from the kind of "user facing script that behaves as if it is a
> complete command, taking its own command line arguments, passing them
> through to the underlying plumbing commands".

I don't understand what distinction you are trying to make here.  Maybe
my mental model of the plumbing/porcelain separation (which is mostly
about interface stability) is wrong?

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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox