git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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

* 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: 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

* 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: [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

* [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] 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: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: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

* 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: 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: 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: [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

* [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: 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] 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] 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

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

* [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: 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

* 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: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-01-20  6:00 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <1327029129-11424-1-git-send-email-spearce@spearce.org>

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

> diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
> index 9b85d42..4723930 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"
> +
> +cat >exp <<EOF
> +remote: error: hook declined to update refs/heads/dev2        

Curious. Where do we get these eight trailing whitespaces?

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

^ permalink raw reply

* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-01-20  5:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1327029129-11424-1-git-send-email-spearce@spearce.org>

Thanks; will queue.

^ permalink raw reply

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

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..4723930 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"
+
+cat >exp <<EOF
+remote: error: hook declined to update refs/heads/dev2        
+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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).