[PATCH] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[PATCH] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[Ævar Arnfjörð Bjarmason]

Using the qw(...) construct as implicit parentheses was deprecated in
perl 5.13.5. Change the relevant code in gitweb to not use the
deprecated construct. The offending code was introduced in 3562198b by
Jakub Narebski.

The issue is that perl will now warn about this:

    $ perl -wE 'for my $i qw(a b) { say $i }'
    Use of qw(...) as parentheses is deprecated at -e line 1.
    a
    b

This caused gitweb.perl to warn on perl 5.13.5 and above, and these
tests to fail on those perl versions:

    ./t9501-gitweb-standalone-http-status.sh           (Wstat: 256 Tests: 11 Failed: 10)
      Failed tests:  2-11
      Non-zero exit status: 1
    ./t9502-gitweb-standalone-parse-output.sh          (Wstat: 256 Tests: 10 Failed: 9)
      Failed tests:  2-10
      Non-zero exit status: 1
    ./t9500-gitweb-standalone-no-errors.sh             (Wstat: 256 Tests: 90 Failed: 84)
      Failed tests:  1-8, 10-36, 38-45, 47-48, 50-88
      Non-zero exit status: 1

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0779f12..b02372c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3501,7 +3501,7 @@ sub print_feed_meta {
 			$href_params{'-title'} = 'log';
 		}
 
-		foreach my $format qw(RSS Atom) {
+		foreach my $format (qw(RSS Atom)) {
 			my $type = lc($format);
 			my %link_attr = (
 				'-rel' => 'alternate',
@@ -3682,7 +3682,7 @@ sub git_footer_html {
 		}
 		$href_params{'-title'} ||= 'log';
 
-		foreach my $format qw(RSS Atom) {
+		foreach my $format (qw(RSS Atom)) {
 			$href_params{'action'} = lc($format);
 			print $cgi->a({-href => href(%href_params),
 			              -title => "$href_params{'-title'} $format feed",
-- 
1.7.2.3

[PATCH v2 0/3] Fix failure-causing warnings in Gitweb + improve gitweb-lib.sh

[Ævar Arnfjörð Bjarmason]

Ignore the previous patch series to fix the qw(...) warning. I missed
some spots.

Ævar Arnfjörð Bjarmason (3):
  t/gitweb-lib.sh: print to stderr when gitweb_run has errors
  gitweb/gitweb.perl: remove use of qw(...) as parentheses
  gitweb/gitweb.perl: don't call S_ISREG() with undef

 gitweb/gitweb.perl |    6 +++---
 t/gitweb-lib.sh    |    7 ++++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

-- 
1.7.2.3

[PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Ævar Arnfjörð Bjarmason]

Change the gitweb_run test subroutine to spew errors to stderr if
there are any, previously it would just silently fail, which made
tests very hard to debug.

Before you'd get this output, when running tests under `--verbose
--immediate --debug`:

    expecting success: git rm renamed_file &&
             rm -f renamed_file &&
             git commit -a -m "File removed." &&
             gitweb_run "p=.git;a=commitdiff"
    rm 'renamed_file'
    [master 8d80741] File removed.
     Author: A U Thor <author@example.com>
     1 files changed, 0 insertions(+), 1 deletions(-)
     delete mode 120000 renamed_file
    not ok - 32 commitdiff(0): file deleted
    #       git rm renamed_file &&
    #                rm -f renamed_file &&
    #                git commit -a -m "File removed." &&
    #                gitweb_run "p=.git;a=commitdiff"

Now you'll get the much more useful:

    expecting success: git rm renamed_file &&
             rm -f renamed_file &&
             git commit -a -m "File removed." &&
             gitweb_run "p=.git;a=commitdiff"
    rm 'renamed_file'
    [master 2a4214e] File removed.
     Author: A U Thor <author@example.com>
     1 files changed, 0 insertions(+), 1 deletions(-)
     delete mode 120000 renamed_file
    [Sat Feb 19 14:32:54 2011] gitweb.perl: Use of uninitialized value in subroutine entry at /home/avar/g/git/t/../gitweb/gitweb.perl line 4415.
    not ok - 32 commitdiff(0): file deleted
    #       git rm renamed_file &&
    #                rm -f renamed_file &&
    #                git commit -a -m "File removed." &&
    #                gitweb_run "p=.git;a=commitdiff"
---
 t/gitweb-lib.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index b9bb95f..2388b0f 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -82,7 +82,12 @@ gitweb_run () {
 		}
 		close O;
 	' gitweb.output &&
-	if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
+	if grep '^[[]' gitweb.log >/dev/null 2>&1; then
+		cat gitweb.log >&2
+		false
+	else
+		true
+	fi
 
 	# gitweb.log is left for debugging
 	# gitweb.output is used to parse HTTP output
-- 
1.7.2.3

Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Jakub Narebski]

On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:

> Change the gitweb_run test subroutine to spew errors to stderr if
> there are any, previously it would just silently fail, which made
> tests very hard to debug.
> 
> Before you'd get this output, when running tests under `--verbose
> --immediate --debug`:

Which test?

[...]
> --- a/t/gitweb-lib.sh
> +++ b/t/gitweb-lib.sh
> @@ -82,7 +82,12 @@ gitweb_run () {
>  		}
>  		close O;
>  	' gitweb.output &&
> -	if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
> +	if grep '^[[]' gitweb.log >/dev/null 2>&1; then
> +		cat gitweb.log >&2
> +		false
> +	else
> +		true
> +	fi

I don't understand this change.  Either it is not necessary, because
test suite (or at least t9500) has

  test_debug 'cat gitweb.log'

after each test, so that error messages would be printed with `--debug`,
or it doesn't go far enough: if the above is used then those test_debug
should be removed.

-- 
Jakub Narebski
Poland

Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Ævar Arnfjörð Bjarmason]

On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sat, 19 Feb 2011, Ęvar Arnfjörš Bjarmason wrote:
>
>> Change the gitweb_run test subroutine to spew errors to stderr if
>> there are any, previously it would just silently fail, which made
>> tests very hard to debug.
>>
>> Before you'd get this output, when running tests under `--verbose
>> --immediate --debug`:
>
> Which test?
>
> [...]
>> --- a/t/gitweb-lib.sh
>> +++ b/t/gitweb-lib.sh
>> @@ -82,7 +82,12 @@ gitweb_run () {
>>               }
>>               close O;
>>       ' gitweb.output &&
>> -     if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
>> +     if grep '^[[]' gitweb.log >/dev/null 2>&1; then
>> +             cat gitweb.log >&2
>> +             false
>> +     else
>> +             true
>> +     fi
>
> I don't understand this change.  Either it is not necessary, because
> test suite (or at least t9500) has
>
>  test_debug 'cat gitweb.log'
>
> after each test, so that error messages would be printed with `--debug`,
> or it doesn't go far enough: if the above is used then those test_debug
> should be removed.

The way you're using test_debug() is incompatible with
--immediate. The test dies, but I'll never see your debug message
because I'm using --immediate.

It would be better to just use test_debug in gitweb_run (instead of my
"cat gitweb.log").

Anyway, if you feel like fixing that feel free, I wan't pursue this
further (going to hack on what I was going to do before gitweb tests
started failing).

Junio, you can drop this patch since it'll produce duplicate output if
the test fails and the user *doesn't* use --immediate, but IMO this
should be fixed by doing the debug output differently.

Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Jakub Narebski]

On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:
> On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sat, 19 Feb 2011, Ęvar Arnfjörš Bjarmason wrote:
>>
>>> Change the gitweb_run test subroutine to spew errors to stderr if
>>> there are any, previously it would just silently fail, which made
>>> tests very hard to debug.
>>>
>>> Before you'd get this output, when running tests under `--verbose
>>> --immediate --debug`:
>>
>> Which test?
>>
>> [...]
>>> --- a/t/gitweb-lib.sh
>>> +++ b/t/gitweb-lib.sh
>>> @@ -82,7 +82,12 @@ gitweb_run () {
>>>               }
>>>               close O;
>>>       ' gitweb.output &&
>>> -     if grep '^[[]' gitweb.log>/dev/null 2>&1; then false; else true; fi
>>> +     if grep '^[[]' gitweb.log>/dev/null 2>&1; then
>>> +             cat gitweb.log>&2
>>> +             false
>>> +     else
>>> +             true
>>> +     fi
>>
>> I don't understand this change.  Either it is not necessary, because
>> test suite (or at least t9500) has
>>
>>  test_debug 'cat gitweb.log'
>>
>> after each test, so that error messages would be printed with `--debug`,
>> or it doesn't go far enough: if the above is used then those test_debug
>> should be removed.
> 
> The way you're using test_debug() is incompatible with
> --immediate. The test dies, but I'll never see your debug message
> because I'm using --immediate.

Ah, so that is what it is about: using --immediate negates --debug.

Note that it is *much* wider issue; it is not only gitweb tests that use
test_debug in such way.  See for example t/t4114-apply-typechange.sh
where you have

  test_expect_success SYMLINKS 'directory becomes symlink' '
        git checkout -f foo-becomes-a-directory &&
        git diff-tree -p HEAD foo-symlinked-to-bar > patch &&
        git apply --index < patch
        '
  test_debug 'cat patch'

This causes the same problem.

> It would be better to just use test_debug in gitweb_run (instead of my
> "cat gitweb.log").
> 
> Anyway, if you feel like fixing that feel free, I wan't pursue this
> further (going to hack on what I was going to do before gitweb tests
> started failing).
> 
> Junio, you can drop this patch since it'll produce duplicate output if
> the test fails and the user *doesn't* use --immediate, but IMO this
> should be fixed by doing the debug output differently.
 
Below there is proposed patch that removes duplication of --debug output...
but does not solve issue for other places where we use test_debug in test
suite and incompatibility with --immediate run.

-- >8 --
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Subject: [PATCH] t/gitweb-lib.sh: Ensure that errors are shown for --debug --immediate

Because '--immediate' stops test suite after first error, therefore in
this mode

  test_debug 'cat gitweb.log'

was never ran, thus in effect negating effect of '--debug' option.
This made finidng the cause of errors in gitweb test sute difficult.

Modify the gitweb_run test subroutine to run test_debug itself in the
case of errors (and also remove "test_debug 'cat gitweb.log'" from
gitweb tests).

This makes it possible to run *gitweb tests* with --immediate ---debug
combination of options; also it makes gitweb tests to not output
spurious debug data that is not considered error.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jakub Narębski <jnareb@gmail.com>
---
 t/gitweb-lib.sh                          |    7 ++
 t/t9500-gitweb-standalone-no-errors.sh   |   86 ------------------------------
 t/t9501-gitweb-standalone-http-status.sh |    1 
 3 files changed, 6 insertions(+), 88 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index b9bb95f..143eb1f 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -82,7 +82,12 @@ gitweb_run () {
 		}
 		close O;
 	' gitweb.output &&
-	if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
+	if grep '^[[]' gitweb.log >/dev/null 2>&1; then
+		test_debug 'cat gitweb.log >&2' &&
+		false
+	else
+		true
+	fi
 
 	# gitweb.log is left for debugging
 	# gitweb.output is used to parse HTTP output
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 21cd286..35c151d 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -18,42 +18,34 @@ or warnings to log.'
 test_expect_success \
 	'no commits: projects_list (implicit)' \
 	'gitweb_run'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: projects_index' \
 	'gitweb_run "a=project_index"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: .git summary (implicit)' \
 	'gitweb_run "p=.git"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: .git commit (implicit HEAD)' \
 	'gitweb_run "p=.git;a=commit"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: .git commitdiff (implicit HEAD)' \
 	'gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: .git tree (implicit HEAD)' \
 	'gitweb_run "p=.git;a=tree"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: .git heads' \
 	'gitweb_run "p=.git;a=heads"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'no commits: .git tags' \
 	'gitweb_run "p=.git;a=tags"'
-test_debug 'cat gitweb.log'
 
 
 # ----------------------------------------------------------------------
@@ -69,52 +61,42 @@ test_expect_success \
 test_expect_success \
 	'projects_list (implicit)' \
 	'gitweb_run'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'projects_index' \
 	'gitweb_run "a=project_index"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git summary (implicit)' \
 	'gitweb_run "p=.git"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git commit (implicit HEAD)' \
 	'gitweb_run "p=.git;a=commit"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git commitdiff (implicit HEAD, root commit)' \
 	'gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git commitdiff_plain (implicit HEAD, root commit)' \
 	'gitweb_run "p=.git;a=commitdiff_plain"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git commit (HEAD)' \
 	'gitweb_run "p=.git;a=commit;h=HEAD"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git tree (implicit HEAD)' \
 	'gitweb_run "p=.git;a=tree"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git blob (file)' \
 	'gitweb_run "p=.git;a=blob;f=file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git blob_plain (file)' \
 	'gitweb_run "p=.git;a=blob_plain;f=file"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # nonexistent objects
@@ -122,37 +104,30 @@ test_debug 'cat gitweb.log'
 test_expect_success \
 	'.git commit (non-existent)' \
 	'gitweb_run "p=.git;a=commit;h=non-existent"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git commitdiff (non-existent)' \
 	'gitweb_run "p=.git;a=commitdiff;h=non-existent"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git commitdiff (non-existent vs HEAD)' \
 	'gitweb_run "p=.git;a=commitdiff;hp=non-existent;h=HEAD"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git tree (0000000000000000000000000000000000000000)' \
 	'gitweb_run "p=.git;a=tree;h=0000000000000000000000000000000000000000"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git tag (0000000000000000000000000000000000000000)' \
 	'gitweb_run "p=.git;a=tag;h=0000000000000000000000000000000000000000"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git blob (non-existent)' \
 	'gitweb_run "p=.git;a=blob;f=non-existent"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'.git blob_plain (non-existent)' \
 	'gitweb_run "p=.git;a=blob_plain;f=non-existent"'
-test_debug 'cat gitweb.log'
 
 
 # ----------------------------------------------------------------------
@@ -161,7 +136,6 @@ test_debug 'cat gitweb.log'
 test_expect_success \
 	'commitdiff(0): root' \
 	'gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): file added' \
@@ -169,21 +143,18 @@ test_expect_success \
 	 git add new_file &&
 	 git commit -a -m "File added." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): mode change' \
 	'test_chmod +x new_file &&
 	 git commit -a -m "Mode changed." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): file renamed' \
 	'git mv new_file renamed_file &&
 	 git commit -a -m "File renamed." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success SYMLINKS \
 	'commitdiff(0): file to symlink' \
@@ -191,7 +162,6 @@ test_expect_success SYMLINKS \
 	 ln -s file renamed_file &&
 	 git commit -a -m "File to symlink." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): file deleted' \
@@ -199,7 +169,6 @@ test_expect_success \
 	 rm -f renamed_file &&
 	 git commit -a -m "File removed." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): file copied / new file' \
@@ -207,7 +176,6 @@ test_expect_success \
 	 git add file2 &&
 	 git commit -a -m "File copied." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): mode change and modified' \
@@ -215,7 +183,6 @@ test_expect_success \
 	 test_chmod +x file2 &&
 	 git commit -a -m "Mode change and modification." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): renamed and modified' \
@@ -233,7 +200,6 @@ EOF
 	 echo "Propter nomen suum." >> file3 &&
 	 git commit -a -m "File rename and modification." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): renamed, mode change and modified' \
@@ -242,7 +208,6 @@ test_expect_success \
 	 test_chmod +x file2 &&
 	 git commit -a -m "File rename, mode change and modification." &&
 	 gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # commitdiff testing (taken from t4114-apply-typechange.sh)
@@ -279,42 +244,34 @@ test_expect_success SYMLINKS 'setup typechange commits' '
 test_expect_success \
 	'commitdiff(2): file renamed from foo to foo/baz' \
 	'gitweb_run "p=.git;a=commitdiff;hp=initial;h=foo-baz-renamed-from-foo"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): file renamed from foo/baz to foo' \
 	'gitweb_run "p=.git;a=commitdiff;hp=foo-baz-renamed-from-foo;h=initial"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): directory becomes file' \
 	'gitweb_run "p=.git;a=commitdiff;hp=foo-becomes-a-directory;h=initial"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): file becomes directory' \
 	'gitweb_run "p=.git;a=commitdiff;hp=initial;h=foo-becomes-a-directory"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): file becomes symlink' \
 	'gitweb_run "p=.git;a=commitdiff;hp=initial;h=foo-symlinked-to-bar"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): symlink becomes file' \
 	'gitweb_run "p=.git;a=commitdiff;hp=foo-symlinked-to-bar;h=foo-back-to-file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): symlink becomes directory' \
 	'gitweb_run "p=.git;a=commitdiff;hp=foo-symlinked-to-bar;h=foo-becomes-a-directory"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(2): directory becomes symlink' \
 	'gitweb_run "p=.git;a=commitdiff;hp=foo-becomes-a-directory;h=foo-symlinked-to-bar"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # commit, commitdiff: merge, large
@@ -330,12 +287,10 @@ test_expect_success \
 test_expect_success \
 	'commit(0): merge commit' \
 	'gitweb_run "p=.git;a=commit"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(0): merge commit' \
 	'gitweb_run "p=.git;a=commitdiff"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'Prepare large commit' \
@@ -371,12 +326,10 @@ test_expect_success \
 test_expect_success \
 	'commit(1): large commit' \
 	'gitweb_run "p=.git;a=commit;h=b"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'commitdiff(1): large commit' \
 	'gitweb_run "p=.git;a=commitdiff;h=b"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # tags testing
@@ -394,17 +347,14 @@ test_expect_success \
 	 git tag lightweight/tag-tree HEAD^{tree} &&
 	 git tag lightweight/tag-blob HEAD:file &&
 	 gitweb_run "p=.git;a=tags"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'tag: Tag to commit object' \
 	'gitweb_run "p=.git;a=tag;h=tag-commit"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'tag: on lightweight tag (invalid)' \
 	'gitweb_run "p=.git;a=tag;h=lightweight/tag-commit"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # logs
@@ -412,22 +362,18 @@ test_debug 'cat gitweb.log'
 test_expect_success \
 	'logs: log (implicit HEAD)' \
 	'gitweb_run "p=.git;a=log"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'logs: shortlog (implicit HEAD)' \
 	'gitweb_run "p=.git;a=shortlog"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'logs: history (implicit HEAD, file)' \
 	'gitweb_run "p=.git;a=history;f=file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'logs: history (implicit HEAD, non-existent file)' \
 	'gitweb_run "p=.git;a=history;f=non-existent"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'logs: history (implicit HEAD, deleted file)' \
@@ -438,55 +384,45 @@ test_expect_success \
 	 git rm deleted_file &&
 	 git commit -m "Delete file" &&
 	 gitweb_run "p=.git;a=history;f=deleted_file"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # path_info links
 test_expect_success \
 	'path_info: project' \
 	'gitweb_run "" "/.git"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/branch' \
 	'gitweb_run "" "/.git/b"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/branch:file' \
 	'gitweb_run "" "/.git/master:file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/branch:dir/' \
 	'gitweb_run "" "/.git/master:foo/"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/branch:file (non-existent)' \
 	'gitweb_run "" "/.git/master:non-existent"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/branch:dir/ (non-existent)' \
 	'gitweb_run "" "/.git/master:non-existent/"'
-test_debug 'cat gitweb.log'
 
 
 test_expect_success \
 	'path_info: project/branch:/file' \
 	'gitweb_run "" "/.git/master:/file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/:/file (implicit HEAD)' \
 	'gitweb_run "" "/.git/:/file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'path_info: project/:/ (implicit HEAD, top tree)' \
 	'gitweb_run "" "/.git/:/"'
-test_debug 'cat gitweb.log'
 
 
 # ----------------------------------------------------------------------
@@ -495,17 +431,14 @@ test_debug 'cat gitweb.log'
 test_expect_success \
 	'feeds: OPML' \
 	'gitweb_run "a=opml"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'feed: RSS' \
 	'gitweb_run "p=.git;a=rss"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'feed: Atom' \
 	'gitweb_run "p=.git;a=atom"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # encoding/decoding
@@ -517,7 +450,6 @@ test_expect_success \
 	 git add file &&
 	 git commit -F "$TEST_DIRECTORY"/t3900/1-UTF-8.txt &&
 	 gitweb_run "p=.git;a=commit"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'encode(commit): iso-8859-1' \
@@ -528,12 +460,10 @@ test_expect_success \
 	 git commit -F "$TEST_DIRECTORY"/t3900/ISO8859-1.txt &&
 	 git config --unset i18n.commitencoding &&
 	 gitweb_run "p=.git;a=commit"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'encode(log): utf-8 and iso-8859-1' \
 	'gitweb_run "p=.git;a=log"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # extra options
@@ -541,27 +471,22 @@ test_debug 'cat gitweb.log'
 test_expect_success \
 	'opt: log --no-merges' \
 	'gitweb_run "p=.git;a=log;opt=--no-merges"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'opt: atom --no-merges' \
 	'gitweb_run "p=.git;a=log;opt=--no-merges"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'opt: "file" history --no-merges' \
 	'gitweb_run "p=.git;a=history;f=file;opt=--no-merges"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'opt: log --no-such-option (invalid option)' \
 	'gitweb_run "p=.git;a=log;opt=--no-such-option"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'opt: tree --no-merges (invalid option for action)' \
 	'gitweb_run "p=.git;a=tree;opt=--no-merges"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # testing config_to_multi / cloneurl
@@ -569,14 +494,12 @@ test_debug 'cat gitweb.log'
 test_expect_success \
        'URL: no project URLs, no base URL' \
        'gitweb_run "p=.git;a=summary"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
        'URL: project URLs via gitweb.url' \
        'git config --add gitweb.url git://example.com/git/trash.git &&
         git config --add gitweb.url http://example.com/git/trash.git &&
         gitweb_run "p=.git;a=summary"'
-test_debug 'cat gitweb.log'
 
 cat >.git/cloneurl <<\EOF
 git://example.com/git/trash.git
@@ -586,7 +509,6 @@ EOF
 test_expect_success \
        'URL: project URLs via cloneurl file' \
        'gitweb_run "p=.git;a=summary"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # gitweb config and repo config
@@ -604,12 +526,10 @@ EOF
 test_expect_success \
 	'config override: projects list (implicit)' \
 	'gitweb_run'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'config override: tree view, features not overridden in repo config' \
 	'gitweb_run "p=.git;a=tree"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'config override: tree view, features disabled in repo config' \
@@ -617,14 +537,12 @@ test_expect_success \
 	 git config gitweb.snapshot none &&
 	 git config gitweb.avatar gravatar &&
 	 gitweb_run "p=.git;a=tree"'
-test_debug 'cat gitweb.log'
 
 test_expect_success \
 	'config override: tree view, features enabled in repo config (1)' \
 	'git config gitweb.blame yes &&
 	 git config gitweb.snapshot "zip,tgz, tbz2" &&
 	 gitweb_run "p=.git;a=tree"'
-test_debug 'cat gitweb.log'
 
 cat >.git/config <<\EOF
 # testing noval and alternate separator
@@ -635,7 +553,6 @@ EOF
 test_expect_success \
 	'config override: tree view, features enabled in repo config (2)' \
 	'gitweb_run "p=.git;a=tree"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # non-ASCII in README.html
@@ -645,7 +562,6 @@ test_expect_success \
 	'echo "<b>UTF-8 example:</b><br />" > .git/README.html &&
 	 cat "$TEST_DIRECTORY"/t3900/1-UTF-8.txt >> .git/README.html &&
 	 gitweb_run "p=.git;a=summary"'
-test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
 # syntax highlighting
@@ -666,7 +582,6 @@ test_expect_success HIGHLIGHT \
 	'syntax highlighting (no highlight, unknown syntax)' \
 	'git config gitweb.highlight yes &&
 	 gitweb_run "p=.git;a=blob;f=file"'
-test_debug 'cat gitweb.log'
 
 test_expect_success HIGHLIGHT \
 	'syntax highlighting (highlighted, shell script)' \
@@ -675,6 +590,5 @@ test_expect_success HIGHLIGHT \
 	 git add test.sh &&
 	 git commit -m "Add test.sh" &&
 	 gitweb_run "p=.git;a=blob;f=test.sh"'
-test_debug 'cat gitweb.log'
 
 test_done
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 18825af..26102ee 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -126,7 +126,6 @@ test_expect_success 'load checking: load too high (default action)' '
 	grep "Status: 503 Service Unavailable" gitweb.headers &&
 	grep "503 - The load average on the server is too high" gitweb.body
 '
-test_debug 'cat gitweb.log' # just in case
 test_debug 'cat gitweb.headers'
 
 # turn off load checking

Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Ævar Arnfjörð Bjarmason]

On Sat, Feb 19, 2011 at 19:16, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:
>> On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Sat, 19 Feb 2011, Ęvar Arnfjörš Bjarmason wrote:
>>>
>>>> Change the gitweb_run test subroutine to spew errors to stderr if
>>>> there are any, previously it would just silently fail, which made
>>>> tests very hard to debug.
>>>>
>>>> Before you'd get this output, when running tests under `--verbose
>>>> --immediate --debug`:
>>>
>>> Which test?
>>>
>>> [...]
>>>> --- a/t/gitweb-lib.sh
>>>> +++ b/t/gitweb-lib.sh
>>>> @@ -82,7 +82,12 @@ gitweb_run () {
>>>>               }
>>>>               close O;
>>>>       ' gitweb.output &&
>>>> -     if grep '^[[]' gitweb.log>/dev/null 2>&1; then false; else true; fi
>>>> +     if grep '^[[]' gitweb.log>/dev/null 2>&1; then
>>>> +             cat gitweb.log>&2
>>>> +             false
>>>> +     else
>>>> +             true
>>>> +     fi
>>>
>>> I don't understand this change.  Either it is not necessary, because
>>> test suite (or at least t9500) has
>>>
>>>  test_debug 'cat gitweb.log'
>>>
>>> after each test, so that error messages would be printed with `--debug`,
>>> or it doesn't go far enough: if the above is used then those test_debug
>>> should be removed.
>>
>> The way you're using test_debug() is incompatible with
>> --immediate. The test dies, but I'll never see your debug message
>> because I'm using --immediate.
>
> Ah, so that is what it is about: using --immediate negates --debug.

Yeah, I didn't realize that when I submitted my patch, I just thought
it would never be printed, period.

> Note that it is *much* wider issue; it is not only gitweb tests that use
> test_debug in such way.  See for example t/t4114-apply-typechange.sh
> where you have

Indeed, it's all over the test suite.

>  test_expect_success SYMLINKS 'directory becomes symlink' '
>        git checkout -f foo-becomes-a-directory &&
>        git diff-tree -p HEAD foo-symlinked-to-bar > patch &&
>        git apply --index < patch
>        '
>  test_debug 'cat patch'
>
> This causes the same problem.
>
>> It would be better to just use test_debug in gitweb_run (instead of my
>> "cat gitweb.log").
>>
>> Anyway, if you feel like fixing that feel free, I wan't pursue this
>> further (going to hack on what I was going to do before gitweb tests
>> started failing).
>>
>> Junio, you can drop this patch since it'll produce duplicate output if
>> the test fails and the user *doesn't* use --immediate, but IMO this
>> should be fixed by doing the debug output differently.
>
> Below there is proposed patch that removes duplication of --debug output...
> but does not solve issue for other places where we use test_debug in test
> suite and incompatibility with --immediate run.

This patch looks good to me.

Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Because '--immediate' stops test suite after first error, therefore in
> this mode
>
>   test_debug 'cat gitweb.log'
>
> was never ran, thus in effect negating effect of '--debug' option.
> This made finidng the cause of errors in gitweb test sute difficult.

I think the patch itself makes sense, but I actually don't agree with the
above reasoning.

The point of --immediate is not just it stops where we find a failure, but
also it leaves the trash directory intact, so that _you_ who started the
test with --immediate can run the "cat gitweb.log" yourself.

In other words, I think test_debug is overused.  In the current setting, I
think a sane way to debug the test script itself would probably be

	sh -x tXXXX-xxxx.sh -i -v -d

so that:

 (1) with -x I can see what commands are being run;
 (2) with -i I can stop the test immediately upon the first failure;
 (3) with -v I can see the output from the commands that we usually do not
     show; and
 (4) with -d I can be sure that trash directory does not go away.

Notice that I do not count "I can see test_debug executed" as a benefit
from (4) above?

My preference, although I do not care too deeply, would be to gradually
remove test_debug calls that were casually sprinkled and left in the test
scripts while they were initially developed, and have the regular test
sequence run them just like your patch does.  Another change I would
welcome would be to make the "-d" option automatically trigger shell
tracing, to make it unnecessary to say "sh -x".

[PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[Ævar Arnfjörð Bjarmason]

Using the qw(...) construct as implicit parentheses was deprecated in
perl 5.13.5. Change the relevant code in gitweb to not use the
deprecated construct. The offending code was introduced in 3562198b by
Jakub Narebski.

The issue is that perl will now warn about this:

    $ perl -wE 'for my $i qw(a b) { say $i }'
    Use of qw(...) as parentheses is deprecated at -e line 1.
    a
    b

This caused gitweb.perl to warn on perl 5.13.5 and above, and these
tests to fail on those perl versions:

    ./t9501-gitweb-standalone-http-status.sh           (Wstat: 256 Tests: 11 Failed: 10)
      Failed tests:  2-11
      Non-zero exit status: 1
    ./t9502-gitweb-standalone-parse-output.sh          (Wstat: 256 Tests: 10 Failed: 9)
      Failed tests:  2-10
      Non-zero exit status: 1

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0779f12..b02372c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3501,7 +3501,7 @@ sub print_feed_meta {
 			$href_params{'-title'} = 'log';
 		}
 
-		foreach my $format qw(RSS Atom) {
+		foreach my $format (qw(RSS Atom)) {
 			my $type = lc($format);
 			my %link_attr = (
 				'-rel' => 'alternate',
@@ -3682,7 +3682,7 @@ sub git_footer_html {
 		}
 		$href_params{'-title'} ||= 'log';
 
-		foreach my $format qw(RSS Atom) {
+		foreach my $format (qw(RSS Atom)) {
 			$href_params{'action'} = lc($format);
 			print $cgi->a({-href => href(%href_params),
 			              -title => "$href_params{'-title'} $format feed",
-- 
1.7.2.3

Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[Jakub Narebski]

On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:

> Using the qw(...) construct as implicit parentheses was deprecated in
> perl 5.13.5. Change the relevant code in gitweb to not use the
> deprecated construct. The offending code was introduced in 3562198b by
> Jakub Narebski.

It is strange that Perl introduces such backwards incompatibile change
(well, actually will introduce, as 5.13.x is development branch leading
to future Perl version 5.14).

qw{} is described in perlop(1) as "word list" operator, so one would
suppose that it generates a list.
 
> The issue is that perl will now warn about this:
> 
>     $ perl -wE 'for my $i qw(a b) { say $i }'
>     Use of qw(...) as parentheses is deprecated at -e line 1.
>     a
>     b

Hmmm... does it affect only foreach loop, or dows it affect also other 
places, like

      use POSIX qw( setlocale localeconv )
      @EXPORT = qw( foo bar baz );
 
Both of those forms are used by gitweb:

      use CGI qw(:standard :escapeHTML -nosticky);

      map { $_ => 'sh'  } qw(bash zsh ksh)
      my @navs = qw(summary shortlog log commit commitdiff tree);

[...]
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3501,7 +3501,7 @@ sub print_feed_meta {
>  			$href_params{'-title'} = 'log';
>  		}
>  
> -		foreach my $format qw(RSS Atom) {
> +		foreach my $format (qw(RSS Atom)) {
>  			my $type = lc($format);
>  			my %link_attr = (
>  				'-rel' => 'alternate',

I am not against futureproofing gitweb in above way for future
Perl 5.14.x

-- 
Jakub Narebski
Poland

Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[Jakub Narebski]

Jakub Narebski wrote:
> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:

> [...]
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -3501,7 +3501,7 @@ sub print_feed_meta {
> >  			$href_params{'-title'} = 'log';
> >  		}
> >  
> > -		foreach my $format qw(RSS Atom) {
> > +		foreach my $format (qw(RSS Atom)) {
> >  			my $type = lc($format);
> >  			my %link_attr = (
> >  				'-rel' => 'alternate',

Ah, sorry, if Perl reqyures to use 'foreach (@array)', then
of course one should use 'forach (qw(A B))'.

-- 
Jakub Narebski
Poland

Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[Ævar Arnfjörð Bjarmason]

On Sat, Feb 19, 2011 at 16:54, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:
>
>> Using the qw(...) construct as implicit parentheses was deprecated in
>> perl 5.13.5. Change the relevant code in gitweb to not use the
>> deprecated construct. The offending code was introduced in 3562198b by
>> Jakub Narebski.
>
> It is strange that Perl introduces such backwards incompatibile change
> (well, actually will introduce, as 5.13.x is development branch leading
> to future Perl version 5.14).
>
> qw{} is described in perlop(1) as "word list" operator, so one would
> suppose that it generates a list.

It does, but it wasn't supposed to generate parens for you.

>> The issue is that perl will now warn about this:
>>
>>     $ perl -wE 'for my $i qw(a b) { say $i }'
>>     Use of qw(...) as parentheses is deprecated at -e line 1.
>>     a
>>     b
>
> Hmmm... does it affect only foreach loop, or dows it affect also other
> places, like
>
>      use POSIX qw( setlocale localeconv )
>      @EXPORT = qw( foo bar baz );
>
> Both of those forms are used by gitweb:
>
>      use CGI qw(:standard :escapeHTML -nosticky);
>
>      map { $_ => 'sh'  } qw(bash zsh ksh)
>      my @navs = qw(summary shortlog log commit commitdiff tree);

No. This is being deprecated because qw(foo bar) is supposed to mean
"foo, "bar", not ("foo", "bar"). I.e. this doesn't compile:

    for my $i "a", "b", "c" { }

So neither should this:

    for my $i qw(a b c) {}

But these both work:

    for my $i ("a", "b", "c") { }
    for my $i (qw(a b c)) {}

All of your other examples could have used a list without implicit
parens. So this is the only change that's needed in gitweb.

Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses

[Jakub Narebski]

Dnia sobota 19. lutego 2011 17:06, Ævar Arnfjörð Bjarmason napisał:
> On Sat, Feb 19, 2011 at 16:54, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:
>>
>>> Using the qw(...) construct as implicit parentheses was deprecated in
>>> perl 5.13.5. Change the relevant code in gitweb to not use the
>>> deprecated construct. The offending code was introduced in 3562198b by
>>> Jakub Narebski.
>>
>> It is strange that Perl introduces such backwards incompatibile change
>> (well, actually will introduce, as 5.13.x is development branch leading
>> to future Perl version 5.14).
>>
>> qw{} is described in perlop(1) as "word list" operator, so one would
>> suppose that it generates a list.
> 
> It does, but it wasn't supposed to generate parens for you.

[...]
>> Hmmm... does it affect only foreach loop, or dows it affect also other
>> places, like
>>
>>      use POSIX qw( setlocale localeconv )
>>      @EXPORT = qw( foo bar baz );

[...]
> No. This is being deprecated because qw(foo bar) is supposed to mean
> "foo, "bar", not ("foo", "bar"). I.e. this doesn't compile:
> 
>     for my $i "a", "b", "c" { }
> 
> So neither should this:
> 
>     for my $i qw(a b c) {}
> 
> But these both work:
> 
>     for my $i ("a", "b", "c") { }
>     for my $i (qw(a b c)) {}
> 
> All of your other examples could have used a list without implicit
> parens. So this is the only change that's needed in gitweb.

Thanks for the explanation.  It makes sense.  So:

Acked-by: Jakub Narębski <jnareb@gmail.com>

-- 
Jakub Narebski
Poland

[PATCH v2 3/3] gitweb/gitweb.perl: don't call S_ISREG() with undef

[Ævar Arnfjörð Bjarmason]

Change S_ISREG($to_mode_oct) to S_ISREG($from_mode_oct) in the branch
that handles from modes, not to modes. This logic appears to have been
caused by copy/paste programming by Jakub Narebski in e8e41a93. It
would be better to rewrite this code not to be duplicated, but I
haven't done so.

This issue caused a failing test on perl 5.13.9, which has a warning
that turned this up:

     gitweb.perl: Use of uninitialized value in subroutine entry at /home/avar/g/git/t/../gitweb/gitweb.perl line 4415.

Which caused the Git test suite to fail on this test:

    ./t9500-gitweb-standalone-no-errors.sh             (Wstat: 256 Tests: 90 Failed: 84)
      Failed tests:  1-8, 10-36, 38-45, 47-48, 50-88
      Non-zero exit status: 1

Reported-by: perl 5.13.9
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b02372c..1b9369d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4412,7 +4412,7 @@ sub git_difftree_body {
 		}
 		if ($diff->{'from_mode'} ne ('0' x 6)) {
 			$from_mode_oct = oct $diff->{'from_mode'};
-			if (S_ISREG($to_mode_oct)) { # only for regular file
+			if (S_ISREG($from_mode_oct)) { # only for regular file
 				$from_mode_str = sprintf("%04o", $from_mode_oct & 0777); # permission bits
 			}
 			$from_file_type = file_type($diff->{'from_mode'});
-- 
1.7.2.3

Re: [PATCH v2 3/3] gitweb/gitweb.perl: don't call S_ISREG() with undef

[Jakub Narebski]

On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:

> Change S_ISREG($to_mode_oct) to S_ISREG($from_mode_oct) in the branch
> that handles from modes, not to modes. This logic appears to have been
> caused by copy/paste programming by Jakub Narebski in e8e41a93. It
> would be better to rewrite this code not to be duplicated, but I
> haven't done so.
> 
> This issue caused a failing test on perl 5.13.9, which has a warning
> that turned this up:
> 
>      gitweb.perl: Use of uninitialized value in subroutine entry at /home/avar/g/git/t/../gitweb/gitweb.perl line 4415.

[...]
> Reported-by: perl 5.13.9
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Thanks for catching this.  I wonder why we didn't caught this earlier...


For what it is worth it:

Acked-by: Jakub Narebski <jnareb@gmail.com>

> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index b02372c..1b9369d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4412,7 +4412,7 @@ sub git_difftree_body {
>  		}
>  		if ($diff->{'from_mode'} ne ('0' x 6)) {
>  			$from_mode_oct = oct $diff->{'from_mode'};
> -			if (S_ISREG($to_mode_oct)) { # only for regular file
> +			if (S_ISREG($from_mode_oct)) { # only for regular file
>  				$from_mode_str = sprintf("%04o", $from_mode_oct & 0777); # permission bits
>  			}
>  			$from_file_type = file_type($diff->{'from_mode'});
> -- 
> 1.7.2.3
> 
> 

-- 
Jakub Narebski
Poland