* Re: [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Adam Spiers @ 2013-02-20 1:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Zoltan Klinger
In-Reply-To: <7vfw0s3qsq.fsf@alter.siamese.dyndns.org>
On Tue, Feb 19, 2013 at 02:03:01PM -0800, Junio C Hamano wrote:
> I started to suspect that may be the right approach. Why not do this?
>
> -- >8 --
> From: Junio C Hamano <gitster@pobox.com>
> Date: Tue, 19 Feb 2013 11:56:44 -0800
> Subject: [PATCH] name-hash: allow hashing an empty string
>
> Usually we do not pass an empty string to the function hash_name()
> because we almost always ask for hash values for a path that is a
> candidate to be added to the index. However, check-ignore (and most
> likely check-attr, but I didn't check) apparently has a callchain
> to ask the hash value for an empty path when it was given a "." from
> the top-level directory to ask "Is the path . excluded by default?"
According to a single gdb run, 'git check-attr -a .' does not hit
hash_name() for me. However that naive experiment doesn't rule out
the possibility of it happening if the right attributes are set, but I
don't know enough about that code to comment.
> Make sure that hash_name() does not overrun the end of the given
> pathname even when it is empty.
>
> Remove a sweep-the-issue-under-the-rug conditional in check-ignore
> that avoided to pass an empty string to the callchain while at it.
> It is a valid question to ask for check-ignore if the top-level is
> set to be ignored by default, even though the answer is most likely
Hmm, I see very little difference between the use of "most likely" and
the use of the words "much" and "typically" which you previously
considered "a sure sign that the design of the fix is iffy".
> no, if only because there is currently no way to specify such an
> entry in the .gitignore file. But it is an unusual thing to ask and
> it is not worth optimizing for it by special casing at the top level
> of the call chain.
Although I agree with your proposed patch's sentiment of avoiding
sweeping this corner case under the rug, 'check-ignore .' still
wouldn't match anything if for example './' was a supported mechanism
for ignoring the top level. So, modulo the obvious advantages that
defensive coding in hash_name() bring, I'm struggling to see a
significant difference between any of the three patches proposed so
far. All of them fix the segfault, and all would require the same
amount of extra work in order to support matching against './'.
But I don't have any strong objections either, so you have my approval
for whichever patch you prefer to take. Thanks.
^ permalink raw reply
* Re: Re* [PATCH 2/2] check-ignore.c: fix segfault with '.' argument from repo root
From: Adam Spiers @ 2013-02-20 2:00 UTC (permalink / raw)
To: git list
In-Reply-To: <7v1ucc5b7n.fsf_-_@alter.siamese.dyndns.org>
On Tue, Feb 19, 2013 at 11:56:44AM -0800, Junio C Hamano wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
> > Fair enough. I'll reply to this with a new version.[0]
> >
> > [0] I wish there was a clean way to include the new version inline,
> > but as I've noted before, there doesn't seem to be:
> >
> > http://article.gmane.org/gmane.comp.version-control.git/146110
>
> I find it easier to later find the patch if you made it a separate
> follow-up like you did, but you can do it this way if you really
> want to, using a scissors line, like so. Please do not try to be
> creative and change the shape of scissors just for the sake of
> chaning it.
[snipped]
OK, thanks for the information. IMHO it would be nice if 'git
format-patch' and 'git am' supported this style of inline patch
inclusion, but maybe there are good reasons to discourage it?
^ permalink raw reply
* Re: [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Junio C Hamano @ 2013-02-20 2:47 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list, Zoltan Klinger
In-Reply-To: <20130220015723.GB7860@pacific.linksys.moosehall>
Adam Spiers <git@adamspiers.org> writes:
>> Remove a sweep-the-issue-under-the-rug conditional in check-ignore
>> that avoided to pass an empty string to the callchain while at it.
>> It is a valid question to ask for check-ignore if the top-level is
>> set to be ignored by default, even though the answer is most likely
>> no, if only because there is currently no way to specify such an
>
> Hmm, I see very little difference between the use of "most likely" and
> the use of the words "much" and "typically" which you previously
> considered "a sure sign that the design of the fix is iffy".
Your patch were "The reason why feeding empty string upsets
hash_name() were not investigated; by punting the '.' as input, and
ignoring the possibility that such a question might make sense, I
can work around the segfault. I do not even question if hash_name()
that misbehaves on an empty string is a bug. Just make sure we do
not tickle the function with a problematic input".
The patch you are responding to declares that hash_name() should
work sensibly on an empty string, and that is the _only_ necessary
change for the fix. We could keep "&& path[0]", but even without
it, by fixing the hash_name(), we will no longer segfault.
My "most likely" is about "the special case '&& path[0]' produces
correct result, and it is likely to stay so in the near future until
we update .gitignore format to allow users to say 'ignore the top by
default', which is not likely to happen soon". It is not about the
nature of the fix at all.
Still do not see the difference?
The removal of the "&& path[0]" is about allowing such a question
whose likeliness may be remote. In the current .gitignore format,
you may not be able to say "ignore the whole thing by default", so
in that sense, the answer to the question this part of the code is
asking when given "." may always be "no". Keeping the "&& path[0]"
will optimize for that case.
And "unusual thing to ask" below is to judge if answering such a
question is worth optimizing for (the verdict is "no, it is not a
common thing to do").
>> entry in the .gitignore file. But it is an unusual thing to ask and
>> it is not worth optimizing for it by special casing at the top level
>> of the call chain.
>
> Although I agree with your proposed patch's sentiment of avoiding
> sweeping this corner case under the rug, 'check-ignore .' still
> wouldn't match anything if for example './' was a supported mechanism
> for ignoring the top level.
It indicates that there may be more bugs (that may not result in
segv) somewhere in check-ignore codepath, if (1)
echo ./ >.gitignore
were to say "ignore everything in the tree by default.", and (2) the
real ignore check does work that way, but (3)
git check-ignore .
says "we do not ignore that one". Such a bug may come from some
code that is not prepared to see an empty pathname that refers to
the top-level in the codepath, which was why I originally asked
Does the callchain that goes down to this function have other places
that assume they will never see an empty string, like this function
does, which I _think_ is the real issue?
in one of the previous messages.
But is
echo ./ >.gitignore
a way to say "ignore everything in the tree by default" in the first
place? I think historically that has never been the case, I recall
that the list had discussions on that topic in the past (it may be
before you appeared here), and I do not recall we reached a concensus
that we should make it mean that nor applied a patch to do so.
^ permalink raw reply
* Re: Re* [PATCH 2/2] check-ignore.c: fix segfault with '.' argument from repo root
From: Junio C Hamano @ 2013-02-20 2:53 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <20130220020046.GC7860@pacific.linksys.moosehall>
Adam Spiers <git@adamspiers.org> writes:
> OK, thanks for the information. IMHO it would be nice if 'git
> format-patch' and 'git am' supported this style of inline patch
> inclusion, but maybe there are good reasons to discourage it?
"git am --scissors" is a way to process such e-mail where the patch
submitter continues discussion in the top part of a message,
concludes the message with:
A patch to do so is attached.
-- >8 --
and then tells the MUA to read in an output from format-patch into
the e-mail buffer. You still need to strip out unneeded headers
like the "From ", "From: " and "Date: " lines when you add the
scissors anyway, and this is applicable only for a single-patch
series, so the "feature" does not fit well as a format-patch option.
^ permalink raw reply
* [PATCH v2 1/4] difftool: silence uninitialized variable warning
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
Git::config() returns `undef` when given keys that do not exist.
Check that the $guitool value is defined to prevent a noisy
"Use of uninitialized variable $guitool in length" warning.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Unchanged from last time but included in the series for convenience.
git-difftool.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..12231fb 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -336,7 +336,7 @@ sub main
}
if ($opts{gui}) {
my $guitool = Git::config('diff.guitool');
- if (length($guitool) > 0) {
+ if (defined($guitool) && length($guitool) > 0) {
$ENV{GIT_DIFF_TOOL} = $guitool;
}
}
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related
* [PATCH v2 2/4] t7800: update copyright notice
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <1361338528-17835-1-git-send-email-davvid@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
t/t7800-difftool.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index eb1d3f8..5b5939b 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -1,6 +1,6 @@
#!/bin/sh
#
-# Copyright (c) 2009, 2010 David Aguilar
+# Copyright (c) 2009, 2010, 2012, 2013 David Aguilar
#
test_description='git-difftool
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related
* [PATCH v2 3/4] t7800: modernize tests
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <1361338528-17835-2-git-send-email-davvid@gmail.com>
Eliminate a lot of redundant work by using test_config().
Catch more return codes by more use of temporary files
and test_cmp.
The original tests relied upon restore_test_defaults()
from the previous test to provide the next test with a sane
environment. Make the tests do their own setup so that they
are not dependent on the success of the previous test.
The end result is shorter tests and better test isolation.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
We no longer export variables into the environment per Jonathan's
suggestion. This covers all of the review notes.
t/t7800-difftool.sh | 360 ++++++++++++++++++++++++----------------------------
1 file changed, 165 insertions(+), 195 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 5b5939b..b577c01 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -10,29 +10,11 @@ Testing basic diff tool invocation
. ./test-lib.sh
-remove_config_vars()
+difftool_test_setup()
{
- # Unset all config variables used by git-difftool
- git config --unset diff.tool
- git config --unset diff.guitool
- git config --unset difftool.test-tool.cmd
- git config --unset difftool.prompt
- git config --unset merge.tool
- git config --unset mergetool.test-tool.cmd
- git config --unset mergetool.prompt
- return 0
-}
-
-restore_test_defaults()
-{
- # Restores the test defaults used by several tests
- remove_config_vars
- unset GIT_DIFF_TOOL
- unset GIT_DIFFTOOL_PROMPT
- unset GIT_DIFFTOOL_NO_PROMPT
- git config diff.tool test-tool &&
- git config difftool.test-tool.cmd 'cat $LOCAL'
- git config difftool.bogus-tool.cmd false
+ test_config diff.tool test-tool &&
+ test_config difftool.test-tool.cmd 'cat $LOCAL' &&
+ test_config difftool.bogus-tool.cmd false
}
prompt_given()
@@ -65,249 +47,237 @@ test_expect_success PERL 'setup' '
# Configure a custom difftool.<tool>.cmd and use it
test_expect_success PERL 'custom commands' '
- restore_test_defaults &&
- git config difftool.test-tool.cmd "cat \$REMOTE" &&
+ difftool_test_setup &&
+ test_config difftool.test-tool.cmd "cat \$REMOTE" &&
+ echo master >expect &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual &&
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "master" &&
-
- restore_test_defaults &&
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch"
+ test_config difftool.test-tool.cmd "cat \$LOCAL" &&
+ echo branch >expect &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual
'
-# Ensures that a custom difftool.<tool>.cmd overrides built-ins
-test_expect_success PERL 'custom commands override built-ins' '
- restore_test_defaults &&
- git config difftool.defaults.cmd "cat \$REMOTE" &&
-
- diff=$(git difftool --tool defaults --no-prompt branch) &&
- test "$diff" = "master" &&
-
- git config --unset difftool.defaults.cmd
+test_expect_success PERL 'custom tool commands override built-ins' '
+ test_config difftool.defaults.cmd "cat \$REMOTE" &&
+ echo master >expect &&
+ git difftool --tool defaults --no-prompt branch >actual &&
+ test_cmp expect actual
'
-# Ensures that git-difftool ignores bogus --tool values
test_expect_success PERL 'difftool ignores bad --tool values' '
- diff=$(git difftool --no-prompt --tool=bad-tool branch)
- test "$?" = 1 &&
- test "$diff" = ""
+ : >expect &&
+ test_expect_code 1 \
+ git difftool --no-prompt --tool=bad-tool branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool forwards arguments to diff' '
+ difftool_test_setup &&
>for-diff &&
git add for-diff &&
echo changes>for-diff &&
git add for-diff &&
- diff=$(git difftool --cached --no-prompt -- for-diff) &&
- test "$diff" = "" &&
+ : >expect &&
+ git difftool --cached --no-prompt -- for-diff >actual &&
+ test_cmp expect actual &&
git reset -- for-diff &&
rm for-diff
'
test_expect_success PERL 'difftool honors --gui' '
- git config merge.tool bogus-tool &&
- git config diff.tool bogus-tool &&
- git config diff.guitool test-tool &&
-
- diff=$(git difftool --no-prompt --gui branch) &&
- test "$diff" = "branch" &&
+ difftool_test_setup &&
+ test_config merge.tool bogus-tool &&
+ test_config diff.tool bogus-tool &&
+ test_config diff.guitool test-tool &&
- restore_test_defaults
+ echo branch >expect &&
+ git difftool --no-prompt --gui branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --gui last setting wins' '
- git config diff.guitool bogus-tool &&
- git difftool --no-prompt --gui --no-gui &&
+ difftool_test_setup &&
+ : >expect &&
+ git difftool --no-prompt --gui --no-gui >actual &&
+ test_cmp expect actual &&
- git config merge.tool bogus-tool &&
- git config diff.tool bogus-tool &&
- git config diff.guitool test-tool &&
- diff=$(git difftool --no-prompt --no-gui --gui branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config merge.tool bogus-tool &&
+ test_config diff.tool bogus-tool &&
+ test_config diff.guitool test-tool &&
+ echo branch >expect &&
+ git difftool --no-prompt --no-gui --gui branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --gui works without configured diff.guitool' '
- git config diff.tool test-tool &&
-
- diff=$(git difftool --no-prompt --gui branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ echo branch >expect &&
+ git difftool --no-prompt --gui branch >actual &&
+ test_cmp expect actual
'
# Specify the diff tool using $GIT_DIFF_TOOL
test_expect_success PERL 'GIT_DIFF_TOOL variable' '
- test_might_fail git config --unset diff.tool &&
- GIT_DIFF_TOOL=test-tool &&
- export GIT_DIFF_TOOL &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ git config --unset diff.tool &&
+ echo branch >expect &&
+ GIT_DIFF_TOOL=test-tool git difftool --no-prompt branch >actual &&
+ test_cmp expect actual
'
# Test the $GIT_*_TOOL variables and ensure
# that $GIT_DIFF_TOOL always wins unless --tool is specified
test_expect_success PERL 'GIT_DIFF_TOOL overrides' '
- git config diff.tool bogus-tool &&
- git config merge.tool bogus-tool &&
-
- GIT_DIFF_TOOL=test-tool &&
- export GIT_DIFF_TOOL &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
+ difftool_test_setup &&
+ test_config diff.tool bogus-tool &&
+ test_config merge.tool bogus-tool &&
- GIT_DIFF_TOOL=bogus-tool &&
- export GIT_DIFF_TOOL &&
+ echo branch >expect &&
+ GIT_DIFF_TOOL=test-tool git difftool --no-prompt branch >actual &&
+ test_cmp expect actual &&
- diff=$(git difftool --no-prompt --tool=test-tool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config diff.tool bogus-tool &&
+ test_config merge.tool bogus-tool &&
+ GIT_DIFF_TOOL=bogus-tool \
+ git difftool --no-prompt --tool=test-tool branch >actual &&
+ test_cmp expect actual
'
# Test that we don't have to pass --no-prompt to difftool
# when $GIT_DIFFTOOL_NO_PROMPT is true
test_expect_success PERL 'GIT_DIFFTOOL_NO_PROMPT variable' '
- GIT_DIFFTOOL_NO_PROMPT=true &&
- export GIT_DIFFTOOL_NO_PROMPT &&
-
- diff=$(git difftool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ echo branch >expect &&
+ GIT_DIFFTOOL_NO_PROMPT=true git difftool branch >actual &&
+ test_cmp expect actual
'
# git-difftool supports the difftool.prompt variable.
# Test that GIT_DIFFTOOL_PROMPT can override difftool.prompt = false
test_expect_success PERL 'GIT_DIFFTOOL_PROMPT variable' '
- git config difftool.prompt false &&
- GIT_DIFFTOOL_PROMPT=true &&
- export GIT_DIFFTOOL_PROMPT &&
-
- prompt=$(echo | git difftool branch | tail -1) &&
- prompt_given "$prompt" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt false &&
+ echo >input &&
+ GIT_DIFFTOOL_PROMPT=true git difftool branch <input >output &&
+ prompt=$(tail -1 <output) &&
+ prompt_given "$prompt"
'
# Test that we don't have to pass --no-prompt when difftool.prompt is false
test_expect_success PERL 'difftool.prompt config variable is false' '
- git config difftool.prompt false &&
-
- diff=$(git difftool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt false &&
+ echo branch >expect &&
+ git difftool branch >actual &&
+ test_cmp expect actual
'
# Test that we don't have to pass --no-prompt when mergetool.prompt is false
test_expect_success PERL 'difftool merge.prompt = false' '
+ difftool_test_setup &&
test_might_fail git config --unset difftool.prompt &&
- git config mergetool.prompt false &&
-
- diff=$(git difftool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config mergetool.prompt false &&
+ echo branch >expect &&
+ git difftool branch >actual &&
+ test_cmp expect actual
'
# Test that the -y flag can override difftool.prompt = true
test_expect_success PERL 'difftool.prompt can overridden with -y' '
- git config difftool.prompt true &&
-
- diff=$(git difftool -y branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt true &&
+ echo branch >expect &&
+ git difftool -y branch >actual &&
+ test_cmp expect actual
'
# Test that the --prompt flag can override difftool.prompt = false
test_expect_success PERL 'difftool.prompt can overridden with --prompt' '
- git config difftool.prompt false &&
-
- prompt=$(echo | git difftool --prompt branch | tail -1) &&
- prompt_given "$prompt" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt false &&
+ echo >input &&
+ git difftool --prompt branch <input >output &&
+ prompt=$(tail -1 <output) &&
+ prompt_given "$prompt"
'
# Test that the last flag passed on the command-line wins
test_expect_success PERL 'difftool last flag wins' '
- diff=$(git difftool --prompt --no-prompt branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults &&
-
- prompt=$(echo | git difftool --no-prompt --prompt branch | tail -1) &&
- prompt_given "$prompt" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ echo branch >expect &&
+ git difftool --prompt --no-prompt branch >actual &&
+ test_cmp expect actual &&
+ echo >input &&
+ git difftool --no-prompt --prompt branch <input >output &&
+ prompt=$(tail -1 <output) &&
+ prompt_given "$prompt"
'
# git-difftool falls back to git-mergetool config variables
# so test that behavior here
test_expect_success PERL 'difftool + mergetool config variables' '
- remove_config_vars &&
- git config merge.tool test-tool &&
- git config mergetool.test-tool.cmd "cat \$LOCAL" &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
+ test_config merge.tool test-tool &&
+ test_config mergetool.test-tool.cmd "cat \$LOCAL" &&
+ echo branch >expect &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual &&
# set merge.tool to something bogus, diff.tool to test-tool
- git config merge.tool bogus-tool &&
- git config diff.tool test-tool &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config merge.tool bogus-tool &&
+ test_config diff.tool test-tool &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool.<tool>.path' '
- git config difftool.tkdiff.path echo &&
- diff=$(git difftool --tool=tkdiff --no-prompt branch) &&
- git config --unset difftool.tkdiff.path &&
- lines=$(echo "$diff" | grep file | wc -l) &&
- test "$lines" -eq 1 &&
-
- restore_test_defaults
+ test_config difftool.tkdiff.path echo &&
+ git difftool --tool=tkdiff --no-prompt branch >output &&
+ lines=$(grep file output | wc -l) &&
+ test "$lines" -eq 1
'
test_expect_success PERL 'difftool --extcmd=cat' '
- diff=$(git difftool --no-prompt --extcmd=cat branch) &&
- test "$diff" = branch"$LF"master
+ echo branch >expect &&
+ echo master >>expect &&
+ git difftool --no-prompt --extcmd=cat branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd cat' '
- diff=$(git difftool --no-prompt --extcmd cat branch) &&
- test "$diff" = branch"$LF"master
+ echo branch >expect &&
+ echo master >>expect &&
+ git difftool --no-prompt --extcmd=cat branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool -x cat' '
- diff=$(git difftool --no-prompt -x cat branch) &&
- test "$diff" = branch"$LF"master
+ echo branch >expect &&
+ echo master >>expect &&
+ git difftool --no-prompt -x cat branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd echo arg1' '
- diff=$(git difftool --no-prompt --extcmd sh\ -c\ \"echo\ \$1\" branch) &&
- test "$diff" = file
+ echo file >expect &&
+ git difftool --no-prompt \
+ --extcmd sh\ -c\ \"echo\ \$1\" branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd cat arg1' '
- diff=$(git difftool --no-prompt --extcmd sh\ -c\ \"cat\ \$1\" branch) &&
- test "$diff" = master
+ echo master >expect &&
+ git difftool --no-prompt \
+ --extcmd sh\ -c\ \"cat\ \$1\" branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd cat arg2' '
- diff=$(git difftool --no-prompt --extcmd sh\ -c\ \"cat\ \$2\" branch) &&
- test "$diff" = branch
+ echo branch >expect &&
+ git difftool --no-prompt \
+ --extcmd sh\ -c\ \"cat\ \$2\" branch >actual &&
+ test_cmp expect actual
'
# Create a second file on master and a different version on branch
@@ -324,26 +294,26 @@ test_expect_success PERL 'setup with 2 files different' '
'
test_expect_success PERL 'say no to the first file' '
- diff=$( (echo n; echo) | git difftool -x cat branch ) &&
-
- echo "$diff" | stdin_contains m2 &&
- echo "$diff" | stdin_contains br2 &&
- echo "$diff" | stdin_doesnot_contain master &&
- echo "$diff" | stdin_doesnot_contain branch
+ (echo n && echo) >input &&
+ git difftool -x cat branch <input >output &&
+ cat output | stdin_contains m2 &&
+ cat output | stdin_contains br2 &&
+ cat output | stdin_doesnot_contain master &&
+ cat output | stdin_doesnot_contain branch
'
test_expect_success PERL 'say no to the second file' '
- diff=$( (echo; echo n) | git difftool -x cat branch ) &&
-
- echo "$diff" | stdin_contains master &&
- echo "$diff" | stdin_contains branch &&
- echo "$diff" | stdin_doesnot_contain m2 &&
- echo "$diff" | stdin_doesnot_contain br2
+ (echo && echo n) >input &&
+ git difftool -x cat branch <input >output &&
+ cat output | stdin_contains master &&
+ cat output | stdin_contains branch &&
+ cat output | stdin_doesnot_contain m2 &&
+ cat output | stdin_doesnot_contain br2
'
test_expect_success PERL 'difftool --tool-help' '
- tool_help=$(git difftool --tool-help) &&
- echo "$tool_help" | stdin_contains tool
+ git difftool --tool-help >output &&
+ cat output | stdin_contains tool
'
test_expect_success PERL 'setup change in subdirectory' '
@@ -359,29 +329,29 @@ test_expect_success PERL 'setup change in subdirectory' '
'
test_expect_success PERL 'difftool -d' '
- diff=$(git difftool -d --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool -d --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
'
test_expect_success PERL 'difftool --dir-diff' '
- diff=$(git difftool --dir-diff --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool --dir-diff --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
'
test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
- diff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool --dir-diff --prompt --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
'
test_expect_success PERL 'difftool --dir-diff from subdirectory' '
(
cd sub &&
- diff=$(git difftool --dir-diff --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool --dir-diff --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
)
'
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related
* [PATCH v2 4/4] t7800: "defaults" is no longer a builtin tool name
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <1361338528-17835-3-git-send-email-davvid@gmail.com>
073678b8e6324a155fa99f40eee0637941a70a34 reworked the
mergetools/ directory so that every file corresponds to a
difftool-supported tool. When this happened the "defaults"
file went away as it was no longer needed by mergetool--lib.
t7800 tests that configured commands can override builtins,
but this test was not adjusted when the "defaults" file was
removed because the test continued to pass.
Adjust the test to use the everlasting "vimdiff" tool name
instead of "defaults" so that it correctly tests against a tool
that is known by mergetool--lib.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Rebased for the new parent patch.
t/t7800-difftool.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index b577c01..18df2c8 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -60,9 +60,9 @@ test_expect_success PERL 'custom commands' '
'
test_expect_success PERL 'custom tool commands override built-ins' '
- test_config difftool.defaults.cmd "cat \$REMOTE" &&
+ test_config difftool.vimdiff.cmd "cat \$REMOTE" &&
echo master >expect &&
- git difftool --tool defaults --no-prompt branch >actual &&
+ git difftool --tool vimdiff --no-prompt branch >actual &&
test_cmp expect actual
'
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related
* Re: [PATCH] l10n: de.po: translate 5 new messages
From: Ralf Thielow @ 2013-02-20 5:51 UTC (permalink / raw)
To: Thomas Rast; +Cc: jk, stimming, git
In-Reply-To: <87ehgcoyxm.fsf@pctrast.inf.ethz.ch>
Hi,
2013/2/19 Thomas Rast <trast@inf.ethz.ch>:
> Ralf Thielow <ralf.thielow@gmail.com> writes:
>
>> msgid "You are currently bisecting branch '%s'."
>> -msgstr "Sie sind gerade beim Halbieren."
>> +msgstr "Sie sind gerade beim Halbieren in Zweig '%s'."
>
> I know this one is already in other messages (and also in the Glossary),
> but I still find it iffy and I might finally have a better idea:
>
> Sie sind gerade an einer binären Suche in Zweig '%s'.
>
> [note to English speakers: I'm just using "binary search" instead of
> "bisect"]
>
> That makes it quite a bit harder to use it in a verbed[1] form, but I
> think it gets the concept across much better. (And in all the usage I
> know in CS, the two things refer to the same.)
>
Very good idea! Thank you. I'll create a patch on top and update the
glossary.
> ACK on the rest.
>
Thanks for review.
>
> Footnotes:
> [1] Verbing weirds language. -- Calvin
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: Google Summer of Code 2013 (GSoC13)
From: Christian Couder @ 2013-02-20 6:17 UTC (permalink / raw)
To: Jens Lehmann
Cc: Jonathan Nieder, Jeff King, Thomas Rast, git, Shawn Pearce,
Jakub Narebski, Pat Thoyts, Paul Mackerras,
Carlos Martín Nieto, Thomas Gummerer, David Michael Barr,
Ramkumar Ramachandra, Nguyen Thai Ngoc Duy
In-Reply-To: <512288B9.6010108@web.de>
On Mon, Feb 18, 2013 at 9:02 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 18.02.2013 20:34, schrieb Jonathan Nieder:
>> That said, I won't have time to mentor a project on my own. It takes
>> a lot of time (or luck, to get the student that doesn't need
>> mentoring).
>
> That's my experience too. Also I think it really makes sense to have a
> co-mentor so you can balance the load a bit.
>
>> I'd be happy to help on a project with 1 or 2 co-mentors.
>
> Same here.
I am ok to be mentor or co-mentor.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Anders Kaseorg @ 2013-02-20 6:20 UTC (permalink / raw)
To: Michael Haggerty
Cc: David Aguilar, Junio C Hamano, Jiang Xin, Lea Wiemann,
David Reiss, Johannes Sixt, git, Lars R. Damerow, Jeff King,
Marc Jordan
In-Reply-To: <508E0FAC.5050600@alum.mit.edu>
On 10/29/2012 01:10 AM, Michael Haggerty wrote:
> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
> slowdown?
Sorry to bring up this old thread again, but I just realized why my
computer has been acting so slow when I’m not connected to the network.
I put various network filesystem paths in $GIT_CEILING_DIRECTORIES,
such as /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its
parents /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and
/afs/athena.mit.edu/user/a/n which all live in different AFS volumes).
Now when I’m not connected to the network, every invocation of Git,
including the __git_ps1 in my shell prompt, waits for AFS to timeout.
Obviously I’m going to stop using $GIT_CEILING_DIRECTORIES now that I
know what the problem is, but I figured you might want to know why this
feature is now useless for me.
Anders
^ permalink raw reply
* Re: Google Summer of Code 2013 (GSoC13)
From: Shawn Pearce @ 2013-02-20 6:50 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, git, Jakub Narebski, Christian Couder, Pat Thoyts,
Paul Mackerras, Carlos Martín Nieto, Thomas Gummerer,
David Michael Barr, Ramkumar Ramachandra, Jens Lehmann,
Nguyen Thai Ngoc Duy
In-Reply-To: <20130218174239.GB22832@sigill.intra.peff.net>
On Mon, Feb 18, 2013 at 9:42 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Feb 18, 2013 at 06:23:01PM +0100, Thomas Rast wrote:
>
>> * We need an org admin. AFAIK this was done by Peff and Shawn in
>> tandem last year. Would you do it again?
>
> I will do it again, if people feel strongly about Git being a part of
> it. However, I have gotten a little soured on the GSoC experience. Not
> because of anything Google has done; it's a good idea, and I think they
> do a fine of administering the program. But I have noticed that the work
> that comes out of GSoC the last few years has quite often not been
> merged, or not made a big impact in the codebase, and nor have the
> participants necessarily stuck around.
This.
I actually think Git should take a year off from GSoC and not
participate. Consequently I will not be volunteering as backup org
admin.
Git has been involved since 2007. In all of that time we have had very
few student projects merge successfully into their upstream project
(e.g. git.git, JGit or libgit2) before the end of GSoC. Even fewer
students have stuck around and remained active contributors. When I
look at the amount of effort we contributors put into GSoC, I think we
are misusing our limited time and resources. The intention of the GSoC
program is to grow new open source developers, and increase our
community of contributors. Somehow I think Git is falling well short
of its potential here. This is especially true if you compare Git's
GSoC program to some other equally long-running GSoC programs.
> And I do not want to blame the students here (some of whom are on the cc
> list :) ). They are certainly under no obligation to stick around after
> GSoC ends, and I know they have many demands on their time. But I am
> also thinking about what Git wants to get out of GSoC (and to my mind,
> the most important thing is contributors).
I agree, our students have been pretty terrific. I think the
shortcomings in our GSoC program are on the mentoring side. Our
program has not really had much success with keeping students active
and engaged post GSoC. I see that primarily as a mentoring failure.
And its one we keep repeating each year.
> As far as merged code, I think part of the problem is that git is fairly
> mature at this point. The most interesting projects are of a bigger
> scope than a student with no experience in the code base can do in a
> summer project. Maybe that means we need to do a better job of breaking
> projects down into reasonably sized sub-components. Or maybe it means
> the project is hitting a point of diminishing returns for GSoC. I don't
> know.
Let me repeat myself. I think our GSoC program has plenty of room for
improvement on the mentoring side. Project scope and size is one of
our most common failure modes. Resumable clone keeps winding up on the
GSoC project idea list. Nobody who knows what they are talking about
has any idea how to approach this feature[1]. Suggesting it to a GSoC
student is just irresponsible[2].
I don't think Git's maturity is a road block for successful GSoC
projects. Peff's toy to insert Lua so `git log` could do fancy
formatting is an interesting one. I suspect there are still fun
archeology sorts of projects that could further improve the type of
data we can mine through log and blame. But touching the core file
formats on disk or the wire protocol is probably far too large for a
GSoC project.
[1] Android's "repo" tool and its /clone.bundle hack on HTTP
transports might work. Peff has talked about putting this into Git
itself one day. Maybe. But its still full of a ton of shortcomings and
somewhat hated by those that have to build the bundles and manage the
server infrastructure. So its probably still outside of the scope of a
successful GSoC project.
[2] I recognize and accept my share of blame for putting it on the
list a few times.
> There are a few counterpoints I can think of:
>
> - Even though not all projects are winners, _some_ are. I see Carlos
> and Ram on the cc list, two people who started as GSoC students and
> stuck around.
I think these interesting cases like Carlos and Ram are places where
the student was able to succeed almost despite our mentoring program.
I am very glad they did.
> - There is also the angle that even if _Git_ doesn't benefit directly
> from people sticking around, those people may float into other open
> source projects and work on them. Which makes the world a better
> place on the whole.
Yes, sure, OK. But if Git doesn't participate in GSoC this year
another org will, and this same benefit will still be had by the
greater open source community.
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Junio C Hamano @ 2013-02-20 6:55 UTC (permalink / raw)
To: Anders Kaseorg
Cc: Michael Haggerty, David Aguilar, Jiang Xin, Lea Wiemann,
David Reiss, Johannes Sixt, git, Lars R. Damerow, Jeff King,
Marc Jordan
In-Reply-To: <51246B31.6000605@mit.edu>
Anders Kaseorg <andersk@MIT.EDU> writes:
> On 10/29/2012 01:10 AM, Michael Haggerty wrote:
>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
>> slowdown?
>
> Sorry to bring up this old thread again, but I just realized why my
> computer has been acting so slow when I’m not connected to the
> network. I put various network filesystem paths in
> $GIT_CEILING_DIRECTORIES, such as /afs/athena.mit.edu/user/a/n/andersk
> (to avoid hitting its parents /afs/athena.mit.edu,
> /afs/athena.mit.edu/user/a, and /afs/athena.mit.edu/user/a/n which all
> live in different AFS volumes). Now when I’m not connected to the
> network, every invocation of Git, including the __git_ps1 in my shell
> prompt, waits for AFS to timeout.
Thanks for a report.
Assuming that this says "yes":
D=/afs/athena.mit.edu/user/a/n/andersk/my/dir
cd "$D"
test "$(/bin/pwd)" = "$D" && echo yes
iow, AFS mounting does not have funny symbolic link issues that make
the logical and physical PWD to be different like some automounter
implementation used to have, perhaps we can introduce a way for the
user to say "The element of this CEILING list do not have any
alias due to funny symlinks" to solve this. Perhaps existing of an
empty element in the list would do? E.g.
GIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk
or something like that. And in such a case, we do not run realpath
on the elements on the list before comparing them with what we get
from getcwd(3).
Of course, you could condionally unset the environment while
offline, but that feels like an ugly hack.
^ permalink raw reply
* Re: [PATCH v2 3/4] t7800: modernize tests
From: Junio C Hamano @ 2013-02-20 7:05 UTC (permalink / raw)
To: David Aguilar; +Cc: git, Jonathan Nieder
In-Reply-To: <1361338528-17835-3-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Eliminate a lot of redundant work by using test_config().
> Catch more return codes by more use of temporary files
> and test_cmp.
>
> The original tests relied upon restore_test_defaults()
> from the previous test to provide the next test with a sane
> environment. Make the tests do their own setup so that they
> are not dependent on the success of the previous test.
> The end result is shorter tests and better test isolation.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> We no longer export variables into the environment per Jonathan's
> suggestion. This covers all of the review notes.
>
> t/t7800-difftool.sh | 360 ++++++++++++++++++++++++----------------------------
> 1 file changed, 165 insertions(+), 195 deletions(-)
>
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 5b5939b..b577c01 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -10,29 +10,11 @@ Testing basic diff tool invocation
>
> . ./test-lib.sh
>
> +difftool_test_setup()
> {
> + test_config diff.tool test-tool &&
> + test_config difftool.test-tool.cmd 'cat $LOCAL' &&
> + test_config difftool.bogus-tool.cmd false
> }
Cute.
Are we sure that $LOCAL is free of $IFS, or is it safer to say 'cat
"$LOCAL"' or something?
> @@ -324,26 +294,26 @@ test_expect_success PERL 'setup with 2 files different' '
> '
>
> test_expect_success PERL 'say no to the first file' '
> - diff=$( (echo n; echo) | git difftool -x cat branch ) &&
> -
> - echo "$diff" | stdin_contains m2 &&
> - echo "$diff" | stdin_contains br2 &&
> - echo "$diff" | stdin_doesnot_contain master &&
> - echo "$diff" | stdin_doesnot_contain branch
> + (echo n && echo) >input &&
> + git difftool -x cat branch <input >output &&
> + cat output | stdin_contains m2 &&
> + cat output | stdin_contains br2 &&
> + cat output | stdin_doesnot_contain master &&
> + cat output | stdin_doesnot_contain branch
Do you need these pipes? In other words, wouldn't
stdin_contains whatever <output
be more straight-forward way to say these?
^ permalink raw reply
* Re: [PATCHv2 0/10] pkt-line and remote-curl cleanups server
From: Shawn Pearce @ 2013-02-20 7:05 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
On Mon, Feb 18, 2013 at 1:12 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Feb 17, 2013 at 05:41:13PM -0800, Jonathan Nieder wrote:
>
>> > I don't think so. Don't ERR lines appear inside their own packets?
>>
>> Yes, I misread get_remote_heads for some reason. Thanks for checking.
>
> Thanks for bringing it up. I had not even thought about ERR at all. So
> it was luck rather than skill that I was right. :)
>
>> I'm not sure whether servers are expected to send a flush after an
>> ERR packet. The only codepath I know of in git itself that sends
>> such packets is git-daemon, which does not flush after the error (but
>> is not used in the stateless-rpc case). http-backend uses HTTP error
>> codes for its errors.
>
> I just checked, and GitHub also does not send flush packets after ERR.
> Which makes sense; ERR is supposed to end the conversation. I can change
> GitHub, of course, but who knows what other implementations exist (e.g.,
> I do not know off-hand whether gitolite has custom ERR responses). So it
> seems pretty clear that just checking for a flush packet is not the
> right thing, and we need to actually parse the packet contents (at least
> to some degree).
JGit (and by extension Gerrit Code Review, android.googlesource.com)
sends ERR with no flush-pkt. I would like to sort of keep the protocol
this way, given how many servers in the wild are running Gerrit and
currently use ERR with no flush-pkt. IMHO its a little late to be
closing that door and stuffing a flush-pkt after the ERR that ends the
conversation.
^ permalink raw reply
* Re: [PATCHv2 0/10] pkt-line and remote-curl cleanups server
From: Shawn Pearce @ 2013-02-20 7:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git
In-Reply-To: <7v8v6mar4e.fsf@alter.siamese.dyndns.org>
On Mon, Feb 18, 2013 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Mon, Feb 18, 2013 at 01:29:16AM -0800, Junio C Hamano wrote:
>>
>>> > I just checked, and GitHub also does not send flush packets after ERR.
>>> > Which makes sense; ERR is supposed to end the conversation.
>>>
>>> Hmph. A flush packet was supposed to be a mark to say "all the
>>> packets before this one can be buffered and kept without getting
>>> passed to write(2), but this and all such buffered data _must_ go on
>>> the wire _now_". So in the sense, ERR not followed by a flush may
>>> not even have a chance to be seen on the other end, no? That is
>>> what the comment before the implementation of packet_flush() is all
>>> about.
>>
>> Despite the name, I always thought of packet_flush as more of a signal
>> for the _receiver_, who then knows that they have gotten all of a
>> particular list. In other words, we seem to use it as a sequence point
>> as much as anything (mostly because we immediately write() out any other
>> packets anyway, so there is no flushing to be done; but perhaps there
>> were originally thoughts that we could do more buffering on the writing
>> side).
>
> Exactly.
This is also my understanding of the flush-pkt ("0000"). Its an
end-of-list/end-of-section marker to let the peer know the protocol is
moving to the next state. Except where the protocol can move to the
next state without a flush-pkt of course (see below).
> The logic behind the comment "we do not buffer, but we should" is
> that flush tells the receiver that the sending end is "done" feeding
> it a class of data, and the data before flush do not have to reach
> the receiver immediately, hence we can afford to buffer on the
> sending end if we can measure that doing so would give us better
> performance. We haven't measure and turned buffering on yet.
So funny story. JGit actually buffers the pkt-line writes in memory
and does flushes to the network socket when any of the following
happen:
- fixed size in-memory buffer is full (8k or 32k by default)
- flush-pkt is needed in the protocol
- JGit forces a flush without a flush-pkt
There are places in the protocol where data needs to be shared with
the peer *despite* not having a flush-pkt present in the data stream
to do that. ERR is one of these places. "done\n" at the end of the
negotiation in a client is another. Sending ACK/NAK in a multi_ack
from upload-pack is another. I may be missing more.
JGit had to define three methods to make the pkt-line protocol work correctly:
- writeString: format a string as a single pkt-line, insert into buffer.
- end: write "000" into the buffer, flush the buffer.
- flush: flush the buffer if it has any content.
I always found that comment in front of that function funny. Its
totally not true. Fixing it is harder than just stuffing a buffer in
there and hoping for the best. The callers need work. At which point
that function isn't really what its author was trying to do.
> But when dying, we may of course have data before flushing. We may
> disconnect (by dying) without showing flush (or preceding ERR) to
> the other side, and for that reason, not relying on the flush packet
> on the receiving end is a good change. Once we turn buffering on, we
> probably need to be careful when sending an ERR indicator by making
> it always drain any buffered data and show the ERR indicator without
> buffering, or something.
Yes.
^ permalink raw reply
* Re: [PATCHv2 10/10] remote-curl: always parse incoming refs
From: Shawn Pearce @ 2013-02-20 7:41 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jeff King, git
In-Reply-To: <20130218105033.GH7049@elie.Belkin>
On Mon, Feb 18, 2013 at 1:30 AM, Jeff King <peff@peff.net> wrote:
> When remote-curl receives a list of refs from a server, it
> keeps the whole buffer intact. When we get a "list" command,
> we feed the result to get_remote_heads, and when we get a
> "fetch" or "push" command, we feed it to fetch-pack or
> send-pack, respectively.
>
> If the HTTP response from the server is truncated for any
> reason,
...
> As a result, fetch-pack hangs, waiting for input. However,
> remote-curl believes it has sent all of the advertisement,
> and therefore waits for fetch-pack to speak. The two
> processes end up in a deadlock.
Eek. Thanks for fixing this.
On Mon, Feb 18, 2013 at 2:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Jeff King wrote:
>
>> remote-curl.c | 23 +++++++++++++----------
>> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> I like.
Me too.
^ permalink raw reply
* [ANNOUNCE] Git v1.8.1.4
From: Junio C Hamano @ 2013-02-20 7:42 UTC (permalink / raw)
To: git; +Cc: Linux Kernel
The latest maintenance release Git v1.8.1.4 is now available at the
usual places.
This is primarily to tighten the host verification when imap-send is
talking to your mail server via TLS/SSL. The topic that was merged to
the tip of 'maint' track consists of 3 patches and is based on the
1.7.6 maintenance track. This is to make it easier for the distro
folks to merge the topic to their older maintenance branches to issue
hotfix binary releases if they wanted to.
This release itself also contains many small updates to the
user-manual.
The release tarballs are found at:
http://code.google.com/p/git-core/downloads/list
and their SHA-1 checksums are:
553191fe02cfac77386d5bb01df0a79eb7f163c8 git-1.8.1.4.tar.gz
bb71df6bc1fdb55b45c59af83102e901d484ef53 git-htmldocs-1.8.1.4.tar.gz
98c41b38d02f09e1fcde335834694616d0a615f7 git-manpages-1.8.1.4.tar.gz
Also the following public repositories all have a copy of the v1.8.1.4
tag and the maint branch that the tag points at:
url = git://repo.or.cz/alt-git.git
url = https://code.google.com/p/git-core/
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
Git 1.8.1.4 Release Notes
=========================
Fixes since v1.8.1.3
--------------------
* "git imap-send" talking over imaps:// did make sure it received a
valid certificate from the other end, but did not check if the
certificate matched the host it thought it was talking to.
Also contains various documentation fixes.
----------------------------------------------------------------
Changes since v1.8.1.3 are as follows:
Junio C Hamano (2):
imap-send: move #ifdef around
Git 1.8.1.4
Matthieu Moy (1):
Replace filepattern with pathspec for consistency
Oswald Buddenhagen (2):
imap-send: the subject of SSL certificate must match the host
imap-send: support subjectAltName as well
W. Trevor King (9):
user-manual: Fix 'both: so' -> 'both; so' typo
user-manual: Fix 'http' -> 'HTTP' typos
user-manual: Fix 'you - Git' -> 'you--Git' typo
user-manual: Rewrite git-gc section for automatic packing
user-manual: use 'remote add' to setup push URLs
user-manual: give 'git push -f' as an alternative to +master
user-manual: mention 'git remote add' for remote branch config
user-manual: use 'git config --global user.*' for setup
user-manual: use -o latest.tar.gz to create a gzipped tarball
^ permalink raw reply
* What's cooking in git.git (Feb 2013, #08; Tue, 19)
From: Junio C Hamano @ 2013-02-20 7:44 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
On the 'maint' front, a maintenance release 1.8.1.4 is out. The
same fixes are also included in the 'master' and upwards.
The tip of the 'master' is a bit past 1.8.2-rc0; new topics that are
not listed in this report are likely to be too late for the upcoming
release.
You can find the changes described here in the integration branches of the
repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* as/check-ignore (2013-02-19) 2 commits
- name-hash: allow hashing an empty string
- t0008: document test_expect_success_multi
"git check-ignore ." segfaulted, as a function it calls deep in its
callchain took a string in the <ptr, length> form but did not stop
when given an empty string.
Will merge to 'next' and will fast-track to 'master' (this is a new
feature in the upcoming release).
* bc/commit-complete-lines-given-via-m-option (2013-02-19) 4 commits
(merged to 'next' on 2013-02-19 at cf622b7)
+ Documentation/git-commit.txt: rework the --cleanup section
+ git-commit: only append a newline to -m mesg if necessary
+ t7502: demonstrate breakage with a commit message with trailing newlines
+ t/t7502: compare entire commit message with what was expected
'git commit -m "$str"' when $str was already terminated with a LF
now avoids adding an extra LF to the message.
Will merge to 'next'.
* ct/autoconf-htmldir (2013-02-19) 1 commit
- Bugfix: undefined htmldir in config.mak.autogen
An earlier change to config.mak.autogen broke a build driven by the
./configure script when --htmldir is not specified on the command
line of ./configure.
Will merge to 'next' and will fast-track to 'master' (this is a
regressin workaround).
* wk/user-manual (2013-02-19) 3 commits
(merged to 'next' on 2013-02-19 at dbc0eb2)
+ user-manual: Flesh out uncommitted changes and submodule updates
+ user-manual: Use request-pull to generate "please pull" text
+ user-manual: Reorganize the reroll sections, adding 'git rebase -i'
Further updates to the user manual.
Will merge to 'next'.
* wk/man-deny-current-branch-is-default-these-days (2013-02-18) 1 commit
- user-manual: typofix (ofthe->of the)
Will merge to 'next' and fast-track to 'master'.
--------------------------------------------------
[Graduated to "master"]
* jc/mention-tracking-for-pull-default (2013-01-31) 1 commit
(merged to 'next' on 2013-02-17 at 043624b)
+ doc: mention tracking for pull.default
We stopped mentioning `tracking` is a deprecated but supported
synonym for `upstream` in pull.default even though we have no
intention of removing the support for it.
* mm/config-intro-in-git-doc (2013-02-15) 1 commit
(merged to 'next' on 2013-02-17 at ff03eb5)
+ git.txt: update description of the configuration mechanism
--------------------------------------------------
[Stalled]
* mb/gitweb-highlight-link-target (2012-12-20) 1 commit
- Highlight the link target line in Gitweb using CSS
Expecting a reroll.
$gmane/211935
* jc/add-delete-default (2012-08-13) 1 commit
- git add: notice removal of tracked paths by default
"git add dir/" updated modified files and added new files, but does
not notice removed files, which may be "Huh?" to some users. They
can of course use "git add -A dir/", but why should they?
Resurrected from graveyard, as I thought it was a worthwhile thing
to do in the longer term.
There seems to be some interest. Let's see if it results in a solid
execution of a sensible transition plan towards Git 2.0.
* mb/remote-default-nn-origin (2012-07-11) 6 commits
- Teach get_default_remote to respect remote.default.
- Test that plain "git fetch" uses remote.default when on a detached HEAD.
- Teach clone to set remote.default.
- Teach "git remote" about remote.default.
- Teach remote.c about the remote.default configuration setting.
- Rename remote.c's default_remote_name static variables.
When the user does not specify what remote to interact with, we
often attempt to use 'origin'. This can now be customized via a
configuration variable.
Expecting a reroll.
$gmane/210151
"The first remote becomes the default" bit is better done as a
separate step.
--------------------------------------------------
[Cooking]
* da/difftool-fixes (2013-02-19) 4 commits
- t7800: "defaults" is no longer a builtin tool name
- t7800: modernize tests
- t7800: update copyright notice
- difftool: silence uninitialized variable warning
Minor maintenance updates to difftool, and updates to its tests.
* jk/smart-http-robustify (2013-02-17) 3 commits
- remote-curl: sanity check ref advertisement from server
- remote-curl: verify smart-http metadata lines
- pkt-line: teach packet_get_line a no-op mode
Parse the HTTP exchange that implements the native Git protocol as
a series of stateless RPC more carefully to diagnose protocol
breakage better.
Will merge to and hold in 'next'.
* nd/read-directory-recursive-optim (2013-02-17) 1 commit
(merged to 'next' on 2013-02-17 at 36ba9f4)
+ read_directory: avoid invoking exclude machinery on tracked files
"git status" has been optimized by taking advantage of the fact
that paths that are already known to the index do not have to be
checked against the .gitignore mechanism under some conditions.
Will cook in 'next'.
* mg/gpg-interface-using-status (2013-02-14) 5 commits
- pretty: make %GK output the signing key for signed commits
- pretty: parse the gpg status lines rather than the output
- gpg_interface: allow to request status return
- log-tree: rely upon the check in the gpg_interface
- gpg-interface: check good signature in a reliable way
Call "gpg" using the right API when validating the signature on
tags.
* jn/shell-disable-interactive (2013-02-11) 2 commits
- shell: pay attention to exit status from 'help' command
- shell doc: emphasize purpose and security model
Expecting a reroll.
$gmane/216229
* jc/fetch-raw-sha1 (2013-02-07) 4 commits
(merged to 'next' on 2013-02-14 at ffa3c65)
+ fetch: fetch objects by their exact SHA-1 object names
+ upload-pack: optionally allow fetching from the tips of hidden refs
+ fetch: use struct ref to represent refs to be fetched
+ parse_fetch_refspec(): clarify the codeflow a bit
Allows requests to fetch objects at any tip of refs (including
hidden ones). It seems that there may be use cases even outside
Gerrit (e.g. $gmane/215701).
Will cook in 'next'.
* mn/send-email-works-with-credential (2013-02-12) 6 commits
- git-send-email: use git credential to obtain password
- Git.pm: add interface for git credential command
- Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
- Git.pm: refactor command_close_bidi_pipe to use _cmd_close
- Git.pm: fix example in command_close_bidi_pipe documentation
- Git.pm: allow command_close_bidi_pipe to be called as method
Hooks the credential system to send-email.
Rerolled.
Waiting for a review.
* nd/branch-show-rebase-bisect-state (2013-02-08) 1 commit
- branch: show rebase/bisect info when possible instead of "(no branch)"
Expecting a reroll.
$gmane/215771
* nd/count-garbage (2013-02-15) 4 commits
(merged to 'next' on 2013-02-17 at b2af923)
+ count-objects: report how much disk space taken by garbage files
+ count-objects: report garbage files in pack directory too
+ sha1_file: reorder code in prepare_packed_git_one()
+ git-count-objects.txt: describe each line in -v output
Will cook in 'next'.
* tz/credential-authinfo (2013-02-05) 1 commit
- Add contrib/credentials/netrc with GPG support
A new read-only credential helper (in contrib/) to interact with
the .netrc/.authinfo files. Hopefully mn/send-email-authinfo topic
can rebuild on top of something like this.
Expecting a reroll.
$gmane/215556
* jl/submodule-deinit (2013-02-17) 1 commit
- submodule: add 'deinit' command
There was no Porcelain way to say "I no longer am interested in
this submodule", once you express your interest in a submodule with
"submodule init". "submodule deinit" is the way to do so.
* jc/remove-export-from-config-mak-in (2013-02-12) 2 commits
(merged to 'next' on 2013-02-12 at eb8af04)
+ Makefile: do not export mandir/htmldir/infodir
(merged to 'next' on 2013-02-07 at 33f7d4f)
+ config.mak.in: remove unused definitions
config.mak.in template had an "export" line to cause a few
common makefile variables to be exported; if they need to be
expoted for autoconf/configure users, they should also be exported
for people who write config.mak the same way. Move the "export" to
the main Makefile. Also, stop exporting mandir that used to be
exported (only) when config.mak.autogen was used. It would have
broken installation of manpages (but not other documentation
formats).
Will cook in 'next'.
* jc/remove-treesame-parent-in-simplify-merges (2013-01-17) 1 commit
(merged to 'next' on 2013-01-30 at b639b47)
+ simplify-merges: drop merge from irrelevant side branch
The --simplify-merges logic did not cull irrelevant parents from a
merge that is otherwise not interesting with respect to the paths
we are following.
This touches a fairly core part of the revision traversal
infrastructure; even though I think this change is correct, please
report immediately if you find any unintended side effect.
Will cook in 'next'.
* jc/push-2.0-default-to-simple (2013-01-16) 14 commits
(merged to 'next' on 2013-01-16 at 23f5df2)
+ t5570: do not assume the "matching" push is the default
+ t5551: do not assume the "matching" push is the default
+ t5550: do not assume the "matching" push is the default
(merged to 'next' on 2013-01-09 at 74c3498)
+ doc: push.default is no longer "matching"
+ push: switch default from "matching" to "simple"
+ t9401: do not assume the "matching" push is the default
+ t9400: do not assume the "matching" push is the default
+ t7406: do not assume the "matching" push is the default
+ t5531: do not assume the "matching" push is the default
+ t5519: do not assume the "matching" push is the default
+ t5517: do not assume the "matching" push is the default
+ t5516: do not assume the "matching" push is the default
+ t5505: do not assume the "matching" push is the default
+ t5404: do not assume the "matching" push is the default
Will cook in 'next' until Git 2.0.
* bc/append-signed-off-by (2013-02-12) 12 commits
- Unify appending signoff in format-patch, commit and sequencer
- format-patch: update append_signoff prototype
- t4014: more tests about appending s-o-b lines
- sequencer.c: teach append_signoff to avoid adding a duplicate newline
- sequencer.c: teach append_signoff how to detect duplicate s-o-b
- sequencer.c: always separate "(cherry picked from" from commit body
- sequencer.c: require a conforming footer to be preceded by a blank line
- sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
- t/t3511: add some tests of 'cherry-pick -s' functionality
- t/test-lib-functions.sh: allow to specify the tag name to test_commit
- commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
- sequencer.c: rework search for start of footer to improve clarity
Waiting for further reviews.
$gmane/216327 may need to be addressed.
--------------------------------------------------
[Discarded]
* jc/maint-name-rev (2012-09-17) 7 commits
- describe --contains: use "name-rev --algorithm=weight"
- name-rev --algorithm=weight: tests and documentation
- name-rev --algorithm=weight: cache the computed weight in notes
- name-rev --algorithm=weight: trivial optimization
- name-rev: --algorithm option
- name_rev: clarify the logic to assign a new tip-name to a commit
- name-rev: lose unnecessary typedef
"git name-rev" names the given revision based on a ref that can be
reached in the smallest number of steps from the rev, but that is
not useful when the caller wants to know which tag is the oldest one
that contains the rev. This teaches a new mode to the command that
uses the oldest ref among those which contain the rev.
I am not sure if this is worth it; for one thing, even with the help
from notes-cache, it seems to make the "describe --contains" even
slower. Also the command will be unusably slow for a user who does
not have a write access (hence unable to create or update the
notes-cache).
Stalled mostly due to lack of responses.
* jc/xprm-generation (2012-09-14) 1 commit
- test-generation: compute generation numbers and clock skews
A toy to analyze how bad the clock skews are in histories of real
world projects.
Stalled mostly due to lack of responses.
* jk/lua-hackery (2012-10-07) 6 commits
- pretty: fix up one-off format_commit_message calls
- Minimum compilation fixup
- Makefile: make "lua" a bit more configurable
- add a "lua" pretty format
- add basic lua infrastructure
- pretty: make some commit-parsing helpers more public
Interesting exercise. When we do this for real, we probably would want
to wrap a commit to make it more like an "object" with methods like
"parents", etc.
* rc/maint-complete-git-p4 (2012-09-24) 1 commit
- Teach git-completion about git p4
Comment from Pete will need to be addressed ($gmane/206172).
^ permalink raw reply
* Re: Git Merge 2013 Conference, Berlin
From: Michael J Gruber @ 2013-02-20 8:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Chacon, Jeff King, Thomas Rast, git list
In-Reply-To: <7vzjyz3lyr.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 20.02.2013 00:47:
> Scott Chacon <schacon@gmail.com> writes:
>
>> Junio, are you interested in attending?
>
> I am interested in meeting our European contributors, but Berlin is
> kind of very far, so give me a few days to think about it.
>
> Thanks.
>
Maybe, we can - for the next time - try to coordinate the date with the
various international IT conferences which take place here, like
Linux-Tag in Berlin (just a few weeks apart), CEBIT in Hannover or the
smaller Chemnitzer Linux-Tage (or coordinate with events somewhere else
in Europe). That would give contributors not only more incentive to come
to the Git event, but also a better chance for successful negotiations
with their employers. Extending a business trip by a few days is a minor
issue, granting one in the first place not always.
Michael
^ permalink raw reply
* Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen
From: Stefano Lattarini @ 2013-02-20 8:22 UTC (permalink / raw)
To: Jiang Xin; +Cc: Junio C Hamano, Christoph J. Thompson, Git List
In-Reply-To: <CANYiYbF0C2qormAWNVhHm45TVQ6woECD7u1cCxxNuwyZxziNpw@mail.gmail.com>
On 02/20/2013 02:39 AM, Jiang Xin wrote:
>
> [SNIP]
>
> I am not familiar with autoconf. After clone autoconf and check,
> I cannot find a neat way to change "htmldir" default location to
> use ${datarootdir} (just like mandir).
>
This one-line change should be enough to do what you want:
diff --git a/configure.ac b/configure.ac
index 1991258..2bfbec9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
config_file=config.mak.autogen
config_in=config.mak.in
+AC_SUBST([htmldir], ['${datarootdir}'])
+
GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
# Directories holding "saner" versions of common or POSIX binaries.
Not sure whether this a good idea though (I haven't really followed the
discussion); but it is easily doable.
HTH,
Stefano
^ permalink raw reply
* [BUG] Infinite make recursion when configure.ac changes
From: Stefano Lattarini @ 2013-02-20 8:44 UTC (permalink / raw)
To: Git List; +Cc: Martin von Zweigbergk, Jonathan Nieder, Jeff King
>From a pristine master checkout:
$ make configure && ./configure make
... # All successfull
$ touch configure.ac
$ make
GEN config.status
make[1]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
make[2]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
make[3]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
make[4]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
make[5]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
...
and I have to hit ^C to interrupt that recursion.
This seems due to the change in commit v1.7.12.4-1-g1226504: the
issue is still present there, but no longer is in the preceding
commit 7e201053 (a.k.a. v1.7.12.4).
I haven't investigated this any further for the moment.
Regards,
Stefano
^ permalink raw reply
* Re: [PATCHv2 04/10] pkt-line: change error message for oversized packet
From: Jeff King @ 2013-02-20 8:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, git, Shawn O. Pearce
In-Reply-To: <20130218213331.GA29200@sigill.intra.peff.net>
On Mon, Feb 18, 2013 at 04:33:31PM -0500, Jeff King wrote:
> On Mon, Feb 18, 2013 at 01:25:35PM -0800, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > But it's easy to do (1), and it starts the clock ticking for
> > > the 1000-byte readers to become obsolete.
> >
> > Yup, I agree with that goal.
>
> Having just looked at the pkt-line callers a lot, I think most of them
> could go for something like:
> [...]
>
> That would actually simplify the callers a bit, and would harmonize the
> buffer sizes at the same time. I'll look into doing a series tonight.
Just a quick update on this series. It ended up taking more nights than
I thought. :) The result looks much better than what I posted before, and
I found several other corner cases and bugs in packet parsing, too.
I'm going to hold off on posting it tonight, as I'm now up to 19
patches, and after several rounds of "rebase -i", I need to give it a
final read-through myself before inflicting it on anyone else. I'll do
that and post it tomorrow.
In the meantime, please hold off on what I've posted so far (that
includes the jk/smart-http-robustify topic).
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #08; Tue, 19)
From: Jeff King @ 2013-02-20 8:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v621nzaxb.fsf@alter.siamese.dyndns.org>
On Tue, Feb 19, 2013 at 11:44:48PM -0800, Junio C Hamano wrote:
> * jk/smart-http-robustify (2013-02-17) 3 commits
> - remote-curl: sanity check ref advertisement from server
> - remote-curl: verify smart-http metadata lines
> - pkt-line: teach packet_get_line a no-op mode
>
> Parse the HTTP exchange that implements the native Git protocol as
> a series of stateless RPC more carefully to diagnose protocol
> breakage better.
>
> Will merge to and hold in 'next'.
I just posted elsewhere, but to make sure I get your attention: please
hold off on this. The sanity check in the top patch is not right
(because of the ERR/flush thing that Jonathan brought up). I'll have an
updated series tomorrow.
-Peff
^ permalink raw reply
* Re: [PATCHv2 04/10] pkt-line: change error message for oversized packet
From: Junio C Hamano @ 2013-02-20 8:54 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git, Shawn O. Pearce
In-Reply-To: <20130220084754.GA17055@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
>
>In the meantime, please hold off on what I've posted so far (that
>includes the jk/smart-http-robustify topic).
Surely. I'm done for the night already. Looking forward to see the reroll tomorrow.
Thanks.
--
Pardon terseness, typo and HTML from a tablet.
^ 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