{"thread":{"id":"26558","subject":"[PATCH] gitweb/gitweb.perl: remove use of qw(...) as parentheses","startedAt":"2011-02-19T14:10:54Z","lastAt":"2011-02-21T06:40:50Z","messageCount":15,"participants":["Ævar Arnfjörð Bjarmason","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"161617","messageId":"1298124654-12051-1-git-send-email-avarab@gmail.com","threadId":"26558","inReplyTo":null,"subject":"[PATCH] gitweb/gitweb.perl: remove use of qw(...) as parentheses","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T14:10:54Z","receivedAt":"2011-02-19T14:10:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Using the qw(...) construct as implicit parentheses was deprecated in\nperl 5.13.5. Change the relevant code in gitweb to not use the\ndeprecated construct. The offending code was introduced in 3562198b by\nJakub Narebski.\n\nThe issue is that perl will now warn about this:\n\n    $ perl -wE 'for my $i qw(a b) { say $i }'\n    Use of qw(...) as parentheses is deprecated at -e line 1.\n    a\n    b\n\nThis caused gitweb.perl to warn on perl 5.13.5 and above, and these\ntests to fail on those perl versions:\n\n    ./t9501-gitweb-standalone-http-status.sh           (Wstat: 256 Tests: 11 Failed: 10)\n      Failed tests:  2-11\n      Non-zero exit status: 1\n    ./t9502-gitweb-standalone-parse-output.sh          (Wstat: 256 Tests: 10 Failed: 9)\n      Failed tests:  2-10\n      Non-zero exit status: 1\n    ./t9500-gitweb-standalone-no-errors.sh             (Wstat: 256 Tests: 90 Failed: 84)\n      Failed tests:  1-8, 10-36, 38-45, 47-48, 50-88\n      Non-zero exit status: 1\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n gitweb/gitweb.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0779f12..b02372c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3501,7 +3501,7 @@ sub print_feed_meta {\n \t\t\t$href_params{'-title'} = 'log';\n \t\t}\n \n-\t\tforeach my $format qw(RSS Atom) {\n+\t\tforeach my $format (qw(RSS Atom)) {\n \t\t\tmy $type = lc($format);\n \t\t\tmy %link_attr = (\n \t\t\t\t'-rel' => 'alternate',\n@@ -3682,7 +3682,7 @@ sub git_footer_html {\n \t\t}\n \t\t$href_params{'-title'} ||= 'log';\n \n-\t\tforeach my $format qw(RSS Atom) {\n+\t\tforeach my $format (qw(RSS Atom)) {\n \t\t\t$href_params{'action'} = lc($format);\n \t\t\tprint $cgi->a({-href => href(%href_params),\n \t\t\t              -title => \"$href_params{'-title'} $format feed\",\n-- \n1.7.2.3\n"},{"id":"161620","messageId":"1298129262-10468-1-git-send-email-avarab@gmail.com","threadId":"26558","inReplyTo":"1298124654-12051-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 0/3] Fix failure-causing warnings in Gitweb + improve gitweb-lib.sh","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T15:27:39Z","receivedAt":"2011-02-19T15:27:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Ignore the previous patch series to fix the qw(...) warning. I missed\nsome spots.\n\nÆvar Arnfjörð Bjarmason (3):\n  t/gitweb-lib.sh: print to stderr when gitweb_run has errors\n  gitweb/gitweb.perl: remove use of qw(...) as parentheses\n  gitweb/gitweb.perl: don't call S_ISREG() with undef\n\n gitweb/gitweb.perl |    6 +++---\n t/gitweb-lib.sh    |    7 ++++++-\n 2 files changed, 9 insertions(+), 4 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"161621","messageId":"1298129262-10468-2-git-send-email-avarab@gmail.com","threadId":"26558","inReplyTo":"1298124654-12051-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T15:27:40Z","receivedAt":"2011-02-19T15:27:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the gitweb_run test subroutine to spew errors to stderr if\nthere are any, previously it would just silently fail, which made\ntests very hard to debug.\n\nBefore you'd get this output, when running tests under `--verbose\n--immediate --debug`:\n\n    expecting success: git rm renamed_file &&\n             rm -f renamed_file &&\n             git commit -a -m \"File removed.\" &&\n             gitweb_run \"p=.git;a=commitdiff\"\n    rm 'renamed_file'\n    [master 8d80741] File removed.\n     Author: A U Thor <author@example.com>\n     1 files changed, 0 insertions(+), 1 deletions(-)\n     delete mode 120000 renamed_file\n    not ok - 32 commitdiff(0): file deleted\n    #       git rm renamed_file &&\n    #                rm -f renamed_file &&\n    #                git commit -a -m \"File removed.\" &&\n    #                gitweb_run \"p=.git;a=commitdiff\"\n\nNow you'll get the much more useful:\n\n    expecting success: git rm renamed_file &&\n             rm -f renamed_file &&\n             git commit -a -m \"File removed.\" &&\n             gitweb_run \"p=.git;a=commitdiff\"\n    rm 'renamed_file'\n    [master 2a4214e] File removed.\n     Author: A U Thor <author@example.com>\n     1 files changed, 0 insertions(+), 1 deletions(-)\n     delete mode 120000 renamed_file\n    [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.\n    not ok - 32 commitdiff(0): file deleted\n    #       git rm renamed_file &&\n    #                rm -f renamed_file &&\n    #                git commit -a -m \"File removed.\" &&\n    #                gitweb_run \"p=.git;a=commitdiff\"\n---\n t/gitweb-lib.sh |    7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex b9bb95f..2388b0f 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -82,7 +82,12 @@ gitweb_run () {\n \t\t}\n \t\tclose O;\n \t' gitweb.output &&\n-\tif grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi\n+\tif grep '^[[]' gitweb.log >/dev/null 2>&1; then\n+\t\tcat gitweb.log >&2\n+\t\tfalse\n+\telse\n+\t\ttrue\n+\tfi\n \n \t# gitweb.log is left for debugging\n \t# gitweb.output is used to parse HTTP output\n-- \n1.7.2.3\n"},{"id":"161623","messageId":"1298129262-10468-3-git-send-email-avarab@gmail.com","threadId":"26558","inReplyTo":"1298124654-12051-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T15:27:41Z","receivedAt":"2011-02-19T15:27:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Using the qw(...) construct as implicit parentheses was deprecated in\nperl 5.13.5. Change the relevant code in gitweb to not use the\ndeprecated construct. The offending code was introduced in 3562198b by\nJakub Narebski.\n\nThe issue is that perl will now warn about this:\n\n    $ perl -wE 'for my $i qw(a b) { say $i }'\n    Use of qw(...) as parentheses is deprecated at -e line 1.\n    a\n    b\n\nThis caused gitweb.perl to warn on perl 5.13.5 and above, and these\ntests to fail on those perl versions:\n\n    ./t9501-gitweb-standalone-http-status.sh           (Wstat: 256 Tests: 11 Failed: 10)\n      Failed tests:  2-11\n      Non-zero exit status: 1\n    ./t9502-gitweb-standalone-parse-output.sh          (Wstat: 256 Tests: 10 Failed: 9)\n      Failed tests:  2-10\n      Non-zero exit status: 1\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n gitweb/gitweb.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0779f12..b02372c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3501,7 +3501,7 @@ sub print_feed_meta {\n \t\t\t$href_params{'-title'} = 'log';\n \t\t}\n \n-\t\tforeach my $format qw(RSS Atom) {\n+\t\tforeach my $format (qw(RSS Atom)) {\n \t\t\tmy $type = lc($format);\n \t\t\tmy %link_attr = (\n \t\t\t\t'-rel' => 'alternate',\n@@ -3682,7 +3682,7 @@ sub git_footer_html {\n \t\t}\n \t\t$href_params{'-title'} ||= 'log';\n \n-\t\tforeach my $format qw(RSS Atom) {\n+\t\tforeach my $format (qw(RSS Atom)) {\n \t\t\t$href_params{'action'} = lc($format);\n \t\t\tprint $cgi->a({-href => href(%href_params),\n \t\t\t              -title => \"$href_params{'-title'} $format feed\",\n-- \n1.7.2.3\n"},{"id":"161622","messageId":"1298129262-10468-4-git-send-email-avarab@gmail.com","threadId":"26558","inReplyTo":"1298124654-12051-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 3/3] gitweb/gitweb.perl: don't call S_ISREG() with undef","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T15:27:42Z","receivedAt":"2011-02-19T15:27:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change S_ISREG($to_mode_oct) to S_ISREG($from_mode_oct) in the branch\nthat handles from modes, not to modes. This logic appears to have been\ncaused by copy/paste programming by Jakub Narebski in e8e41a93. It\nwould be better to rewrite this code not to be duplicated, but I\nhaven't done so.\n\nThis issue caused a failing test on perl 5.13.9, which has a warning\nthat turned this up:\n\n     gitweb.perl: Use of uninitialized value in subroutine entry at /home/avar/g/git/t/../gitweb/gitweb.perl line 4415.\n\nWhich caused the Git test suite to fail on this test:\n\n    ./t9500-gitweb-standalone-no-errors.sh             (Wstat: 256 Tests: 90 Failed: 84)\n      Failed tests:  1-8, 10-36, 38-45, 47-48, 50-88\n      Non-zero exit status: 1\n\nReported-by: perl 5.13.9\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n gitweb/gitweb.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b02372c..1b9369d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4412,7 +4412,7 @@ sub git_difftree_body {\n \t\t}\n \t\tif ($diff->{'from_mode'} ne ('0' x 6)) {\n \t\t\t$from_mode_oct = oct $diff->{'from_mode'};\n-\t\t\tif (S_ISREG($to_mode_oct)) { # only for regular file\n+\t\t\tif (S_ISREG($from_mode_oct)) { # only for regular file\n \t\t\t\t$from_mode_str = sprintf(\"%04o\", $from_mode_oct & 0777); # permission bits\n \t\t\t}\n \t\t\t$from_file_type = file_type($diff->{'from_mode'});\n-- \n1.7.2.3\n"},{"id":"161626","messageId":"201102191646.02543.jnareb@gmail.com","threadId":"26558","inReplyTo":"1298129262-10468-2-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-19T15:46:00Z","receivedAt":"2011-02-19T15:46:00Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n\n> Change the gitweb_run test subroutine to spew errors to stderr if\n> there are any, previously it would just silently fail, which made\n> tests very hard to debug.\n> \n> Before you'd get this output, when running tests under `--verbose\n> --immediate --debug`:\n\nWhich test?\n\n[...]\n> --- a/t/gitweb-lib.sh\n> +++ b/t/gitweb-lib.sh\n> @@ -82,7 +82,12 @@ gitweb_run () {\n>  \t\t}\n>  \t\tclose O;\n>  \t' gitweb.output &&\n> -\tif grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi\n> +\tif grep '^[[]' gitweb.log >/dev/null 2>&1; then\n> +\t\tcat gitweb.log >&2\n> +\t\tfalse\n> +\telse\n> +\t\ttrue\n> +\tfi\n\nI don't understand this change.  Either it is not necessary, because\ntest suite (or at least t9500) has\n\n  test_debug 'cat gitweb.log'\n\nafter each test, so that error messages would be printed with `--debug`,\nor it doesn't go far enough: if the above is used then those test_debug\nshould be removed.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"161627","messageId":"201102191654.41601.jnareb@gmail.com","threadId":"26558","inReplyTo":"1298129262-10468-3-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-19T15:54:41Z","receivedAt":"2011-02-19T15:54:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n\n> Using the qw(...) construct as implicit parentheses was deprecated in\n> perl 5.13.5. Change the relevant code in gitweb to not use the\n> deprecated construct. The offending code was introduced in 3562198b by\n> Jakub Narebski.\n\nIt is strange that Perl introduces such backwards incompatibile change\n(well, actually will introduce, as 5.13.x is development branch leading\nto future Perl version 5.14).\n\nqw{} is described in perlop(1) as \"word list\" operator, so one would\nsuppose that it generates a list.\n \n> The issue is that perl will now warn about this:\n> \n>     $ perl -wE 'for my $i qw(a b) { say $i }'\n>     Use of qw(...) as parentheses is deprecated at -e line 1.\n>     a\n>     b\n\nHmmm... does it affect only foreach loop, or dows it affect also other \nplaces, like\n\n      use POSIX qw( setlocale localeconv )\n      @EXPORT = qw( foo bar baz );\n \nBoth of those forms are used by gitweb:\n\n      use CGI qw(:standard :escapeHTML -nosticky);\n\n      map { $_ => 'sh'  } qw(bash zsh ksh)\n      my @navs = qw(summary shortlog log commit commitdiff tree);\n\n[...]\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3501,7 +3501,7 @@ sub print_feed_meta {\n>  \t\t\t$href_params{'-title'} = 'log';\n>  \t\t}\n>  \n> -\t\tforeach my $format qw(RSS Atom) {\n> +\t\tforeach my $format (qw(RSS Atom)) {\n>  \t\t\tmy $type = lc($format);\n>  \t\t\tmy %link_attr = (\n>  \t\t\t\t'-rel' => 'alternate',\n\nI am not against futureproofing gitweb in above way for future\nPerl 5.14.x\n\n-- \nJakub Narebski\nPoland\n"},{"id":"161630","messageId":"201102191657.02431.jnareb@gmail.com","threadId":"26558","inReplyTo":"1298129262-10468-4-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 3/3] gitweb/gitweb.perl: don't call S_ISREG() with undef","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-19T15:57:02Z","receivedAt":"2011-02-19T15:57:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n\n> Change S_ISREG($to_mode_oct) to S_ISREG($from_mode_oct) in the branch\n> that handles from modes, not to modes. This logic appears to have been\n> caused by copy/paste programming by Jakub Narebski in e8e41a93. It\n> would be better to rewrite this code not to be duplicated, but I\n> haven't done so.\n> \n> This issue caused a failing test on perl 5.13.9, which has a warning\n> that turned this up:\n> \n>      gitweb.perl: Use of uninitialized value in subroutine entry at /home/avar/g/git/t/../gitweb/gitweb.perl line 4415.\n\n[...]\n> Reported-by: perl 5.13.9\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nThanks for catching this.  I wonder why we didn't caught this earlier...\n\n\nFor what it is worth it:\n\nAcked-by: Jakub Narebski <jnareb@gmail.com>\n\n> ---\n>  gitweb/gitweb.perl |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index b02372c..1b9369d 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -4412,7 +4412,7 @@ sub git_difftree_body {\n>  \t\t}\n>  \t\tif ($diff->{'from_mode'} ne ('0' x 6)) {\n>  \t\t\t$from_mode_oct = oct $diff->{'from_mode'};\n> -\t\t\tif (S_ISREG($to_mode_oct)) { # only for regular file\n> +\t\t\tif (S_ISREG($from_mode_oct)) { # only for regular file\n>  \t\t\t\t$from_mode_str = sprintf(\"%04o\", $from_mode_oct & 0777); # permission bits\n>  \t\t\t}\n>  \t\t\t$from_file_type = file_type($diff->{'from_mode'});\n> -- \n> 1.7.2.3\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"161629","messageId":"201102191702.29648.jnareb@gmail.com","threadId":"26558","inReplyTo":"201102191654.41601.jnareb@gmail.com","subject":"Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-19T16:02:29Z","receivedAt":"2011-02-19T16:02:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n\n> [...]\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -3501,7 +3501,7 @@ sub print_feed_meta {\n> >  \t\t\t$href_params{'-title'} = 'log';\n> >  \t\t}\n> >  \n> > -\t\tforeach my $format qw(RSS Atom) {\n> > +\t\tforeach my $format (qw(RSS Atom)) {\n> >  \t\t\tmy $type = lc($format);\n> >  \t\t\tmy %link_attr = (\n> >  \t\t\t\t'-rel' => 'alternate',\n\nAh, sorry, if Perl reqyures to use 'foreach (@array)', then\nof course one should use 'forach (qw(A B))'.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"161631","messageId":"AANLkTi==3c4ZLckUPE6NUt-sqWBsywQh3Gu+4B5vOBpj@mail.gmail.com","threadId":"26558","inReplyTo":"201102191654.41601.jnareb@gmail.com","subject":"Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T16:06:20Z","receivedAt":"2011-02-19T16:06:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Feb 19, 2011 at 16:54, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Using the qw(...) construct as implicit parentheses was deprecated in\n>> perl 5.13.5. Change the relevant code in gitweb to not use the\n>> deprecated construct. The offending code was introduced in 3562198b by\n>> Jakub Narebski.\n>\n> It is strange that Perl introduces such backwards incompatibile change\n> (well, actually will introduce, as 5.13.x is development branch leading\n> to future Perl version 5.14).\n>\n> qw{} is described in perlop(1) as \"word list\" operator, so one would\n> suppose that it generates a list.\n\nIt does, but it wasn't supposed to generate parens for you.\n\n>> The issue is that perl will now warn about this:\n>>\n>>     $ perl -wE 'for my $i qw(a b) { say $i }'\n>>     Use of qw(...) as parentheses is deprecated at -e line 1.\n>>     a\n>>     b\n>\n> Hmmm... does it affect only foreach loop, or dows it affect also other\n> places, like\n>\n>      use POSIX qw( setlocale localeconv )\n>      @EXPORT = qw( foo bar baz );\n>\n> Both of those forms are used by gitweb:\n>\n>      use CGI qw(:standard :escapeHTML -nosticky);\n>\n>      map { $_ => 'sh'  } qw(bash zsh ksh)\n>      my @navs = qw(summary shortlog log commit commitdiff tree);\n\nNo. This is being deprecated because qw(foo bar) is supposed to mean\n\"foo, \"bar\", not (\"foo\", \"bar\"). I.e. this doesn't compile:\n\n    for my $i \"a\", \"b\", \"c\" { }\n\nSo neither should this:\n\n    for my $i qw(a b c) {}\n\nBut these both work:\n\n    for my $i (\"a\", \"b\", \"c\") { }\n    for my $i (qw(a b c)) {}\n\nAll of your other examples could have used a list without implicit\nparens. So this is the only change that's needed in gitweb.\n"},{"id":"161632","messageId":"AANLkTimFyue2F6aRg7KdqOJZw9dOHY+B0XWCiVBYNOeg@mail.gmail.com","threadId":"26558","inReplyTo":"201102191646.02543.jnareb@gmail.com","subject":"Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T16:17:39Z","receivedAt":"2011-02-19T16:17:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Sat, 19 Feb 2011, Ęvar Arnfjörš Bjarmason wrote:\n>\n>> Change the gitweb_run test subroutine to spew errors to stderr if\n>> there are any, previously it would just silently fail, which made\n>> tests very hard to debug.\n>>\n>> Before you'd get this output, when running tests under `--verbose\n>> --immediate --debug`:\n>\n> Which test?\n>\n> [...]\n>> --- a/t/gitweb-lib.sh\n>> +++ b/t/gitweb-lib.sh\n>> @@ -82,7 +82,12 @@ gitweb_run () {\n>>               }\n>>               close O;\n>>       ' gitweb.output &&\n>> -     if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi\n>> +     if grep '^[[]' gitweb.log >/dev/null 2>&1; then\n>> +             cat gitweb.log >&2\n>> +             false\n>> +     else\n>> +             true\n>> +     fi\n>\n> I don't understand this change.  Either it is not necessary, because\n> test suite (or at least t9500) has\n>\n>  test_debug 'cat gitweb.log'\n>\n> after each test, so that error messages would be printed with `--debug`,\n> or it doesn't go far enough: if the above is used then those test_debug\n> should be removed.\n\nThe way you're using test_debug() is incompatible with\n--immediate. The test dies, but I'll never see your debug message\nbecause I'm using --immediate.\n\nIt would be better to just use test_debug in gitweb_run (instead of my\n\"cat gitweb.log\").\n\nAnyway, if you feel like fixing that feel free, I wan't pursue this\nfurther (going to hack on what I was going to do before gitweb tests\nstarted failing).\n\nJunio, you can drop this patch since it'll produce duplicate output if\nthe test fails and the user *doesn't* use --immediate, but IMO this\nshould be fixed by doing the debug output differently.\n"},{"id":"161638","messageId":"201102191916.21164.jnareb@gmail.com","threadId":"26558","inReplyTo":"AANLkTimFyue2F6aRg7KdqOJZw9dOHY+B0XWCiVBYNOeg@mail.gmail.com","subject":"Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-19T18:16:19Z","receivedAt":"2011-02-19T18:16:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n> On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Sat, 19 Feb 2011, Ęvar Arnfjörš Bjarmason wrote:\n>>\n>>> Change the gitweb_run test subroutine to spew errors to stderr if\n>>> there are any, previously it would just silently fail, which made\n>>> tests very hard to debug.\n>>>\n>>> Before you'd get this output, when running tests under `--verbose\n>>> --immediate --debug`:\n>>\n>> Which test?\n>>\n>> [...]\n>>> --- a/t/gitweb-lib.sh\n>>> +++ b/t/gitweb-lib.sh\n>>> @@ -82,7 +82,12 @@ gitweb_run () {\n>>>               }\n>>>               close O;\n>>>       ' gitweb.output &&\n>>> -     if grep '^[[]' gitweb.log>/dev/null 2>&1; then false; else true; fi\n>>> +     if grep '^[[]' gitweb.log>/dev/null 2>&1; then\n>>> +             cat gitweb.log>&2\n>>> +             false\n>>> +     else\n>>> +             true\n>>> +     fi\n>>\n>> I don't understand this change.  Either it is not necessary, because\n>> test suite (or at least t9500) has\n>>\n>>  test_debug 'cat gitweb.log'\n>>\n>> after each test, so that error messages would be printed with `--debug`,\n>> or it doesn't go far enough: if the above is used then those test_debug\n>> should be removed.\n> \n> The way you're using test_debug() is incompatible with\n> --immediate. The test dies, but I'll never see your debug message\n> because I'm using --immediate.\n\nAh, so that is what it is about: using --immediate negates --debug.\n\nNote that it is *much* wider issue; it is not only gitweb tests that use\ntest_debug in such way.  See for example t/t4114-apply-typechange.sh\nwhere you have\n\n  test_expect_success SYMLINKS 'directory becomes symlink' '\n        git checkout -f foo-becomes-a-directory &&\n        git diff-tree -p HEAD foo-symlinked-to-bar > patch &&\n        git apply --index < patch\n        '\n  test_debug 'cat patch'\n\nThis causes the same problem.\n\n> It would be better to just use test_debug in gitweb_run (instead of my\n> \"cat gitweb.log\").\n> \n> Anyway, if you feel like fixing that feel free, I wan't pursue this\n> further (going to hack on what I was going to do before gitweb tests\n> started failing).\n> \n> Junio, you can drop this patch since it'll produce duplicate output if\n> the test fails and the user *doesn't* use --immediate, but IMO this\n> should be fixed by doing the debug output differently.\n \nBelow there is proposed patch that removes duplication of --debug output...\nbut does not solve issue for other places where we use test_debug in test\nsuite and incompatibility with --immediate run.\n\n-- >8 --\nFrom: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSubject: [PATCH] t/gitweb-lib.sh: Ensure that errors are shown for --debug --immediate\n\nBecause '--immediate' stops test suite after first error, therefore in\nthis mode\n\n  test_debug 'cat gitweb.log'\n\nwas never ran, thus in effect negating effect of '--debug' option.\nThis made finidng the cause of errors in gitweb test sute difficult.\n\nModify the gitweb_run test subroutine to run test_debug itself in the\ncase of errors (and also remove \"test_debug 'cat gitweb.log'\" from\ngitweb tests).\n\nThis makes it possible to run *gitweb tests* with --immediate ---debug\ncombination of options; also it makes gitweb tests to not output\nspurious debug data that is not considered error.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Jakub Narębski <jnareb@gmail.com>\n---\n t/gitweb-lib.sh                          |    7 ++\n t/t9500-gitweb-standalone-no-errors.sh   |   86 ------------------------------\n t/t9501-gitweb-standalone-http-status.sh |    1 \n 3 files changed, 6 insertions(+), 88 deletions(-)\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex b9bb95f..143eb1f 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -82,7 +82,12 @@ gitweb_run () {\n \t\t}\n \t\tclose O;\n \t' gitweb.output &&\n-\tif grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi\n+\tif grep '^[[]' gitweb.log >/dev/null 2>&1; then\n+\t\ttest_debug 'cat gitweb.log >&2' &&\n+\t\tfalse\n+\telse\n+\t\ttrue\n+\tfi\n \n \t# gitweb.log is left for debugging\n \t# gitweb.output is used to parse HTTP output\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 21cd286..35c151d 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -18,42 +18,34 @@ or warnings to log.'\n test_expect_success \\\n \t'no commits: projects_list (implicit)' \\\n \t'gitweb_run'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: projects_index' \\\n \t'gitweb_run \"a=project_index\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: .git summary (implicit)' \\\n \t'gitweb_run \"p=.git\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: .git commit (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=commit\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: .git commitdiff (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: .git tree (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=tree\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: .git heads' \\\n \t'gitweb_run \"p=.git;a=heads\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'no commits: .git tags' \\\n \t'gitweb_run \"p=.git;a=tags\"'\n-test_debug 'cat gitweb.log'\n \n \n # ----------------------------------------------------------------------\n@@ -69,52 +61,42 @@ test_expect_success \\\n test_expect_success \\\n \t'projects_list (implicit)' \\\n \t'gitweb_run'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'projects_index' \\\n \t'gitweb_run \"a=project_index\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git summary (implicit)' \\\n \t'gitweb_run \"p=.git\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git commit (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=commit\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git commitdiff (implicit HEAD, root commit)' \\\n \t'gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git commitdiff_plain (implicit HEAD, root commit)' \\\n \t'gitweb_run \"p=.git;a=commitdiff_plain\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git commit (HEAD)' \\\n \t'gitweb_run \"p=.git;a=commit;h=HEAD\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git tree (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=tree\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git blob (file)' \\\n \t'gitweb_run \"p=.git;a=blob;f=file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git blob_plain (file)' \\\n \t'gitweb_run \"p=.git;a=blob_plain;f=file\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # nonexistent objects\n@@ -122,37 +104,30 @@ test_debug 'cat gitweb.log'\n test_expect_success \\\n \t'.git commit (non-existent)' \\\n \t'gitweb_run \"p=.git;a=commit;h=non-existent\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git commitdiff (non-existent)' \\\n \t'gitweb_run \"p=.git;a=commitdiff;h=non-existent\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git commitdiff (non-existent vs HEAD)' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=non-existent;h=HEAD\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git tree (0000000000000000000000000000000000000000)' \\\n \t'gitweb_run \"p=.git;a=tree;h=0000000000000000000000000000000000000000\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git tag (0000000000000000000000000000000000000000)' \\\n \t'gitweb_run \"p=.git;a=tag;h=0000000000000000000000000000000000000000\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git blob (non-existent)' \\\n \t'gitweb_run \"p=.git;a=blob;f=non-existent\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'.git blob_plain (non-existent)' \\\n \t'gitweb_run \"p=.git;a=blob_plain;f=non-existent\"'\n-test_debug 'cat gitweb.log'\n \n \n # ----------------------------------------------------------------------\n@@ -161,7 +136,6 @@ test_debug 'cat gitweb.log'\n test_expect_success \\\n \t'commitdiff(0): root' \\\n \t'gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): file added' \\\n@@ -169,21 +143,18 @@ test_expect_success \\\n \t git add new_file &&\n \t git commit -a -m \"File added.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): mode change' \\\n \t'test_chmod +x new_file &&\n \t git commit -a -m \"Mode changed.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): file renamed' \\\n \t'git mv new_file renamed_file &&\n \t git commit -a -m \"File renamed.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success SYMLINKS \\\n \t'commitdiff(0): file to symlink' \\\n@@ -191,7 +162,6 @@ test_expect_success SYMLINKS \\\n \t ln -s file renamed_file &&\n \t git commit -a -m \"File to symlink.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): file deleted' \\\n@@ -199,7 +169,6 @@ test_expect_success \\\n \t rm -f renamed_file &&\n \t git commit -a -m \"File removed.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): file copied / new file' \\\n@@ -207,7 +176,6 @@ test_expect_success \\\n \t git add file2 &&\n \t git commit -a -m \"File copied.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): mode change and modified' \\\n@@ -215,7 +183,6 @@ test_expect_success \\\n \t test_chmod +x file2 &&\n \t git commit -a -m \"Mode change and modification.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): renamed and modified' \\\n@@ -233,7 +200,6 @@ EOF\n \t echo \"Propter nomen suum.\" >> file3 &&\n \t git commit -a -m \"File rename and modification.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): renamed, mode change and modified' \\\n@@ -242,7 +208,6 @@ test_expect_success \\\n \t test_chmod +x file2 &&\n \t git commit -a -m \"File rename, mode change and modification.\" &&\n \t gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # commitdiff testing (taken from t4114-apply-typechange.sh)\n@@ -279,42 +244,34 @@ test_expect_success SYMLINKS 'setup typechange commits' '\n test_expect_success \\\n \t'commitdiff(2): file renamed from foo to foo/baz' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=initial;h=foo-baz-renamed-from-foo\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): file renamed from foo/baz to foo' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=foo-baz-renamed-from-foo;h=initial\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): directory becomes file' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=foo-becomes-a-directory;h=initial\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): file becomes directory' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=initial;h=foo-becomes-a-directory\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): file becomes symlink' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=initial;h=foo-symlinked-to-bar\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): symlink becomes file' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=foo-symlinked-to-bar;h=foo-back-to-file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): symlink becomes directory' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=foo-symlinked-to-bar;h=foo-becomes-a-directory\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(2): directory becomes symlink' \\\n \t'gitweb_run \"p=.git;a=commitdiff;hp=foo-becomes-a-directory;h=foo-symlinked-to-bar\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # commit, commitdiff: merge, large\n@@ -330,12 +287,10 @@ test_expect_success \\\n test_expect_success \\\n \t'commit(0): merge commit' \\\n \t'gitweb_run \"p=.git;a=commit\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(0): merge commit' \\\n \t'gitweb_run \"p=.git;a=commitdiff\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'Prepare large commit' \\\n@@ -371,12 +326,10 @@ test_expect_success \\\n test_expect_success \\\n \t'commit(1): large commit' \\\n \t'gitweb_run \"p=.git;a=commit;h=b\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'commitdiff(1): large commit' \\\n \t'gitweb_run \"p=.git;a=commitdiff;h=b\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # tags testing\n@@ -394,17 +347,14 @@ test_expect_success \\\n \t git tag lightweight/tag-tree HEAD^{tree} &&\n \t git tag lightweight/tag-blob HEAD:file &&\n \t gitweb_run \"p=.git;a=tags\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'tag: Tag to commit object' \\\n \t'gitweb_run \"p=.git;a=tag;h=tag-commit\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'tag: on lightweight tag (invalid)' \\\n \t'gitweb_run \"p=.git;a=tag;h=lightweight/tag-commit\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # logs\n@@ -412,22 +362,18 @@ test_debug 'cat gitweb.log'\n test_expect_success \\\n \t'logs: log (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=log\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'logs: shortlog (implicit HEAD)' \\\n \t'gitweb_run \"p=.git;a=shortlog\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'logs: history (implicit HEAD, file)' \\\n \t'gitweb_run \"p=.git;a=history;f=file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'logs: history (implicit HEAD, non-existent file)' \\\n \t'gitweb_run \"p=.git;a=history;f=non-existent\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'logs: history (implicit HEAD, deleted file)' \\\n@@ -438,55 +384,45 @@ test_expect_success \\\n \t git rm deleted_file &&\n \t git commit -m \"Delete file\" &&\n \t gitweb_run \"p=.git;a=history;f=deleted_file\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # path_info links\n test_expect_success \\\n \t'path_info: project' \\\n \t'gitweb_run \"\" \"/.git\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/branch' \\\n \t'gitweb_run \"\" \"/.git/b\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/branch:file' \\\n \t'gitweb_run \"\" \"/.git/master:file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/branch:dir/' \\\n \t'gitweb_run \"\" \"/.git/master:foo/\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/branch:file (non-existent)' \\\n \t'gitweb_run \"\" \"/.git/master:non-existent\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/branch:dir/ (non-existent)' \\\n \t'gitweb_run \"\" \"/.git/master:non-existent/\"'\n-test_debug 'cat gitweb.log'\n \n \n test_expect_success \\\n \t'path_info: project/branch:/file' \\\n \t'gitweb_run \"\" \"/.git/master:/file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/:/file (implicit HEAD)' \\\n \t'gitweb_run \"\" \"/.git/:/file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'path_info: project/:/ (implicit HEAD, top tree)' \\\n \t'gitweb_run \"\" \"/.git/:/\"'\n-test_debug 'cat gitweb.log'\n \n \n # ----------------------------------------------------------------------\n@@ -495,17 +431,14 @@ test_debug 'cat gitweb.log'\n test_expect_success \\\n \t'feeds: OPML' \\\n \t'gitweb_run \"a=opml\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'feed: RSS' \\\n \t'gitweb_run \"p=.git;a=rss\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'feed: Atom' \\\n \t'gitweb_run \"p=.git;a=atom\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # encoding/decoding\n@@ -517,7 +450,6 @@ test_expect_success \\\n \t git add file &&\n \t git commit -F \"$TEST_DIRECTORY\"/t3900/1-UTF-8.txt &&\n \t gitweb_run \"p=.git;a=commit\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'encode(commit): iso-8859-1' \\\n@@ -528,12 +460,10 @@ test_expect_success \\\n \t git commit -F \"$TEST_DIRECTORY\"/t3900/ISO8859-1.txt &&\n \t git config --unset i18n.commitencoding &&\n \t gitweb_run \"p=.git;a=commit\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'encode(log): utf-8 and iso-8859-1' \\\n \t'gitweb_run \"p=.git;a=log\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # extra options\n@@ -541,27 +471,22 @@ test_debug 'cat gitweb.log'\n test_expect_success \\\n \t'opt: log --no-merges' \\\n \t'gitweb_run \"p=.git;a=log;opt=--no-merges\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'opt: atom --no-merges' \\\n \t'gitweb_run \"p=.git;a=log;opt=--no-merges\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'opt: \"file\" history --no-merges' \\\n \t'gitweb_run \"p=.git;a=history;f=file;opt=--no-merges\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'opt: log --no-such-option (invalid option)' \\\n \t'gitweb_run \"p=.git;a=log;opt=--no-such-option\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'opt: tree --no-merges (invalid option for action)' \\\n \t'gitweb_run \"p=.git;a=tree;opt=--no-merges\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # testing config_to_multi / cloneurl\n@@ -569,14 +494,12 @@ test_debug 'cat gitweb.log'\n test_expect_success \\\n        'URL: no project URLs, no base URL' \\\n        'gitweb_run \"p=.git;a=summary\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n        'URL: project URLs via gitweb.url' \\\n        'git config --add gitweb.url git://example.com/git/trash.git &&\n         git config --add gitweb.url http://example.com/git/trash.git &&\n         gitweb_run \"p=.git;a=summary\"'\n-test_debug 'cat gitweb.log'\n \n cat >.git/cloneurl <<\\EOF\n git://example.com/git/trash.git\n@@ -586,7 +509,6 @@ EOF\n test_expect_success \\\n        'URL: project URLs via cloneurl file' \\\n        'gitweb_run \"p=.git;a=summary\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # gitweb config and repo config\n@@ -604,12 +526,10 @@ EOF\n test_expect_success \\\n \t'config override: projects list (implicit)' \\\n \t'gitweb_run'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'config override: tree view, features not overridden in repo config' \\\n \t'gitweb_run \"p=.git;a=tree\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'config override: tree view, features disabled in repo config' \\\n@@ -617,14 +537,12 @@ test_expect_success \\\n \t git config gitweb.snapshot none &&\n \t git config gitweb.avatar gravatar &&\n \t gitweb_run \"p=.git;a=tree\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success \\\n \t'config override: tree view, features enabled in repo config (1)' \\\n \t'git config gitweb.blame yes &&\n \t git config gitweb.snapshot \"zip,tgz, tbz2\" &&\n \t gitweb_run \"p=.git;a=tree\"'\n-test_debug 'cat gitweb.log'\n \n cat >.git/config <<\\EOF\n # testing noval and alternate separator\n@@ -635,7 +553,6 @@ EOF\n test_expect_success \\\n \t'config override: tree view, features enabled in repo config (2)' \\\n \t'gitweb_run \"p=.git;a=tree\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # non-ASCII in README.html\n@@ -645,7 +562,6 @@ test_expect_success \\\n \t'echo \"<b>UTF-8 example:</b><br />\" > .git/README.html &&\n \t cat \"$TEST_DIRECTORY\"/t3900/1-UTF-8.txt >> .git/README.html &&\n \t gitweb_run \"p=.git;a=summary\"'\n-test_debug 'cat gitweb.log'\n \n # ----------------------------------------------------------------------\n # syntax highlighting\n@@ -666,7 +582,6 @@ test_expect_success HIGHLIGHT \\\n \t'syntax highlighting (no highlight, unknown syntax)' \\\n \t'git config gitweb.highlight yes &&\n \t gitweb_run \"p=.git;a=blob;f=file\"'\n-test_debug 'cat gitweb.log'\n \n test_expect_success HIGHLIGHT \\\n \t'syntax highlighting (highlighted, shell script)' \\\n@@ -675,6 +590,5 @@ test_expect_success HIGHLIGHT \\\n \t git add test.sh &&\n \t git commit -m \"Add test.sh\" &&\n \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n-test_debug 'cat gitweb.log'\n \n test_done\ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex 18825af..26102ee 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -126,7 +126,6 @@ test_expect_success 'load checking: load too high (default action)' '\n \tgrep \"Status: 503 Service Unavailable\" gitweb.headers &&\n \tgrep \"503 - The load average on the server is too high\" gitweb.body\n '\n-test_debug 'cat gitweb.log' # just in case\n test_debug 'cat gitweb.headers'\n \n # turn off load checking\n"},{"id":"161640","messageId":"AANLkTinwV0zFTR3zYtN53JJOZYDhtAkhoY4TC1RstZHC@mail.gmail.com","threadId":"26558","inReplyTo":"201102191916.21164.jnareb@gmail.com","subject":"Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-19T19:11:36Z","receivedAt":"2011-02-19T19:11:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Feb 19, 2011 at 19:16, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n>> On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@gmail.com> wrote:\n>>> On Sat, 19 Feb 2011, Ęvar Arnfjörš Bjarmason wrote:\n>>>\n>>>> Change the gitweb_run test subroutine to spew errors to stderr if\n>>>> there are any, previously it would just silently fail, which made\n>>>> tests very hard to debug.\n>>>>\n>>>> Before you'd get this output, when running tests under `--verbose\n>>>> --immediate --debug`:\n>>>\n>>> Which test?\n>>>\n>>> [...]\n>>>> --- a/t/gitweb-lib.sh\n>>>> +++ b/t/gitweb-lib.sh\n>>>> @@ -82,7 +82,12 @@ gitweb_run () {\n>>>>               }\n>>>>               close O;\n>>>>       ' gitweb.output &&\n>>>> -     if grep '^[[]' gitweb.log>/dev/null 2>&1; then false; else true; fi\n>>>> +     if grep '^[[]' gitweb.log>/dev/null 2>&1; then\n>>>> +             cat gitweb.log>&2\n>>>> +             false\n>>>> +     else\n>>>> +             true\n>>>> +     fi\n>>>\n>>> I don't understand this change.  Either it is not necessary, because\n>>> test suite (or at least t9500) has\n>>>\n>>>  test_debug 'cat gitweb.log'\n>>>\n>>> after each test, so that error messages would be printed with `--debug`,\n>>> or it doesn't go far enough: if the above is used then those test_debug\n>>> should be removed.\n>>\n>> The way you're using test_debug() is incompatible with\n>> --immediate. The test dies, but I'll never see your debug message\n>> because I'm using --immediate.\n>\n> Ah, so that is what it is about: using --immediate negates --debug.\n\nYeah, I didn't realize that when I submitted my patch, I just thought\nit would never be printed, period.\n\n> Note that it is *much* wider issue; it is not only gitweb tests that use\n> test_debug in such way.  See for example t/t4114-apply-typechange.sh\n> where you have\n\nIndeed, it's all over the test suite.\n\n>  test_expect_success SYMLINKS 'directory becomes symlink' '\n>        git checkout -f foo-becomes-a-directory &&\n>        git diff-tree -p HEAD foo-symlinked-to-bar > patch &&\n>        git apply --index < patch\n>        '\n>  test_debug 'cat patch'\n>\n> This causes the same problem.\n>\n>> It would be better to just use test_debug in gitweb_run (instead of my\n>> \"cat gitweb.log\").\n>>\n>> Anyway, if you feel like fixing that feel free, I wan't pursue this\n>> further (going to hack on what I was going to do before gitweb tests\n>> started failing).\n>>\n>> Junio, you can drop this patch since it'll produce duplicate output if\n>> the test fails and the user *doesn't* use --immediate, but IMO this\n>> should be fixed by doing the debug output differently.\n>\n> Below there is proposed patch that removes duplication of --debug output...\n> but does not solve issue for other places where we use test_debug in test\n> suite and incompatibility with --immediate run.\n\nThis patch looks good to me.\n"},{"id":"161759","messageId":"201102201542.47590.jnareb@gmail.com","threadId":"26558","inReplyTo":"AANLkTi==3c4ZLckUPE6NUt-sqWBsywQh3Gu+4B5vOBpj@mail.gmail.com","subject":"Re: [PATCH v2 2/3] gitweb/gitweb.perl: remove use of qw(...) as parentheses","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-20T14:42:46Z","receivedAt":"2011-02-20T14:42:46Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia sobota 19. lutego 2011 17:06, Ævar Arnfjörð Bjarmason napisał:\n> On Sat, Feb 19, 2011 at 16:54, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Sat, 19 Feb 2011, Ævar Arnfjörð Bjarmason wrote:\n>>\n>>> Using the qw(...) construct as implicit parentheses was deprecated in\n>>> perl 5.13.5. Change the relevant code in gitweb to not use the\n>>> deprecated construct. The offending code was introduced in 3562198b by\n>>> Jakub Narebski.\n>>\n>> It is strange that Perl introduces such backwards incompatibile change\n>> (well, actually will introduce, as 5.13.x is development branch leading\n>> to future Perl version 5.14).\n>>\n>> qw{} is described in perlop(1) as \"word list\" operator, so one would\n>> suppose that it generates a list.\n> \n> It does, but it wasn't supposed to generate parens for you.\n\n[...]\n>> Hmmm... does it affect only foreach loop, or dows it affect also other\n>> places, like\n>>\n>>      use POSIX qw( setlocale localeconv )\n>>      @EXPORT = qw( foo bar baz );\n\n[...]\n> No. This is being deprecated because qw(foo bar) is supposed to mean\n> \"foo, \"bar\", not (\"foo\", \"bar\"). I.e. this doesn't compile:\n> \n>     for my $i \"a\", \"b\", \"c\" { }\n> \n> So neither should this:\n> \n>     for my $i qw(a b c) {}\n> \n> But these both work:\n> \n>     for my $i (\"a\", \"b\", \"c\") { }\n>     for my $i (qw(a b c)) {}\n> \n> All of your other examples could have used a list without implicit\n> parens. So this is the only change that's needed in gitweb.\n\nThanks for the explanation.  It makes sense.  So:\n\nAcked-by: Jakub Narębski <jnareb@gmail.com>\n\n-- \nJakub Narebski\nPoland\n"},{"id":"161775","messageId":"7vmxlplwjx.fsf@alter.siamese.dyndns.org","threadId":"26558","inReplyTo":"201102191916.21164.jnareb@gmail.com","subject":"Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-21T06:40:50Z","receivedAt":"2011-02-21T06:40:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Because '--immediate' stops test suite after first error, therefore in\n> this mode\n>\n>   test_debug 'cat gitweb.log'\n>\n> was never ran, thus in effect negating effect of '--debug' option.\n> This made finidng the cause of errors in gitweb test sute difficult.\n\nI think the patch itself makes sense, but I actually don't agree with the\nabove reasoning.\n\nThe point of --immediate is not just it stops where we find a failure, but\nalso it leaves the trash directory intact, so that _you_ who started the\ntest with --immediate can run the \"cat gitweb.log\" yourself.\n\nIn other words, I think test_debug is overused.  In the current setting, I\nthink a sane way to debug the test script itself would probably be\n\n\tsh -x tXXXX-xxxx.sh -i -v -d\n\nso that:\n\n (1) with -x I can see what commands are being run;\n (2) with -i I can stop the test immediately upon the first failure;\n (3) with -v I can see the output from the commands that we usually do not\n     show; and\n (4) with -d I can be sure that trash directory does not go away.\n\nNotice that I do not count \"I can see test_debug executed\" as a benefit\nfrom (4) above?\n\nMy preference, although I do not care too deeply, would be to gradually\nremove test_debug calls that were casually sprinkled and left in the test\nscripts while they were initially developed, and have the regular test\nsequence run them just like your patch does.  Another change I would\nwelcome would be to make the \"-d\" option automatically trigger shell\ntracing, to make it unnecessary to say \"sh -x\".\n"}]}