Git development
 help / color / mirror / Atom feed
* Re: [ANNOUNCE] Git User's Survey 2011
From: Miles Bader @ 2011-09-05  5:37 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <201109050243.21299.jnareb@gmail.com>

Hmmm, a cool thing about the git survey is that I learn quite a few
new things (commands/concepts/websites) from it ...

Especially when it comes to various documentation/help/tool resources,
it's a nice concise list!

-miles

-- 
The key to happiness
 is having dreams.      [from a fortune cookie]

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Johannes Sixt @ 2011-09-05  7:03 UTC (permalink / raw)
  To: Naohiro Aota; +Cc: git, gitster, tarmigan+git
In-Reply-To: <8762l73758.fsf@elisp.net>

Am 9/5/2011 7:11, schrieb Naohiro Aota:
> Variable expansions like "${foo#bar}" or "${foo%bar}" doesn't work on
> shells like FreeBSD sh and they made the test to fail. This patch
> replace such variable expansions with sed.
> 
> Signed-off-by: Naohiro Aota <naota@elisp.net>
> ---
> 
> Testing on FreeBSD failed because of this "bash-ism".

These are not bashism, but features require by POSIX.

I'd rather suspect that the failures are not because FreeBSD sh does not
have ${%} or ${#}, but rather that it interprets the meaning of the
backslash in this case in a way different from other shells.

>  run_backend() {
>  	echo "$2" |
> -	QUERY_STRING="${1#*\?}" \
> -	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \

What happens if you write these as

	QUERY_STRING=${1#*\?} \
	PATH_TRANSLATED=$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*} \

i.e., drop the double-quotes?

-- Hannes

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Junio C Hamano @ 2011-09-05  7:09 UTC (permalink / raw)
  To: Naohiro Aota; +Cc: git, tarmigan+git, David Barr
In-Reply-To: <8762l73758.fsf@elisp.net>

Naohiro Aota <naota@elisp.net> writes:

> Variable expansions like "${foo#bar}" or "${foo%bar}" doesn't work on
> shells like FreeBSD sh and they made the test to fail.

Sorry, I do appreciate the effort, but a patch like this takes us in the
wrong direction.

While we do not allow blatant bashisms like ${parameter:offset:length}
(substring expansion), ${parameter/pattern/string} (pattern substitution),
"local" variables, "function" noiseword, and shell arrays in our shell
scripts, the two kinds of substitution you quoted above are purely POSIX,
and our coding guideline does allow them to be used in the scripts.

Even though you may be able to rewrite trivial cases easily in some
scripts (either tests or Porcelain), some Porcelain scripts we ship
(e.g. "git bisect", "git stash", "git pull", etc.) do use these POSIX
constructs, and we do not want to butcher them with extra forks and
reduced readability.

Please use $SHELL_PATH and point to a POSIX compliant shell on your
platform instead. "make test" should pick it up and pass it down to
t/Makefile to be used when it runs these test scripts.

Besides, even inside t/ directory, there are many other instances of these
prefix/postfix substitution, not just 5560. Do the following tests pass on
your box without a similar patch?

$ git grep -n -e '\${[^}]*[#%]' -- t/\*.sh
t/t1410-reflog.sh:33:	aa=${1%??????????????????????????????????????} zz=${1#??}
t/t1410-reflog.sh:38:	aa=${1%??????????????????????????????????????} zz=${1#??}
t/t2030-unresolve-info.sh:125:	rerere_id=${rerere_id%/postimage} &&
t/t2030-unresolve-info.sh:151:	rerere_id=${rerere_id%/postimage} &&
t/t5560-http-backend-noserver.sh:12:	QUERY_STRING="${1#*\?}" \
t/t5560-http-backend-noserver.sh:13:	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \
t/t6050-replace.sh:124:     aa=${HASH2%??????????????????????????????????????} &&
t/t9010-svn-fe.sh:17:		printf "%s\n" "K ${#property}" &&
t/t9010-svn-fe.sh:19:		printf "%s\n" "V ${#value}" &&
t/t9010-svn-fe.sh:30:	printf "%s\n" "Text-content-length: ${#text}" &&
t/t9010-svn-fe.sh:31:	printf "%s\n" "Content-length: $((${#text} + 10))" &&
t/test-lib.sh:838:		test_results_path="$test_results_dir/${0%.sh}-$$.counts"
t/test-lib.sh:1047:this_test=${0##*/}
t/test-lib.sh:1048:this_test=${this_test%%-*}
t/valgrind/analyze.sh:98:				test $output = ${output%.message} &&

Looking at the above output, I suspect that it _might_ be that your shell
is almost POSIX but does not handle the backslash-quoted question mark
correctly or something silly like that, in which case a stupid patch like
the attached might be an acceptable compromise, until the shell is fixed.

By the way, t9010 uses ${#parameter} (strlen) which is bashism we forbid,
and it needs to be rewritten (David CC'ed).

Thanks.

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index 0ad7ce0..c8bbacc 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -9,8 +9,8 @@ test_have_prereq MINGW && export GREP_OPTIONS=-U
 
 run_backend() {
 	echo "$2" |
-	QUERY_STRING="${1#*\?}" \
-	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \
+	QUERY_STRING="${1#*[?]}" \
+	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%[?]*}" \
 	git http-backend >act.out 2>act.err
 }
 

^ permalink raw reply related

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Junio C Hamano @ 2011-09-05  7:15 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Naohiro Aota, git, tarmigan+git
In-Reply-To: <4E647442.9000005@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

>>  run_backend() {
>>  	echo "$2" |
>> -	QUERY_STRING="${1#*\?}" \
>> -	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \
>
> What happens if you write these as
>
> 	QUERY_STRING=${1#*\?} \
> 	PATH_TRANSLATED=$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*} \
>
> i.e., drop the double-quotes?

Interesting. Your conjecture is that the shell may be dropping the
backslash inside dq context when it does not understand what follows the
backslash, i.e. "\?"  -> "?", losing the quote. I find it very plausible.

If that is the case, either the above or my [?] would work it around, I
would think.

Thanks.

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Johannes Sixt @ 2011-09-05  7:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Naohiro Aota, git, tarmigan+git
In-Reply-To: <7v7h5nxxwf.fsf@alter.siamese.dyndns.org>

Am 9/5/2011 9:15, schrieb Junio C Hamano:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>>>  run_backend() {
>>>  	echo "$2" |
>>> -	QUERY_STRING="${1#*\?}" \
>>> -	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \
>>
>> What happens if you write these as
>>
>> 	QUERY_STRING=${1#*\?} \
>> 	PATH_TRANSLATED=$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*} \
>>
>> i.e., drop the double-quotes?
> 
> Interesting. Your conjecture is that the shell may be dropping the
> backslash inside dq context when it does not understand what follows the
> backslash, i.e. "\?"  -> "?", losing the quote. I find it very plausible.

Actually, it's the opposite: Within double-quotes, a backslash is only
removed when the next character has a special meaning (essentially $, `,
", \), otherwise, it remains and loses its quoting ability. This means,
that the backslash would remain as a literal character in our patterns on
the right of % or #, and they would not work anymore as intended.

Other shells seem to parse the pattern following % and # in a different
mode, which keeps the quoting ability of the backslash even inside
double-quotes... (And to me it looks like those shells are wrong.)

Without double-quotes, backslashes (that are not themselves quoted) are
always removed and give the subsequent character its literal meaning.
Hence, in my version, the question mark would unambiguously (I think) act
as a literal rather than a wildcard.

> If that is the case, either the above or my [?] would work it around, I
> would think.

[?] instead of \? is certainly also worth a try.

-- Hannes

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Junio C Hamano @ 2011-09-05  7:45 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Naohiro Aota, git, tarmigan+git
In-Reply-To: <4E647BD5.8060609@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Actually, it's the opposite: Within double-quotes, a backslash is only
> removed when the next character has a special meaning (essentially $, `,
> ", \), otherwise, it remains and loses its quoting ability. This means,
> that the backslash would remain as a literal character in our patterns on
> the right of % or #, and they would not work anymore as intended.

That's strange...

I thought that VAR=<any string without $IFS character in it> would behave
identically to VAR="<the same string as above>". You seem to be saying
that they should act differently.

>> If that is the case, either the above or my [?] would work it around, I
>> would think.
>
> [?] instead of \? is certainly also worth a try.

I obviously agree. Besides, [?] would sidestep the tricky backslash vs
double quote issue entirely, so it would be a more robust solution to
leave it around than "sometimes you need to avoid double-quote and some
other times you would need double-quote" for other people to mimic writing
tests later.

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Johannes Sixt @ 2011-09-05  7:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Naohiro Aota, git, tarmigan+git, David Barr
In-Reply-To: <7vbouzxy7g.fsf@alter.siamese.dyndns.org>

Am 9/5/2011 9:09, schrieb Junio C Hamano:
> By the way, t9010 uses ${#parameter} (strlen) which is bashism we forbid,
> and it needs to be rewritten (David CC'ed).

Actually, no. It is perfectly valid POSIX. So we would need this patch.

--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] CodingGuidelines: ${#parameter} is POSIX and should be allowed

See http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Documentation/CodingGuidelines |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index fe1c1e5..df0b620 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -52,8 +52,6 @@ For shell scripts specifically (not exhaustive):
 
    - No shell arrays.
 
-   - No strlen ${#parameter}.
-
    - No pattern replacement ${parameter/pattern/string}.
 
  - We use Arithmetic Expansion $(( ... )).
-- 
1.7.7.rc0.211.g5ac7e

^ permalink raw reply related

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Naohiro Aota @ 2011-09-05  7:55 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, gitster, tarmigan+git
In-Reply-To: <4E647442.9000005@viscovery.net>

Ah, seems I was misundertanding much of things :(

Johannes Sixt <j.sixt@viscovery.net> writes:

> What happens if you write these as
>
> 	QUERY_STRING=${1#*\?} \
> 	PATH_TRANSLATED=$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*} \
>
> i.e., drop the double-quotes?

not worked, even increased the number of failure...

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

> Naohiro Aota <naota@elisp.net> writes:
>
>> Variable expansions like "${foo#bar}" or "${foo%bar}" doesn't work on
>> shells like FreeBSD sh and they made the test to fail.
>
> Sorry, I do appreciate the effort, but a patch like this takes us in the
> wrong direction.
>
> While we do not allow blatant bashisms like ${parameter:offset:length}
> (substring expansion), ${parameter/pattern/string} (pattern substitution),
> "local" variables, "function" noiseword, and shell arrays in our shell
> scripts, the two kinds of substitution you quoted above are purely POSIX,
> and our coding guideline does allow them to be used in the scripts.
>
> Even though you may be able to rewrite trivial cases easily in some
> scripts (either tests or Porcelain), some Porcelain scripts we ship
> (e.g. "git bisect", "git stash", "git pull", etc.) do use these POSIX
> constructs, and we do not want to butcher them with extra forks and
> reduced readability.
>
> Please use $SHELL_PATH and point to a POSIX compliant shell on your
> platform instead. "make test" should pick it up and pass it down to
> t/Makefile to be used when it runs these test scripts.

Thanks, I'll try this.

> Besides, even inside t/ directory, there are many other instances of these
> prefix/postfix substitution, not just 5560. Do the following tests pass on
> your box without a similar patch?
>
> $ git grep -n -e '\${[^}]*[#%]' -- t/\*.sh
> t/t1410-reflog.sh:33:	aa=${1%??????????????????????????????????????} zz=${1#??}
> t/t1410-reflog.sh:38:	aa=${1%??????????????????????????????????????} zz=${1#??}
> t/t2030-unresolve-info.sh:125:	rerere_id=${rerere_id%/postimage} &&
> t/t2030-unresolve-info.sh:151:	rerere_id=${rerere_id%/postimage} &&
> t/t5560-http-backend-noserver.sh:12:	QUERY_STRING="${1#*\?}" \
> t/t5560-http-backend-noserver.sh:13:	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \
> t/t6050-replace.sh:124:     aa=${HASH2%??????????????????????????????????????} &&
> t/t9010-svn-fe.sh:17:		printf "%s\n" "K ${#property}" &&
> t/t9010-svn-fe.sh:19:		printf "%s\n" "V ${#value}" &&
> t/t9010-svn-fe.sh:30:	printf "%s\n" "Text-content-length: ${#text}" &&
> t/t9010-svn-fe.sh:31:	printf "%s\n" "Content-length: $((${#text} + 10))" &&
> t/test-lib.sh:838:		test_results_path="$test_results_dir/${0%.sh}-$$.counts"
> t/test-lib.sh:1047:this_test=${0##*/}
> t/test-lib.sh:1048:this_test=${this_test%%-*}
> t/valgrind/analyze.sh:98:				test $output = ${output%.message} &&

I've tried t[0-9]{4}-*.sh and all of them passed. (t9010 had some known
breakages) yeah, my patch was taking wrong way.

> Looking at the above output, I suspect that it _might_ be that your shell
> is almost POSIX but does not handle the backslash-quoted question mark
> correctly or something silly like that, in which case a stupid patch like
> the attached might be an acceptable compromise, until the shell is fixed.
>
> diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
> index 0ad7ce0..c8bbacc 100755
> --- a/t/t5560-http-backend-noserver.sh
> +++ b/t/t5560-http-backend-noserver.sh
> @@ -9,8 +9,8 @@ test_have_prereq MINGW && export GREP_OPTIONS=-U
>  
>  run_backend() {
>  	echo "$2" |
> -	QUERY_STRING="${1#*\?}" \
> -	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%\?*}" \
> +	QUERY_STRING="${1#*[?]}" \
> +	PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%[?]*}" \
>  	git http-backend >act.out 2>act.err
>  }

This worked on my box. hm, then the problem should be in /bin/sh

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Junio C Hamano @ 2011-09-05  8:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Naohiro Aota, git, tarmigan+git, David Barr
In-Reply-To: <4E648031.6050607@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 9/5/2011 9:09, schrieb Junio C Hamano:
>> By the way, t9010 uses ${#parameter} (strlen) which is bashism we forbid,
>> and it needs to be rewritten (David CC'ed).
>
> Actually, no. It is perfectly valid POSIX. So we would need this patch.

I know it is in POSIX, but not in the subset we allowed so far. I do not
recall the details offhand, but we must have seen some shell that lacked
it or something.

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Johannes Sixt @ 2011-09-05  8:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Naohiro Aota, git, tarmigan+git
In-Reply-To: <7v39gbxwi6.fsf@alter.siamese.dyndns.org>

Am 9/5/2011 9:45, schrieb Junio C Hamano:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> Actually, it's the opposite: Within double-quotes, a backslash is only
>> removed when the next character has a special meaning (essentially $, `,
>> ", \), otherwise, it remains and loses its quoting ability. This means,
>> that the backslash would remain as a literal character in our patterns on
>> the right of % or #, and they would not work anymore as intended.
> 
> That's strange...
> 
> I thought that VAR=<any string without $IFS character in it> would behave
> identically to VAR="<the same string as above>". You seem to be saying
> that they should act differently.

They are not the same.

First of all, the value of $IFS is irrelevant whether or not you need
double-quotes on the RHS of an assignment, because it is purely a
syntactic matter; $IFS plays no role during syntax analysis. It is only
the presence of white-space that sometimes[*] requires quoting of some form.

The most visible difference is a backslash that is followed by a character
that is not special:

$ foo="a\xb" env | grep foo; foo=a\xb env | grep foo
foo=a\xb
foo=axb

But it is the same elsewhere in a command:

$ echo "a\xb"; echo a\xb
a\xb
axb

The reason is that a backslash inside double-quotes remains as a literal
character when it is not followed by a special character, whereas outside
double-quotes an unquoted backslash is always removed.

[*] No quoting is required in cases like this: VAR=$(echo foo)

>> [?] instead of \? is certainly also worth a try.
> 
> I obviously agree. Besides, [?] would sidestep the tricky backslash vs
> double quote issue entirely, so it would be a more robust solution to
> leave it around than "sometimes you need to avoid double-quote and some
> other times you would need double-quote" for other people to mimic writing
> tests later.

Good point, and I shall prefer this solution as well.

-- Hannes

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Johannes Sixt @ 2011-09-05  8:21 UTC (permalink / raw)
  Cc: Junio C Hamano, Naohiro Aota, git, tarmigan+git
In-Reply-To: <4E648546.8060303@viscovery.net>

Am 9/5/2011 10:16, schrieb Johannes Sixt:
> The most visible difference is a backslash that is followed by a character
> that is not special:

I should have said: "The difference that I can see immediately is...". I
suspect there are other subtle differences that are not so obvious to me.

-- Hannes

^ permalink raw reply

* Re: [PATCH] shell portability: Use sed instead of non-portable variable expansion
From: Junio C Hamano @ 2011-09-05  8:22 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Naohiro Aota, git, tarmigan+git, David Barr
In-Reply-To: <4E648031.6050607@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 9/5/2011 9:09, schrieb Junio C Hamano:
>> By the way, t9010 uses ${#parameter} (strlen) which is bashism we forbid,
>> and it needs to be rewritten (David CC'ed).
>
> Actually, no. It is perfectly valid POSIX. So we would need this patch.
>
> --- 8< ---
> From: Johannes Sixt <j6t@kdbg.org>
> Subject: [PATCH] CodingGuidelines: ${#parameter} is POSIX and should be allowed
>
> See http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---

I would prefer to play it safe at least for now, especially before 1.7.7
ships.

 Documentation/CodingGuidelines |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index fe1c1e5..594fb76 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -52,7 +52,7 @@ For shell scripts specifically (not exhaustive):
 
    - No shell arrays.
 
-   - No strlen ${#parameter}.
+   - No strlen ${#parameter} (even though it is in POSIX).
 
    - No pattern replacement ${parameter/pattern/string}.
 

^ permalink raw reply related

* Re: [ANNOUNCE] Git User's Survey 2011
From: Michael J Gruber @ 2011-09-05  8:33 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <201109050243.21299.jnareb@gmail.com>

Jakub Narebski venit, vidit, dixit 05.09.2011 02:43:
> Hello all,
> 
> We would like to ask you a few questions about your use of the Git
> version control system. This survey is mainly to understand who is
> using Git, how and why.
> 
> The results will be published to the Git wiki on the GitSurvey2011
> page (https://git.wiki.kernel.org/index.php/GitSurvey2011) and
> discussed on the git mailing list.

Jakub, thanks for your work!

I've made a few last minute minor edits on the wiki (language-wise) and
linked to it from identi.ca, twitter and g+. Hope that's alright.

Let the results come in!

Michael

^ permalink raw reply

* Re: [PATCH 3/3] push: old receive-pack does not understand --quiet
From: Junio C Hamano @ 2011-09-05  8:35 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: git, tobiasu, Michael J Gruber
In-Reply-To: <1315067656-2846-4-git-send-email-drizzd@aon.at>

Hmm, with this whole series merged to 'pu', I somehow am getting this
error:

$ make -j8 \
    DEFAULT_TEST_TARGET=prove \
    GIT_PROVE_OPTS=-j8 \
    T=t5541-http-push.sh test
*** prove ***
t5541-http-push.sh .. All 1 subtests passed 

Test Summary Report
-------------------
t5541-http-push.sh (Wstat: 0 Tests: 1 Failed: 0)
  Parse errors: No plan found in TAP output
Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.06 cusr  0.01 csys =  0.09 CPU)
Result: FAIL
make[1]: *** [prove] Error 1
make[1]: Leaving directory `/srv/project/git/git.git/t'
make: *** [test] Error 2

Without prove (drop "DEFAULT_TEST_TARGET=prove" from the command line),
I do not see the breakage.

*** t5541-http-push.sh ***
ok 1 - set up terminal for tests
# passed all 1 test(s)
1..1 # SKIP Network testing disabled (define GIT_TEST_HTTPD to enable)
make aggregate-results
make[3]: Entering directory `/srv/project/git/git.git/t'
for f in test-results/t*-*.counts; do \
                echo "$f"; \
        done | '/bin/sh' ./aggregate-results.sh
fixed   0
success 1
failed  0
broken  0
total   1

^ permalink raw reply

* Re: [PATCH 3/3] push: old receive-pack does not understand --quiet
From: Michael J Gruber @ 2011-09-05  9:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Clemens Buchacher, git, tobiasu
In-Reply-To: <7v62l7crpg.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 05.09.2011 10:35:
> Hmm, with this whole series merged to 'pu', I somehow am getting this
> error:
> 
> $ make -j8 \
>     DEFAULT_TEST_TARGET=prove \
>     GIT_PROVE_OPTS=-j8 \
>     T=t5541-http-push.sh test
> *** prove ***
> t5541-http-push.sh .. All 1 subtests passed 
> 
> Test Summary Report
> -------------------
> t5541-http-push.sh (Wstat: 0 Tests: 1 Failed: 0)
>   Parse errors: No plan found in TAP output
> Files=1, Tests=1,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.06 cusr  0.01 csys =  0.09 CPU)
> Result: FAIL
> make[1]: *** [prove] Error 1
> make[1]: Leaving directory `/srv/project/git/git.git/t'
> make: *** [test] Error 2
> 
> Without prove (drop "DEFAULT_TEST_TARGET=prove" from the command line),
> I do not see the breakage.
> 
> *** t5541-http-push.sh ***
> ok 1 - set up terminal for tests
> # passed all 1 test(s)
> 1..1 # SKIP Network testing disabled (define GIT_TEST_HTTPD to enable)
> make aggregate-results
> make[3]: Entering directory `/srv/project/git/git.git/t'
> for f in test-results/t*-*.counts; do \
>                 echo "$f"; \
>         done | '/bin/sh' ./aggregate-results.sh
> fixed   0
> success 1
> failed  0
> broken  0
> total   1
> 

Being cc'ed makes me feel guilty but I don't know what for... Anyway, in
case you need more test points, with pu at v1.7.7-rc0-315-g55af6ac  and
prove I get:

*** prove ***
t5541-http-push.sh .. skipped: Network testing disabled (define
GIT_TEST_HTTPD to enable)
Files=1, Tests=0,  0 wallclock secs ( 0.04 usr  0.01 sys +  0.00 cusr
0.02 csys =  0.07 CPU)
Result: NOTESTS

Patch 3/3 does not apply (am) to pu but leaves neither changes nor
conflicts in my wt. -3 gives a conflict which I don't know how to
resolve without digging further, see below.

Michael

The conflict in send-pack.c is simple, but what about:

diff --cc builtin/receive-pack.c
index 0b42e21,06c481a..0000000
--- i/builtin/receive-pack.c
+++ w/builtin/receive-pack.c
@@@ -29,9 -29,9 +29,10 @@@ static int receive_fsck_objects
  static int receive_unpack_limit = -1;
  static int transfer_unpack_limit = -1;
  static int unpack_limit = 100;
 +static unsigned long limit_pack_size, limit_commit_count,
limit_object_count;
  static int report_status;
  static int use_sideband;
+ static int quiet;
  static int prefer_ofs_delta = 1;
  static int auto_update_server_info;
  static int auto_gc = 1;
@@@ -144,8 -113,10 +145,15 @@@ static int show_ref(const char *path, c
        if (sent_capabilities)
                packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
        else
++<<<<<<< HEAD
 +              packet_write(1, "%s %s%c%s\n",
 +                           sha1_to_hex(sha1), path, 0, capabilities());
++=======
+               packet_write(1, "%s %s%c%s%s\n",
+                            sha1_to_hex(sha1), path, 0,
+                            " report-status delete-refs side-band-64k
quiet",
+                            prefer_ofs_delta ? " ofs-delta" : "");
++>>>>>>> push: old receive-pack does not understand --quiet
        sent_capabilities = 1;
        return 0;
  }

^ permalink raw reply

* [BUG] git bisect start fails when stale bisect data is left behind
From: Joel Kaasinen @ 2011-09-05 11:15 UTC (permalink / raw)
  To: git

Hi,

Just bumped into a weird bug: bisect refused to start. It seems some
previous bisect had left around state that referred to a
now-nonexistent branch.

How to reproduce:
$ echo foo > .git/BISECT_START
$ git bisect start HEAD HEAD^

Fails with "fatal: invalid reference:" on git 1.7.6.

–J

PS. Please CC me when answering, I'm not subscribed

^ permalink raw reply

* survey
From: Mihamina Rakotomandimby @ 2011-09-05 11:16 UTC (permalink / raw)
  To: git

Hi all,

I just filled the Git survey, thank you guys to care about users 
experience ;-)

-- 
RMA.

^ permalink raw reply

* Re: [PATCH 10/10] want_color: automatically fallback to color.ui
From: Steffen Daode Nurpmeso @ 2011-09-05 11:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Martin von Zweigbergk, git, Ingo Brückl
In-Reply-To: <20110904125312.GA21724@sigill.intra.peff.net>

@ Jeff King <peff@peff.net> wrote (2011-09-04 14:53+0200):
> Sorry, this is a regression.

I should have found that.
I apologize.

--Steffen
Ciao, sdaoden(*)(gmail.com)
ASCII ribbon campaign           ( ) More nuclear fission plants
  against HTML e-mail            X    can serve more coloured
    and proprietary attachments / \     and sounding animations

^ permalink raw reply

* [PATCH 1/2] Add strtoimax() compatibility function.
From: Nix @ 2011-09-05 11:45 UTC (permalink / raw)
  To: git; +Cc: Nix

Since systems that omit strtoumax() will likely omit strtomax() too,
and likewise for strtoull() and strtoll(), we also adjust the
compatibility #defines from NO_STRTOUMAX to NO_STRTOMAX and from
NO_STRTOULL to NO_STRTOLL, and have them cover both the signed and
unsigned functions.

Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
 Makefile           |   36 ++++++++++++++++++------------------
 compat/strtoimax.c |   10 ++++++++++
 compat/strtoumax.c |    2 +-
 3 files changed, 29 insertions(+), 19 deletions(-)
 create mode 100644 compat/strtoimax.c

diff --git a/Makefile b/Makefile
index 6bf7d6c..0959e07 100644
--- a/Makefile
+++ b/Makefile
@@ -57,9 +57,9 @@ all::
 #
 # Define NO_STRLCPY if you don't have strlcpy.
 #
-# Define NO_STRTOUMAX if you don't have strtoumax in the C library.
-# If your compiler also does not support long long or does not have
-# strtoull, define NO_STRTOULL.
+# Define NO_STRTOMAX if you don't have both strtoimax and strtoumax in the
+# C library.  If your compiler also does not support long long or does
+# not have strtoll or strtoull, define NO_STRTOLL.
 #
 # Define NO_SETENV if you don't have setenv in the C library.
 #
@@ -804,7 +804,7 @@ ifeq ($(uname_S),OSF1)
 	# Need this for u_short definitions et al
 	BASIC_CFLAGS += -D_OSF_SOURCE
 	SOCKLEN_T = int
-	NO_STRTOULL = YesPlease
+	NO_STRTOLL = YesPlease
 	NO_NSEC = YesPlease
 endif
 ifeq ($(uname_S),Linux)
@@ -890,7 +890,7 @@ ifeq ($(uname_S),SunOS)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
 		NO_STRLCPY = YesPlease
-		NO_STRTOUMAX = YesPlease
+		NO_STRTOMAX = YesPlease
 		GIT_TEST_CMP = cmp
 	endif
 	ifeq ($(uname_R),5.7)
@@ -900,19 +900,19 @@ ifeq ($(uname_S),SunOS)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
 		NO_STRLCPY = YesPlease
-		NO_STRTOUMAX = YesPlease
+		NO_STRTOMAX = YesPlease
 		GIT_TEST_CMP = cmp
 	endif
 	ifeq ($(uname_R),5.8)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
-		NO_STRTOUMAX = YesPlease
+		NO_STRTOMAX = YesPlease
 		GIT_TEST_CMP = cmp
 	endif
 	ifeq ($(uname_R),5.9)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
-		NO_STRTOUMAX = YesPlease
+		NO_STRTOMAX = YesPlease
 		GIT_TEST_CMP = cmp
 	endif
 	INSTALL = /usr/ucb/install
@@ -954,7 +954,7 @@ ifeq ($(uname_S),FreeBSD)
 	ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
 		PTHREAD_LIBS = -pthread
 		NO_UINTMAX_T = YesPlease
-		NO_STRTOUMAX = YesPlease
+		NO_STRTOMAX = YesPlease
 	endif
 	PYTHON_PATH = /usr/local/bin/python
 	HAVE_PATHS_H = YesPlease
@@ -1092,8 +1092,8 @@ ifeq ($(uname_S),Windows)
 	NO_MEMMEM = YesPlease
 	# NEEDS_LIBICONV = YesPlease
 	NO_ICONV = YesPlease
-	NO_STRTOUMAX = YesPlease
-	NO_STRTOULL = YesPlease
+	NO_STRTOMAX = YesPlease
+	NO_STRTOLL = YesPlease
 	NO_MKDTEMP = YesPlease
 	NO_MKSTEMPS = YesPlease
 	SNPRINTF_RETURNS_BOGUS = YesPlease
@@ -1139,7 +1139,7 @@ ifeq ($(uname_S),Interix)
 	NO_IPV6 = YesPlease
 	NO_MEMMEM = YesPlease
 	NO_MKDTEMP = YesPlease
-	NO_STRTOUMAX = YesPlease
+	NO_STRTOMAX = YesPlease
 	NO_NSEC = YesPlease
 	NO_MKSTEMPS = YesPlease
 	ifeq ($(uname_R),3.5)
@@ -1184,7 +1184,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_MEMMEM = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	OLD_ICONV = YesPlease
-	NO_STRTOUMAX = YesPlease
+	NO_STRTOMAX = YesPlease
 	NO_MKDTEMP = YesPlease
 	NO_MKSTEMPS = YesPlease
 	NO_SVN_TESTS = YesPlease
@@ -1440,12 +1440,12 @@ ifdef NO_STRLCPY
 	COMPAT_CFLAGS += -DNO_STRLCPY
 	COMPAT_OBJS += compat/strlcpy.o
 endif
-ifdef NO_STRTOUMAX
-	COMPAT_CFLAGS += -DNO_STRTOUMAX
-	COMPAT_OBJS += compat/strtoumax.o
+ifdef NO_STRTOMAX
+	COMPAT_CFLAGS += -DNO_STRTOMAX
+	COMPAT_OBJS += compat/strtoumax.o compat/strtoimax.o
 endif
-ifdef NO_STRTOULL
-	COMPAT_CFLAGS += -DNO_STRTOULL
+ifdef NO_STRTOLL
+	COMPAT_CFLAGS += -DNO_STRTOLL
 endif
 ifdef NO_STRTOK_R
 	COMPAT_CFLAGS += -DNO_STRTOK_R
diff --git a/compat/strtoimax.c b/compat/strtoimax.c
new file mode 100644
index 0000000..fca07a3
--- /dev/null
+++ b/compat/strtoimax.c
@@ -0,0 +1,10 @@
+#include "../git-compat-util.h"
+
+intmax_t gitstrtoimax (const char *nptr, char **endptr, int base)
+{
+#if defined(NO_STRTOLL)
+	return strtol(nptr, endptr, base);
+#else
+	return strtoll(nptr, endptr, base);
+#endif
+}
diff --git a/compat/strtoumax.c b/compat/strtoumax.c
index 5541353..7feedfd 100644
--- a/compat/strtoumax.c
+++ b/compat/strtoumax.c
@@ -2,7 +2,7 @@
 
 uintmax_t gitstrtoumax (const char *nptr, char **endptr, int base)
 {
-#if defined(NO_STRTOULL)
+#if defined(NO_STRTOLL)
 	return strtoul(nptr, endptr, base);
 #else
 	return strtoull(nptr, endptr, base);
-- 
1.7.6.1.139.gcb612

^ permalink raw reply related

* [PATCH 2/2] Support sizes >=2G in various config options accepting 'g' sizes.
From: Nix @ 2011-09-05 11:45 UTC (permalink / raw)
  To: git; +Cc: Nix
In-Reply-To: <1315223155-4218-1-git-send-email-nix@esperi.org.uk>

The config options core.packedGitWindowSize, core.packedGitLimit,
core.deltaBaseCacheLimit, core.bigFileThreshold, pack.windowMemory and
pack.packSizeLimit all claim to support suffixes up to and including
'g'.  This implies that they should accept sizes >=2G on 64-bit
systems: certainly, specifying a size of 3g should not silently be
translated to zero or transformed into a large negative value due to
integer overflow. However, due to use of git_config_int() rather than
git_config_ulong(), that is exactly what happens:

% git config core.bigFileThreshold 2g
% git gc --aggressive # with extra debugging code to print out
                      # core.bigfilethreshold after parsing
bigfilethreshold: -2147483648
[...]

This is probably irrelevant for core.deltaBaseCacheLimit, but is
problematic for the other values. (It is particularly problematic for
core.packedGitLimit, which can't even be set to its default value in
the config file due to this bug.)

This fixes things for 32-bit platforms as well. They get the usual bad
config error if an overlarge value is specified, e.g.:

fatal: bad config value for 'core.bigfilethreshold' in /home/nix/.gitconfig

32-bit platforms with no type larger than 'long' cannot detect this
case and will continue to silently misbehave, but the misbehaviour
will be somewhat different and more useful, since bigFileThreshold was
also being mistakenly treated as a signed value when it should have
been unsigned.

Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
 config.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/config.c b/config.c
index 4183f80..b19df66 100644
--- a/config.c
+++ b/config.c
@@ -333,7 +333,7 @@ static int git_parse_file(config_fn_t fn, void *data)
 	die("bad config file line %d in %s", cf->linenr, cf->name);
 }
 
-static int parse_unit_factor(const char *end, unsigned long *val)
+static int parse_unit_factor(const char *end, uintmax_t *val)
 {
 	if (!*end)
 		return 1;
@@ -356,11 +356,14 @@ static int git_parse_long(const char *value, long *ret)
 {
 	if (value && *value) {
 		char *end;
-		long val = strtol(value, &end, 0);
-		unsigned long factor = 1;
+		intmax_t val = strtoimax(value, &end, 0);
+		uintmax_t factor = 1;
 		if (!parse_unit_factor(end, &factor))
 			return 0;
-		*ret = val * factor;
+		val *= factor;
+		if (val > maximum_signed_value_of_type(long))
+			return 0;
+		*ret = val;
 		return 1;
 	}
 	return 0;
@@ -370,9 +373,11 @@ int git_parse_ulong(const char *value, unsigned long *ret)
 {
 	if (value && *value) {
 		char *end;
-		unsigned long val = strtoul(value, &end, 0);
+		uintmax_t val = strtoumax(value, &end, 0);
 		if (!parse_unit_factor(end, &val))
 			return 0;
+		if (val > maximum_unsigned_value_of_type(long))
+			return 0;
 		*ret = val;
 		return 1;
 	}
@@ -391,6 +396,8 @@ int git_config_int(const char *name, const char *value)
 	long ret = 0;
 	if (!git_parse_long(value, &ret))
 		die_bad_config(name);
+	if (ret > maximum_signed_value_of_type(int))
+		die_bad_config(name);
 	return ret;
 }
 
@@ -550,7 +557,7 @@ static int git_default_core_config(const char *var, const char *value)
 
 	if (!strcmp(var, "core.packedgitwindowsize")) {
 		int pgsz_x2 = getpagesize() * 2;
-		packed_git_window_size = git_config_int(var, value);
+		packed_git_window_size = git_config_ulong(var, value);
 
 		/* This value must be multiple of (pagesize * 2) */
 		packed_git_window_size /= pgsz_x2;
@@ -561,18 +568,17 @@ static int git_default_core_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.bigfilethreshold")) {
-		long n = git_config_int(var, value);
-		big_file_threshold = 0 < n ? n : 0;
+		big_file_threshold = git_config_ulong(var, value);
 		return 0;
 	}
 
 	if (!strcmp(var, "core.packedgitlimit")) {
-		packed_git_limit = git_config_int(var, value);
+		packed_git_limit = git_config_ulong(var, value);
 		return 0;
 	}
 
 	if (!strcmp(var, "core.deltabasecachelimit")) {
-		delta_base_cache_limit = git_config_int(var, value);
+		delta_base_cache_limit = git_config_ulong(var, value);
 		return 0;
 	}
 
-- 
1.7.6.1.139.gcb612

^ permalink raw reply related

* Re: Special characters in file name
From: Tajti Ákos @ 2011-09-05 12:14 UTC (permalink / raw)
  To: Alexey Shumkin; +Cc: Jonathan Nieder, git
In-Reply-To: <20110904000933.195d5bf4@zappedws>

Thanks four your answer. However, it didn't help. After setting the 
core.quotepath option to false I get this:

diff --git a/<E1>rv<ED>zt<FB>r<F5> b/<E1>rv<ED>zt<FB>r<F5>

Is there a solution?

Thanks,
Ákos Tajti

2011.09.03. 22:09 keltezéssel, Alexey Shumkin írta:
>> Hi Alexey,
>>
>> Alexey Shumkin wrote:
>>> Tajti Ákos writes:
>>>> I have a file named "árvíz.txt" in my repository. When modify that
>>>> file and execute git diff, the first line looks like this:
>>>> diff --git "a/\303\241rv\303\255z.txt" "b/\303\241rv\303\255z.txt"
>>>>
>>>> Is there an option that (if specified) will get git to print
>>>> "árvíz.txt" instead of this escaped string?
>> [...]
>>> please, refresh your memory )))
>>> http://comments.gmane.org/gmane.comp.version-control.git/177849
>>>
>>> see my comment
>>> http://permalink.gmane.org/gmane.comp.version-control.git/177857
>> Cc-ing Ákos so he can actually get your message this time. :)  (FWIW
>> the convention on this list is always to reply-to-all.)
> Yes, I know, but I replied with the gmane.org site form, and it did not
> CC-ed

^ permalink raw reply

* Re: Special characters in file name
From: Tajti Ákos @ 2011-09-05 13:12 UTC (permalink / raw)
  To: Шумкин Алексей
  Cc: git@vger.kernel.org
In-Reply-To: <20110905170237.3ed59c6a@ashu.dyn.rarus.ru>

Both the terminal encoding and the filename encoding are UTF-8.

Ákos

2011.09.05. 15:02 keltezéssel, Шумкин Алексей írta:
>> Thanks four your answer. However, it didn't help. After setting the
>> core.quotepath option to false I get this:
>>
>> diff --git a/<E1>rv<ED>zt<FB>r<F5>  b/<E1>rv<ED>zt<FB>r<F5>
>>
>> Is there a solution?
> What terminal encoding do you use? and what is its encoding?
> It seems to me terminal encoding is UTF-8 but filename encoding is not
> the same.
> I get the same behaviour on Linux UTF-8 terminal for the project
> created on Windows with filenames in cp1251-encoding.
>
>> Thanks,
>> Ákos Tajti
>>
>> 2011.09.03. 22:09 keltezéssel, Alexey Shumkin írta:
>>>> Hi Alexey,
>>>>
>>>> Alexey Shumkin wrote:
>>>>> Tajti Ákos writes:
>>>>>> I have a file named "árvíz.txt" in my repository. When modify
>>>>>> that file and execute git diff, the first line looks like this:
>>>>>> diff --git "a/\303\241rv\303\255z.txt"
>>>>>> "b/\303\241rv\303\255z.txt"
>>>>>>
>>>>>> Is there an option that (if specified) will get git to print
>>>>>> "árvíz.txt" instead of this escaped string?
>>>> [...]
>>>>> please, refresh your memory )))
>>>>> http://comments.gmane.org/gmane.comp.version-control.git/177849
>>>>>
>>>>> see my comment
>>>>> http://permalink.gmane.org/gmane.comp.version-control.git/177857
>>>> Cc-ing Ákos so he can actually get your message this time. :)
>>>> (FWIW the convention on this list is always to reply-to-all.)
>>> Yes, I know, but I replied with the gmane.org site form, and it did
>>> not CC-ed

^ permalink raw reply

* Re: Special characters in file name
From: Johannes Sixt @ 2011-09-05 13:16 UTC (permalink / raw)
  To: Tajti Ákos; +Cc: Alexey Shumkin, Jonathan Nieder, git
In-Reply-To: <4E64BD3D.6060900@intland.com>

Am 9/5/2011 14:14, schrieb Tajti Ákos:
> Thanks four your answer. However, it didn't help. After setting the
> core.quotepath option to false I get this:
> 
> diff --git a/<E1>rv<ED>zt<FB>r<F5> b/<E1>rv<ED>zt<FB>r<F5>
> 
> Is there a solution?

This is because the output goes through the pager (less), which treats
non-ASCII characters.

Tweak your settings (LESSCHARSET) or use a different or no pager.

-- Hannes

^ permalink raw reply

* Re: Special characters in file name
From: Tajti Ákos @ 2011-09-05 13:33 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Alexey Shumkin, Jonathan Nieder, git
In-Reply-To: <4E64CBB0.7020902@viscovery.net>

Thank you very much, setting LESSCHARSET solved this last problem!

Ákos

2011.09.05. 15:16 keltezéssel, Johannes Sixt írta:
> Am 9/5/2011 14:14, schrieb Tajti Ákos:
>> Thanks four your answer. However, it didn't help. After setting the
>> core.quotepath option to false I get this:
>>
>> diff --git a/<E1>rv<ED>zt<FB>r<F5>  b/<E1>rv<ED>zt<FB>r<F5>
>>
>> Is there a solution?
> This is because the output goes through the pager (less), which treats
> non-ASCII characters.
>
> Tweak your settings (LESSCHARSET) or use a different or no pager.
>
> -- Hannes

^ permalink raw reply

* Re: [PATCH 2/2] Support sizes >=2G in various config options accepting 'g' sizes.
From: Sverre Rabbelier @ 2011-09-05 13:49 UTC (permalink / raw)
  To: Nix; +Cc: git
In-Reply-To: <1315223155-4218-2-git-send-email-nix@esperi.org.uk>

Heya,

On Mon, Sep 5, 2011 at 13:45, Nix <nix@esperi.org.uk> wrote:
> 32-bit platforms with no type larger than 'long' cannot detect this
> case and will continue to silently misbehave, but the misbehaviour
> will be somewhat different and more useful, since bigFileThreshold was
> also being mistakenly treated as a signed value when it should have
> been unsigned.

Is it not possible to detect that the target value won't fit in the
max size of an int when parsing the config value?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox