* [PATCH 2/2] t1300: test mixed-case variable retrieval
From: Jeff King @ 2011-10-12 18:30 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <20111012182742.GA14543@sigill.intra.peff.net>
We should be able to ask for a config value both by its
canonical all-lowercase name (as git does internally), as
well as by random mixed-case (which will be canonicalized by
git-config for us).
Subsections are a tricky point, though. Since we have both
[section "Foo"]
and
[section.Foo]
you might want git-config to canonicalize the subsection or
not, depending on which you are expecting. But there's no
way to communicate this; git-config sees only the key, and
doesn't know which type of section name will be in the
config file.
So it must leave the subsection intact, and it is up to the
caller to provide a canonical version of the subsection if
they want to match the latter form.
Signed-off-by: Jeff King <peff@peff.net>
---
I was surprised this wasn't tested anywhere, but I couldn't find any
such place. I think it makes sense to document the desired behavior in
the form of a few tests.
t/t1300-repo-config.sh | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index cf508af..8a37f96 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -73,6 +73,33 @@ EOF
test_expect_success 'non-match result' 'test_cmp expect .git/config'
+test_expect_success 'find mixed-case key by canonical name' '
+ echo Second >expect &&
+ git config cores.whatever >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'find mixed-case key by non-canonical name' '
+ echo Second >expect &&
+ git config CoReS.WhAtEvEr >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'subsections are not canonicalized by git-config' '
+ cat >>.git/config <<-\EOF &&
+ [section.SubSection]
+ key = one
+ [section "SubSection"]
+ key = two
+ EOF
+ echo one >expect &&
+ git config section.subsection.key >actual &&
+ test_cmp expect actual &&
+ echo two >expect &&
+ git config section.SubSection.key >actual &&
+ test_cmp expect actual
+'
+
cat > .git/config <<\EOF
[alpha]
bar = foo
--
1.7.7.rc2.21.gb9948
^ permalink raw reply related
* [PATCH 1/2] t1300: put git invocations inside test function
From: Jeff King @ 2011-10-12 18:29 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <20111012182742.GA14543@sigill.intra.peff.net>
This is a very old script, and did a lot of:
echo whatever >expect
git config foo bar
test_expect_success 'cmp .git/config expect'
which meant that we didn't actually check that the call to
git-config succeeded. Fix this, and while we're at it,
modernize the style to use test_cmp.
Signed-off-by: Jeff King <peff@peff.net>
---
There are still a few 'cp' and 'rm' calls outside of the test functions,
but we can generally expect those to work (as we do with the 'cat'
calls).
t/t1300-repo-config.sh | 176 +++++++++++++++++++++++++-----------------------
1 files changed, 93 insertions(+), 83 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3e140c1..cf508af 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -7,28 +7,28 @@ test_description='Test git config in different settings'
. ./test-lib.sh
-test -f .git/config && rm .git/config
-
-git config core.penguin "little blue"
+test_expect_success 'clear default config' '
+ rm -f .git/config
+'
cat > expect << EOF
[core]
penguin = little blue
EOF
-
-test_expect_success 'initial' 'cmp .git/config expect'
-
-git config Core.Movie BadPhysics
+test_expect_success 'initial' '
+ git config core.penguin "little blue" &&
+ test_cmp expect .git/config
+'
cat > expect << EOF
[core]
penguin = little blue
Movie = BadPhysics
EOF
-
-test_expect_success 'mixed case' 'cmp .git/config expect'
-
-git config Cores.WhatEver Second
+test_expect_success 'mixed case' '
+ git config Core.Movie BadPhysics &&
+ test_cmp expect .git/config
+'
cat > expect << EOF
[core]
@@ -37,10 +37,10 @@ cat > expect << EOF
[Cores]
WhatEver = Second
EOF
-
-test_expect_success 'similar section' 'cmp .git/config expect'
-
-git config CORE.UPPERCASE true
+test_expect_success 'similar section' '
+ git config Cores.WhatEver Second
+ test_cmp expect .git/config
+'
cat > expect << EOF
[core]
@@ -50,8 +50,10 @@ cat > expect << EOF
[Cores]
WhatEver = Second
EOF
-
-test_expect_success 'similar section' 'cmp .git/config expect'
+test_expect_success 'uppercase section' '
+ git config CORE.UPPERCASE true &&
+ test_cmp expect .git/config
+'
test_expect_success 'replace with non-match' \
'git config core.penguin kingpin !blue'
@@ -69,7 +71,7 @@ cat > expect << EOF
WhatEver = Second
EOF
-test_expect_success 'non-match result' 'cmp .git/config expect'
+test_expect_success 'non-match result' 'test_cmp expect .git/config'
cat > .git/config <<\EOF
[alpha]
@@ -88,7 +90,7 @@ bar = foo
[beta]
EOF
-test_expect_success 'unset with cont. lines is correct' 'cmp .git/config expect'
+test_expect_success 'unset with cont. lines is correct' 'test_cmp expect .git/config'
cat > .git/config << EOF
[beta] ; silly comment # another comment
@@ -116,7 +118,7 @@ noIndent= sillyValue ; 'nother silly comment
[nextSection] noNewline = ouch
EOF
-test_expect_success 'multiple unset is correct' 'cmp .git/config expect'
+test_expect_success 'multiple unset is correct' 'test_cmp expect .git/config'
cp .git/config2 .git/config
@@ -140,9 +142,7 @@ noIndent= sillyValue ; 'nother silly comment
[nextSection] noNewline = ouch
EOF
-test_expect_success 'all replaced' 'cmp .git/config expect'
-
-git config beta.haha alpha
+test_expect_success 'all replaced' 'test_cmp expect .git/config'
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -153,10 +153,10 @@ noIndent= sillyValue ; 'nother silly comment
haha = alpha
[nextSection] noNewline = ouch
EOF
-
-test_expect_success 'really mean test' 'cmp .git/config expect'
-
-git config nextsection.nonewline wow
+test_expect_success 'really mean test' '
+ git config beta.haha alpha &&
+ test_cmp expect .git/config
+'
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -168,11 +168,12 @@ noIndent= sillyValue ; 'nother silly comment
[nextSection]
nonewline = wow
EOF
-
-test_expect_success 'really really mean test' 'cmp .git/config expect'
+test_expect_success 'really really mean test' '
+ git config nextsection.nonewline wow &&
+ test_cmp expect .git/config
+'
test_expect_success 'get value' 'test alpha = $(git config beta.haha)'
-git config --unset beta.haha
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -183,10 +184,10 @@ noIndent= sillyValue ; 'nother silly comment
[nextSection]
nonewline = wow
EOF
-
-test_expect_success 'unset' 'cmp .git/config expect'
-
-git config nextsection.NoNewLine "wow2 for me" "for me$"
+test_expect_success 'unset' '
+ git config --unset beta.haha &&
+ test_cmp expect .git/config
+'
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -198,8 +199,10 @@ noIndent= sillyValue ; 'nother silly comment
nonewline = wow
NoNewLine = wow2 for me
EOF
-
-test_expect_success 'multivar' 'cmp .git/config expect'
+test_expect_success 'multivar' '
+ git config nextsection.NoNewLine "wow2 for me" "for me$" &&
+ test_cmp expect .git/config
+'
test_expect_success 'non-match' \
'git config --get nextsection.nonewline !for'
@@ -214,8 +217,6 @@ test_expect_success 'ambiguous get' '
test_expect_success 'get multivar' \
'git config --get-all nextsection.nonewline'
-git config nextsection.nonewline "wow3" "wow$"
-
cat > expect << EOF
[beta] ; silly comment # another comment
noIndent= sillyValue ; 'nother silly comment
@@ -226,8 +227,10 @@ noIndent= sillyValue ; 'nother silly comment
nonewline = wow3
NoNewLine = wow2 for me
EOF
-
-test_expect_success 'multivar replace' 'cmp .git/config expect'
+test_expect_success 'multivar replace' '
+ git config nextsection.nonewline "wow3" "wow$" &&
+ test_cmp expect .git/config
+'
test_expect_success 'ambiguous value' '
test_must_fail git config nextsection.nonewline
@@ -241,8 +244,6 @@ test_expect_success 'invalid unset' '
test_must_fail git config --unset somesection.nonewline
'
-git config --unset nextsection.nonewline "wow3$"
-
cat > expect << EOF
[beta] ; silly comment # another comment
noIndent= sillyValue ; 'nother silly comment
@@ -253,7 +254,10 @@ noIndent= sillyValue ; 'nother silly comment
NoNewLine = wow2 for me
EOF
-test_expect_success 'multivar unset' 'cmp .git/config expect'
+test_expect_success 'multivar unset' '
+ git config --unset nextsection.nonewline "wow3$" &&
+ test_cmp expect .git/config
+'
test_expect_success 'invalid key' 'test_must_fail git config inval.2key blabla'
@@ -276,7 +280,7 @@ noIndent= sillyValue ; 'nother silly comment
Alpha = beta
EOF
-test_expect_success 'hierarchical section value' 'cmp .git/config expect'
+test_expect_success 'hierarchical section value' 'test_cmp expect .git/config'
cat > expect << EOF
beta.noindent=sillyValue
@@ -304,15 +308,16 @@ EOF
test_expect_success '--get-regexp' \
'git config --get-regexp in > output && cmp output expect'
-git config --add nextsection.nonewline "wow4 for you"
-
cat > expect << EOF
wow2 for me
wow4 for you
EOF
-test_expect_success '--add' \
- 'git config --get-all nextsection.nonewline > output && cmp output expect'
+test_expect_success '--add' '
+ git config --add nextsection.nonewline "wow4 for you" &&
+ git config --get-all nextsection.nonewline > output &&
+ test_cmp expect output
+'
cat > .git/config << EOF
[novalue]
@@ -361,8 +366,6 @@ cat > .git/config << EOF
c = d
EOF
-git config a.x y
-
cat > expect << EOF
[a.b]
c = d
@@ -370,10 +373,10 @@ cat > expect << EOF
x = y
EOF
-test_expect_success 'new section is partial match of another' 'cmp .git/config expect'
-
-git config b.x y
-git config a.b c
+test_expect_success 'new section is partial match of another' '
+ git config a.x y &&
+ test_cmp expect .git/config
+'
cat > expect << EOF
[a.b]
@@ -385,7 +388,11 @@ cat > expect << EOF
x = y
EOF
-test_expect_success 'new variable inserts into proper section' 'cmp .git/config expect'
+test_expect_success 'new variable inserts into proper section' '
+ git config b.x y &&
+ git config a.b c &&
+ test_cmp expect .git/config
+'
test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' \
'test_must_fail git config --file non-existing-config -l'
@@ -399,9 +406,10 @@ cat > expect << EOF
ein.bahn=strasse
EOF
-GIT_CONFIG=other-config git config -l > output
-
-test_expect_success 'alternative GIT_CONFIG' 'cmp output expect'
+test_expect_success 'alternative GIT_CONFIG' '
+ GIT_CONFIG=other-config git config -l >output &&
+ test_cmp expect output
+'
test_expect_success 'alternative GIT_CONFIG (--file)' \
'git config --file other-config -l > output && cmp output expect'
@@ -417,8 +425,6 @@ test_expect_success 'refer config from subdirectory' '
'
-GIT_CONFIG=other-config git config anwohner.park ausweis
-
cat > expect << EOF
[ein]
bahn = strasse
@@ -426,7 +432,10 @@ cat > expect << EOF
park = ausweis
EOF
-test_expect_success '--set in alternative GIT_CONFIG' 'cmp other-config expect'
+test_expect_success '--set in alternative GIT_CONFIG' '
+ GIT_CONFIG=other-config git config anwohner.park ausweis &&
+ test_cmp expect other-config
+'
cat > .git/config << EOF
# Hallo
@@ -531,7 +540,7 @@ test_expect_success 'section ending' '
git config gitcvs.enabled true &&
git config gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite &&
git config gitcvs.dbname %Ggitcvs2.%a.%m.sqlite &&
- cmp .git/config expect
+ test_cmp expect .git/config
'
@@ -750,13 +759,6 @@ test_expect_success NOT_MINGW 'get --path copes with unset $HOME' '
test_cmp expect result
'
-rm .git/config
-
-git config quote.leading " test"
-git config quote.ending "test "
-git config quote.semicolon "test;test"
-git config quote.hash "test#test"
-
cat > expect << EOF
[quote]
leading = " test"
@@ -764,8 +766,14 @@ cat > expect << EOF
semicolon = "test;test"
hash = "test#test"
EOF
-
-test_expect_success 'quoting' 'cmp .git/config expect'
+test_expect_success 'quoting' '
+ rm .git/config &&
+ git config quote.leading " test" &&
+ git config quote.ending "test " &&
+ git config quote.semicolon "test;test" &&
+ git config quote.hash "test#test" &&
+ test_cmp expect .git/config
+'
test_expect_success 'key with newline' '
test_must_fail git config "key.with
@@ -790,9 +798,10 @@ section.noncont=not continued
section.quotecont=cont;inued
EOF
-git config --list > result
-
-test_expect_success 'value continued on next line' 'cmp result expect'
+test_expect_success 'value continued on next line' '
+ git config --list > result &&
+ cmp result expect
+'
cat > .git/config <<\EOF
[section "sub=section"]
@@ -813,16 +822,17 @@ barQsection.sub=section.val3
Qsection.sub=section.val4
Qsection.sub=section.val5Q
EOF
+test_expect_success '--null --list' '
+ git config --null --list | nul_to_q >result &&
+ echo >>result &&
+ test_cmp expect result
+'
-git config --null --list | perl -pe 'y/\000/Q/' > result
-echo >>result
-
-test_expect_success '--null --list' 'cmp result expect'
-
-git config --null --get-regexp 'val[0-9]' | perl -pe 'y/\000/Q/' > result
-echo >>result
-
-test_expect_success '--null --get-regexp' 'cmp result expect'
+test_expect_success '--null --get-regexp' '
+ git config --null --get-regexp "val[0-9]" | nul_to_q >result &&
+ echo >>result &&
+ test_cmp expect result
+'
test_expect_success 'inner whitespace kept verbatim' '
git config section.val "foo bar" &&
--
1.7.7.rc2.21.gb9948
^ permalink raw reply related
* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 18:27 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <20111012174643.GA14336@sigill.intra.peff.net>
On Wed, Oct 12, 2011 at 01:46:43PM -0400, Jeff King wrote:
> On Wed, Oct 12, 2011 at 12:29:39PM -0400, Jeff King wrote:
>
> > The explanation matches what we do now, but it did end up a bit longer
> > than I had hoped. We could make it a lot shorter by:
> >
> > 1. Canonicalizing the section and key names that the caller gives to
> > git-config.
>
> Hmm. Scratch that. We seem to do this already in my tests. I'll look
> further and try to make a better documentation patch.
OK, I was all set to do a patch to git-config for this, but it seems the
code is already there. It's only the subsections which are the sticking
point, and those can't be canonicalized, because in most cases we need
to match them exactly.
In the process, I did some cleanup and added some new tests to t1300,
which I think are probably worth applying anyway.
[1/2]: t1300: put git invocations inside test function
[2/2]: t1300: test mixed-case variable retrieval
-Peff
^ permalink raw reply
* Re: [PATCH] Make is_gitfile a non-static generic function
From: Junio C Hamano @ 2011-10-12 18:08 UTC (permalink / raw)
To: Phil Hord; +Cc: Phil Hord, git@vger.kernel.org
In-Reply-To: <CABURp0p4_oVMmkFDPue4kvhO_bEr_dBh-XFGArdSJFMz-Eboeg@mail.gmail.com>
Phil Hord <phil.hord@gmail.com> writes:
> On Tue, Oct 11, 2011 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> After looking at this patch and the way the other caller in transport.c
>> uses it, I am more and more convinced that "is_gitfile()" is a stupid and
>> horrible mistake.
>
> I think I misunderstood your objection before. Now I think I
> understand. Tell me if I am right.
>
>
> I think you mean that instead of this:
> } else if (is_local(url) && is_file(url) && !is_gitfile(url)) {
>
> you would like to see this:
> } else if (is_local(url) && is_file(url) && is_bundle(url)) {
>
> Or maybe even this:
> } else if (is_bundle(url)) {
Exactly.
^ permalink raw reply
* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-12 18:07 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <4E95D60B.5030904@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 10/12/2011 07:49 PM, Junio C Hamano wrote:
>> diff --git a/refs.c b/refs.c
>> index e3692bd..e54c482 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
>> NULL
>> };
>>
>> +static int refname_ok_at_root_level(const char *str, int len)
>> +{
>> + int seen_non_root_char = 0;
>> +
>> + while (len--) {
>> + char ch = *str++;
>> +
>> + if (ch == '/')
>> + return 1;
>> + /*
>> + * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
>> + * the root level as a ref.
>> + */
>> + if (ch != '_' && (ch < 'A' || 'Z' < ch))
>> + seen_non_root_char = 1;
>> + }
>> + return !seen_non_root_char;
>> +}
>> +
>
> Nit: the seen_non_root_char variable can be replaced by an early "return
> 0" from the loop and "return 1" if the loop falls through.
Hmm, I thought that would fail when you feed "refs/heads/master" to the
function.
^ permalink raw reply
* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Michael Haggerty @ 2011-10-12 18:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <7vvcru9k22.fsf_-_@alter.siamese.dyndns.org>
On 10/12/2011 07:49 PM, Junio C Hamano wrote:
> diff --git a/refs.c b/refs.c
> index e3692bd..e54c482 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
> NULL
> };
>
> +static int refname_ok_at_root_level(const char *str, int len)
> +{
> + int seen_non_root_char = 0;
> +
> + while (len--) {
> + char ch = *str++;
> +
> + if (ch == '/')
> + return 1;
> + /*
> + * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
> + * the root level as a ref.
> + */
> + if (ch != '_' && (ch < 'A' || 'Z' < ch))
> + seen_non_root_char = 1;
> + }
> + return !seen_non_root_char;
> +}
> +
Nit: the seen_non_root_char variable can be replaced by an early "return
0" from the loop and "return 1" if the loop falls through.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] Make is_gitfile a non-static generic function
From: Phil Hord @ 2011-10-12 18:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phil Hord, git@vger.kernel.org
In-Reply-To: <7vipnvccso.fsf@alter.siamese.dyndns.org>
On Tue, Oct 11, 2011 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> After looking at this patch and the way the other caller in transport.c
> uses it, I am more and more convinced that "is_gitfile()" is a stupid and
> horrible mistake.
I think I misunderstood your objection before. Now I think I
understand. Tell me if I am right.
I think you mean that instead of this:
} else if (is_local(url) && is_file(url) && !is_gitfile(url)) {
you would like to see this:
} else if (is_local(url) && is_file(url) && is_bundle(url)) {
Or maybe even this:
} else if (is_bundle(url)) {
Phil
^ permalink raw reply
* [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-12 17:49 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <20111012045004.GA22413@sigill.intra.peff.net>
We have always dwimmed the user input $string into a ref by first looking
directly inside $GIT_DIR, and then in $GIT_DIR/refs, $GIT_DIR/refs/tags,
etc., and that is what made
git log HEAD..MERGE_HEAD
work correctly. This however means that
git rev-parse config
git log index
would look at $GIT_DIR/config and $GIT_DIR/index and see if they are valid
refs.
To reduce confusion, let's not dwim a path immediately below $GIT_DIR that
is not all-caps.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* And this adds coverage to refname_match() and shorten_unambiguous_ref()
on top of the one from yesterday.
refs.c | 36 ++++++++++++++++++++++++++++++++++--
1 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index e3692bd..e54c482 100644
--- a/refs.c
+++ b/refs.c
@@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
NULL
};
+static int refname_ok_at_root_level(const char *str, int len)
+{
+ int seen_non_root_char = 0;
+
+ while (len--) {
+ char ch = *str++;
+
+ if (ch == '/')
+ return 1;
+ /*
+ * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+ * the root level as a ref.
+ */
+ if (ch != '_' && (ch < 'A' || 'Z' < ch))
+ seen_non_root_char = 1;
+ }
+ return !seen_non_root_char;
+}
+
int refname_match(const char *abbrev_name, const char *full_name, const char **rules)
{
const char **p;
const int abbrev_name_len = strlen(abbrev_name);
for (p = rules; *p; p++) {
+ if (p == rules &&
+ !refname_ok_at_root_level(abbrev_name, abbrev_name_len))
+ continue;
if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {
return 1;
}
@@ -1100,6 +1122,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
unsigned char *this_result;
int flag;
+ if (p == ref_rev_parse_rules && !refname_ok_at_root_level(str, len))
+ continue;
this_result = refs_found ? sha1_from_ref : sha1;
mksnpath(fullref, sizeof(fullref), *p, len, str);
r = resolve_ref(fullref, this_result, 1, &flag);
@@ -1128,6 +1152,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
char path[PATH_MAX];
const char *ref, *it;
+ if (p == ref_rev_parse_rules && !refname_ok_at_root_level(str, len))
+ continue;
mksnpath(path, sizeof(path), *p, len, str);
ref = resolve_ref(path, hash, 1, NULL);
if (!ref)
@@ -2045,12 +2071,14 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
/* buffer for scanf result, at most ref must fit */
short_name = xstrdup(ref);
- /* skip first rule, it will always match */
- for (i = nr_rules - 1; i > 0 ; --i) {
+ for (i = nr_rules - 1; i >= 0; i--) {
int j;
int rules_to_fail = i;
int short_name_len;
+ if (!i && !refname_ok_at_root_level(ref, strlen(ref)))
+ continue;
+
if (1 != sscanf(ref, scanf_fmts[i], short_name))
continue;
@@ -2076,6 +2104,10 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
if (i == j)
continue;
+ if (!j &&
+ !refname_ok_at_root_level(short_name, short_name_len))
+ continue;
+
/*
* the short name is ambiguous, if it resolves
* (with this previous rule) to a valid ref
--
1.7.7.213.g8b0e1
^ permalink raw reply related
* [PATCH 1/2] refs.c: move dwim_ref()/dwim_log() from sha1_name.c
From: Junio C Hamano @ 2011-10-12 17:48 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <20111012045004.GA22413@sigill.intra.peff.net>
OK, so here is a re-roll of the one from yesterday. This one is new but is
a mere preparatory step.
-- >8 --
Both dwim_ref()/dwim_log() functions are intimately related to the ref
parsing rules defined in refs.c and better fits there. Move them together
with substitute_branch_name(), a file scope static helper function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
sha1_name.c | 85 -----------------------------------------------------------
2 files changed, 85 insertions(+), 85 deletions(-)
diff --git a/refs.c b/refs.c
index 832a52f..e3692bd 100644
--- a/refs.c
+++ b/refs.c
@@ -1067,6 +1067,91 @@ static int is_refname_available(const char *ref, const char *oldref,
return 1;
}
+/*
+ * *string and *len will only be substituted, and *string returned (for
+ * later free()ing) if the string passed in is a magic short-hand form
+ * to name a branch.
+ */
+static char *substitute_branch_name(const char **string, int *len)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int ret = interpret_branch_name(*string, &buf);
+
+ if (ret == *len) {
+ size_t size;
+ *string = strbuf_detach(&buf, &size);
+ *len = size;
+ return (char *)*string;
+ }
+
+ return NULL;
+}
+
+int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
+{
+ char *last_branch = substitute_branch_name(&str, &len);
+ const char **p, *r;
+ int refs_found = 0;
+
+ *ref = NULL;
+ for (p = ref_rev_parse_rules; *p; p++) {
+ char fullref[PATH_MAX];
+ unsigned char sha1_from_ref[20];
+ unsigned char *this_result;
+ int flag;
+
+ this_result = refs_found ? sha1_from_ref : sha1;
+ mksnpath(fullref, sizeof(fullref), *p, len, str);
+ r = resolve_ref(fullref, this_result, 1, &flag);
+ if (r) {
+ if (!refs_found++)
+ *ref = xstrdup(r);
+ if (!warn_ambiguous_refs)
+ break;
+ } else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD"))
+ warning("ignoring dangling symref %s.", fullref);
+ }
+ free(last_branch);
+ return refs_found;
+}
+
+int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
+{
+ char *last_branch = substitute_branch_name(&str, &len);
+ const char **p;
+ int logs_found = 0;
+
+ *log = NULL;
+ for (p = ref_rev_parse_rules; *p; p++) {
+ struct stat st;
+ unsigned char hash[20];
+ char path[PATH_MAX];
+ const char *ref, *it;
+
+ mksnpath(path, sizeof(path), *p, len, str);
+ ref = resolve_ref(path, hash, 1, NULL);
+ if (!ref)
+ continue;
+ if (!stat(git_path("logs/%s", path), &st) &&
+ S_ISREG(st.st_mode))
+ it = path;
+ else if (strcmp(ref, path) &&
+ !stat(git_path("logs/%s", ref), &st) &&
+ S_ISREG(st.st_mode))
+ it = ref;
+ else
+ continue;
+ if (!logs_found++) {
+ *log = xstrdup(it);
+ hashcpy(sha1, hash);
+ }
+ if (!warn_ambiguous_refs)
+ break;
+ }
+ free(last_branch);
+ return logs_found;
+}
+
static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int flags, int *type_p)
{
char *ref_file;
diff --git a/sha1_name.c b/sha1_name.c
index 143fd97..d423635 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -241,91 +241,6 @@ static int ambiguous_path(const char *path, int len)
return slash;
}
-/*
- * *string and *len will only be substituted, and *string returned (for
- * later free()ing) if the string passed in is a magic short-hand form
- * to name a branch.
- */
-static char *substitute_branch_name(const char **string, int *len)
-{
- struct strbuf buf = STRBUF_INIT;
- int ret = interpret_branch_name(*string, &buf);
-
- if (ret == *len) {
- size_t size;
- *string = strbuf_detach(&buf, &size);
- *len = size;
- return (char *)*string;
- }
-
- return NULL;
-}
-
-int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
-{
- char *last_branch = substitute_branch_name(&str, &len);
- const char **p, *r;
- int refs_found = 0;
-
- *ref = NULL;
- for (p = ref_rev_parse_rules; *p; p++) {
- char fullref[PATH_MAX];
- unsigned char sha1_from_ref[20];
- unsigned char *this_result;
- int flag;
-
- this_result = refs_found ? sha1_from_ref : sha1;
- mksnpath(fullref, sizeof(fullref), *p, len, str);
- r = resolve_ref(fullref, this_result, 1, &flag);
- if (r) {
- if (!refs_found++)
- *ref = xstrdup(r);
- if (!warn_ambiguous_refs)
- break;
- } else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD"))
- warning("ignoring dangling symref %s.", fullref);
- }
- free(last_branch);
- return refs_found;
-}
-
-int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
-{
- char *last_branch = substitute_branch_name(&str, &len);
- const char **p;
- int logs_found = 0;
-
- *log = NULL;
- for (p = ref_rev_parse_rules; *p; p++) {
- struct stat st;
- unsigned char hash[20];
- char path[PATH_MAX];
- const char *ref, *it;
-
- mksnpath(path, sizeof(path), *p, len, str);
- ref = resolve_ref(path, hash, 1, NULL);
- if (!ref)
- continue;
- if (!stat(git_path("logs/%s", path), &st) &&
- S_ISREG(st.st_mode))
- it = path;
- else if (strcmp(ref, path) &&
- !stat(git_path("logs/%s", ref), &st) &&
- S_ISREG(st.st_mode))
- it = ref;
- else
- continue;
- if (!logs_found++) {
- *log = xstrdup(it);
- hashcpy(sha1, hash);
- }
- if (!warn_ambiguous_refs)
- break;
- }
- free(last_branch);
- return logs_found;
-}
-
static inline int upstream_mark(const char *string, int len)
{
const char *suffix[] = { "@{upstream}", "@{u}" };
--
1.7.7.213.g8b0e1
^ permalink raw reply related
* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Junio C Hamano @ 2011-10-12 17:46 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, Jeff King
In-Reply-To: <1318434726-5556-1-git-send-email-cmn@elego.de>
Carlos Martín Nieto <cmn@elego.de> writes:
> This bit me recently when I was creating a parser. See Jeff's
> explanation here:
> http://thread.gmane.org/gmane.comp.version-control.git/179569/focus=180290
I think you meant to focus on 180123 but anyway I think the updated text reads
much better.
Thanks.
> Documentation/config.txt | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 0658ffb..1212c47 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -45,9 +45,10 @@ lines. Variables may belong directly to a section or to a given subsection.
> You can have `[section]` if you have `[section "subsection"]`, but you
> don't need to.
>
> -There is also a case insensitive alternative `[section.subsection]` syntax.
> -In this syntax, subsection names follow the same restrictions as for section
> -names.
> +There is also a deprecated `[section.subsection]` syntax. With this
> +syntax, the subsection name is converted to lower-case and is also
> +compared case sensitively. These subsection names follow the same
> +restrictions as section names.
>
> All the other lines (and the remainder of the line after the section
> header) are recognized as setting variables, in the form
^ permalink raw reply
* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 17:46 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <20111012162939.GA3055@sigill.intra.peff.net>
On Wed, Oct 12, 2011 at 12:29:39PM -0400, Jeff King wrote:
> The explanation matches what we do now, but it did end up a bit longer
> than I had hoped. We could make it a lot shorter by:
>
> 1. Canonicalizing the section and key names that the caller gives to
> git-config.
Hmm. Scratch that. We seem to do this already in my tests. I'll look
further and try to make a better documentation patch.
-Peff
^ permalink raw reply
* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-12 16:52 UTC (permalink / raw)
To: Lars Hjemli, Jeff King
Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <20111011225434.GA24142@sigill.intra.peff.net>
Subject: [PATCH] branch -m/-M: remove undocumented RENAMED-REF
The commit message for c976d41 (git-branch: add options and tests for
branch renaming, 2006-11-28) mentions RENAME_REF but otherwise this is not
documented anywhere, and it does not appear in any of the tests.
Worse yet, the name of the actual file is "RENAMED-REF".
This was supposed to hold the commit object name at the tip of the branch
the most recent "branch -m/-M" renamed, but that is not necessary in order
to be able to recover from a mistake. Even when "branch -M A B" overwrites
an existing branch B, what is kept in RENAMED-REF is the commit at the tip
of the original branch A, not the commit B from the now-lost branch.
Just remove this unused "feature".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Let's do this.
refs.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index cdedb45..ecfc22c 100644
--- a/refs.c
+++ b/refs.c
@@ -786,7 +786,6 @@ int delete_ref(const char *refname, unsigned char *sha1)
int rename_ref(const char *oldref, const char *newref)
{
- static const char renamed_ref[] = "RENAMED-REF";
unsigned char sha1[20], orig_sha1[20];
int flag = 0, logmoved = 0;
struct ref_lock *lock;
@@ -809,13 +808,6 @@ int rename_ref(const char *oldref, const char *newref)
if (snprintf(msg, sizeof(msg), "renamed %s to %s", oldref, newref) > sizeof(msg))
return error("Refnames to long");
- lock = lock_ref_sha1_basic(renamed_ref, NULL, NULL);
- if (!lock)
- return error("unable to lock %s", renamed_ref);
- lock->force_write = 1;
- if (write_ref_sha1(lock, orig_sha1, msg))
- return error("unable to save current sha1 in %s", renamed_ref);
-
if (log && rename(git_path("logs/%s", oldref), git_path("tmp-renamed-log")))
return error("unable to move logfile logs/%s to tmp-renamed-log: %s",
oldref, strerror(errno));
--
1.7.7.213.g8b0e1
^ permalink raw reply related
* Re: [PATCH 00/20] [GIT PULL][v3.2] tracing: queued updates
From: Ingo Molnar @ 2011-10-12 16:29 UTC (permalink / raw)
To: Jeff King
Cc: Valdis.Kletnieks, git, Steven Rostedt, linux-kernel,
Andrew Morton, Thomas Gleixner, Frederic Weisbecker
In-Reply-To: <20111012141939.GA25085@sigill.intra.peff.net>
* Jeff King <peff@peff.net> wrote:
> > It's really useful for a painless UI flow to disambiguate failure
> > messages into clearly actionable variants.
>
> I agree. I think some people are concerned with leaking information
> about which repos exist and how they are configured. That is
> probably not a big problem for a public site like kernel.org,
> though.
Well, a gitconfig option could be provided to not leak that small
amount of info - but it would otherwise be weird for an OSS SCM to
default to a behavior that only makes sense with closed source,
right? :-)
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 16:29 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <1318434726-5556-1-git-send-email-cmn@elego.de>
On Wed, Oct 12, 2011 at 05:52:06PM +0200, Carlos Martín Nieto wrote:
> -There is also a case insensitive alternative `[section.subsection]` syntax.
> -In this syntax, subsection names follow the same restrictions as for section
> -names.
> +There is also a deprecated `[section.subsection]` syntax. With this
> +syntax, the subsection name is converted to lower-case and is also
> +compared case sensitively. These subsection names follow the same
> +restrictions as section names.
Hmm. While technically more correct, I think it is a little more
confusing to read. The lower-case canonicalization thing is actually
used for the other case-insensitive parts, too. So maybe it makes sense
to describe that in detail, and then just note that
"[section.subsection]" uses the same mechanism.
The patch below does this, and then the original text in the section you
tweaked above hopefully makes more sense in the new context.
The explanation matches what we do now, but it did end up a bit longer
than I had hoped. We could make it a lot shorter by:
1. Canonicalizing the section and key names that the caller gives to
git-config.
2. Not mentioning the "section.foo" syntax. We can't canonicalize the
subsection in (1) because of this syntax. But we can at least gloss
over the detail, and then maybe just mention it much later in the
file format. Or even deprecate it.
-- >8 --
Subject: [PATCH] docs/config: explain case-insensitive matching
We generally think of key matching as case-insensitive, but
it's not exactly. It's about canonicalizing one side, and
comparing it byte-wise with the canonical key name given to
git-config.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-config.txt | 50 +++++++++++++++++++++++++++++++++++++++++-
1 files changed, 49 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e7ecf5d..e92aee9 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -28,7 +28,7 @@ DESCRIPTION
-----------
You can query/set/replace/unset options with this command. The name is
actually the section and the key separated by a dot, and the value will be
-escaped.
+escaped. See the section on name matching below.
Multiple lines can be added to an option by using the '--add' option.
If you want to update or unset an option which can occur on multiple
@@ -178,6 +178,54 @@ See also <<FILES>>.
Opens an editor to modify the specified config file; either
'--system', '--global', or repository (default).
+
+NAME MATCHING
+-------------
+
+Configuration key names are matched using an algorithm that allows for
+partial case sensitivity. Section and key names are read from the config
+files, canonicalized according to the rules below, and then compared
+case-sensitively with the input given to git-config. Therefore any
+callers to git-config should request the canonicalized version of the
+name. This typically means lowercasing the section and key names, and
+leaving the subsection (if any) intact. For example, ask for
+`git config core.eol`, not `git config CoRe.EOL`.
+
+The canonicalization rules are:
+
+1. Lowercase the section and key names.
+
+2. If a literal subsection (like `[section "foo"]`) is used, leave it
+ intact.
+
+3. If a non-literal subsection (like `[section.foo]`) is used, lowercase
+ the subsection.
+
+4. Concatenate the resulting section, subsection, and key, separated by
+ a dot ('.').
+
+For example, this configuration file:
+
+-----------------------------------------------
+[CORE]
+eol = true
+
+[branch "Foo"]
+REMOTE = origin
+
+[color.DIFF]
+new = blue
+-----------------------------------------------
+
+would yield the following three canonicalized names:
+
+-----------------------------------------------
+core.eol
+branch.Foo.remote
+color.diff.new
+-----------------------------------------------
+
+
[[FILES]]
FILES
-----
--
1.7.7.rc2.21.gb9948
^ permalink raw reply related
* [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Carlos Martín Nieto @ 2011-10-12 15:52 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
Using the [section.subsection] syntax, the subsection is transformed
to lower-case and is matched case sensitively. Say so in the
documentation and mention that you shouldn't be using it anyway.
Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
This bit me recently when I was creating a parser. See Jeff's
explanation here:
http://thread.gmane.org/gmane.comp.version-control.git/179569/focus=180290
Documentation/config.txt | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0658ffb..1212c47 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -45,9 +45,10 @@ lines. Variables may belong directly to a section or to a given subsection.
You can have `[section]` if you have `[section "subsection"]`, but you
don't need to.
-There is also a case insensitive alternative `[section.subsection]` syntax.
-In this syntax, subsection names follow the same restrictions as for section
-names.
+There is also a deprecated `[section.subsection]` syntax. With this
+syntax, the subsection name is converted to lower-case and is also
+compared case sensitively. These subsection names follow the same
+restrictions as section names.
All the other lines (and the remainder of the line after the section
header) are recognized as setting variables, in the form
--
1.7.6.557.gcee4
^ permalink raw reply related
* Re: [PATCH v3] gitk: Teach gitk to respect log.showroot
From: Marcus Karlsson @ 2011-10-12 14:36 UTC (permalink / raw)
To: Paul Mackerras; +Cc: zbyszek, gitster, git
In-Reply-To: <20111008064704.GA27056@bloggs.ozlabs.ibm.com>
On Sat, Oct 08, 2011 at 05:47:04PM +1100, Paul Mackerras wrote:
> On Tue, Oct 04, 2011 at 10:08:13PM +0200, Marcus Karlsson wrote:
> > Teach gitk to respect log.showroot.
>
> Sounds reasonable, ...
>
> > - set cmd [concat | git diff-tree -r $flags $ids]
> > + set cmd [concat | git diff-tree -r]
> > + if {$log_showroot eq true} {
> > + set cmd [concat $cmd --root]
> > + }
> > + set cmd [concat $cmd $flags $ids]
>
> but is there any reason not to do it like this?
>
> if {$log_showroot} {
> lappend flags --root
> }
> set cmd [concat | git diff-tree -r $flags $ids]
>
> I.e., do you particularly want the --root before the other flags?
>
> Paul.
Not really, that would work very well.
Marcus
^ permalink raw reply
* Re: [PATCH] fix "git apply --index ..." not to deref NULL
From: Jim Meyering @ 2011-10-12 14:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111012142750.GB25085@sigill.intra.peff.net>
Jeff King wrote:
> On Wed, Oct 12, 2011 at 10:18:01AM +0200, Jim Meyering wrote:
>
>> I noticed this when "git am CORRUPTED" unexpectedly failed with an
>> odd diagnostic, and even removed one of the files it was supposed
>> to have patched.
>>
>> Reproduce with any valid old/new patch from which you have removed
>> the "+++ b/FILE" line. You'll see a diagnostic like this
>>
>> fatal: unable to write file '(null)' mode 100644: Bad address
>>
>> and you'll find that FILE has been removed.
>
> Yikes. Your fix looks right to me.
>
>> builtin/apply.c | 3 +++
>> t/t4254-am-corrupt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 46 insertions(+), 0 deletions(-)
>> create mode 100644 t/t4254-am-corrupt.sh
>
> Missing executable bit on the new test.
Thanks.
Fixed with this:
-- >8 --
Subject: [PATCH] fix "git apply --index ..." not to deref NULL
I noticed this when "git am CORRUPTED" unexpectedly failed with an
odd diagnostic, and even removed one of the files it was supposed
to have patched.
Reproduce with any valid old/new patch from which you have removed
the "+++ b/FILE" line. You'll see a diagnostic like this
fatal: unable to write file '(null)' mode 100644: Bad address
and you'll find that FILE has been removed.
The above is on glibc-based systems. On other systems, rather than
getting "null", you may provoke a segfault as git tries to
dereference the NULL file name.
Signed-off-by: Jim Meyering <meyering@redhat.com>
---
builtin/apply.c | 3 +++
t/t4254-am-corrupt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+), 0 deletions(-)
create mode 100755 t/t4254-am-corrupt.sh
diff --git a/builtin/apply.c b/builtin/apply.c
index f2edc52..aaa39fe 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1407,6 +1407,9 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
"%d leading pathname components (line %d)" , p_value, linenr);
patch->old_name = patch->new_name = patch->def_name;
}
+ if (!patch->is_delete && !patch->new_name)
+ die("git diff header lacks filename information "
+ "(line %d)", linenr);
patch->is_toplevel_relative = 1;
*hdrsize = git_hdr_len;
return offset;
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
new file mode 100755
index 0000000..b7da95f
--- /dev/null
+++ b/t/t4254-am-corrupt.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='git am with corrupt input'
+. ./test-lib.sh
+
+# Note the missing "+++" line:
+cat > bad-patch.diff <<'EOF'
+From: A U Thor <au.thor@example.com>
+diff --git a/f b/f
+index 7898192..6178079 100644
+--- a/f
+@@ -1 +1 @@
+-a
++b
+EOF
+
+test_expect_success setup '
+ test $? = 0 &&
+ echo a > f &&
+ git add f &&
+ test_tick &&
+ git commit -m initial
+'
+
+# This used to fail before, too, but with a different diagnostic.
+# fatal: unable to write file '(null)' mode 100644: Bad address
+# Also, it had the unwanted side-effect of deleting f.
+test_expect_success 'try to apply corrupted patch' '
+ git am bad-patch.diff 2> actual
+ test $? = 1
+'
+
+cat > expected <<EOF
+fatal: git diff header lacks filename information (line 4)
+EOF
+
+test_expect_success 'compare diagnostic; ensure file is still here' '
+ test $? = 0 &&
+ test -f f &&
+ test_cmp expected actual
+'
+
+test_done
--
1.7.7
^ permalink raw reply related
* Re: [PATCH] fix "git apply --index ..." not to deref NULL
From: Jeff King @ 2011-10-12 14:27 UTC (permalink / raw)
To: Jim Meyering; +Cc: git
In-Reply-To: <87lisq8vye.fsf@rho.meyering.net>
On Wed, Oct 12, 2011 at 10:18:01AM +0200, Jim Meyering wrote:
> I noticed this when "git am CORRUPTED" unexpectedly failed with an
> odd diagnostic, and even removed one of the files it was supposed
> to have patched.
>
> Reproduce with any valid old/new patch from which you have removed
> the "+++ b/FILE" line. You'll see a diagnostic like this
>
> fatal: unable to write file '(null)' mode 100644: Bad address
>
> and you'll find that FILE has been removed.
Yikes. Your fix looks right to me.
> builtin/apply.c | 3 +++
> t/t4254-am-corrupt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+), 0 deletions(-)
> create mode 100644 t/t4254-am-corrupt.sh
Missing executable bit on the new test.
-Peff
^ permalink raw reply
* Re: [PATCH 00/20] [GIT PULL][v3.2] tracing: queued updates
From: Jeff King @ 2011-10-12 14:19 UTC (permalink / raw)
To: Ingo Molnar
Cc: Valdis.Kletnieks, git, Steven Rostedt, linux-kernel,
Andrew Morton, Thomas Gleixner, Frederic Weisbecker
In-Reply-To: <20111012080711.GM18618@elte.hu>
On Wed, Oct 12, 2011 at 10:07:14AM +0200, Ingo Molnar wrote:
> > On Tue, 11 Oct 2011 07:50:17 +0200, Ingo Molnar said:
> >
> > > $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/perf/core
> > > fatal: The remote end hung up unexpectedly
> >
> > Is it possible to get 'git' to say something more informative than
> > "hung up unexpectedly"? "Tree not found, check URL" or similar
> > would be nice...
It's not possible for the client to say anything more. The server sees
that the request isn't valid and hangs up without saying anything. So
the server needs to be changed to output better responses.
> Firstly, arguably, typoing something is not 'fatal' really - it's
> just a resource that was not found on the server.
>
> Secondly, and more importantly, the reason for the failed pull is
> indeed important to know, if you want to resolve the problem with a
> minimum fuss:
>
> - Was it the tree that didnt exist?
> - Or the branch?
> - Or was there some other problem [such as a truly unexpectedly
> closed transport socket]?
>
> It's really useful for a painless UI flow to disambiguate failure
> messages into clearly actionable variants.
I agree. I think some people are concerned with leaking information
about which repos exist and how they are configured. That is probably
not a big problem for a public site like kernel.org, though.
You might find this thread interesting:
http://thread.gmane.org/gmane.comp.version-control.git/182529/focus=182642
It seems to have resulted in a patch that will at least say "access
denied" for every error. Which is a step up from "the remote end hung up
unexpectedly", but I do think most users would appreciate it being more
specific.
Perhaps we just need a config option to turn on more verbose messages,
if the site decides that there's no security implications to doing so.
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] t5403.1: simplify commit creation
From: Johannes Sixt @ 2011-10-12 14:14 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1318412105-13595-1-git-send-email-pclouds@gmail.com>
Am 10/12/2011 11:35, schrieb Nguyễn Thái Ngọc Duy:
> test_expect_success setup '
> echo Data for commit0. >a &&
> echo Data for commit0. >b &&
> - git update-index --add a &&
> - git update-index --add b &&
> - tree0=$(git write-tree) &&
> - commit0=$(echo setup | git commit-tree $tree0) &&
> - git update-ref refs/heads/master $commit0 &&
> + git add a b &&
> + git commit -m setup &&
> git clone ./. clone1 &&
> git clone ./. clone2 &&
> GIT_DIR=clone2/.git git branch new2 &&
I don't think this change is necessary. It doesn't hurt to use plumbing
commands here and there in the test suite to exercise them to a degree
that they deserve.
-- Hannes
^ permalink raw reply
* Re: [RFC] teach --edit to git rebase
From: Jean Privat @ 2011-10-12 13:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcrubyz1.fsf@alter.siamese.dyndns.org>
> The only thing you can do with this new option is "update one commit
> buried in the history, and rebase everything that build on top of it",
It is a good summary indeed.
> as
> far as I can tell. It feels a shame to waste the generic word "--edit" for
> such a narrow option.
I'm bad at bikesheeding. The 'edit' come from the 'edit' command in
rebase interactive. I am open to other names. Note that in the
following I keep the --edit name, not because I am suborn but because
I do not have better to propose yet.
> At the UI level, "git commit --amend HEAD~4" might be a more natural way
> to invoke such an operation, I would think.
As I say in the original email the point of the 'rebase --edit
some-commit' is to temporally checkout some-commit so that edits are
done in the context of the commit and not in the context of the head
of the branch.
One has to do a rebase --edit prior to modification (although we can
imagine a possibility to bring back the index or the content of the
working directory with us either automatically as with a branch
checkout or manually with the help of stash).
Unless I misinterpret the 'git commit --amend HEAD~4' you suggest, it
means that you have to prepare the commit in the head of the branch.
It may be difficult if what was in HEAD~4 is altered by HEAD~2.
My argument is that if preparing a patch to HEAD~4 in HEAD is easy, a
git commit --fixup will do the tick.
If the preparation is difficult because I have to work on (or more
insidious, near) change that occurs between HEAD~4 and HEAD, I need
something like my proposal. For example I added a line in HEAD~2 but I
prefer now to have this line to appears in HEAD~4.
The workflow I propose is :
$ # we are on master
$ git rebase --edit HEAD~4 # workdir is a detached master~4 like with a
# git checkout master~4
$ hack hack hack; git add files
$ git commit --amend
$ git rebase --continue # conflict is detected with master~2, resolve it
# manually
$ git rebase --continue # workdir is now a rebased master
I do no see what is the workflow with an extend git commit --amend
Do you mean something like the following ?
$ git checkout HEAD~4 -- . # bring back the content of master~4 but
# HEAD still points master
$ hack hack hack; git add files # And try to now be disturbed by the fact
# that diff and status are polluted
# with things related to master
$ git commit --amend HEAD~4 # conflict is detected with master~2, resolve it
$ git rebase --continue # Do we really want using "git rebase"?
This last workflow seems so awkward to me that I might miss something.
-- Jean Privat
^ permalink raw reply
* Re: [PATCH] Make is_gitfile a non-static generic function
From: Phil Hord @ 2011-10-12 13:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phil Hord, git@vger.kernel.org
In-Reply-To: <7vipnvccso.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>> The new is_gitfile is an amalgam of similar functional checks
>> from different places in the code....
>
>
> After looking at this patch and the way the other caller in transport.c
> uses it, I am more and more convinced that "is_gitfile()" is a stupid and
> horrible mistake.
I think it's a simple and low-impact change that fixes a bug with a
minimum of disruption. But I also think it is lazy.
> The caller in transport.c says "I am about to read from a regular file,
> and usually I would treat it as a bundle, but I want to avoid that
> codepath if that regular file is not a bundle. So I use the codepath only
> when that file is not a gitfile".
>
> It should be saying "Is it a bundle? Then I'd use the codepath to read
> from the bundle" to begin with. Otherwise the code will break when we add
> yet another regular file we can fetch from that is not a bundle nor a
> gitfile.
Yes, and this is part of the kind of distraction that held back my
update over the weekend.
When we do add another file type we'll wind up with a half-dozen
places that get affected in slightly different ways again. Wouldn't
it be nice to have a function to tell us what kind of thing it is
we've been asked to look at? Something like git_type(url) that
returns GIT_BUNDLE, GIT_DIRECTORY or GIT_FILE, maybe.
Except I didn't see many examples in the code using this sort of
enumerated decision function.
> I think the hand-crafted check in builtin/clone.c you removed originated
> from laziness to avoid teaching read_gitfile() to read potential gitfile
> silently (and signal errors by simply returning NULL).
I made a read_gitfile(... , gently) function, but I didn't like it
much. When !gently, I think it should be rather explicit about the
type of failure. This makes the code look like 20% of it is repeated
"if (!gently) die... ;\n return;" sequences. It's almost enough to
lead me to macros.
And what about when fopen() fails and we are running silently. Do we
just shrug and say "Not a gitfile"? I don't think it's good enough.
We need to be able to say all of these:
It's a gitfile, here's the internal path.
It's not a gitfile, it is something else.
It looked like a gitfile until I ran into E_ACCES or some other error.
Making the one function run silently or not complicates the code further.
I tried to find a similar style to mimic elsewhere in the code, but I
didn't find any consistency. Pointers to clean examples would be
welcome.
I started working on more of an API. But it's still very ugly and not
ready for even a strawman discussion.
But I don't know how much time I have for a full writeup atm. Without
something, though, I cannot easily fetch from a submodule, because
submodules all use gitfiles now, and git:master does not know how to
fetch from them.
And that's the itch I had to scratch.
> I also suspect the
> codepath may become simpler if we had a way to ask "Is this a bundle?".
>
> I think read_bundle_header() in bundle.c can be refactored to a silent
> interface that allows us to ask "Is this a bundle?" question properly.
I'll take a look at it. But I won't have much time for it this week.
Thanks,
Phil
^ permalink raw reply
* [PATCH 3/3] t5403: do not use access repos with GIT_DIR when worktree is involved
From: Nguyễn Thái Ngọc Duy @ 2011-10-12 9:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1318412105-13595-1-git-send-email-pclouds@gmail.com>
Setting GIT_DIR alone means worktree is current directory for legacy
reasons. Avoid using that, instead go to the worktree and execute
commands there.
The troublesome command is "GIT_DIR=clone2/.git git add clone2/b". The
real worktree is clone2, but that command tells git worktree is $(pwd).
What does user expect to add then? Should the new entry in index be "b"
or "clone2/b"?
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/t5403-post-checkout-hook.sh | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 6643f32..3459539 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -13,10 +13,13 @@ test_expect_success setup '
git commit -m setup &&
git clone ./. clone1 &&
git clone ./. clone2 &&
- GIT_DIR=clone2/.git git branch new2 &&
- echo Data for commit1. >clone2/b &&
- GIT_DIR=clone2/.git git add clone2/b &&
- GIT_DIR=clone2/.git git commit -m new2
+ (
+ cd clone2 &&
+ git branch new2 &&
+ echo Data for commit1. >b &&
+ git add b &&
+ git commit -m new2
+ )
'
for clone in 1 2; do
@@ -45,7 +48,7 @@ test_expect_success 'post-checkout runs as expected ' '
'
test_expect_success 'post-checkout args are correct with git checkout -b ' '
- GIT_DIR=clone1/.git git checkout -b new1 &&
+ ( cd clone1 && git checkout -b new1 ) &&
old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
@@ -53,7 +56,7 @@ test_expect_success 'post-checkout args are correct with git checkout -b ' '
'
test_expect_success 'post-checkout receives the right args with HEAD changed ' '
- GIT_DIR=clone2/.git git checkout new2 &&
+ ( cd clone2 && git checkout new2 ) &&
old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
@@ -61,7 +64,7 @@ test_expect_success 'post-checkout receives the right args with HEAD changed ' '
'
test_expect_success 'post-checkout receives the right args when not switching branches ' '
- GIT_DIR=clone2/.git git checkout master b &&
+ ( cd clone2 && git checkout master b ) &&
old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH 2/3] t5403: convert leading spaces to tabs
From: Nguyễn Thái Ngọc Duy @ 2011-10-12 9:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1318412105-13595-1-git-send-email-pclouds@gmail.com>
The first and last tests use tabs. The rest uses spaces. Convert all
to tabs.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/t5403-post-checkout-hook.sh | 46 ++++++++++++++++++++--------------------
1 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 0c126d7..6643f32 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -28,44 +28,44 @@ EOF
done
test_expect_success 'post-checkout runs as expected ' '
- GIT_DIR=clone1/.git git checkout master &&
- test -e clone1/.git/post-checkout.args
+ GIT_DIR=clone1/.git git checkout master &&
+ test -e clone1/.git/post-checkout.args
'
test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
- old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
- test $old = $new -a $flag = 1
+ old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
+ new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
+ flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+ test $old = $new -a $flag = 1
'
test_expect_success 'post-checkout runs as expected ' '
- GIT_DIR=clone1/.git git checkout master &&
- test -e clone1/.git/post-checkout.args
+ GIT_DIR=clone1/.git git checkout master &&
+ test -e clone1/.git/post-checkout.args
'
test_expect_success 'post-checkout args are correct with git checkout -b ' '
- GIT_DIR=clone1/.git git checkout -b new1 &&
- old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
- test $old = $new -a $flag = 1
+ GIT_DIR=clone1/.git git checkout -b new1 &&
+ old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
+ new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
+ flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+ test $old = $new -a $flag = 1
'
test_expect_success 'post-checkout receives the right args with HEAD changed ' '
- GIT_DIR=clone2/.git git checkout new2 &&
- old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
- test $old != $new -a $flag = 1
+ GIT_DIR=clone2/.git git checkout new2 &&
+ old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
+ new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
+ flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+ test $old != $new -a $flag = 1
'
test_expect_success 'post-checkout receives the right args when not switching branches ' '
- GIT_DIR=clone2/.git git checkout master b &&
- old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
- test $old = $new -a $flag = 0
+ GIT_DIR=clone2/.git git checkout master b &&
+ old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
+ new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
+ flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+ test $old = $new -a $flag = 0
'
if test "$(git config --bool core.filemode)" = true; then
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH 1/3] t5403.1: simplify commit creation
From: Nguyễn Thái Ngọc Duy @ 2011-10-12 9:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/t5403-post-checkout-hook.sh | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index d05a913..0c126d7 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,11 +9,8 @@ test_description='Test the post-checkout hook.'
test_expect_success setup '
echo Data for commit0. >a &&
echo Data for commit0. >b &&
- git update-index --add a &&
- git update-index --add b &&
- tree0=$(git write-tree) &&
- commit0=$(echo setup | git commit-tree $tree0) &&
- git update-ref refs/heads/master $commit0 &&
+ git add a b &&
+ git commit -m setup &&
git clone ./. clone1 &&
git clone ./. clone2 &&
GIT_DIR=clone2/.git git branch new2 &&
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
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