Git development
 help / color / mirror / Atom feed
* 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

* Re: [PATCH v2 1/2] am: learn passing -b to mailinfo
From: Thomas Rast @ 2012-01-20 13:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3afidt3.fsf@alter.siamese.dyndns.org>

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

> After re-reading the code that parses the command line options given to
> "am" and the previous invocation state we read from $dotest/*, however, I
> think the way this change uses $keep makes things somewhat inconsistent
> and harder to follow.
>
> Currently the variables are given abstract meaning (e.g. "are we told to
> record to utf8? yes or no") when we parse our command line options and
> read from the previous invocation state, and then based on that abstract
> meaning, a later code decides what exact option we throw at the git
> commands we invoke (e.g. "utf8=t" -> "-u").
>
> How about doing something like this instead at least for now?  It might be
> better to decide when we parse our options and $dotest/* immediately what
> options we give to the git commands we run (which your patch does but only
> to $keep option), but that kind of change (1) belongs to a separate topic
> and should be done consistently to all options, and (2) I am not convinced
> if it is necessarily a good change.

Yes, at second glance it's probably better to remain consistent.  I
didn't like it at first because it's layering complexity on it, but you
are right, the existing code follows the same pattern.

> diff --git a/git-am.sh b/git-am.sh
> index 6cdd591..8b755d9 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -15,6 +15,7 @@ q,quiet         be quiet
>  s,signoff       add a Signed-off-by line to the commit message
>  u,utf8          recode into utf8 (default)
>  k,keep          pass -k flag to git-mailinfo
> +keep-non-patch  pass -b flag to git-mailinfo
>  keep-cr         pass --keep-cr flag to git-mailsplit for mbox format
>  no-keep-cr      do not pass --keep-cr flag to git-mailsplit independent of am.keepcr
>  c,scissors      strip everything before a scissors line
> @@ -345,6 +346,8 @@ do
>  		utf8= ;;
>  	-k|--keep)
>  		keep=t ;;
> +	--keep-non-patch)
> +		keep=b ;;
>  	-c|--scissors)
>  		scissors=t ;;
>  	--no-scissors)
> @@ -522,16 +525,25 @@ case "$resolved" in
>  	fi
>  esac
>  
> +# Now, decide what command line options we will give to the git
> +# commands we invoke, based on the result of parsing command line
> +# options and previous invocation state stored in $dotest/ files.
> +
>  if test "$(cat "$dotest/utf8")" = t
>  then
>  	utf8=-u
>  else
>  	utf8=-n
>  fi
> -if test "$(cat "$dotest/keep")" = t
> -then
> -	keep=-k
> -fi
> +keep=$(cat "$dotest/keep")
> +case "$keep" in
> +t)
> +	keep=-k ;;
> +b)
> +	keep=-b ;;
> +*)
> +	keep= ;;
> +esac
>  case "$(cat "$dotest/keepcr")" in
>  t)
>  	keepcr=--keep-cr ;;

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

^ permalink raw reply

* [Q] Determing if a commit is reachable from the HEAD ?
From: Brian Foster @ 2012-01-20 13:33 UTC (permalink / raw)
  To: git mailing list


Hello,

 Whilst I have found answers to my question on the Web,
 only one seems to do exactly what I want ....

                         x---Y---y---y---y  HEAD
                        /
  ...--o---o---C---o---S
                        \
                         n---n---N---*---*  other

 In a script, how can I determine commit Y is reachable
 from the current HEAD ?   And, much more importantly
 for my purposes, that commit N is _not_-reachable from
 the current HEAD ?  "A is reachable from B" meaning B
 is an ancestor of A (B could possibly part of a merge
 (not shown in the above diagram)):

  ✓ Commits o(all), C, S, x, y(all), and Y: YES  (reachable).
  ✗ Commits n(all), N, and *(all): NO  (not-reachable).

 The only readily script-able answer I've found is to use
 `git branch --contains=Y' and check to see if the current
 branch (conveniently marked `*') is/isn't included in the
 output.  That is probably Ok, but I'm wondering if there
 is some "better" method (with the IMPORTANT caveat it must
 work with GIT 1.5.x or later  ;-\ ).

 git-rev-list(1) may be a answer (see script below), but ....
 `git rev-list ^Y HEAD' lists all the y commits, which is Ok
 (correct for my purposes).  However, `git rev-list ^N HEAD'
 lists commits x, Y, and y(all) on branch "other", which is
 not-Ok:  As per above, the answer I want is "NO".

 Apologies if I've overlooked something totally obvious !

cheers,
	-blf-

=====(cut here and below)=====reach02.sh=====
#!/bin/bash
#
#                         x---Y---y---y---y  HEAD
#                        /
#  ...--o---o---C---o---S
#                        \
#                         n---n---N---*---*  other
#

: ${GIT:=git}	# Allow use of different installed GIT versions.

update_file() {
	date +"%c: $*" >>file
	$GIT add file  &&  $GIT commit -m "$*" file
}

set -e		# Terminate if problem creating diagrammed situation.

rm -rf -- reachable
mkdir  -- reachable
cd reachable

$GIT version
$GIT init

update_file     o1
update_file     o2
update_file "C (o3)";	$GIT tag C
update_file     o4
update_file "S (o5)";	$GIT tag S

$GIT checkout -b other
update_file     n1
update_file     n2
update_file "N (n3)";	$GIT tag N
update_file    "*1"
update_file    "*2"

$GIT checkout master
update_file     x
update_file "Y (y1)";	$GIT tag Y
update_file     y2
update_file     y3
update_file     y4

declare -i wrong=0

echo "Is Y reachable from HEAD?  Wanted answer: Yes."
$GIT log --oneline ^Y HEAD
if $GIT rev-list --quiet ^Y HEAD; then
	echo "rev-list: YES (wanted answer)!"
else
	echo "rev-list: No! *** not-wanted, oops... ***"
	wrong=$(( wrong + 1 ))
fi
if $GIT branch --contains=Y | grep -q -e '^\*'; then
	echo "contains: YES (wanted answer)!"
else
	echo "contains: No! *** not-wanted, oops... ***"
	wrong=$(( wrong + 1 ))
fi

echo "Is N reachable from HEAD?  Wanted answer: No."
$GIT log --oneline ^N HEAD
if $GIT rev-list --quiet ^N HEAD; then
	echo "rev-list: YES? *** not-wanted, oops... ***"
	wrong=$(( wrong + 1 ))
else
	echo "rev-list: No (wanted answer)!"
fi
if $GIT branch --contains=N | grep -q -e '^\*'; then
	echo "contains: YES? *** not-wanted, oops... ***"
	wrong=$(( wrong + 1 ))
else
	echo "contains: No (wanted answer)!"
fi

exit $wrong
=====(cut here and above)=====reach02.sh=====

-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Maxim Integrated Products      |  Web:  http://www.maxim-ic.com/

^ permalink raw reply

* Re: [PATCH] git-sh-i18n: detect and avoid broken gettext(1) implementation
From: Alex Riesen @ 2012-01-20 14:02 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano, Johannes Sixt
In-Reply-To: <1327063775-28420-1-git-send-email-avarab@gmail.com>

On Fri, Jan 20, 2012 at 13:49, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 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.

FWIW, I confirm it works (which is quite obvious).

Just for giggles, I even risked running the tests and, of course, crashed
that piece of junk with broken Cygwin installation.
Please don't ask me to do that again :) restarting it is PITA as well.

Just for future reference to all poor Cygwin users:

I also left NO_GETTEXT in the config.mak. This, BTW, explains why git
works, while gettext binary does not: one .dll dependency less.

^ permalink raw reply

* Re: [PATCH v3] mergetool: Provide an empty file when needed
From: Jason Wenger @ 2012-01-20 14:03 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git
In-Reply-To: <CAJDDKr71Q9ihLZdP1Pu=ebpvX0EPvW-9bN6kZz9MeuhYKZzh+Q@mail.gmail.com>

On Fri, Jan 20, 2012 at 01:53, David Aguilar <davvid@gmail.com> wrote:
> 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.

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

--jcw

^ permalink raw reply

* Re: [Q] Determing if a commit is reachable from the HEAD ?
From: Andreas Schwab @ 2012-01-20 14:13 UTC (permalink / raw)
  To: Brian Foster; +Cc: git mailing list
In-Reply-To: <201201201433.30267.brian.foster@maxim-ic.com>

Brian Foster <brian.foster@maxim-ic.com> writes:

>  In a script, how can I determine commit Y is reachable
>  from the current HEAD ?

test $(git merge-base HEAD Y) = $(git rev-parse Y)

>  And, much more importantly
>  for my purposes, that commit N is _not_-reachable from
>  the current HEAD ?

test $(git merge-base HEAD N) != $(git rev-parse N)

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* crash with git and latest mainline: EIP: 0060:[<c106c2d2>] EFLAGS: 00210006 CPU: 0
From: Justin P. Mattock @ 2012-01-20 15:00 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org; +Cc: git

not sure if this means anything or not, but I was doing a apt-get 
upgrade and a git pull at the same time and had a total system freeze.
I was only able to capture an image of the crash. here are the images of it:

http://www.flickr.com/photos/44066293@N08/6730938347/in/photostream
http://www.flickr.com/photos/44066293@N08/6730937479/in/photostream
http://www.flickr.com/photos/44066293@N08/6730936509/in/photostream

will keep an eye out or for this, tried to reproduce but with no luck 
doing so.

Justin P. Mattock

^ permalink raw reply

* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Shawn Pearce @ 2012-01-20 15:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobtyhq16.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 22:00, Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> +cat >exp <<EOF
>> +remote: error: hook declined to update refs/heads/dev2
>
> Curious. Where do we get these eight trailing whitespaces?

I think this is padding being added to the end of the line by
recv_sideband(). I noticed the trailing whitespace in the diff, but
the test passed with it present, so I had to leave it in.

> The call to rp_error("hook declined to update %s", name) seems to be
> giving the name properly.

Yea, I think the server is sending the correct data in the sideband
channel, its just the sideband client padding out the line. I think
this padding is a fudge against progress meters that are being written
and over-written with \r lines in subsequent sideband packets.

^ permalink raw reply

* Re: Rebase and incrementing version numbers
From: Carlos Martín Nieto @ 2012-01-20 15:53 UTC (permalink / raw)
  To: mike; +Cc: git
In-Reply-To: <CADo4Y9iJyirdkEr1GCg9BD5rwX9=1uKptqHsiWB0_MiDKb_wUA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4169 bytes --]

On Thu, 2012-01-19 at 15:02 -0500, Michael Nahas wrote:
> I'm guessing here, but I believe the "version number" is used to make
> a directory on the production machine.  Thus, older versions of the
> javascript are available on the production machines under their older
> version number.  If there's an issue in production with the new
> version, code can be redirected to use the older version that is still
> in its directory.
> 
> So it probably looks like:
> /100/js/<files>
> /101/js/<files>
> /103/js/<files>
> /104/js/<files>
> 
> If something goes wrong with version 104, the admin can just tell the
> machine to use version 103 instead of 104.

So your team has developed a VCS to run on top of the VCS you're using.
This is a bit disconcerting. What's the point of svn if you're tracking
the versions manually? Rolling back changes is one of the things that
svn is there to help you with. There is no need for an extra layer.
Separating production-ready changes with experimental changes is what
branches are for.

From the way you explain the development/deployment cycle, it doesn't
sound like any of the changes you dcommit should increase the version
number except for the last one. If you increase the version number three
times in one dcommit but you introduced the bug in the first of those,
now you have to manually go back and try each version, which seems
contrary to what the point of the scheme is.

   cmn

> 
> You're right that incrementing this version number is probably not an
> issue for SVN users because they put N features in a single commit and
> they update the version number once.   With git, a user can put N
> features in N commits and changing the version number really belongs
> in each commit.  This makes rebasing suck.
> 
> 
> On Thu, Jan 19, 2012 at 2:20 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> > On Thu, 2012-01-19 at 12:20 -0500, Michael Nahas wrote:
> >> I'm at a new job and using Git-SVN at a place that is accustomed to SVN.
> >>
> >> The problem I'm running into is that whenever I change a file in a
> >> directory, I have to bump up the version number in the configuration
> >> file.  The larger version value in the config file causes my changes
> >> to be loaded over the old ones.
> >
> > Is this a deployment script that does this? Why can't it look at whether
> > files have changed? If a feature isn't ready for production, why is it
> > in a branch that gets deployed?
> >
> >>
> >> Most of my commits are edits to a file like "foo.js" and an increment
> >> to the version number in "config".  Ideally, each of my features
> >> should live in a single commit and I should be able to make a sequence
> >> of them, each time incrementing the version number in config.
> >>
> >
> > So if you've changed the file but don't increase the config file's
> > version, it means that the change isn't ready for production? If that's
> > the case, you've just implemented branches, poorly.
> >
> > Contrary to what apparently many people think, subversion does support
> > branches. Get your team to use them.
> >
> >> The problem I'm running into starts with me editing version=100.  I
> >> create new commits where I set the version to 101, 102, 103, 104.
> >> When I go to push ("git svn dcommit"), my coworkers have incremented
> >> the version to 103.  So, I rebase my changes, and get conflicts every
> >> time because of the version number!
> >
> > This sounds like a race condition that the svn users might be avoiding
> > by committing everything immediately. Sounds like a buggy development
> > process.
> >
> >>
> >> Is there a good way to avoid these conflicts?  Is there a hook I can
> >> write?  Is there a change to this process that would work smoother
> >> with Git and its distributed development?  It's okay if the version
> >> number skips numbers (e.g., jumps from 100 to 104), as long as it
> >> increases.
> >
> > You could write a merge driver that detects this situation and writes in
> > a higher number, but it's all working around the fact that it's a race
> > condition.
> >
> >   cmn
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Thomas Rast @ 2012-01-20 16:17 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <CAJo=hJtCb=WFfuSKWvPk+S4sRQmSGemG_Ugqj+k1TZCOJj9vLQ@mail.gmail.com>

Shawn Pearce <spearce@spearce.org> writes:

> On Thu, Jan 19, 2012 at 22:00, Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>>> +cat >exp <<EOF
>>> +remote: error: hook declined to update refs/heads/dev2
>>
>> Curious. Where do we get these eight trailing whitespaces?
>
> I think this is padding being added to the end of the line by
> recv_sideband(). I noticed the trailing whitespace in the diff, but
> the test passed with it present, so I had to leave it in.

ISTR we had a policy to guard such whitespace at EOL?  Compare
e.g. c1376c12b7.

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

^ permalink raw reply

* Autocompletion - commands no longer work as stand alone
From: Nathan Bullock @ 2012-01-20 16:32 UTC (permalink / raw)
  To: git
In-Reply-To: <CAPx=Vfp_HVr5W1fFic_1k+JsKr2RAKd-RK=VkfSgo7qkb5GsAw@mail.gmail.com>

I have for a number of years had the following in my .bashrc

alias br="git branch"
complete -F _git_branch br

As well as similar commands for co and log.

Recently though this broke, now when I type something like "br
mas<command completion>" it will occasionally complain with messages
like:
bash: [: 1: unary operator expected

From digging through the source it looks like this was broken back in
April. (The commit is show at the bottom of this email.)

So my questions are:
1. Is it reasonable for things like _git_branch to work as a
standalone autocompletion function instead of having to go through
_git? I certainly like it to work as a standalone function. I also use
it to add autocompletion to other bash scripts that I use frequently.

2. If I add code that verifies that the variable cword exists at the
start of these functions and only if not call something like
_get_comp_words_by_ref -n =: cur words cword prev. Would that be
reasonable? I think this should address the performance concerns that
caused these to be removed in the first place, but it may make the
code uglier.

I have already added wrapper functions in my bashrc so that this is no
longer a problem for me, but there may be other people who start
hitting this as well once they start using newer versions of git.

Nathan


commit da4902a73017ad82b9926d03101ec69a2802d1e7
Author: SZEDER Gábor <szeder@ira.uka.de>
Date:   Thu Apr 28 18:01:52 2011 +0200

   completion: remove unnecessary _get_comp_words_by_ref() invocations

   In v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work
   with bash v4, 2010-12-02) we started to use _get_comp_words_by_ref()
   to access completion-related variables.  That was large change, and to
   make it easily reviewable, we invoked _get_comp_words_by_ref() in each
   completion function and systematically replaced every occurance of
   bash's completion-related variables ($COMP_WORDS and $COMP_CWORD) with
   variables set by _get_comp_words_by_ref().

   This has the downside that _get_comp_words_by_ref() is invoked several
   times during a single completion.  The worst offender is perhaps 'git
   log mas<TAB>': during the completion of 'master'
   _get_comp_words_by_ref() is invoked no less than six times.

   However, the variables $prev, $cword, and $words provided by
   _get_comp_words_by_ref() are not modified in any of the completion
   functions, and the previous commit ensures that the $cur variable is
   not modified as well.  This makes it possible to invoke
   _get_comp_words_by_ref() to get those variables only once in our
   toplevel completion functions _git() and _gitk(), and all other
   completion functions will inherit them.

   Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
   Signed-off-by: Junio C Hamano <gitster@pobox.com>

^ permalink raw reply

* [PATCH] remote-curl: Fix push status report when all branches fail
From: Shawn O. Pearce @ 2012-01-20 17:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <8739bacpql.fsf@thomas.inf.ethz.ch>

From: "Shawn O. Pearce" <spearce@spearce.org>

The protocol between transport-helper.c and remote-curl requires
remote-curl to always print a blank line after the push command
has run. If the blank line is ommitted, transport-helper kills its
container process (the git push the user started) with exit(128)
and no message indicating a problem, assuming the helper already
printed reasonable error text to the console.

However if the remote rejects all branches with "ng" commands in the
report-status reply, send-pack terminates with non-zero status, and
in turn remote-curl exited with non-zero status before outputting
the blank line after the helper status printed by send-pack. No
error messages reach the user.

This caused users to see the following from git push over HTTP
when the remote side's update hook rejected the branch:

  $ git push http://... master
  Counting objects: 4, done.
  Delta compression using up to 6 threads.
  Compressing objects: 100% (2/2), done.
  Writing objects: 100% (3/3), 301 bytes, done.
  Total 3 (delta 0), reused 0 (delta 0)
  $

Always print a blank line after the send-pack process terminates,
ensuring the helper status report (if it was output) will be
correctly parsed by the calling transport-helper.c. This ensures
the helper doesn't abort before the status report can be shown to
the user.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 remote-curl.c        |    9 +++++----
 t/t5541-http-push.sh |   27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 48c20b8..25c1af7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -805,7 +805,7 @@ static int push(int nr_spec, char **specs)
 static void parse_push(struct strbuf *buf)
 {
 	char **specs = NULL;
-	int alloc_spec = 0, nr_spec = 0, i;
+	int alloc_spec = 0, nr_spec = 0, i, ret;
 
 	do {
 		if (!prefixcmp(buf->buf, "push ")) {
@@ -822,12 +822,13 @@ static void parse_push(struct strbuf *buf)
 			break;
 	} while (1);
 
-	if (push(nr_spec, specs))
-		exit(128); /* error already reported */
-
+	ret = push(nr_spec, specs);
 	printf("\n");
 	fflush(stdout);
 
+	if (ret)
+		exit(128); /* error already reported */
+
  free_specs:
 	for (i = 0; i < nr_spec; i++)
 		free(specs[i]);
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 9b85d42..c68cbf3 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -95,6 +95,31 @@ test_expect_success 'create and delete remote branch' '
 	test_must_fail git show-ref --verify refs/remotes/origin/dev
 '
 
+cat >"$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update" <<EOF
+#!/bin/sh
+exit 1
+EOF
+chmod a+x "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update"
+
+printf 'remote: error: hook declined to update refs/heads/dev2        \n' >exp
+cat >>exp <<EOF
+To http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git
+ ! [remote rejected] dev2 -> dev2 (hook declined)
+error: failed to push some refs to 'http://127.0.0.1:5541/smart/test_repo.git'
+EOF
+
+test_expect_success 'rejected update prints status' '
+	cd "$ROOT_PATH"/test_repo_clone &&
+	git checkout -b dev2 &&
+	: >path4 &&
+	git add path4 &&
+	test_tick &&
+	git commit -m dev2 &&
+	git push origin dev2 2>act
+	test_cmp exp act
+'
+rm -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update"
+
 cat >exp <<EOF
 
 GET  /smart/test_repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
@@ -106,6 +131,8 @@ GET  /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
 POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
 GET  /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
 POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
+GET  /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
+POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
 EOF
 test_expect_success 'used receive-pack service' '
 	sed -e "
-- 
1.7.9.rc2.124.g1c075

^ permalink raw reply related

* Re: [PATCH] git-add: allow --ignore-missing always, not just in dry run
From: Junio C Hamano @ 2012-01-20 18:03 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git
In-Reply-To: <87mx9icz28.fsf@thomas.inf.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

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

What's so hard to understand that these tests are very different from
end-user scripts?

We could have shipped you a written instruction to type these commands
from your shell and made you responsible for running them every time we
release a new version. That would have been more true to the intent of
these test scripts. The test being implemented as scripts is merely a
substitute for hiring one Thomas Rast as a test engineer to type them from
the terminal ;-).

User-facing scripts (Porcelain enhancements) people write are in a totally
different boat. They take input and have code to make their own decision
what kind of arguments and inputs to feed to their underlying building
blocks. They may even parse output from the commands they invoke to base
their decision that affects what happens next. Our tests start from a
known state (i.e. empty trash directory), take input from neither command
line, human interaction nor the filesystem content of the day, that affect
the input to the commands they drive.

To put it another way, if you have a cron job that does

    cd $HOME/diary && git add MyDiary.txt

that is perfectly fine. You are letting the machine do the typing for you
every hour, instead of having you type these yourself. It is even OK if
the filename was derived from `date` or something, i.e.

    N=$(date +'%Y-%m-%d').txt &&
    if test -f "$N"
    then
	git add "$N"
    fi

What is not OK is to attempt parsing from Porcelain output to decide what
to do next. "git branch | sed -ne 's/^\* //p'" is a typical example.

Our tests are different for another important reason you seem to be
missing. The tests we ship are tied very closely with the version of Git
they are testing. Even parsing the command output is acceptable for our
tests for this reason (obviously that is the only way to make sure that we
are issuing an appropriate error, warning, or advice message to the end
user). End-user scripts do not have that property.

And the biggest thing you should consider is that 99% of users are too
busy to bother thinking for themselves and instead prefer to be handed
down a concise recipe to follow blindly. You could include "in this, that,
and that other situation, it is OK to use Porcelain command" to the
recipe, but doing so defeats the whole purpose of having a recipe to begin
with, by making the readers responsible for thinking for themselves again.
That is why we just give a concise "Do not use Porcelain commands in your
scripts as their behaviour is subject to change."

^ permalink raw reply

* Re: [Q] Determing if a commit is reachable from the HEAD ?
From: David Brown @ 2012-01-20 18:06 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Brian Foster, git mailing list
In-Reply-To: <m2ehuu8nrt.fsf@igel.home>

On Fri, Jan 20, 2012 at 03:13:58PM +0100, Andreas Schwab wrote:
> Brian Foster <brian.foster@maxim-ic.com> writes:
> 
> >  In a script, how can I determine commit Y is reachable
> >  from the current HEAD ?
> 
> test $(git merge-base HEAD Y) = $(git rev-parse Y)

Almost.  It works as long as there is only one merge base.  You really
need to check if $(git rev-parse Y) is one of $(git merge-base --all
HEAD Y)

if HEAD is a named branch, you can do

  git name-rev --refs=refs/heads/branchname Y

which will give you Y relative to branchname if it is contained within
it.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply

* Re: [BUG] Git bisect not finding the right commit
From: Junio C Hamano @ 2012-01-20 18:09 UTC (permalink / raw)
  To: git; +Cc: Andreas J. Koenig
In-Reply-To: <87ehuu958d.fsf@franz.ak.mind.de>

andreas.koenig.7os6VVqR@franz.ak.mind.de (Andreas J. Koenig) writes:

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

(This is somewhat off-topic, so Andreas is on Cc: and the list is on To:)

I doubt --simplify-by-decoration alone would make it easier to view such a
complex and long history, but I wonder if we can use the same logic to
help users in a case like this. Instead of only keeping tagged versions in
the result to show topology, perhaps we can allow the user to feed a list
of "key points in history" as command line arguments and apply the same
kind of simplification to help visualizing the topology?

^ permalink raw reply

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

On Thu, 19 Jan 2012 13:26:40 -0800
Junio C Hamano <gitster@pobox.com> wrote:

> Dieter Plaetinck <dieter@plaetinck.be> writes:
> 
> > So basically, if this tool needs to check which files
> > still/no-longer exist before calling git-add, that's vulnerable to
> > race conditions.
> 
> I do not think you are solving the real problem in your script even
> if you allowed "add --ignore-missing".
> 
> I suspect you are making things even worse by using
> "--ignore-missing" in your script. If a user is actively updating the
> files in the filesystem, at least "git add" without
> "--ignore-missing" would catch the case where you _thought_ the user
> modified but still has the file, but in reality the further updates
> in the working tree removed the file, which is a clear indication
> that the rate you are processing the notify stream is slower than the
> activity generated by the user and allows you to notice that you may
> be better off waiting a bit until things calm down before running
> your automated commit.

I don't understand what you mean. if this happened:
1) modify file
2) modify file
3) remove file
but my script tries to git add --ignore-missing, when 3) already happened
but the script only sees event 2)
then it will not fail, then catch the 3rd event, then do a git rm.
if however, i don't enable ignore-missing, there's a failure on the git add.
I guess what you're saying is "better to get an error, interpret it as `this may not be a "real" error`, just continue"

> 
> Also, with or without "--ignore-missing", I think we have safety
> valves to cause "git add" fail if the file being added is updated
> while git is working on it (i.e. we read and compute the object name,
> and then store it compressed, and check the hash of what is stored
> matches the object name we computed earlier, which would fail if the
> file is updated in the middle at the right time).
> 
> This means that the "--ignore-missing" option will _not_ eliminate all
> cases where "git add" may detect an error and fails. In other words,
> your script needs to deal with error return from "git add" anyway
> even if we applied your patch and you used "--ignore-missing" in your
> script.
> 
> I have to say that the basic premise of your script is simply broken,
> and I am afraid that it is unfixable without an atomic snapshot
> support from the underlying filesystem (i.e. take a snapshot, run
> 'git add' on it, and then release the snapshot).

I assumed that `git add` works atomically. (as in: if you git add a file and
modify that file at the same time, either the old or the new version will be added
to the index, but always successfully).
If I understand this correctly, `git add` can only be successful if at least the file
remains untouched while the git command runs.
This means I should change my entire approach and be aware that git may return failures when the user is changing files while we are adding to the index. 
Maybe I should do something like:
once >0 inotify events happened,
* run git status, for each file in git status, if we've seen events, try to add file to the index.
if the above fails, wait a few seconds, then try again. if failures persist a few times in a row, only then we can be reasonbly sure something is really wrong.

but there will always be the case where a file gets deleted and (re)added, there is always the risk for races:
1) my script runs "git rm" after seeing a "delete" event but the file has been added in the meanwhile... git removes the file => bad
2) my script runs "git add" after seeing a new/changed file but the file has been removed in the meanwhile... git gives an error.
and in the latter case you can't just apply the "just retry" trick I mentioned above because the next event is delete, which would trigger a "git rm",
potentially causing unrecoverable data loss as described in 1).

one example of such things happening in real life is vim which creates (and removes) tmp files called `4913`.
We could just add such filenames to gitignore,
but that seems like a bit of a burden which shouldn't be needed.

To avoid the risks mentioned above and the "file can be altered during git add command",
what i'm really looking for is a way to, for a given file (file for which I've seen inotify events):
synchronize changes of that file to the index even if the file was unknown to git, or doesn't exist anymore;
and do that in an atomic way (or: i'll just retry a few times until it fails consistently, but removal race conditions can lead to data loss)

I tried `git update-index` as you suggested but couldn't have it behave like that.
And something like `git add --all` can still cause a file removal when it shouldn't, and lead to uncoverable data loss on race conditions.

Sorry for the long mail, I tried to keep it as minimal as possible.

thanks!
Dieter

^ 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