{"thread":{"id":"24094","subject":"[PATCH] gitweb: Return or exit after done serving request","startedAt":"2010-06-13T10:09:32Z","lastAt":"2010-06-14T20:54:29Z","messageCount":4,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143601","messageId":"20100613100800.28221.77529.stgit@localhost.localdomain","threadId":"24094","inReplyTo":null,"subject":"[PATCH] gitweb: Return or exit after done serving request","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-13T10:09:32Z","receivedAt":"2010-06-13T10:09:32Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Check if there is a caller in top frame of gitweb, and either 'return'\nif gitweb code is wrapped in subroutine, or 'exit' if it is not.\n\nThis should avoid\n\n  gitweb.cgi: Subroutine git_SOMETHING redefined at gitweb.cgi line NNN\n\nwarnings in error_log when running gitweb with mod_perl (using\nModPerl::Registry handler)\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nI have noticed bunch of\n\n  [Sun Jun 13 11:58:02 2010] gitweb.cgi: Subroutine git_opml redefined\n  at /var/www/perl/gitweb/gitweb.cgi line 6808.\n\nwarnings in /var/log/httpd/error_log, after running gitweb from mod_perl.\nThis fixes it.\n\n gitweb/gitweb.perl |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e108bbc..02f366d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -987,7 +987,16 @@ if ($action !~ m/^(?:opml|project_list|project_index)$/ &&\n \tdie_error(400, \"Project needed\");\n }\n $actions{$action}->();\n+\n DONE_GITWEB:\n+if (defined caller) {\n+\t# wrapped in a subroutine processing requests,\n+\t# e.g. mod_perl with ModPerl::Registry, or PSGI with Plack::App::WrapCGI\n+\treturn;\n+} else {\n+\t# pure CGI script, serving single request\n+\texit;\n+}\n 1;\n \n ## ======================================================================\n"},{"id":"143634","messageId":"7v1vcach4x.fsf@alter.siamese.dyndns.org","threadId":"24094","inReplyTo":"20100613100800.28221.77529.stgit@localhost.localdomain","subject":"Re: [PATCH] gitweb: Return or exit after done serving request","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-14T05:33:50Z","receivedAt":"2010-06-14T05:33: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> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index e108bbc..02f366d 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -987,7 +987,16 @@ if ($action !~ m/^(?:opml|project_list|project_index)$/ &&\n>  \tdie_error(400, \"Project needed\");\n>  }\n>  $actions{$action}->();\n> +\n>  DONE_GITWEB:\n> +if (defined caller) {\n> +\t# wrapped in a subroutine processing requests,\n> +\t# e.g. mod_perl with ModPerl::Registry, or PSGI with Plack::App::WrapCGI\n> +\treturn;\n> +} else {\n> +\t# pure CGI script, serving single request\n> +\texit;\n> +}\n>  1;\n\nIs the last \"1;\" still needed if we did this?\n\nI am guessing that this new codeblock will go inside \"sub run\" when\nmerging with your c2394fe (gitweb: Put all per-connection code in run()\nsubroutine, 2010-05-07) and Sam's a0446e7 (gitweb: Add support for\nFastCGI, using CGI::Fast, 2010-05-07).  If I am mistaken, please advise.\n\nThanks.\n"},{"id":"143646","messageId":"201006141029.00310.jnareb@gmail.com","threadId":"24094","inReplyTo":"7v1vcach4x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: Return or exit after done serving request","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-14T08:28:58Z","receivedAt":"2010-06-14T08:28:58Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 14 June 2010, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index e108bbc..02f366d 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -987,7 +987,16 @@ if ($action !~ m/^(?:opml|project_list|project_index)$/ &&\n> >  \tdie_error(400, \"Project needed\");\n> >  }\n> >  $actions{$action}->();\n> > +\n> >  DONE_GITWEB:\n> > +if (defined caller) {\n> > +\t# wrapped in a subroutine processing requests,\n> > +\t# e.g. mod_perl with ModPerl::Registry, or PSGI with Plack::App::WrapCGI\n> > +\treturn;\n> > +} else {\n> > +\t# pure CGI script, serving single request\n> > +\texit;\n> > +}\n> >  1;\n> \n> Is the last \"1;\" still needed if we did this?\n\nProbably not, and probably we never need this.\n\n> \n> I am guessing that this new codeblock will go inside \"sub run\" when\n> merging with your c2394fe (gitweb: Put all per-connection code in run()\n> subroutine, 2010-05-07) and Sam's a0446e7 (gitweb: Add support for\n> FastCGI, using CGI::Fast, 2010-05-07).  If I am mistaken, please advise.\n\nNo, the code is meant to be after last code to be run in gitweb, \nin *top level* scope, otherwise it would always return.\n\nSo the patch merged with commits mentioned above ('jn/gitweb-fastcgi',\nold or new) would look like the following... but I am not sure if it\nis needed, as I tried to reproduce error / warning that I got to ensure\nthat the patch really fixes it (as in new post \"sub run\" code there\nare subroutine definitions before end of 'main' code, i.e. run(); in\nnew post \"sub run\" code, $actions{$action}->(); in pre \"sub run\" code)\nand I couldn't get again this:\n\n  /var/log/httpd/error_log:\n  ...\n  [Sun Jun 13 11:58:02 2010] gitweb.cgi: Subroutine git_atom redefined at /var/www/perl/gitweb/gitweb.cgi line 6804.\n  [Sun Jun 13 11:58:02 2010] gitweb.cgi: Subroutine git_opml redefined at /var/www/perl/gitweb/gitweb.cgi line 6808.\n  ...gi-bin\n\nLets table it now.\n\n-- >8 --\nSubject: [RFC/PATCH] gitweb: Return or exit after done serving request\n\nCheck if there is a caller in top frame of gitweb, and either 'return'\nif gitweb code is wrapped in subroutine, or 'exit' if it is not.\n\nThis should avoid\n\n  gitweb.cgi: Subroutine git_SOMETHING redefined at gitweb.cgi line NNN\n\nwarnings in error_log when running gitweb with mod_perl (using\nModPerl::Registry handler)\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 14ef50c..43589b5 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1111,6 +1111,16 @@ sub run {\n \n run();\n \n+if (defined caller) {\n+\t# wrapped in a subroutine processing requests,\n+\t# e.g. mod_perl with ModPerl::Registry, or PSGI with Plack::App::WrapCGI\n+\treturn;\n+} else {\n+\t# pure CGI script, serving single request\n+\t# or 'exit' hijacked appropriately\n+\texit;\n+}\n+\n ## ======================================================================\n ## action links\n \n-- \n1.7.0.1\n"},{"id":"143697","messageId":"7v8w6h9vy2.fsf@alter.siamese.dyndns.org","threadId":"24094","inReplyTo":"201006141029.00310.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Return or exit after done serving request","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-14T20:54:29Z","receivedAt":"2010-06-14T20:54:29Z","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> On Mon, 14 June 2010, Junio C Hamano wrote:\n> ...\n>> Is the last \"1;\" still needed if we did this?\n>\n> Probably not, and probably we never need this.\n>> \n>> I am guessing that this new codeblock will go inside \"sub run\" when\n>> merging with your c2394fe (gitweb: Put all per-connection code in run()\n>> subroutine, 2010-05-07) and Sam's a0446e7 (gitweb: Add support for\n>> FastCGI, using CGI::Fast, 2010-05-07).  If I am mistaken, please advise.\n>\n> No, the code is meant to be after last code to be run in gitweb, \n> in *top level* scope, otherwise it would always return.\n\nOk, tentatively I fixed the mismerge of 'pu' from the last night and\npushed it out.\n"}]}