* 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
* Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default
From: Tay Ray Chuan @ 2012-01-20 1:14 UTC (permalink / raw)
To: Thomas Rast; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <87bopzofir.fsf@thomas.inf.ethz.ch>
On Thu, Jan 19, 2012 at 11:53 PM, Thomas Rast <trast@student.ethz.ch> wrote:
>[snip]
> Under [^[:space:]]+ neither of the examples would work. Actually,
> [^[:space:]]+ is the same as today's default, the [^[:space:]]* I
> mentioned later is (strictly speaking) broken as it allows for a
> 0-length match. (It doesn't really matter because IIRC the engine
> ignores 0-length words.)
My bad.
>[snip]
> I tried measuring it across a few commits, but it mostly gets drowned
> out by the diff effort. For a commit with stat
>
> exercises/cgal/cover/cover.cpp | 5 +-
> exercises/cgal/cover/cover.in1 |27014 +++++++++++++++-----
> exercises/cgal/cover/cover.in2 |48996 +++++++++++++++++++++++------------
> exercises/cgal/cover/cover.in3 |55041 +++++++++++++++++++++++++--------------
> exercises/cgal/cover/cover.in4 |47600 ++++++++++++++++++++--------------
> exercises/cgal/cover/cover.int |43491 ++++++++++++++++++++++---------
> exercises/cgal/cover/cover.out1 | 53 +-
> exercises/cgal/cover/cover.out2 | 24 +-
> exercises/cgal/cover/cover.out3 | 11 +-
> exercises/cgal/cover/cover.out4 | 2 +-
> exercises/cgal/cover/cover.outt | 23 +-
> exercises/cgal/cover/gen | 39 +-
> exercises/cgal/cover/gen-1.cpp | 4 +-
> exercises/cgal/cover/gen-2.cpp | 6 +-
> exercises/cgal/cover/gen-3.cpp | 6 +-
>
> (sorry, can't share as those testcases are secret) I get best-of-5
> timings
>
> --word-diff-regex='[^[:space:]]+' 0:07.50real 7.40user 0.07system
> --word-diff 0:07.47real 7.41user 0.03system
>
> In conclusion, "meh". I think ripping out the isspace() part would make
> for a nice code reduction.
Thanks for the numbers. Well, that agrees with the intuition that
regex is slower than isspace(), since you have run it through the
regex engine.
>>> and your proposal is equivalent to
>>>
>>> [^[:space:]]|UTF_8_GUARD
>>>
>>> I think there is a case to be made for a default of
>>>
>>> [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+
>>>
>>> or some such. There's a lot of bikeshedding lurking in the (non)extent
>>> of the [[:alnum:]] here, however.
>>
>> Care to explain further? Not to sure what you mean here.
>
> For natural language, it may or may not make sense to match numbers as
> part of a word.
>
> For typical use in e.g. emails, a lot of punctuation has a double role;
> breaking words in
>
> http://article.gmane.org/gmane.comp.version-control.git/188391
>
> may or may not make sense.
>
> For some uses, especially source code, it would be better to match an
> underscore _ as part of a complete word, too.
>
> For some programming languages, say lisp, a dash - would also belong in
> the same category.
>
> There's no real reason other than ease of implementation why the pattern
> handles ASCII non-alphanumerics separately, but non-ASCII UTF-8
> non-alnums (like, say, unicode NO-BREAK SPACE which would show as \xc2
> \xa0) always goes into a word. But if you were to make UTF-8 sequences
> a single word, text in (say) many European languages would become
> chunked at accented letters.
>
> I'm sure you can find more items for this list. It's a grey area.
Thanks.
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Santi Béjar @ 2012-01-19 23:31 UTC (permalink / raw)
To: mike; +Cc: demerphq, git
In-Reply-To: <CADo4Y9is9mBOJaU+YRTMedTz7FfDrMFoDiqiUvQpVxQpyariPQ@mail.gmail.com>
On Thu, Jan 19, 2012 at 7:48 PM, Michael Nahas <mike.nahas@gmail.com> wrote:
> On Thu, Jan 19, 2012 at 1:12 PM, demerphq <demerphq@gmail.com> wrote:
>> Stop using version numbers and start using the git sha1 of the code
>> you are using.
>>
>> Yves
>
[...]
> 2. The version number needs to be increasing, to work with the current
> process. SHA1's are random.
Yes, but you can use "git describe" output:
$ git describe
v1.7.6-180-gdf3f3d8
HTH,
Santi
^ permalink raw reply
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-01-19 22:57 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1327011899-18883-1-git-send-email-spearce@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> 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>
> ---
Anybody wants to add a simple test for this failure mode?
> remote-curl.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 48c20b8..d6054e2 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,11 @@ static void parse_push(struct strbuf *buf)
> break;
> } while (1);
>
> - if (push(nr_spec, specs))
> + ret = push(nr_spec, specs);
> + xwrite(1, "\n", 1);
> + if (ret)
> exit(128); /* error already reported */
>
> - printf("\n");
> - fflush(stdout);
> -
This is not a fault of this patch, but could we fix this ugly mixture of
xwrite() and printf() in the same program? I can see that the loop in the
main() function carefully tries to call fflush(stdout) to make sure that
nothing is pending after processing a single command so using xwrite() may
not cause any harm here, but the thing is that you do not check the error
return from this xwrite(), so use of it is not giving us any potential
benefit of being able to detect I/O errors in a finer grained manner,
i.e. it is no better than the printf("\n"); fflush(stdout); sequence it
replaces.
Thanks.
^ permalink raw reply
* [PATCH] remote-curl: Fix push status report when all branches fail
From: Shawn O. Pearce @ 2012-01-19 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
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 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 48c20b8..d6054e2 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,11 @@ static void parse_push(struct strbuf *buf)
break;
} while (1);
- if (push(nr_spec, specs))
+ ret = push(nr_spec, specs);
+ xwrite(1, "\n", 1);
+ if (ret)
exit(128); /* error already reported */
- printf("\n");
- fflush(stdout);
-
free_specs:
for (i = 0; i < nr_spec; i++)
free(specs[i]);
--
1.7.8.4.dirty
^ permalink raw reply related
* Re: More support on branch description?
From: Junio C Hamano @ 2012-01-19 22:22 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <CACsJy8D0_EB6jN7KxpzLtnPnj0HjdU6sNHJRyqXJf-2-ZNatFA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> The coming v1.7.9 will introduce branch description, mainly used in
> integration process. I think we could make it useful for users who
> don't extensively use request-pull/format-patch. Showing a short
> summary along with branch name in "git branch" would be nice.
I agree that it would be nice to give users access to the information even
if the branch ends up being merged to the master branch by you and never
leaves your repository by itself.
You however are misguided to say "Showing a short summary along...".
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.
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.
^ permalink raw reply
* Re: Unexpected "clean -Xd" behavior
From: pgit @ 2012-01-19 22:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, Pete Harlan, Git Mailing List, Shawn Bohrer
In-Reply-To: <CACsJy8AE+rwmOVUZez5GRXRHJsTy+W8ekzr59NTd7_C+gB0Byw@mail.gmail.com>
Thank you very much for looking at this.
2012/1/19 "Nguyen Thai Ngoc Duy" <pclouds@gmail.com>:
> 2012/1/19 Jonathan Nieder <jrnieder@gmail.com>:
>> Pete Harlan wrote:
>>
>>> When a directory contains nothing but an ignored subdirectory, that
>>> subdirectory does not get removed by "git clean -Xdf".
>>>
>>> For example, in a new directory:
>>>
>>> # git init
>>> Initialized empty Git repository in /tmp/foo/.git/
>>> # echo a/ >.gitignore
>>> # git add .gitignore
>>> # git commit -m "Initial commit"
>>> [master (root-commit) c3af24c] Initial commit
>>> Â 1 files changed, 1 insertions(+), 0 deletions(-)
>>> Â create mode 100644 .gitignore
>>> # mkdir -p foo/a
>>> # touch foo/a/junk.o
>>> # git status
>>> # On branch master
>>> nothing to commit (working directory clean)
>>> # git clean -Xdn  # <--- DOES NOT MENTION foo/a
>
> -X is to remove ignored files _only_ (DIR_SHOW_IGNORED flag). And
> "foo" is not ignored according to .gitignore, so it cuts short there
> and never gets to "foo/a". -x works.
But the presence of a tracked file in foo makes it not cut short there, so
the logic seems a bit off. (If we're interested in removing ignored files
only, then the ignored files (not a tracked file) should trigger us
looking into foo. I don't know Git internals but I'm guessing it's not
quite that simple.)
> May be intentional, may be not
> (we hit a corner case). I don't know. Commit message b991625 might
> help:
>
> dir.c: Omit non-excluded directories with dir->show_ignored
>
> This makes "git-ls-files --others --directory --ignored" behave
> as documented and consequently also fixes "git-clean -d -X".
> Previously, git-clean would remove non-excluded directories
> even when using the -X option.
It can (and does) leave foo behind (because it's not ignored), but it
would conform better to the -X documentation if the ignored files were
removed.
BTW the above commit doesn't affect the behavior in this example.
If a fix isn't desirable then as Jonathan said updating the documentation
makes sense. (And those of us using it as a poor man's "make clean" can
just fix our Makefiles instead...)
Thanks,
--Pete
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox