* [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script @ 2010-05-07 12:54 Jakub Narebski 2010-05-07 12:54 ` [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine Jakub Narebski ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-07 12:54 UTC (permalink / raw) To: git Cc: Jakub Narebski, Sam Vilain, Eric Wong, Juan Jose Comellas, Peter Vereshagin, John Goerzen This series requires the following patch: [PATCH 3/5] gitweb: Use nonlocal jump instead of 'exit' in die_error http://article.gmane.org/gmane.comp.version-control.git/145678 which is present as c42b00c commit in 'pu', as part of 'jn/gitweb-caching-prep' branch, which was merged into 'pu' at 97153ab7. This patch adds support for FastCGI directly to gitweb.perl (so it would be present in gitweb.cgi); selecting between FastCGI and CGI is done via command-line switch (command-line option). It uses CGI::Fast, which is core Perl module. It is port of old patch by Sam Vilain from 2006 to new gitweb announced in "Re: gitweb testing with non-apache web server" http://article.gmane.org/gmane.comp.version-control.git/24718 The preparatory patch is refactoring work, to not need to have request loop around large parts of code. I think the preparatory patch makes gitweb code more clean. The alternate solution would be to add gitweb.fcgi wrapper, like e.g.: in the following patch by Eric Wong "[PATCH 1/2] gitweb: add a simple wrapper for FCGI support" http://thread.gmane.org/gmane.comp.version-control.git/35920/focus=35921 which was part of the "[0/2 PATCH] FastCGI and nginx support for gitweb" series. (Note that the patch does 'do $gitweb_cgi;' without checking for errors, see the bottom of `perldoc -f do` documentation on how it should be done). Some other references: * "GitWeb in FastCGI" by Peter Vereshagin http://thread.gmane.org/gmane.comp.version-control.git/142132 * "FastCGI support in gitweb" by Juan Jose Comellas http://thread.gmane.org/gmane.comp.version-control.git/75704 Table of contents: ~~~~~~~~~~~~~~~~~~ [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine [PATCH/RFC 2/2] gitweb: Add support for FastCGI, using CGI::Fast Jakub Narebski (1): gitweb: Put all per-connection code in run() subroutine Sam Vilain (1): gitweb: Add support for FastCGI, using CGI::Fast Diffstat: ~~~~~~~~~ gitweb/gitweb.perl | 409 ++++++++++++++++++++++++++++++++-------------------- 1 files changed, 255 insertions(+), 154 deletions(-) Diffstat -w: ~~~~~~~~~~~~ When ignoring whitespace change (reindenting) gitweb/gitweb.perl | 117 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 109 insertions(+), 8 deletions(-) -- Jakub Narebski ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine 2010-05-07 12:54 [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski @ 2010-05-07 12:54 ` Jakub Narebski 2010-05-07 12:54 ` [RFC/PATCH 2/2] gitweb: Add support for FastCGI, using CGI::Fast Jakub Narebski 2010-05-08 22:41 ` [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski 2 siblings, 0 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-07 12:54 UTC (permalink / raw) To: git Cc: Jakub Narebski, Sam Vilain, Eric Wong, Juan Jose Comellas, Peter Vereshagin, John Goerzen All code that is run per-connection (as opposed to those parts of gitweb code that can be run once) is put into appropriate subroutines: - evaluate_uri - evaluate_gitweb_config - evaluate_git_version (here only because $GIT can be set in config) - check_loadavg (as soon as possible; $git_version must be defined) - evaluate_query_params (counterpart to evaluate_path_info) - evaluate_and_validate_params - evaluate_git_dir (requires $project) - configure_gitweb_features (@snapshot_fmts, $git_avatar) - dispatch (includes setting default $action) The difference is best viewed with '-w', '--ignore-all-space' option, because of reindent caused by putting code in subroutines. Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- This patch requires the following commit from 'jn/gitweb-caching-prep' branch c42b00c (gitweb: Use nonlocal jump instead of 'exit' in die_error, 2010-04-24) This is an RFC patch because selecting which parts of gitweb code are run-once, and which parts are per-request (and are to be put into subroutines and run from run() subroutine) is at, least in part, a bit subjective. gitweb/gitweb.perl | 359 ++++++++++++++++++++++++++++++---------------------- 1 files changed, 205 insertions(+), 154 deletions(-) When ignoring whitespace changes (reindenting): gitweb/gitweb.perl | 67 +++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 59 insertions(+), 8 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 4074300..41bf992 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -28,34 +28,42 @@ BEGIN { CGI->compile() if $ENV{'MOD_PERL'}; } -our $cgi = new CGI; our $version = "++GIT_VERSION++"; -our $my_url = $cgi->url(); -our $my_uri = $cgi->url(-absolute => 1); -# Base URL for relative URLs in gitweb ($logo, $favicon, ...), -# needed and used only for URLs with nonempty PATH_INFO -our $base_url = $my_url; +our ($my_url, $my_uri, $base_url, $path_info, $home_link); +sub evaluate_uri { + our $cgi; -# When the script is used as DirectoryIndex, the URL does not contain the name -# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we -# have to do it ourselves. We make $path_info global because it's also used -# later on. -# -# Another issue with the script being the DirectoryIndex is that the resulting -# $my_url data is not the full script URL: this is good, because we want -# generated links to keep implying the script name if it wasn't explicitly -# indicated in the URL we're handling, but it means that $my_url cannot be used -# as base URL. -# Therefore, if we needed to strip PATH_INFO, then we know that we have -# to build the base URL ourselves: -our $path_info = $ENV{"PATH_INFO"}; -if ($path_info) { - if ($my_url =~ s,\Q$path_info\E$,, && - $my_uri =~ s,\Q$path_info\E$,, && - defined $ENV{'SCRIPT_NAME'}) { - $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'}; + our $my_url = $cgi->url(); + our $my_uri = $cgi->url(-absolute => 1); + + # Base URL for relative URLs in gitweb ($logo, $favicon, ...), + # needed and used only for URLs with nonempty PATH_INFO + our $base_url = $my_url; + + # When the script is used as DirectoryIndex, the URL does not contain the name + # of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we + # have to do it ourselves. We make $path_info global because it's also used + # later on. + # + # Another issue with the script being the DirectoryIndex is that the resulting + # $my_url data is not the full script URL: this is good, because we want + # generated links to keep implying the script name if it wasn't explicitly + # indicated in the URL we're handling, but it means that $my_url cannot be used + # as base URL. + # Therefore, if we needed to strip PATH_INFO, then we know that we have + # to build the base URL ourselves: + our $path_info = $ENV{"PATH_INFO"}; + if ($path_info) { + if ($my_url =~ s,\Q$path_info\E$,, && + $my_uri =~ s,\Q$path_info\E$,, && + defined $ENV{'SCRIPT_NAME'}) { + $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'}; + } } + + # target of the home link on top of all pages + our $home_link = $my_uri || "/"; } # core git executable to use @@ -70,9 +78,6 @@ our $projectroot = "++GITWEB_PROJECTROOT++"; # the number is relative to the projectroot our $project_maxdepth = "++GITWEB_PROJECT_MAXDEPTH++"; -# target of the home link on top of all pages -our $home_link = $my_uri || "/"; - # string of the home link on top of all pages our $home_link_str = "++GITWEB_HOME_LINK_STR++"; @@ -566,15 +571,18 @@ sub filter_snapshot_fmts { !$known_snapshot_formats{$_}{'disabled'}} @fmts; } -our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++"; -our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++"; -# die if there are errors parsing config file -if (-e $GITWEB_CONFIG) { - do $GITWEB_CONFIG; - die $@ if $@; -} elsif (-e $GITWEB_CONFIG_SYSTEM) { - do $GITWEB_CONFIG_SYSTEM; - die $@ if $@; +our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM); +sub evaluate_gitweb_config { + our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++"; + our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++"; + # die if there are errors parsing config file + if (-e $GITWEB_CONFIG) { + do $GITWEB_CONFIG; + die $@ if $@; + } elsif (-e $GITWEB_CONFIG_SYSTEM) { + do $GITWEB_CONFIG_SYSTEM; + die $@ if $@; + } } # Get loadavg of system, to compare against $maxload. @@ -600,13 +608,16 @@ sub get_loadavg { } # version of the core git binary -our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown"; -$number_of_git_cmds++; - -$projects_list ||= $projectroot; +our $git_version; +sub evaluate_git_version { + our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown"; + $number_of_git_cmds++; +} -if (defined $maxload && get_loadavg() > $maxload) { - die_error(503, "The load average on the server is too high"); +sub check_loadavg { + if (defined $maxload && get_loadavg() > $maxload) { + die_error(503, "The load average on the server is too high"); + } } # ====================================================================== @@ -693,11 +704,15 @@ our %allowed_options = ( # should be single values, but opt can be an array. We should probably # build an array of parameters that can be multi-valued, but since for the time # being it's only this one, we just single it out -while (my ($name, $symbol) = each %cgi_param_mapping) { - if ($symbol eq 'opt') { - $input_params{$name} = [ $cgi->param($symbol) ]; - } else { - $input_params{$name} = $cgi->param($symbol); +sub evaluate_query_params { + our $cgi; + + while (my ($name, $symbol) = each %cgi_param_mapping) { + if ($symbol eq 'opt') { + $input_params{$name} = [ $cgi->param($symbol) ]; + } else { + $input_params{$name} = $cgi->param($symbol); + } } } @@ -844,149 +859,185 @@ sub evaluate_path_info { } } } -evaluate_path_info(); -our $action = $input_params{'action'}; -if (defined $action) { - if (!validate_action($action)) { - die_error(400, "Invalid action parameter"); +our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base, + $hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp, + $searchtext, $search_regexp); +sub evaluate_and_validate_params { + our $action = $input_params{'action'}; + if (defined $action) { + if (!validate_action($action)) { + die_error(400, "Invalid action parameter"); + } } -} -# parameters which are pathnames -our $project = $input_params{'project'}; -if (defined $project) { - if (!validate_project($project)) { - undef $project; - die_error(404, "No such project"); + # parameters which are pathnames + our $project = $input_params{'project'}; + if (defined $project) { + if (!validate_project($project)) { + undef $project; + die_error(404, "No such project"); + } } -} -our $file_name = $input_params{'file_name'}; -if (defined $file_name) { - if (!validate_pathname($file_name)) { - die_error(400, "Invalid file parameter"); + our $file_name = $input_params{'file_name'}; + if (defined $file_name) { + if (!validate_pathname($file_name)) { + die_error(400, "Invalid file parameter"); + } } -} -our $file_parent = $input_params{'file_parent'}; -if (defined $file_parent) { - if (!validate_pathname($file_parent)) { - die_error(400, "Invalid file parent parameter"); + our $file_parent = $input_params{'file_parent'}; + if (defined $file_parent) { + if (!validate_pathname($file_parent)) { + die_error(400, "Invalid file parent parameter"); + } } -} -# parameters which are refnames -our $hash = $input_params{'hash'}; -if (defined $hash) { - if (!validate_refname($hash)) { - die_error(400, "Invalid hash parameter"); + # parameters which are refnames + our $hash = $input_params{'hash'}; + if (defined $hash) { + if (!validate_refname($hash)) { + die_error(400, "Invalid hash parameter"); + } } -} -our $hash_parent = $input_params{'hash_parent'}; -if (defined $hash_parent) { - if (!validate_refname($hash_parent)) { - die_error(400, "Invalid hash parent parameter"); + our $hash_parent = $input_params{'hash_parent'}; + if (defined $hash_parent) { + if (!validate_refname($hash_parent)) { + die_error(400, "Invalid hash parent parameter"); + } } -} -our $hash_base = $input_params{'hash_base'}; -if (defined $hash_base) { - if (!validate_refname($hash_base)) { - die_error(400, "Invalid hash base parameter"); + our $hash_base = $input_params{'hash_base'}; + if (defined $hash_base) { + if (!validate_refname($hash_base)) { + die_error(400, "Invalid hash base parameter"); + } } -} -our @extra_options = @{$input_params{'extra_options'}}; -# @extra_options is always defined, since it can only be (currently) set from -# CGI, and $cgi->param() returns the empty array in array context if the param -# is not set -foreach my $opt (@extra_options) { - if (not exists $allowed_options{$opt}) { - die_error(400, "Invalid option parameter"); - } - if (not grep(/^$action$/, @{$allowed_options{$opt}})) { - die_error(400, "Invalid option parameter for this action"); + our @extra_options = @{$input_params{'extra_options'}}; + # @extra_options is always defined, since it can only be (currently) set from + # CGI, and $cgi->param() returns the empty array in array context if the param + # is not set + foreach my $opt (@extra_options) { + if (not exists $allowed_options{$opt}) { + die_error(400, "Invalid option parameter"); + } + if (not grep(/^$action$/, @{$allowed_options{$opt}})) { + die_error(400, "Invalid option parameter for this action"); + } } -} -our $hash_parent_base = $input_params{'hash_parent_base'}; -if (defined $hash_parent_base) { - if (!validate_refname($hash_parent_base)) { - die_error(400, "Invalid hash parent base parameter"); + our $hash_parent_base = $input_params{'hash_parent_base'}; + if (defined $hash_parent_base) { + if (!validate_refname($hash_parent_base)) { + die_error(400, "Invalid hash parent base parameter"); + } } -} -# other parameters -our $page = $input_params{'page'}; -if (defined $page) { - if ($page =~ m/[^0-9]/) { - die_error(400, "Invalid page parameter"); + # other parameters + our $page = $input_params{'page'}; + if (defined $page) { + if ($page =~ m/[^0-9]/) { + die_error(400, "Invalid page parameter"); + } } -} -our $searchtype = $input_params{'searchtype'}; -if (defined $searchtype) { - if ($searchtype =~ m/[^a-z]/) { - die_error(400, "Invalid searchtype parameter"); + our $searchtype = $input_params{'searchtype'}; + if (defined $searchtype) { + if ($searchtype =~ m/[^a-z]/) { + die_error(400, "Invalid searchtype parameter"); + } } -} -our $search_use_regexp = $input_params{'search_use_regexp'}; + our $search_use_regexp = $input_params{'search_use_regexp'}; -our $searchtext = $input_params{'searchtext'}; -our $search_regexp; -if (defined $searchtext) { - if (length($searchtext) < 2) { - die_error(403, "At least two characters are required for search parameter"); + our $searchtext = $input_params{'searchtext'}; + our $search_regexp; + if (defined $searchtext) { + if (length($searchtext) < 2) { + die_error(403, "At least two characters are required for search parameter"); + } + $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext; } - $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext; } # path to the current git repository our $git_dir; -$git_dir = "$projectroot/$project" if $project; +sub evaluate_git_dir { + our $git_dir = "$projectroot/$project" if $project; +} -# list of supported snapshot formats -our @snapshot_fmts = gitweb_get_feature('snapshot'); -@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts); +our (@snapshot_fmts, $git_avatar); +sub configure_gitweb_features { + # list of supported snapshot formats + our @snapshot_fmts = gitweb_get_feature('snapshot'); + @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts); -# check that the avatar feature is set to a known provider name, -# and for each provider check if the dependencies are satisfied. -# if the provider name is invalid or the dependencies are not met, -# reset $git_avatar to the empty string. -our ($git_avatar) = gitweb_get_feature('avatar'); -if ($git_avatar eq 'gravatar') { - $git_avatar = '' unless (eval { require Digest::MD5; 1; }); -} elsif ($git_avatar eq 'picon') { - # no dependencies -} else { - $git_avatar = ''; + # check that the avatar feature is set to a known provider name, + # and for each provider check if the dependencies are satisfied. + # if the provider name is invalid or the dependencies are not met, + # reset $git_avatar to the empty string. + our ($git_avatar) = gitweb_get_feature('avatar'); + if ($git_avatar eq 'gravatar') { + $git_avatar = '' unless (eval { require Digest::MD5; 1; }); + } elsif ($git_avatar eq 'picon') { + # no dependencies + } else { + $git_avatar = ''; + } } # dispatch -if (!defined $action) { - if (defined $hash) { - $action = git_get_type($hash); - } elsif (defined $hash_base && defined $file_name) { - $action = git_get_type("$hash_base:$file_name"); - } elsif (defined $project) { - $action = 'summary'; - } else { - $action = 'project_list'; +sub dispatch { + if (!defined $action) { + if (defined $hash) { + $action = git_get_type($hash); + } elsif (defined $hash_base && defined $file_name) { + $action = git_get_type("$hash_base:$file_name"); + } elsif (defined $project) { + $action = 'summary'; + } else { + $action = 'project_list'; + } } + if (!defined($actions{$action})) { + die_error(400, "Unknown action"); + } + if ($action !~ m/^(?:opml|project_list|project_index)$/ && + !$project) { + die_error(400, "Project needed"); + } + $actions{$action}->(); } -if (!defined($actions{$action})) { - die_error(400, "Unknown action"); -} -if ($action !~ m/^(?:opml|project_list|project_index)$/ && - !$project) { - die_error(400, "Project needed"); + +sub run { + our $t0 = [Time::HiRes::gettimeofday()] + if defined $t0; + + evaluate_uri(); + evaluate_gitweb_config(); + evaluate_git_version(); + check_loadavg(); + + # $projectroot and $projects_list might be set in gitweb config file + $projects_list ||= $projectroot; + + evaluate_query_params(); + evaluate_path_info(); + evaluate_and_validate_params(); + evaluate_git_dir(); + + configure_gitweb_features(); + + dispatch(); + + DONE_GITWEB: + 1; } -$actions{$action}->(); -DONE_GITWEB: -1; +our $cgi = CGI->new(); +run(); ## ====================================================================== ## action links -- 1.7.0.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC/PATCH 2/2] gitweb: Add support for FastCGI, using CGI::Fast 2010-05-07 12:54 [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski 2010-05-07 12:54 ` [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine Jakub Narebski @ 2010-05-07 12:54 ` Jakub Narebski 2010-05-08 7:59 ` [RFC/PATCHv2 " Jakub Narebski 2010-05-08 22:41 ` [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski 2 siblings, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-07 12:54 UTC (permalink / raw) To: git Cc: Jakub Narebski, Sam Vilain, Eric Wong, Juan Jose Comellas, Peter Vereshagin, John Goerzen From: Sam Vilain <sam.vilain@catalyst.net.nz> Former run() subroutine got renamed to run_request(). The new run() subroutine can run multiple requests at once if run as FastCGI script. To run gitweb as FastCGI script you must specify '--fastcgi' / '-f' command line option to gitweb, otherwise it runs as an ordinary CGI script. [jn: cherry picked from 56d7d436644ab296155a697552ea1345f2701620 in http://utsl.gen.nz/gitweb/?p=gitweb which was originally based on v264 (2326acfa95ac86a53804ca8eeeb482c2f9265e34) by Kay Sievers; updated to reflect current gitweb code] TODO: update 'gitweb/README' and/or 'gitweb/INSTALL' files. Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz> Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- This is straighforward port of Sam Vilain patch to new gitweb code. It is an RFC because while I have checked that it doesn't cause problems when running without the '--fastcgi' parameter: * as CGI script (from mod_cgi), * as ModPerl::Registry script (from mod_perl), * as standalone script configured via gitweb_config.perl (from command line), * as PSGI script via gitweb.psgi wrapper (from plackup, using Plack::App::WrapCGI, which in turn uses CGI::Emulate::PSGI), I haven't actually checked that it runs correctly with *FastCGI server* (because I don't have one installed). gitweb/gitweb.perl | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 52 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 41bf992..a4194d7 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1012,7 +1012,7 @@ sub dispatch { $actions{$action}->(); } -sub run { +sub run_request { our $t0 = [Time::HiRes::gettimeofday()] if defined $t0; @@ -1032,11 +1032,61 @@ sub run { configure_gitweb_features(); dispatch(); +} + +our $is_last_request = sub { 1 }; +our ($pre_dispatch_hook, $post_dispatch_hook, $pre_listen_hook); +our $CGI = 'CGI'; +our $cgi; +sub evaluate_argv { + return unless (@ARGV); + + require Getopt::Long; + Getopt::Long::GetOptions( + 'fastcgi|fcgi|f' => sub { + require CGI::Fast; + our $CGI = 'CGI::Fast'; + + my $request_number = 0; + # let each child service 100 requests + our $is_last_request = sub { ++$request_number > 100 }; + }, + 'nproc|n=i' => sub { + my ($arg, $val) = @_; + return unless eval { require FCGI::ProcManager; 1; }; + my $proc_manager = FCGI::ProcManager->new({ + n_processes => $val, + }); + our $pre_listen_hook = sub { $proc_manager->pm_manage() }; + our $pre_dispatch_hook = sub { $proc_manager->pm_pre_dispatch() }; + our $post_dispatch_hook = sub { $proc_manager->pm_post_dispatch() }; + }, + ); +} + +sub run { + evaluate_argv(); + + $pre_listen_hook->() + if $pre_listen_hook; + + REQUEST: + while ($cgi = $CGI->new()) { + $pre_dispatch_hook->() + if $pre_dispatch_hook; + + run_request(); + + $pre_dispatch_hook->() + if $post_dispatch_hook; + + last REQUEST if ($is_last_request->()); + } DONE_GITWEB: 1; } -our $cgi = CGI->new(); + run(); ## ====================================================================== -- 1.7.0.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC/PATCHv2 2/2] gitweb: Add support for FastCGI, using CGI::Fast 2010-05-07 12:54 ` [RFC/PATCH 2/2] gitweb: Add support for FastCGI, using CGI::Fast Jakub Narebski @ 2010-05-08 7:59 ` Jakub Narebski 0 siblings, 0 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-08 7:59 UTC (permalink / raw) To: git Cc: Sam Vilain, Eric Wong, Juan Jose Comellas, Peter Vereshagin, John Goerzen From: Sam Vilain <sam.vilain@catalyst.net.nz> Former run() subroutine got renamed to run_request(). The new run() subroutine can run multiple requests at once if run as FastCGI script. To run gitweb as FastCGI script you must specify '--fastcgi' / '-f' command line option to gitweb, otherwise it runs as an ordinary CGI script. [jn: cherry picked from 56d7d436644ab296155a697552ea1345f2701620 in http://utsl.gen.nz/gitweb/?p=gitweb which was originally based on v264 (2326acfa95ac86a53804ca8eeeb482c2f9265e34) by Kay Sievers; updated to reflect current gitweb code] TODO: update 'gitweb/README' and/or 'gitweb/INSTALL' files. Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz> Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- Changes since v1: * Fix $pre_dispatch_hook -> $post_dispatch_hook typo. * Leave DONE_GITWEB label in run_request() subroutine. This way "HTTP exceptions" thrown using die_error(), such as '404 Not Found', would correctly end current request, instead of exiting FCGI script. Note that in original patch by Sam Vilain "HTTP exceptions" would not run $post_dispatch_hook. gitweb/gitweb.perl | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 52 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 41bf992..9a3eaf5 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1012,7 +1012,7 @@ sub dispatch { $actions{$action}->(); } -sub run { +sub run_request { our $t0 = [Time::HiRes::gettimeofday()] if defined $t0; @@ -1036,7 +1036,57 @@ sub run { DONE_GITWEB: 1; } -our $cgi = CGI->new(); + +our $is_last_request = sub { 1 }; +our ($pre_dispatch_hook, $post_dispatch_hook, $pre_listen_hook); +our $CGI = 'CGI'; +our $cgi; +sub evaluate_argv { + return unless (@ARGV); + + require Getopt::Long; + Getopt::Long::GetOptions( + 'fastcgi|fcgi|f' => sub { + require CGI::Fast; + our $CGI = 'CGI::Fast'; + + my $request_number = 0; + # let each child service 100 requests + our $is_last_request = sub { ++$request_number > 100 }; + }, + 'nproc|n=i' => sub { + my ($arg, $val) = @_; + return unless eval { require FCGI::ProcManager; 1; }; + my $proc_manager = FCGI::ProcManager->new({ + n_processes => $val, + }); + our $pre_listen_hook = sub { $proc_manager->pm_manage() }; + our $pre_dispatch_hook = sub { $proc_manager->pm_pre_dispatch() }; + our $post_dispatch_hook = sub { $proc_manager->pm_post_dispatch() }; + }, + ); +} + +sub run { + evaluate_argv(); + + $pre_listen_hook->() + if $pre_listen_hook; + + REQUEST: + while ($cgi = $CGI->new()) { + $pre_dispatch_hook->() + if $pre_dispatch_hook; + + run_request(); + + $post_dispatch_hook->() + if $post_dispatch_hook; + + last REQUEST if ($is_last_request->()); + } +} + run(); ## ====================================================================== -- 1.7.0.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-07 12:54 [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski 2010-05-07 12:54 ` [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine Jakub Narebski 2010-05-07 12:54 ` [RFC/PATCH 2/2] gitweb: Add support for FastCGI, using CGI::Fast Jakub Narebski @ 2010-05-08 22:41 ` Jakub Narebski 2010-05-09 9:31 ` Eric Wong 2 siblings, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-08 22:41 UTC (permalink / raw) To: Eric Wong Cc: git, Sam Vilain, Juan Jose Comellas, Peter Vereshagin, John Goerzen On Fri, 7 May 2010, Jakub Narebski wrote: > The alternate solution would be to add gitweb.fcgi wrapper, like e.g.: > in the following patch by Eric Wong > > "[PATCH 1/2] gitweb: add a simple wrapper for FCGI support" > http://thread.gmane.org/gmane.comp.version-control.git/35920/focus=35921 > > which was part of the "[0/2 PATCH] FastCGI and nginx support for gitweb" > series. (Note that the patch does 'do $gitweb_cgi;' without checking for > errors, see the bottom of `perldoc -f do` documentation on how it should > be done). I think a better solution here would be to use CGI::Compile instead of 'do $gitweb_cgi;'. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-08 22:41 ` [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski @ 2010-05-09 9:31 ` Eric Wong 2010-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2010-05-09 12:39 ` Jakub Narebski 0 siblings, 2 replies; 30+ messages in thread From: Eric Wong @ 2010-05-09 9:31 UTC (permalink / raw) To: Jakub Narebski Cc: git, Sam Vilain, Juan Jose Comellas, Peter Vereshagin, John Goerzen Jakub Narebski <jnareb@gmail.com> wrote: > On Fri, 7 May 2010, Jakub Narebski wrote: > > > The alternate solution would be to add gitweb.fcgi wrapper, like e.g.: > > in the following patch by Eric Wong > > > > "[PATCH 1/2] gitweb: add a simple wrapper for FCGI support" > > http://thread.gmane.org/gmane.comp.version-control.git/35920/focus=35921 > > > > which was part of the "[0/2 PATCH] FastCGI and nginx support for gitweb" > > series. (Note that the patch does 'do $gitweb_cgi;' without checking for > > errors, see the bottom of `perldoc -f do` documentation on how it should > > be done). > > I think a better solution here would be to use CGI::Compile instead > of 'do $gitweb_cgi;'. Possibly, now that CGI::Compile exists. Can that be used with a standalone Perl HTTP server? It's 2010 now and I have long abandoned FastCGI in favor of using HTTP to the application backends. In my experience, having only one plain-text protocol for both frontend web serving and backend application RPC makes development/monitoring/testing much easier. I just use Ruby WEBrick nowadays for any instaweb instances I run to share with a few cow-orkers. I do a reasonable amount of development in Ruby, so it's always installed and ready for me. It would be nice if there were something standalone and as ubiquitous as WEBrick in the Perl world. -- Eric Wong ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-09 9:31 ` Eric Wong @ 2010-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2010-05-09 12:39 ` Jakub Narebski 1 sibling, 0 replies; 30+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-05-09 11:48 UTC (permalink / raw) To: Eric Wong Cc: Jakub Narebski, git, Sam Vilain, Juan Jose Comellas, Peter Vereshagin, John Goerzen On Sun, May 9, 2010 at 09:31, Eric Wong <normalperson@yhbt.net> wrote: > I just use Ruby WEBrick nowadays for any instaweb instances I run to > share with a few cow-orkers. I do a reasonable amount of development in > Ruby, so it's always installed and ready for me. It would be nice if > there were something standalone and as ubiquitous as WEBrick in the Perl > world. There is: http://search.cpan.org/perldoc?Plack You can run applications under everything from stand-alone webservers to cgi, fastcgi and mod_perl if you interface with Plack. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-09 9:31 ` Eric Wong 2010-05-09 11:48 ` Ævar Arnfjörð Bjarmason @ 2010-05-09 12:39 ` Jakub Narebski 2010-05-09 16:47 ` Peter Vereshagin 1 sibling, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-09 12:39 UTC (permalink / raw) To: Eric Wong Cc: git, Sam Vilain, Juan Jose Comellas, Peter Vereshagin, John Goerzen On Sun, 9 May 2010, Eric Wong wrote: > Jakub Narebski <jnareb@gmail.com> wrote: >> On Fri, 7 May 2010, Jakub Narebski wrote: >> >>> The alternate solution would be to add gitweb.fcgi wrapper, like e.g.: >>> in the following patch by Eric Wong >>> >>> "[PATCH 1/2] gitweb: add a simple wrapper for FCGI support" >>> http://thread.gmane.org/gmane.comp.version-control.git/35920/focus=35921 >>> >>> which was part of the "[0/2 PATCH] FastCGI and nginx support for gitweb" >>> series. (Note that the patch does 'do $gitweb_cgi;' without checking for >>> errors, see the bottom of `perldoc -f do` documentation on how it should >>> be done). >> >> I think a better solution here would be to use CGI::Compile instead >> of 'do $gitweb_cgi;'. > > Possibly, now that CGI::Compile exists. Can that be used with a > standalone Perl HTTP server? Yes, it can. CGI::Compile is used for example by CGI::Emulate::PSGI, and you can run PSGI app on standalone Perl web server (pure Perl HTTP::Server::PSGI, or HTTP::Server::Simple::PSGI which in turn uses HTTP::Server::Simple, or Starman, or Twiggy, or Perlbal). CGI::Compile just compiles given CGI script into a subroutine, which can be called many times in a persistent web environment like FastCGI. > > It's 2010 now and I have long abandoned FastCGI in favor of using HTTP > to the application backends. In my experience, having only one > plain-text protocol for both frontend web serving and backend > application RPC makes development/monitoring/testing much easier. Do you mean here standalone web server in the language of web application? > > I just use Ruby WEBrick nowadays for any instaweb instances I run to > share with a few cow-orkers. I do a reasonable amount of development in > Ruby, so it's always installed and ready for me. It would be nice if > there were something standalone and as ubiquitous as WEBrick in the Perl > world. Modern Perl has PSGI/Plack (http://plackperl.org), which was inspired by Python's WSGI and Ruby's Rack. The Plack reference implementation includes 'plackup' tool (inspired by Ruby's 'rackup'), which can be used to run a PSGI application on any supported web server (with Plackup's adapter), like HTTP::Server::PSGI, HTTP::Server::Simple::PSGI, etc. PSGI/Plack goal is to be THE superglue interface between perl web application frameworks and web servers. P.S. BTW, I use the following wrapper for gitweb.cgi (in gitweb.psgi) -- 8< -- #!/usr/bin/env plackup # gitweb - simple web interface to track changes in git repositories # PSGI wrapper (see http://plackperl.org) use strict; use warnings; use Plack::Builder; use Plack::App::WrapCGI; use CGI::Emulate::PSGI 0.07; # minimum version required to work use File::Spec; # __DIR__ is taken from Dir::Self __DIR__ fragment sub __DIR__ () { File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]); } builder { enable 'Static', path => sub { m!\.(js|css|png)$! && s!^/gitweb/!! }, root => __DIR__."/"; Plack::App::WrapCGI->new(script => __DIR__."/gitweb.cgi")->to_app; } __END__ -- >8 -- Thanks to the she-bang line (I don't have plackup installed globally, but it is in my $PATH) I can just run ./gitweb.psgi to start server (http://0:5000/) with gitweb running. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-09 12:39 ` Jakub Narebski @ 2010-05-09 16:47 ` Peter Vereshagin 2010-05-09 18:18 ` Jakub Narebski 0 siblings, 1 reply; 30+ messages in thread From: Peter Vereshagin @ 2010-05-09 16:47 UTC (permalink / raw) To: Jakub Narebski Cc: Eric Wong, git, Sam Vilain, Juan Jose Comellas, Peter Vereshagin, John Goerzen I'm face to face with Jakub who sold the world? 2010/05/09 14:39:23 +0200 Jakub Narebski <jnareb@gmail.com> => To Eric Wong : JN> Yes, it can. CGI::Compile is used for example by CGI::Emulate::PSGI, JN> and you can run PSGI app on standalone Perl web server (pure Perl JN> HTTP::Server::PSGI, or HTTP::Server::Simple::PSGI which in turn uses JN> HTTP::Server::Simple, or Starman, or Twiggy, or Perlbal). CGI::Compile JN> just compiles given CGI script into a subroutine, which can be called JN> many times in a persistent web environment like FastCGI. Thanks a lot about that! I took a quick look at the patches and see this: - FastCGI people are not always happy with CGI.pm anmd thus with CGI::Fast that derives from it. They prefer CGI::Simple, e. g. for the Catalyst on fastcgi and other CGI.pm replacements. Despite the CGI::Fast is somehow the part of the perl core distribution the FCGI.pm and CGI.pm which are the required dependencies are not. Needless to say that the CGI.pm is not at all ( because it tries too much to be ) a 'killer app'. I myself is about to stop using CGI::Fast in FCGI::Spawn in favor of regular FCGI.pm and the CGI.pm variant chosen by the user. Needless to say that this can make the CGI.pm patching for FCGI::Spawn unecessary. - FCGI::ProcManager is a piece of cake in any way, but there are 'more than one way to do it' (c) and it should be mentioned on a docs as a dependency since there are modules on CPAN too for the same purpose but promiseful of features like OO/etc. The special thank for getting rid of exit()! I'd like to propose the Git to have the Perl interface for common functions that can make it easy to create trhe bunch of tools like those made with (likely XS'ed) SVN:* namespace, e. g. git-svn. It makes me wonder why gitweb is packaged with Git but no Perl API seen: looks like its storage is simple enough to realize all of that in PP. Don't just disappoint me saying that git is used to be exec()'uted on some of the gitweb calls. ;) 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-09 16:47 ` Peter Vereshagin @ 2010-05-09 18:18 ` Jakub Narebski 2010-05-10 7:13 ` Peter Vereshagin 0 siblings, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-09 18:18 UTC (permalink / raw) To: Peter Vereshagin, Petr Baudis, Petr Baudis Cc: Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Sun, 9 May 2010, Peter Vereshagin wrote: > I'm face to face with Jakub who sold the world? > 2010/05/09 14:39:23 +0200 Jakub Narebski <jnareb@gmail.com> => To Eric Wong : > > JN> Yes, it can. CGI::Compile is used for example by CGI::Emulate::PSGI, > JN> and you can run PSGI app on standalone Perl web server (pure Perl > JN> HTTP::Server::PSGI, or HTTP::Server::Simple::PSGI which in turn uses > JN> HTTP::Server::Simple, or Starman, or Twiggy, or Perlbal). CGI::Compile > JN> just compiles given CGI script into a subroutine, which can be called > JN> many times in a persistent web environment like FastCGI. > > Thanks a lot about that! > > I took a quick look at the patches and see this: > - FastCGI people are not always happy with CGI.pm and thus with CGI::Fast that > derives from it. They prefer CGI::Simple, e. g. for the Catalyst on fastcgi > and other CGI.pm replacements. CGI::Simple is not in core. For Catalyst folks it is not a problem, because Catalyst (one of Perl MVC web frameworks) is not in core either. > Despite the CGI::Fast is somehow the part of the perl core distribution > the FCGI.pm and CGI.pm which are the required dependencies are not. Actually both CGI and CGI::Fast are in Perl core distribution since perl 5.004 (Perl 5.4.0). I assume that CGI::Fast simply degrades to CGI if FCGI module is not present. FCGI is the single non-core dependency of CGI, see http://deps.cpantesters.org/?module=CGI;perl=latest so when I upgraded CGI (locally, using cpan client and local::lib), I also installed FCGI. > Needless to say that the CGI.pm is not at all (because > it tries too much to be) a 'killer app'. I myself is about to stop using > CGI::Fast in FCGI::Spawn in favor of regular FCGI.pm and the CGI.pm variant > chosen by the user. Needless to say that this can make the CGI.pm patching for > FCGI::Spawn unecessary. That's nice. What are required changes to gitweb to use FCGI::Spawn to run gitweb as a FastCGI script? Alternatively, how the wrapper script for gitweb (gitweb.fcgi) to be run as FastCGI should look like to use FCGI::Spawn? > - FCGI::ProcManager is a piece of cake in any way, but there are 'more than one > way to do it' (c) and it should be mentioned on a docs as a dependency since > there are modules on CPAN too for the same purpose but promiseful of features > like OO/etc. As PATCH 2/2 was straighforward port of Sam Vilain patch, I don't even know what exactly the part containing FCGI::ProcManager does. Note however that FCGI::ProcManager is require'd on demand; you have to run gitweb with '--nproc=<n>'. Also if FCGI::ProcManager is not found, then the '--nproc=<n>' command line option is a no-op (does nothing). > > The special thank for getting rid of exit()! You are welcome. > > I'd like to propose the Git to have the Perl interface for common functions > that can make it easy to create the bunch of tools like those made with > (likely XS'ed) SVN::* namespace, e. g. git-svn. >From what I remember from watching questions and discussion here on git mailing list, the SVN::* Subversion bindings (from subversion-perl package) are serious PITA. Git.pm started (by Petr Baudis) with XS parts, but because of lack of Perl hacker their compilation was unportable (IIRC it required support for -fPIC), and was therefore abandoned. The difficulty of creating Git::XS is exacerbated by the fact that there is no libgit to make Perl bindings againts, although hopefully GSoC 2010 project "Completing libgit2" would help. Git.pm currently wraps git commands in a *portable* way; that was the reason behind creating it, to e.g. not have to write the same workarounds for ActiveState Perl. The other reason was to have safe way of invoking git commands. There are some utility functions there, but not much. The "Gitweb caching" GSoC 2008 project[1] by Lea Wiemann included improvements to git Perl interface in the form of Git::Repo, Git::RepoRoot, Git::Object, Git::Commit and Git::Tag. They are available in Lea's repository[2], but were not merged into git core. [1]: http://git.wiki.kernel.org/index.php/SoC2008Projects#Gitweb_caching [2]: http://repo.or.cz/w/git/gitweb-caching.git > > It makes me wonder why gitweb is packaged with Git but no Perl API seen: If you ask why gitweb does not use Git.pm, the reason is twofold. First, gitweb predates Git.pm. It also used safe form of invokeing git commands (list form of magic "-|" pipeline open) at least since b918298 (gitweb: Use list for of open for running git commands..., 2006-07-30), and was intended to run on POSIX (like e.g. "/" as path separator) and therefore ActiveState Perl workarounds were not needed. Second, in a few places gitweb needs either to pipe output of git command to other command (to compressor in snapshot, and newly introduced to highlighter when syntax highlighting is turned on), or silence errors redirecting STDERR to /dev/null (in 'object' action to check if object exists). This is not yet supported in Git.pm. Note that using IPC::Run for piping git command output to other command would be counter to gitweb's goal of minimal non-core dependencies. > looks like its storage is simple enough > to realize all of that in PP. Don't just disappoint me saying that git is used > to be exec()'uted on some of the gitweb calls. ;) Gitalist[3], which started as port of gitweb to Catalyst web framework, uses Git::PurePerl, a pure Perl interface to Git repositories (which was mostly based on Grit, a git Ruby library, which includes a partial native Ruby implementation). Or used to use; there was some discussion whether to use Git::PurePerl or wrap git commands. Note that Gitalist was based on IIRC 2008 version of gitweb, so while it includes features that gitweb doesn't have, the opposite also might be true (features in gitweb that Gitalist doesn't have). Also performance matters for gitweb. [3]: https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools#Gitalist -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-09 18:18 ` Jakub Narebski @ 2010-05-10 7:13 ` Peter Vereshagin 2010-05-10 15:29 ` Jakub Narebski 0 siblings, 1 reply; 30+ messages in thread From: Peter Vereshagin @ 2010-05-10 7:13 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen Hey Jakub don't wanna cause you pain but the big boys feel no sorrow! 2010/05/09 20:18:52 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : Great! I was just about to ask on caching, etc. What a complex history on all of that, will be on those tracks after some of my whiles. ;-) JN> What are required changes to gitweb to use FCGI::Spawn to run gitweb as JN> a FastCGI script? Alternatively, how the wrapper script for gitweb JN> (gitweb.fcgi) to be run as FastCGI should look like to use FCGI::Spawn? By far it's only an exit() of the what I use (1.6.0.6): --- /usr/local/share/examples/git/gitweb/gitweb.cgi 2010-02-25 13:49:30.068287112 +0300 +++ www/gitweb.cgi 2010-03-13 14:28:45.326244103 +0300 @@ -933,7 +933,7 @@ die_error(400, "Project needed"); } $actions{$action}->(); -exit; +# exit; ## ====================================================================== ## action links @@ -3371,7 +3371,7 @@ </div> EOF git_footer_html(); - exit; +# exit; } ## ---------------------------------------------------------------------- but it's probably even not necessary with -e parameter: http://search.cpan.org/~veresc/FCGI-Spawn-0.16.1/fcgi_spawn#Command_line_options which is definitely required for bugzilla, the worst boy in that sandbox. The parameter does just this: === my $cref = sub { if( 'FCGI::ProcManager' eq scalar caller ){ CORE::exit @_; } else { no warnings; last CALLED_OUT; } }; *CORE::GLOBAL::exit = $cref; *CORE::GLOBAL::exit; === so this requires configuration ( $PREFIX/etc/fcgi_spawn/preload_nonprepared_01.pl, in my case ) for fcgi_spawn daemon like this: === $spawn->{ callout } = sub{ do shift; CALLED_OUT: }; === all of that is not needed without exit() in gitweb, now. I didn't mean FCGI::PM is a problem by itself. The standalone gitweb daemon is great thing for those who need such a choice. FCGI::Spawn is just for some different task: to put several ( wish to say: any CGI app ) applications inside the same fork()ed processes. It should be just obviously documented for a user as a dependency for implementation of a gitweb fastcgi daemon. Although I'm not sure if the FCGI::PM package should be a dependency for git package for any OS: for those modules use()d in eval() my guess is: particular user's choice to be offered. So FCGI::PM usage I think makes a flavor taste for any daemon and thus should be explicit. YMMV for those uninvolved in daemonizing, of course. ;-) Is it probable that gitweb doesn't take any POSTs requests? The main trick around FCGI::Spawn is the need to patch the CGI.pm but if that is the case... I'd try to redefine the STDIN to /dev/null or zero so FCGI.Spawn.CGI.pm.patch should be unnecessary for one who only wants to run the gitweb in FCGI::Spawn. If switch to FCGI.pm will be way complicated to me. 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-10 7:13 ` Peter Vereshagin @ 2010-05-10 15:29 ` Jakub Narebski 2010-05-11 6:24 ` Peter Vereshagin 0 siblings, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-10 15:29 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Mon, 10 May 2010, Peter Vereshagin wrote: > 2010/05/09 20:18:52 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > > Great! I was just about to ask on caching, etc. What a complex history on all > of that, will be on those tracks after some of my whiles. ;-) You can find current state of my take on gitweb output caching (based on / inspired by work by John 'Warthog9' Hawley) in my repository on repo.or.cz, in the 'gitweb/cache-kernel-pu' branch: http://repo.or.cz/w/git/jnareb-git.git gitweb/cache-kernel-pu You can find progress reports (and what current show-stoppers are) in git mailing list archives. Note that http://repo.or.cz does its own gitweb caching, IIRC by caching Perl data, and only for 'projects_list' page (the most costly one). There was also "Gitweb caching" projects in GSoC 2008 by Lea Wiemann, which IIUC cached output of git commands. This project was, I think, completed but didn't get merged into git. > JN> What are required changes to gitweb to use FCGI::Spawn to run gitweb as > JN> a FastCGI script? Alternatively, how the wrapper script for gitweb > JN> (gitweb.fcgi) to be run as FastCGI should look like to use FCGI::Spawn? > > By far it's only an exit() of the what I use (1.6.0.6): Why so old git? Current version is git version 1.7.1 > > --- /usr/local/share/examples/git/gitweb/gitweb.cgi 2010-02-25 13:49:30.068287112 +0300 > +++ www/gitweb.cgi 2010-03-13 14:28:45.326244103 +0300 Hrmph. Why not use "git diff --no-index <file1> <file2>" here? The Perl-aware equivalent of '-p' option of GNU diff, i.e. showing in which function we are in hunk headers, would help here. > @@ -933,7 +933,7 @@ > die_error(400, "Project needed"); > } > $actions{$action}->(); > -exit; > +# exit; This 'exit' was here just in case there were some forgotten code below this line outside subroutines (that should not be run). It can be safely removed. > > ## ====================================================================== > ## action links > @@ -3371,7 +3371,7 @@ sub die_error { I have added my guess of in which subroutine this code is above. > </div> > EOF > git_footer_html(); > - exit; > +# exit; > } Err... and gitweb works correctly with this change? This 'exit' was required for die_error to function like 'die' in that it finishes serving request, and should not continue subroutine it was called from. I have changed this 'exit' to non-local goto to toplevel. It could be done instead by redefining 'exit' subroutine, like shown below, but I feel that would be hacky if you can change gitweb code (it is not black box you should not touch). > > ## ---------------------------------------------------------------------- > > but it's probably even not necessary with -e parameter: > http://search.cpan.org/~veresc/FCGI-Spawn-0.16.1/fcgi_spawn#Command_line_options > which is definitely required for bugzilla, the worst boy in that sandbox. The > parameter does just this: > === > my $cref = sub { > if ('FCGI::ProcManager' eq scalar caller) { > CORE::exit @_; > } else { > no warnings; > last CALLED_OUT; > } > }; > *CORE::GLOBAL::exit = $cref; > *CORE::GLOBAL::exit; > === This is quite nice idea to replace 'exit' by subroutine that does non-local jump to outside of application, at the end of request loop. Such "monkey patching" is the only solution if you can't or shouldn't modify application code (like FCGI::Spawn being generic solution). > so this requires configuration > ( $PREFIX/etc/fcgi_spawn/preload_nonprepared_01.pl, in my case ) for fcgi_spawn > daemon like this: > === > $spawn->{ callout } = sub{ do shift; > CALLED_OUT: > }; > === Here $spawn->{'callout'} = sub { my $cgi_app = shift; do $cgi_app; # this is needed for sane error handling die "Couldn't parse $cgi_app: $@" if $@; CALLED_OUT: }; could be simply replaced by use CGI::Compile; # ... $spawn->{'callout'} = \&{CGI::Compile->compile} or something like that. See CGI::Compile manpage and CGI::Compile source: http://cpansearch.perl.org/src/MIYAGAWA/CGI-Compile-0.11/lib/CGI/Compile.pm > > All of that is not needed without exit() in gitweb, now. BTW I wonder what are the consequences for performance on replacing 'exit' by non-local jump. It can degrade performance a bit for gitweb run as pure CGI (mod_cgi / mod_cgid), but should improve performance for mod_perl, at least if there are more connections... unless ModPerl::Registry does similar trick with exit(). > > I didn't mean FCGI::PM is a problem by itself. The standalone gitweb daemon is > great thing for those who need such a choice. FCGI::Spawn is just for some > different task: to put several ( wish to say: any CGI app ) applications inside > the same fork()ed processes. It should be just obviously documented for a user > as a dependency for implementation of a gitweb fastcgi daemon. Although I'm not > sure if the FCGI::PM package should be a dependency for git package for any OS: > for those modules use()d in eval() my guess is: particular user's choice to be > offered. > > So FCGI::PM usage I think makes a flavor taste for any daemon and thus should > be explicit. YMMV for those uninvolved in daemonizing, of course. ;-) Hmmm... is FCGI::Spawn really needed, or can it be replaced by simple PSGI wrapper using either Plack::App::CGIBin, use Plack::App::CGIBin; use Plack::Builder; my $app = Plack::App::CGIBin->new(root => "/path/to/cgi-bin")->to_app; builder { mount "/cgi-bin" => $app; }; or Plack::App::WrapCGI plus Plack::App::URLMap, the last indirectly via Plack::Builder DSL: use Plack::Builder; use Plack::App::WrapCGI; builder { mount "/foo" => Plack::App::WrapCGI->new(script => "foo.cgi")->to_app; mount "/bar" => Plack::App::WrapCGI->new(script => "bar.cgi")->to_app; }; > > Is it probable that gitweb doesn't take any POSTs requests? The main trick > around FCGI::Spawn is the need to patch the CGI.pm but if that is the case... > I'd try to redefine the STDIN to /dev/null or zero so FCGI.Spawn.CGI.pm.patch > should be unnecessary for one who only wants to run the gitweb in FCGI::Spawn. > If switch to FCGI.pm will be way complicated to me. Errr... excuse me, what you wanted to say in the paragraph above? Gitweb doesn't use no POST requests: it is read-only web repository browser... well, except for the 'show_ctags' action. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-10 15:29 ` Jakub Narebski @ 2010-05-11 6:24 ` Peter Vereshagin 2010-05-11 8:35 ` Petr Baudis 2010-05-11 10:58 ` Jakub Narebski 0 siblings, 2 replies; 30+ messages in thread From: Peter Vereshagin @ 2010-05-11 6:24 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen I know St. Peter won't call your name, Jakub! 2010/05/10 17:29:03 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> On Mon, 10 May 2010, Peter Vereshagin wrote: JN> > 2010/05/09 20:18:52 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> > JN> > Great! I was just about to ask on caching, etc. What a complex history on all JN> > of that, will be on those tracks after some of my whiles. ;-) JN> JN> You can find current state of my take on gitweb output caching (based JN> on / inspired by work by John 'Warthog9' Hawley) in my repository on JN> repo.or.cz, in the 'gitweb/cache-kernel-pu' branch: JN> JN> http://repo.or.cz/w/git/jnareb-git.git gitweb/cache-kernel-pu JN> JN> You can find progress reports (and what current show-stoppers are) in JN> git mailing list archives. I will. JN> Note that http://repo.or.cz does its own gitweb caching, IIRC by JN> caching Perl data, and only for 'projects_list' page (the most costly JN> one). JN> JN> There was also "Gitweb caching" projects in GSoC 2008 by Lea Wiemann, JN> which IIUC cached output of git commands. This project was, I think, JN> completed but didn't get merged into git. JN> JN> > JN> What are required changes to gitweb to use FCGI::Spawn to run gitweb as JN> > JN> a FastCGI script? Alternatively, how the wrapper script for gitweb JN> > JN> (gitweb.fcgi) to be run as FastCGI should look like to use FCGI::Spawn? JN> > JN> > By far it's only an exit() of the what I use (1.6.0.6): JN> JN> Why so old git? Current version is git version 1.7.1 Update or die sounds too consumerical to me ;-) JN> JN> > JN> > --- /usr/local/share/examples/git/gitweb/gitweb.cgi 2010-02-25 13:49:30.068287112 +0300 JN> > +++ www/gitweb.cgi 2010-03-13 14:28:45.326244103 +0300 JN> JN> Hrmph. Why not use "git diff --no-index <file1> <file2>" here? JN> JN> The Perl-aware equivalent of '-p' option of GNU diff, i.e. showing in JN> which function we are in hunk headers, would help here. It's obvious that I just made a more simple thing about this patch: grep exit gitweb.cgi. Meat and potatoes ;-) JN> JN> > @@ -933,7 +933,7 @@ JN> > die_error(400, "Project needed"); JN> > } JN> > $actions{$action}->(); JN> > -exit; JN> > +# exit; JN> JN> This 'exit' was here just in case there were some forgotten code below JN> this line outside subroutines (that should not be run). It can be JN> safely removed. JN> JN> > JN> > ## ====================================================================== JN> > ## action links JN> > @@ -3371,7 +3371,7 @@ sub die_error { JN> JN> I have added my guess of in which subroutine this code is above. Right. JN> JN> > </div> JN> > EOF JN> > git_footer_html(); JN> > - exit; JN> > +# exit; JN> > } JN> JN> Err... and gitweb works correctly with this change? This 'exit' was JN> required for die_error to function like 'die' in that it finishes JN> serving request, and should not continue subroutine it was called JN> from. Does at least on 'non-existent diff' page: http://gitweb.vereshagin.org/fcgiproxy/commitdiff/abcd JN> I have changed this 'exit' to non-local goto to toplevel. It could be JN> done instead by redefining 'exit' subroutine, like shown below, but I JN> feel that would be hacky if you can change gitweb code (it is not JN> black box you should not touch). Right, one shouldn't ever redefine perl built-in functions. I did only because of no other way to 'get things working' JN> JN> > JN> > ## ---------------------------------------------------------------------- JN> > JN> > but it's probably even not necessary with -e parameter: JN> > http://search.cpan.org/~veresc/FCGI-Spawn-0.16.1/fcgi_spawn#Command_line_options JN> > which is definitely required for bugzilla, the worst boy in that sandbox. The JN> > parameter does just this: JN> > === JN> > my $cref = sub { JN> > if ('FCGI::ProcManager' eq scalar caller) { JN> > CORE::exit @_; JN> > } else { JN> > no warnings; JN> > last CALLED_OUT; JN> > } JN> > }; JN> > *CORE::GLOBAL::exit = $cref; JN> > *CORE::GLOBAL::exit; JN> > === JN> JN> This is quite nice idea to replace 'exit' by subroutine that does JN> non-local jump to outside of application, at the end of request loop. JN> Such "monkey patching" is the only solution if you can't or shouldn't JN> modify application code (like FCGI::Spawn being generic solution). Yes, this is quick-n-dirty to apply for those monkeys who are just busy to care about re-writing CGI apps. JN> > so this requires configuration JN> > ( $PREFIX/etc/fcgi_spawn/preload_nonprepared_01.pl, in my case ) for fcgi_spawn JN> > daemon like this: JN> > === JN> > $spawn->{ callout } = sub{ do shift; JN> > CALLED_OUT: JN> > }; JN> > === JN> JN> Here JN> JN> $spawn->{'callout'} = sub { JN> my $cgi_app = shift; JN> do $cgi_app; JN> JN> # this is needed for sane error handling JN> die "Couldn't parse $cgi_app: $@" if $@; JN> JN> CALLED_OUT: JN> }; in a forked application, die() is a PITA on any reasonable load. It makes the CoW-shared memory to be copied into separate area and being marked as unusable before the process is dead. This is the only case I saw load averages on the servers valued as crazy ~700. So just exit there, not die. By far, die can not be redefined the same way as I propose for exit in FCGI::Spawn. JN> JN> could be simply replaced by JN> JN> use CGI::Compile; JN> JN> # ... JN> JN> $spawn->{'callout'} = \&{CGI::Compile->compile} JN> JN> or something like that. See CGI::Compile manpage and CGI::Compile source: JN> http://cpansearch.perl.org/src/MIYAGAWA/CGI-Compile-0.11/lib/CGI/Compile.pm JN> JN> > JN> > All of that is not needed without exit() in gitweb, now. JN> JN> BTW I wonder what are the consequences for performance on replacing JN> 'exit' by non-local jump. It can degrade performance a bit for gitweb JN> run as pure CGI (mod_cgi / mod_cgid), but should improve performance JN> for mod_perl, at least if there are more connections... unless JN> ModPerl::Registry does similar trick with exit(). I knew out about such a trick somewhere in modperlbook. For mod_perl, this should be done in startup.pl JN> > I didn't mean FCGI::PM is a problem by itself. The standalone gitweb daemon is JN> > great thing for those who need such a choice. FCGI::Spawn is just for some JN> > different task: to put several ( wish to say: any CGI app ) applications inside JN> > the same fork()ed processes. It should be just obviously documented for a user JN> > as a dependency for implementation of a gitweb fastcgi daemon. Although I'm not JN> > sure if the FCGI::PM package should be a dependency for git package for any OS: JN> > for those modules use()d in eval() my guess is: particular user's choice to be JN> > offered. JN> > JN> > So FCGI::PM usage I think makes a flavor taste for any daemon and thus should JN> > be explicit. YMMV for those uninvolved in daemonizing, of course. ;-) JN> JN> Hmmm... is FCGI::Spawn really needed, or can it be replaced by simple JN> PSGI wrapper using either Plack::App::CGIBin, JN> JN> use Plack::App::CGIBin; JN> use Plack::Builder; JN> JN> my $app = Plack::App::CGIBin->new(root => "/path/to/cgi-bin")->to_app; JN> builder { JN> mount "/cgi-bin" => $app; JN> }; You use the predefined paths here on initialization. FCGI::Spawn knows about the CGI application's path at the right moment it takes the request. JN> or Plack::App::WrapCGI plus Plack::App::URLMap, the last indirectly JN> via Plack::Builder DSL: JN> JN> use Plack::Builder; JN> use Plack::App::WrapCGI; JN> JN> builder { JN> mount "/foo" => JN> Plack::App::WrapCGI->new(script => "foo.cgi")->to_app; JN> mount "/bar" => JN> Plack::App::WrapCGI->new(script => "bar.cgi")->to_app; JN> }; Sounds no more simple than simplicity of php deployment. That is whom the FCGI::Spawn combats for. Probably, 'the directories and scripts cache' should help to defer such an initialization? like it is done about the DBI handles in Apache::DBI. You can perform that init on a first request per fork, and keep it built for all of the process lifetime for the requests coming next. JN> > Is it probable that gitweb doesn't take any POSTs requests? The main trick JN> > around FCGI::Spawn is the need to patch the CGI.pm but if that is the case... JN> > I'd try to redefine the STDIN to /dev/null or zero so FCGI.Spawn.CGI.pm.patch JN> > should be unnecessary for one who only wants to run the gitweb in FCGI::Spawn. JN> > If switch to FCGI.pm will be way complicated to me. JN> JN> Errr... excuse me, what you wanted to say in the paragraph above? CGI::Fast use CGI.pm for POST input. With FCGI.pm I'll use CGI.pm for POST input only on an application's demand in FCGI::Spawn. In case of gitweb.cgi POST is not used. This makes the CGI.pm patch supplied with FCGI::Spawn not needed. But web user may send the POST request and FCGI::Spawn should feed a dummy input for CGI.pm in the CGI script waiting for input from STDIN when request method is POST. Not sure if this feature is needed at all for FCGI::Spawn though. JN> Gitweb doesn't use no POST requests: it is read-only web repository JN> browser... well, except for the 'show_ctags' action. Tag cloud? Is there an example of usable tag cloud on any public gitweb out there? 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-11 6:24 ` Peter Vereshagin @ 2010-05-11 8:35 ` Petr Baudis 2010-05-11 10:58 ` Jakub Narebski 1 sibling, 0 replies; 30+ messages in thread From: Petr Baudis @ 2010-05-11 8:35 UTC (permalink / raw) To: Peter Vereshagin Cc: Jakub Narebski, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Tue, May 11, 2010 at 10:24:15AM +0400, Peter Vereshagin wrote: > JN> Gitweb doesn't use no POST requests: it is read-only web repository > JN> browser... well, except for the 'show_ctags' action. > > Tag cloud? Is there an example of usable tag cloud on any public gitweb out > there? See http://repo.or.cz/ for an example. I don't think it's essential to support ctags under all configurations, if doing so is too troublesome. But I expect our current GSoC to want to add some more POST forms too. -- Petr "Pasky" Baudis When I feel like exercising, I just lie down until the feeling goes away. -- xed_over ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-11 6:24 ` Peter Vereshagin 2010-05-11 8:35 ` Petr Baudis @ 2010-05-11 10:58 ` Jakub Narebski 2010-05-11 12:09 ` Peter Vereshagin 1 sibling, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-11 10:58 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Tue, 11 May 2010, Peter Vereshagin <peter@vereshagin.org> wrote: > 2010/05/10 17:29:03 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > > On Mon, 10 May 2010, Peter Vereshagin wrote: > > > ## ====================================================================== > > > ## action links > > > @@ -3371,7 +3371,7 @@ sub die_error { > > > > I have added my guess of in which subroutine this code is above. [...] > > > git_footer_html(); > > > - exit; > > > +# exit; > > > } > > > > Err... and gitweb works correctly with this change? This 'exit' was > > required for die_error to function like 'die' in that it finishes > > serving request, and should not continue subroutine it was called > > from. > > Does at least on 'non-existent diff' page: > > http://gitweb.vereshagin.org/fcgiproxy/commitdiff/abcd Hmmm... strange that it works. > > I have changed this 'exit' to non-local goto to toplevel. It could be > > done instead by redefining 'exit' subroutine, like shown below, but I > > feel that would be hacky if you can change gitweb code (it is not > > black box you should not touch). > > Right, one shouldn't ever redefine perl built-in functions. I did only because > of no other way to 'get things working' Why not? For example CGI::Carp redefines 'die' to log errors. BEGIN { require Carp; *CORE::GLOBAL::die = \&CGI::Carp::die; } Sub::Uplevel and Test::Exception redefines 'caller' (perhaps locally). CGI::Compile redefines 'exit': our $USE_REAL_EXIT; BEGIN { $USE_REAL_EXIT = 1; *CORE::GLOBAL::exit = sub (;$) { my $exit_code = shift; CORE::exit(defined $exit_code ? $exit_code : 0) if $USE_REAL_EXIT; die [ "EXIT\n", $exit_code || 0 ] }; } > > This is quite nice idea to replace 'exit' by subroutine that does > > non-local jump to outside of application, at the end of request loop. > > Such "monkey patching" is the only solution if you can't or shouldn't > > modify application code (like FCGI::Spawn being generic solution). > > Yes, this is quick-n-dirty to apply for those monkeys who are just busy to care > about re-writing CGI apps. Errr... "monkey patching" is the name of technique of extending and modifying runtime code in dynamic languages, see http://en.wikipedia.org/wiki/Monkey_patch Although I am not entirely sure if I correctly applied this name to described (used) techique. > > Here > > > > $spawn->{'callout'} = sub { > > my $cgi_app = shift; > > do $cgi_app; > > > > # this is needed for sane error handling > > die "Couldn't parse $cgi_app: $@" if $@; > > > > CALLED_OUT: > > }; > > In a forked application, die() is a PITA on any reasonable load. It makes the > CoW-shared memory to be copied into separate area and being marked as unusable > before the process is dead. This is the only case I saw load averages on the > servers valued as crazy ~700. > > So just exit there, not die. Well, it might be 'exit' not 'die', but you really, really need to check if there were problems parsing file. Otheriwse you can get error messages somewhere further on that doesn't absolutely make sense. I know this from painful experience of trying to find bug in a test... when the error was in parsing file in 'do $file;'. > By far, die can not be redefined the same way as I propose for exit in > FCGI::Spawn. It can't? CGI::Carp redefines 'die'. > > > [...] FCGI::Spawn is just for some different task: to put several > > > (wish to say: any CGI app) applications inside the same fork()ed > > > processes. [...] > > > > Hmmm... is FCGI::Spawn really needed, or can it be replaced by simple > > PSGI wrapper using either Plack::App::CGIBin, > > > > use Plack::App::CGIBin; > > use Plack::Builder; > > > > my $app = Plack::App::CGIBin->new(root => "/path/to/cgi-bin")->to_app; > > builder { > > mount "/cgi-bin" => $app; > > }; > > You use the predefined paths here on initialization. FCGI::Spawn knows about > the CGI application's path at the right moment it takes the request. No, you need to provide only *root*, i.e. where Perl CGI applications are, so that e.g. accessing 'http://0:5000/cgi-bin/foo/bar.cgi' would run PSGI-ized (via CGI::Emulate::PSGI) '/path/to/cgi-bin/foo/bar.cgi' application. You don't need to mount it at "/cgi-bin", you can just builder { $app; } or even without it ($app should be the last expression). Or did you mean here something like mod_rewrite, or Plack::Middleware::Rewrite? [...] > > Gitweb doesn't use no POST requests: it is read-only web repository > > browser... well, except for the 'show_ctags' action. > > Tag cloud? Is there an example of usable tag cloud on any public gitweb out > there? Tag cloud are optional feature in stock gitweb, named 'ctag' in %feature hash. It is disabled by default. If I understand correctly POST is used here to populate which tags one wants to use... but perhaps GET request would be enough here (at the cost of less readable URL). See http://repo.or.cz for example usage of this feature. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-11 10:58 ` Jakub Narebski @ 2010-05-11 12:09 ` Peter Vereshagin 2010-05-11 13:51 ` Jakub Narebski 0 siblings, 1 reply; 30+ messages in thread From: Peter Vereshagin @ 2010-05-11 12:09 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen I know St. Peter won't call your name, Jakub! 2010/05/11 12:58:50 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> > > I have added my guess of in which subroutine this code is above. JN> JN> [...] JN> > > > git_footer_html(); JN> > > > - exit; JN> > > > +# exit; JN> > > > } JN> > > JN> > > Err... and gitweb works correctly with this change? This 'exit' was JN> > > required for die_error to function like 'die' in that it finishes JN> > > serving request, and should not continue subroutine it was called JN> > > from. JN> > JN> > Does at least on 'non-existent diff' page: JN> > JN> > http://gitweb.vereshagin.org/fcgiproxy/commitdiff/abcd JN> JN> Hmmm... strange that it works. just break it (c) ;-) There are many other cases for this function to use, I just don't care. It's all yet readonly after all ;-) JN> JN> > > I have changed this 'exit' to non-local goto to toplevel. It could be JN> > > done instead by redefining 'exit' subroutine, like shown below, but I JN> > > feel that would be hacky if you can change gitweb code (it is not JN> > > black box you should not touch). JN> > JN> > Right, one shouldn't ever redefine perl built-in functions. I did only because JN> > of no other way to 'get things working' JN> JN> Why not? For example CGI::Carp redefines 'die' to log errors. Ouch, sorry, I meant last() or something like that. I just believe any non-system application development for end-user being a non-developer doesn't need to redefine perl built-in functions. Just a sane bone tone for common functioning in a sandbox. For example, I remember the Linux kernel ( or Glibc? ) was criticised much of being possible to override the str*cmp() inside. Because most of the existing commerceware were protected from copying by password, e. g. serial number, etc. sometimes by authors. So criticants supposed it's impossible to 'protect' their software this way. And thus Linux was 'bad'. ;-) Nowadays we have all of those possible actions ( trying to investigate and substitute the password hash in the str*cmp() called by user-space software ) classified by Crime Code and aoplied apparently widely. Kind of confessional debates between those who suppose the potentially dangerous thing should be restricted because it can be used as harm and those who use it regularly without any idea to use it in any different dangerous way. Last time I saw them on bbc about islam - 'hey, your islam spells to cut thievs' arms' - 'hey islam of no danger, it's just very powerful and you shouldn't use it in dangerous way'. This will last for ages, anyway. You may find them even in "Just For Fun", where the 'pubs' restricted topics' persist. Even there. So one who use CORE:: namespace in their sources should always know it can be grepped and considered as dangerous, especially if those are 3rd+ party sources, not approved by any reasonable authority, and there are lots of such a software off the shelves to choose. And most of them doesn't use to override perl built-in functions. ;-) JN> JN> BEGIN { JN> require Carp; JN> *CORE::GLOBAL::die = \&CGI::Carp::die; JN> } JN> JN> Sub::Uplevel and Test::Exception redefines 'caller' (perhaps locally). JN> CGI::Compile redefines 'exit': JN> JN> our $USE_REAL_EXIT; JN> BEGIN { JN> $USE_REAL_EXIT = 1; JN> *CORE::GLOBAL::exit = sub (;$) { JN> my $exit_code = shift; JN> JN> CORE::exit(defined $exit_code ? $exit_code : 0) if $USE_REAL_EXIT; JN> JN> die [ "EXIT\n", $exit_code || 0 ] JN> }; JN> } JN> JN> JN> > > This is quite nice idea to replace 'exit' by subroutine that does JN> > > non-local jump to outside of application, at the end of request loop. JN> > > Such "monkey patching" is the only solution if you can't or shouldn't JN> > > modify application code (like FCGI::Spawn being generic solution). JN> > JN> > Yes, this is quick-n-dirty to apply for those monkeys who are just busy to care JN> > about re-writing CGI apps. JN> JN> Errr... "monkey patching" is the name of technique of extending and JN> modifying runtime code in dynamic languages, see JN> JN> http://en.wikipedia.org/wiki/Monkey_patch What's about 'patching'? Mokeys I meant to be the OpenBSD ( or non-OpenBSD ) crowd ;-) http://article.gmane.org/gmane.linux.kernel/706950 JN> Although I am not entirely sure if I correctly applied this name to JN> described (used) techique. 'Replace methods/attributes/functions at runtime, e.g. to stub out a function during testing;' sounds like 'dynamic modules reloading' to me. Yes, that is about FCGI::Spawn, its 'stats' disablable feature 'Modify/extend behaviour of a third-party product without maintaining a private copy of the source code; ' sounds like 'run any 3rd-party CGI app in FCGI::Spawn' Yep, uncle Darwin cries out loud. ;-) JN> JN> > > Here JN> > > JN> > > $spawn->{'callout'} = sub { JN> > > my $cgi_app = shift; JN> > > do $cgi_app; JN> > > JN> > > # this is needed for sane error handling JN> > > die "Couldn't parse $cgi_app: $@" if $@; JN> > > JN> > > CALLED_OUT: JN> > > }; JN> > JN> > In a forked application, die() is a PITA on any reasonable load. It makes the JN> > CoW-shared memory to be copied into separate area and being marked as unusable JN> > before the process is dead. This is the only case I saw load averages on the JN> > servers valued as crazy ~700. JN> > JN> > So just exit there, not die. JN> JN> Well, it might be 'exit' not 'die', but you really, really need to check JN> if there were problems parsing file. Otheriwse you can get error JN> messages somewhere further on that doesn't absolutely make sense. php is very successful with this: it can put error messages to both STDOUT and STDERR or separate log. FCGI::Spawn is the something about that because it's the 'what the most people want' (c) Junio. ;-) Anyway, if the file was not parsed, it did not change anything in perl's compile-cache/variables and therefore it's quite safe for any othe application on the same sandbox. Why should the fork die if some file gets unparsed? JN> I know this from painful experience of trying to find bug in a JN> test... when the error was in parsing file in 'do $file;'. I handle them just fine like in any other CGI program using CGI::Carp:fatalsToBrowser. Are you about to 'make test' via the http? ;-) [...] JN> builder { JN> $app; JN> } that's the wow to try. I will after some of my whiles. JN> or even without it ($app should be the last expression). JN> Or did you mean here something like mod_rewrite, or JN> Plack::Middleware::Rewrite? No, nginx rewrites just fine, it's a matter of another application level I believe. The scoop is meat and potatoes: here is the cgi app, just do it over FastCGI. There are no such a thing as a mandatory mounts and paths tweaks in php's fastcgi. Hope PSGI has no them either. JN> [...] JN> > > Gitweb doesn't use no POST requests: it is read-only web repository JN> > > browser... well, except for the 'show_ctags' action. JN> > Tag cloud? Is there an example of usable tag cloud on any public gitweb out JN> > there? JN> JN> Tag cloud are optional feature in stock gitweb, named 'ctag' in %feature JN> hash. It is disabled by default. If I understand correctly POST is JN> used here to populate which tags one wants to use... but perhaps GET JN> request would be enough here (at the cost of less readable URL). JN> JN> See http://repo.or.cz for example usage of this feature. Ouch, it was the first for me to look for them. It's just not named like that there ( and looked like linkspam ;-. Anyway. user registration .cgi is a part of gitweb distribution? It contains POST form and it's not preferable stuff to omit for too many cases to consider such a gitweb-based web site to be 'mostly read-only' for a user. Or those .cgi's are nothing in common with gitweb? 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-11 12:09 ` Peter Vereshagin @ 2010-05-11 13:51 ` Jakub Narebski 2010-05-13 13:10 ` Peter Vereshagin 0 siblings, 1 reply; 30+ messages in thread From: Jakub Narebski @ 2010-05-11 13:51 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Tue, 11 May 2010, Peter Vereshagin wrote: > 2010/05/11 12:58:50 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > > > > I have changed this 'exit' to non-local goto to toplevel. It could be > > > > done instead by redefining 'exit' subroutine, like shown below, but I > > > > feel that would be hacky if you can change gitweb code (it is not > > > > black box you should not touch). > > > > > > Right, one shouldn't ever redefine perl built-in functions. I did only because > > > of no other way to 'get things working' > > > > Why not? For example CGI::Carp redefines 'die' to log errors. > > Ouch, sorry, I meant 'last' or something like that. "last" / "last LABEL" is a command, not a function, therefore you cannot redefine it. Well, perhaps you can with heavy hackery involving opcodes and the like, or something debugger-like, or/and something like B::* modules, taking over Perl parser. See e.g. Devel::Declare or Template::Declare Perl modules on CPAN. :-) > I just believe any non-system application development for end-user being a > non-developer doesn't need to redefine perl built-in functions. Just a sane > bone tone for common functioning in a sandbox. > > For example, I remember the Linux kernel ( or Glibc? ) was criticised much of > being possible to override the str*cmp() inside. Because most of the existing > commerceware were protected from copying by password, e. g. serial number, etc. > sometimes by authors. So criticants supposed it's impossible to 'protect' their > software this way. And thus Linux was 'bad'. ;-) What about libsafe (?) and similar security solutions, which replace str* functions from (g)libc with safer but slower counterparts? What about Dmalloc, Electric Fence and the like which replace malloc etc.? > So one who use CORE:: namespace in their sources should always know it can be > grepped and considered as dangerous, especially if those are 3rd+ party > sources, not approved by any reasonable authority, and there are lots of such a > software off the shelves to choose. And most of them doesn't use to override > perl built-in functions. ;-) It is true that messing with / overriding things from CORE:: (or UNIVERSAL:: for OOP) namespace is dangerous, and should be avoided if possible... but well, sometimes it is a best solution. > > I know this from painful experience of trying to find bug in a > > test... when the error was in parsing file in 'do $file;'. > > I handle them just fine like in any other CGI program using > CGI::Carp:fatalsToBrowser. Are you about to 'make test' via the http? ;-) I don't think you understand what I wanted to say there. If you don't check if there were parse errors from 'do $file;', you can get later some error message which is totally unrelated to the parsing error. If you don't know or forget that you should check $@ after 'do $file;', and are unlucky, you can chase elusive error from there to kingdom come... For example when debugging gitweb output caching code using automated tests, I got the following error: 'Undefined subroutine &GitwebCache::SimpleFileCache::compute called' The subroutine was defined, but there was a bug in parsing included file, so Perl didn't make it to definition of said compute() subroutine. > [...] > > > builder { > > $app; > > } > > that's the wow to try. I will after some of my whiles. Check out http://plackperl.org, especially presentations and Perl Advent Calendar which describes PSGI/Plack step by step (links at the bottom of the page). > > or even without it ($app should be the last expression). > > Or did you mean here something like mod_rewrite, or > > Plack::Middleware::Rewrite? > > No, nginx rewrites just fine, it's a matter of another application level I > believe. > > The scoop is meat and potatoes: here is the CGI app, just do it over FastCGI. > There are no such a thing as a mandatory mounts and paths tweaks in PHP's > FastCGI. Hope PSGI has no them either. PSGI is interface, Plack is reference implementation. You can run PSGI app on any supported web server; this includes running PSGI apps on FastCGI. > > > > Gitweb doesn't use no POST requests: it is read-only web repository > > > > browser... well, except for the 'show_ctags' action. > > > > > > Tag cloud? Is there an example of usable tag cloud on any public gitweb out > > > there? > > > > Tag cloud are optional feature in stock gitweb, named 'ctag' in %feature > > hash. It is disabled by default. If I understand correctly POST is > > used here to populate which tags one wants to use... but perhaps GET > > request would be enough here (at the cost of less readable URL). > > > > See http://repo.or.cz for example usage of this feature. > > Ouch, it was the first for me to look for them. It's just not named like that > there ( and looked like linkspam ;-. Anyway. user registration .cgi is a part > of gitweb distribution? It contains POST form and it's not preferable stuff to > omit for too many cases to consider such a gitweb-based web site to be 'mostly > read-only' for a user. > > Or those .cgi's are nothing in common with gitweb? The repository management part of http://repo.or.cz is not part of gitweb. It is a separate tool, named Girocco. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-11 13:51 ` Jakub Narebski @ 2010-05-13 13:10 ` Peter Vereshagin 2010-05-13 17:13 ` Ævar Arnfjörð Bjarmason 2010-05-14 10:53 ` Jakub Narebski 0 siblings, 2 replies; 30+ messages in thread From: Peter Vereshagin @ 2010-05-13 13:10 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen Hey Mr(s) Jakub show some good to me! 2010/05/11 15:51:15 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> On Tue, 11 May 2010, Peter Vereshagin wrote: JN> > 2010/05/11 12:58:50 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> JN> > > > > I have changed this 'exit' to non-local goto to toplevel. It could be JN> > > > > done instead by redefining 'exit' subroutine, like shown below, but I JN> > > > > feel that would be hacky if you can change gitweb code (it is not JN> > > > > black box you should not touch). JN> > > > JN> > > > Right, one shouldn't ever redefine perl built-in functions. I did only because JN> > > > of no other way to 'get things working' JN> > > JN> > > Why not? For example CGI::Carp redefines 'die' to log errors. JN> > JN> > Ouch, sorry, I meant 'last' or something like that. JN> JN> "last" / "last LABEL" is a command, not a function, therefore you cannot JN> redefine it. it's a flow control statement thus it is a built-in thing same way as any other functions are explained in a 'perldoc -f' Therefore it is treated by monkeys crowd as function. It's obvious for me to stay out here ( here != maillist ) yet in such an environment. Anyway, I compare last() here with exit() and die() which look to user just like the same kind of: the flow control statements. I guess any perl user who makes things like gitweb ( at least as a CGI-only app ) shouldn't care about such an internal difference of flow control statements those are hidden/incapsulated inside the implementation of those statements? Needless to mention that the 'last LABEL' ( goto, gosub, ... named them ) is a bad and a very deprecated style which is every schoolboy is aware about nowadays to keep from using in the application, not system, programming in imho every language. JN> Well, perhaps you can with heavy hackery involving opcodes and the like, JN> or something debugger-like, or/and something like B::* modules, taking JN> over Perl parser. See e.g. Devel::Declare or Template::Declare Perl JN> modules on CPAN. :-) JN> JN> > I just believe any non-system application development for end-user being a JN> > non-developer doesn't need to redefine perl built-in functions. Just a sane JN> > bone tone for common functioning in a sandbox. JN> > JN> > For example, I remember the Linux kernel ( or Glibc? ) was criticised much of JN> > being possible to override the str*cmp() inside. Because most of the existing JN> > commerceware were protected from copying by password, e. g. serial number, etc. JN> > sometimes by authors. So criticants supposed it's impossible to 'protect' their JN> > software this way. And thus Linux was 'bad'. ;-) JN> JN> What about libsafe (?) and similar security solutions, which replace JN> str* functions from (g)libc with safer but slower counterparts? What That was bad sound for commerceware vendors. Because such a in-core functions substititions can make the user safer but not the investments ( targeted on squeezing users' pursues). JN> about Dmalloc, Electric Fence and the like which replace malloc etc.? I think malloc implementation details cannot keep software's serial number from being verified. ;-) Back to perl built-ins: should it be normal if gitweb will be dependent on a usage of a particular malloc implementation? In my perl, I can have a choice of them. ;-) JN> > So one who use CORE:: namespace in their sources should always know it can be JN> > grepped and considered as dangerous, especially if those are 3rd+ party JN> > sources, not approved by any reasonable authority, and there are lots of such a JN> > software off the shelves to choose. And most of them doesn't use to override JN> > perl built-in functions. ;-) JN> JN> It is true that messing with / overriding things from CORE:: (or JN> UNIVERSAL:: for OOP) namespace is dangerous, and should be avoided if JN> possible... but well, sometimes it is a best solution. I think the state line between area to avoid one and the area where it can ever happen to be the best of the solutions is built socially: it is where system coder's work about daemon interfaces like the FCGI/PSGI/SCGI/etc. and the applied coder one: the application architecture, used libraries, application layers, etc. This is just where FCGI::Spawn is about to help. Because 'regular system admin' is typically unaware of details of usage of system daemon interfaces in perl. But (s)he could be the perl application coder since perl is that easy as a language tool. This is just who and when, and thus in what parts of the code used by the same Perl interpreter shouldn't play with built-ins like the CORE:: namespace and thanks perl it can be easily grepped. JN> > > I know this from painful experience of trying to find bug in a JN> > > test... when the error was in parsing file in 'do $file;'. JN> > JN> > I handle them just fine like in any other CGI program using JN> > CGI::Carp:fatalsToBrowser. Are you about to 'make test' via the http? ;-) JN> JN> I don't think you understand what I wanted to say there. JN> JN> If you don't check if there were parse errors from 'do $file;', you can JN> get later some error message which is totally unrelated to the parsing JN> error. If you don't know or forget that you should check $@ after JN> 'do $file;', and are unlucky, you can chase elusive error from there JN> to kingdom come... Got it, it's about the inclusion failure via the do() which is the development, not a production, situation. I think this should be an adjective noun to use the both strict and the warnings? And yes, since it's about development but not production use, die is just fine in the inclusion code like this: eval( 'use Module;' ); die $@ if $@; as always, require() can do the trick, not to mention usual use Module; This all will cause die() when it's necessary as only the application developer knows how strict is the dependence on the Module. In some cases, application can work without some Module but it's just better with it. JN> For example when debugging gitweb output caching code using automated JN> tests, I got the following error: JN> JN> 'Undefined subroutine &GitwebCache::SimpleFileCache::compute called' JN> JN> The subroutine was defined, but there was a bug in parsing included JN> file, so Perl didn't make it to definition of said compute() subroutine. What is the code? Where and what file was included via the do()? Interesting situation. If the sub was compiled, was it present then in the symbol table? I can't see the code of ... GitwebCache::SimpleFileCache package to contain the do()? JN> > [...] JN> > JN> > > builder { JN> > > $app; JN> > > } JN> > JN> > that's the wow to try. I will after some of my whiles. JN> JN> Check out http://plackperl.org, especially presentations and Perl Advent JN> Calendar which describes PSGI/Plack step by step (links at the bottom of JN> the page). JN> JN> > > or even without it ($app should be the last expression). JN> > > Or did you mean here something like mod_rewrite, or JN> > > Plack::Middleware::Rewrite? JN> > JN> > No, nginx rewrites just fine, it's a matter of another application level I JN> > believe. JN> > JN> > The scoop is meat and potatoes: here is the CGI app, just do it over FastCGI. JN> > There are no such a thing as a mandatory mounts and paths tweaks in PHP's JN> > FastCGI. Hope PSGI has no them either. JN> JN> PSGI is interface, Plack is reference implementation. You can run PSGI JN> app on any supported web server; this includes running PSGI apps on JN> FastCGI. Existing problem FCGI::Spawn for is not the PSGI applications to be run as a FastCGI, but the bunch of existing CGI.pm applications ( even gitorious ) need to be more effective with the widest-spread protocol FastCGI. Best without any patching of the application, deployed the same simple way as with apache's cgi implementation. Will check on this. 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-13 13:10 ` Peter Vereshagin @ 2010-05-13 17:13 ` Ævar Arnfjörð Bjarmason 2010-05-14 15:58 ` Peter Vereshagin 2010-05-14 10:53 ` Jakub Narebski 1 sibling, 1 reply; 30+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-05-13 17:13 UTC (permalink / raw) To: Peter Vereshagin Cc: Jakub Narebski, Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen 2010/5/13 Peter Vereshagin <peter@vereshagin.org>: > Hey Mr(s) Jakub show some good to me! > 2010/05/11 15:51:15 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > JN> On Tue, 11 May 2010, Peter Vereshagin wrote: > JN> > 2010/05/11 12:58:50 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > JN> > JN> > > > > I have changed this 'exit' to non-local goto to toplevel. It could be > JN> > > > > done instead by redefining 'exit' subroutine, like shown below, but I > JN> > > > > feel that would be hacky if you can change gitweb code (it is not > JN> > > > > black box you should not touch). > JN> > > > > JN> > > > Right, one shouldn't ever redefine perl built-in functions. I did only because > JN> > > > of no other way to 'get things working' > JN> > > > JN> > > Why not? For example CGI::Carp redefines 'die' to log errors. > JN> > > JN> > Ouch, sorry, I meant 'last' or something like that. > JN> > JN> "last" / "last LABEL" is a command, not a function, therefore you cannot > JN> redefine it. > > it's a flow control statement thus it is a built-in thing same way as any other > functions are explained in a 'perldoc -f' > Therefore it is treated by monkeys crowd as function. It's obvious for me to > stay out here ( here != maillist ) yet in such an environment. These things are called "operators" in Perl, some of them (like exit) you can redefine. Some (like last) you can't. At least not without some deep magic. > Anyway, I compare last() here with exit() and die() which look to user just > like the same kind of: the flow control statements. I guess any perl user who > makes things like gitweb ( at least as a CGI-only app ) shouldn't care about > such an internal difference of flow control statements those are > hidden/incapsulated inside the implementation of those statements? > Needless to mention that the 'last LABEL' ( goto, gosub, ... named them ) is a > bad and a very deprecated style which is every schoolboy is aware about > nowadays to keep from using in the application, not system, programming in imho > every language. `last LABEL' is not bad or deprecated. It's what you use to get out of nested for-loops in Perl: OUTER: for my $i (1 .. 10) { for my $j (1 .. 10) { last OUTER if $i == 5 and $j == 5; } } goto is also recommended in some cases in Perl. That's because it doesn't do the same thing as in C: # Don't create a stack frame sub foo { goto &bar } Anyway, arguing over which control flow operator is evil in an imperitive language is just splitting hairs. Certain uses of them are a bad idea, not the operators themselves. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-13 17:13 ` Ævar Arnfjörð Bjarmason @ 2010-05-14 15:58 ` Peter Vereshagin 0 siblings, 0 replies; 30+ messages in thread From: Peter Vereshagin @ 2010-05-14 15:58 UTC (permalink / raw) To: ??var Arnfj??r?? Bjarmason Cc: Peter Vereshagin, Jakub Narebski, Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen God love is hard to find. You got lucky ??var! 2010/05/13 17:13:12 +0000 ??var Arnfj??r?? Bjarmason <avarab@gmail.com> => To Peter Vereshagin : vArB> 2010/5/13 Peter Vereshagin <peter@vereshagin.org>: vArB> > Hey Mr(s) Jakub show some good to me! vArB> > 2010/05/11 15:51:15 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : vArB> > JN> On Tue, 11 May 2010, Peter Vereshagin wrote: vArB> > JN> > 2010/05/11 12:58:50 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : vArB> > JN> vArB> > JN> > > > > I have changed this 'exit' to non-local goto to toplevel. It could be vArB> > JN> > > > > done instead by redefining 'exit' subroutine, like shown below, but I vArB> > JN> > > > > feel that would be hacky if you can change gitweb code (it is not vArB> > JN> > > > > black box you should not touch). vArB> > JN> > > > vArB> > JN> > > > Right, one shouldn't ever redefine perl built-in functions. I did only because vArB> > JN> > > > of no other way to 'get things working' vArB> > JN> > > vArB> > JN> > > Why not? For example CGI::Carp redefines 'die' to log errors. vArB> > JN> > vArB> > JN> > Ouch, sorry, I meant 'last' or something like that. vArB> > JN> vArB> > JN> "last" / "last LABEL" is a command, not a function, therefore you cannot vArB> > JN> redefine it. vArB> > vArB> > it's a flow control statement thus it is a built-in thing same way as any other vArB> > functions are explained in a 'perldoc -f' vArB> > Therefore it is treated by monkeys crowd as function. It's obvious for me to vArB> > stay out here ( here != maillist ) yet in such an environment. vArB> vArB> These things are called "operators" in Perl, some of them (like exit) vArB> you can redefine. Some (like last) you can't. At least not without vArB> some deep magic. problem is not the naming, but that those are built-in and supposed to be used 'as is'. Operators or functions are whatever, but for perldoc they are the '-f' so think not a big problem I named them functions. vArB> > Anyway, I compare last() here with exit() and die() which look to user just vArB> > like the same kind of: the flow control statements. I guess any perl user who vArB> > makes things like gitweb ( at least as a CGI-only app ) shouldn't care about vArB> > such an internal difference of flow control statements those are vArB> > hidden/incapsulated inside the implementation of those statements? vArB> > Needless to mention that the 'last LABEL' ( goto, gosub, ... named them ) is a vArB> > bad and a very deprecated style which is every schoolboy is aware about vArB> > nowadays to keep from using in the application, not system, programming in imho vArB> > every language. vArB> vArB> `last LABEL' is not bad or deprecated. It's what you use to get out of vArB> nested for-loops in Perl: vArB> vArB> OUTER: for my $i (1 .. 10) { vArB> for my $j (1 .. 10) { vArB> last OUTER if $i == 5 and $j == 5; vArB> } vArB> } vArB> vArB> goto is also recommended in some cases in Perl. That's because it vArB> doesn't do the same thing as in C: vArB> vArB> # Don't create a stack frame vArB> sub foo { goto &bar } vArB> vArB> Anyway, arguing over which control flow operator is evil in an vArB> imperitive language is just splitting hairs. Certain uses of them are vArB> a bad idea, not the operators themselves. correct, just use-cases are a thing to change like cgi to fastcgi environment, this is where exit() is intended to be redefined for performance reasons. Thus original uses are not as certain as they were supposed to be at the moment of applications' coding: there were no idea why the END{}'s exit() is any better than the explicit in-code one. It's just can cause the lack of the performance and should be avoided in persistent perl processes to serve such a CGI-like applications. 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-13 13:10 ` Peter Vereshagin 2010-05-13 17:13 ` Ævar Arnfjörð Bjarmason @ 2010-05-14 10:53 ` Jakub Narebski 2010-05-14 15:36 ` Peter Vereshagin 2010-05-15 11:51 ` Petr Baudis 1 sibling, 2 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-14 10:53 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Thu, 13 May 2010, Peter Vereshagin wrote: > 2010/05/11 15:51:15 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : >> On Tue, 11 May 2010, Peter Vereshagin wrote: >>> 2010/05/11 12:58:50 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : >>>>>> I have changed this 'exit' to non-local goto to toplevel. It could be >>>>>> done instead by redefining 'exit' subroutine, like shown below, but I >>>>>> feel that would be hacky if you can change gitweb code (it is not >>>>>> black box you should not touch). >>>>> >>>>> Right, one shouldn't ever redefine perl built-in functions. I did only because >>>>> of no other way to 'get things working' >>>> >>>> Why not? For example CGI::Carp redefines 'die' to log errors. >>> >>> Ouch, sorry, I meant 'last' or something like that. >> >> "last" / "last LABEL" is a command, not a function, therefore you cannot >> redefine it. > > It's a flow control statement, thus it is a built-in thing; same way as any other > functions are explained in a 'perldoc -f'. `perldoc -f exit` says 'The exit() function ...', while `perldoc -f last` says 'The "last" command is like the "break" statement in C ...'. > Therefore it is treated by monkeys crowd as function. It's obvious for me to > stay out here (here != maillist) yet in such an environment. Sidenote: The 'Monkey patch' article on Wikipedia says that the technique of adding method dirctly to class instead of subclassing was originally called "guerilla patching", then it mutated into "gorilla patching", and finally into "monkey patching". > Anyway, I compare "last" here with exit() and die() which look to user just > like the same kind of: the flow control statements. I guess any Perl user who > makes things like gitweb (at least as a CGI-only app) shouldn't care about > such an internal difference of flow control statements those are > hidden/incapsulated inside the implementation of those statements? Perl hacker should know the difference between command such as "last" and "next", and functions such as exit() and die(). Just like C programmer should know the difference between "break" statement and exit() function. > Needless to mention that the 'last LABEL' ( goto, gosub, ... named them ) is a > bad and a very deprecated style which is every schoolboy is aware about > nowadays to keep from using in the application [...] Not true. The 'last LABEL;' command is very useful to exit nested loops. If used right it makes code much simpler (allowing to avoid extra flag variable and/or complicating loop conditional). If I remember correctly in O.-J. Dahl, Edsger W. Dijkstra, C. A. R. Hoare "Structured Programming" the programming language described includes "break <n>" statement, with similar purpose as "last LABEL" in Perl. Note also that Dijkstra wrote in seminal article "Go To Statement Considered Harmful" that the problem with abused 'goto' is that it compilcates and muddles control flow of program. But there are legitimate uses of 'goto' that make the program simpler to understand, and not harder,... among those is handling exceptions. >>>> I know this from painful experience of trying to find bug in a >>>> test... when the error was in parsing file in 'do $file;'. >>> >>> I handle them just fine like in any other CGI program using >>> CGI::Carp:fatalsToBrowser. Are you about to 'make test' via the http? ;-) >> >> I don't think you understand what I wanted to say there. >> >> If you don't check if there were parse errors from 'do $file;', you can >> get later some error message which is totally unrelated to the parsing >> error. If you don't know or forget that you should check $@ after >> 'do $file;', and are unlucky, you can chase elusive error from there >> to kingdom come... > > Got it, it's about the inclusion failure via the do() which is the > development, not a production, situation. Yes, it is a problem mainly in developemtn, where changes to the file included via "do <file>" might introduce parsing errors. > I think this should be an adjective noun to use the both strict and > the warnings? The problem is that "do <file>;" is similar to "eval `cat <file>`;" (except that it's more efficient and concise), it that it silences parsing errors. From `perldoc -f do`: If "do" cannot read the file, it returns undef and sets $! to the error. If "do" can read the file but cannot compile it, it returns undef and sets an error message in $@. If the file is successfully compiled, "do" returns the value of the last expression evaluated. > And yes, since it's about development but not production use, die is just fine > in the inclusion code like this: > > eval( 'use Module;' ); die $@ if $@; Wrong! > as always, require() can do the trick, not to mention usual > > use Module; > > This all will cause die() when it's necessary as only the application developer > knows how strict is the dependence on the Module. In some cases, application > can work without some Module but it's just better with it. First, both "use Module;" and "require Module;" (and "require '<file>';") do automatic error checking and raise an exception if there is problem. Second, "use Module <LIST>;" is equivalent to BEGIN { require Module; import Module <LIST>; } and therefore it doesn't make sense to use it for conditional inclusion. Therefore, to load Perl module / file, if you can 'die' you can simply use require "<file>"; If you don't want to die, but want to know if loading and parsing file succeeded or not, you should use the following syntax: if (eval { require "<file>"; 1 }) { ... } else { ... } If you want to use 'do "<file>";' (it is preferred in some circumstances), you really should check for error conditins: unless (my $return = do "<file>") { if ($@) { # couldn't parse <file> } elsif (!defined $return) { # couldn't do <file> (e.g. couldn't find <file>) } ... } [...] >> PSGI is interface, Plack is reference implementation. You can run PSGI >> app on any supported web server; this includes running PSGI apps on >> FastCGI. > > Existing problem FCGI::Spawn for is not the PSGI applications to be run as a > FastCGI, but the bunch of existing CGI.pm applications (even gitorious) need > to be more effective with the widest-spread protocol FastCGI. Best without any > patching of the application, deployed the same simple way as with apache's cgi > implementation. Gitorious is in Ruby, therefore is not a CGI.pm application, as it is not even in Perl. By using Plack::App::CGIBin you can load CGI scripts from a directory and convert them into a <persistent> PSGI application. You can use Plack::App::WrapCGI to convert single CGI script into PSGI application. You can use Plack::Buuilder's domain specific language to join (map) together a bunch of PSGI applications (in different paths) in a single app (via Plack::App::URLMap). You can then run PSGI application (for example the PSGI app which loads CGI apps via Plack::App::CGIBin) on any supported web server, which includes FCGI (FastCGI). -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-14 10:53 ` Jakub Narebski @ 2010-05-14 15:36 ` Peter Vereshagin 2010-05-14 17:58 ` Jakub Narebski 2010-05-15 11:51 ` Petr Baudis 1 sibling, 1 reply; 30+ messages in thread From: Peter Vereshagin @ 2010-05-14 15:36 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen God love is hard to find. You got lucky Jakub! 2010/05/14 12:53:42 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> legitimate uses of 'goto' that make the program simpler to understand, JN> and not harder,... among those is handling exceptions. so did you change the exception-related exit()s on your patch to the last()s ? JN> >>>> I know this from painful experience of trying to find bug in a JN> >>>> test... when the error was in parsing file in 'do $file;'. JN> >>> JN> >>> I handle them just fine like in any other CGI program using JN> >>> CGI::Carp:fatalsToBrowser. Are you about to 'make test' via the http? ;-) JN> >> JN> >> I don't think you understand what I wanted to say there. JN> >> JN> >> If you don't check if there were parse errors from 'do $file;', you can JN> >> get later some error message which is totally unrelated to the parsing JN> >> error. If you don't know or forget that you should check $@ after JN> >> 'do $file;', and are unlucky, you can chase elusive error from there JN> >> to kingdom come... JN> > JN> > Got it, it's about the inclusion failure via the do() which is the JN> > development, not a production, situation. JN> JN> Yes, it is a problem mainly in developemtn, where changes to the file JN> included via "do <file>" might introduce parsing errors. JN> JN> > I think this should be an adjective noun to use the both strict and JN> > the warnings? JN> JN> The problem is that "do <file>;" is similar to "eval `cat <file>`;" JN> (except that it's more efficient and concise), it that it silences JN> parsing errors. From `perldoc -f do`: JN> JN> If "do" cannot read the file, it returns undef and sets $! to the error. JN> If "do" can read the file but cannot compile it, it returns undef and sets JN> an error message in $@. If the file is successfully compiled, "do" JN> returns the value of the last expression evaluated. JN> JN> > And yes, since it's about development but not production use, die is just fine JN> > in the inclusion code like this: JN> > JN> > eval( 'use Module;' ); die $@ if $@; JN> JN> Wrong! The problem was you can't see the reason of the inclusion-via-do() parsing failure. But you may see it with use warnings; right? Is there any applied example of do()-caused failures? JN> > as always, require() can do the trick, not to mention usual JN> > JN> > use Module; JN> > JN> > This all will cause die() when it's necessary as only the application developer JN> > knows how strict is the dependence on the Module. In some cases, application JN> > can work without some Module but it's just better with it. JN> JN> First, both "use Module;" and "require Module;" (and "require '<file>';") JN> do automatic error checking and raise an exception if there is problem. for web applications, half of exceptions or more are generated when the user isn't the develioper. Notifications() via the logs are just enough and more than it: should be the prefered way of exceptions' notifications in a production. Why worry about return code then? JN> Second, "use Module <LIST>;" is equivalent to JN> BEGIN { require Module; import Module <LIST>; } JN> and therefore it doesn't make sense to use it for conditional inclusion. eval() is used there. JN> Therefore, to load Perl module / file, if you can 'die' you can simply JN> use JN> JN> require "<file>"; JN> JN> If you don't want to die, but want to know if loading and parsing file JN> succeeded or not, you should use the following syntax: JN> JN> if (eval { require "<file>"; 1 }) { JN> ... JN> } else { JN> ... JN> } JN> JN> If you want to use 'do "<file>";' (it is preferred in some JN> circumstances), you really should check for error conditins: JN> JN> unless (my $return = do "<file>") { JN> if ($@) { JN> # couldn't parse <file> JN> } elsif (!defined $return) { JN> # couldn't do <file> (e.g. couldn't find <file>) JN> } JN> ... JN> } So you propose to use the return code either way. Is it a key point? And what is the real difference from $@ usage? You mention 'The subroutine was defined, but there was a bug in parsing included file' just where is the code? How come file was not parsed but sub was defined? JN> [...] JN> >> PSGI is interface, Plack is reference implementation. You can run PSGI JN> >> app on any supported web server; this includes running PSGI apps on JN> >> FastCGI. JN> > JN> > Existing problem FCGI::Spawn for is not the PSGI applications to be run as a JN> > FastCGI, but the bunch of existing CGI.pm applications (even gitorious) need JN> > to be more effective with the widest-spread protocol FastCGI. Best without any JN> > patching of the application, deployed the same simple way as with apache's cgi JN> > implementation. JN> JN> Gitorious is in Ruby, therefore is not a CGI.pm application, as it is JN> not even in Perl. It was Girocco you mentioned earlier JN> By using Plack::App::CGIBin you can load CGI scripts from a directory JN> and convert them into a <persistent> PSGI application. You can use Such a conversion is more than a compilation? Does it mean converted CGI app should be stored before to become a persistent application? JN> Plack::App::WrapCGI to convert single CGI script into PSGI application. JN> You can use Plack::Buuilder's domain specific language to join (map) JN> together a bunch of PSGI applications (in different paths) in a single JN> app (via Plack::App::URLMap). And can the same process of that application server run for the several applications depending on the FastCGI request? JN> You can then run PSGI application (for example the PSGI app which loads JN> CGI apps via Plack::App::CGIBin) on any supported web server, which JN> includes FCGI (FastCGI). 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-14 15:36 ` Peter Vereshagin @ 2010-05-14 17:58 ` Jakub Narebski 2010-05-14 18:43 ` Jakub Narebski 2010-05-15 10:06 ` Peter Vereshagin 0 siblings, 2 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-14 17:58 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Fri, 14 May 2010, Peter Vereshagin wrote: > 2010/05/14 12:53:42 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : >> legitimate uses of 'goto' that make the program simpler to understand, >> and not harder,... among those is handling exceptions. > > so did you change the exception-related exit()s on your patch to the > "last" ? Yes, die_error(), which had "exit" that got replaced by non-local "goto" is exception-related subroutine. >> The problem is that "do <file>;" is similar to "eval `cat <file>`;" >> (except that it's more efficient and concise), it that it silences >> parsing errors. From `perldoc -f do`: >> >> If "do" cannot read the file, it returns undef and sets $! to the error. >> If "do" can read the file but cannot compile it, it returns undef and sets >> an error message in $@. If the file is successfully compiled, "do" >> returns the value of the last expression evaluated. >> >>> And yes, since it's about development but not production use, die is just fine >>> in the inclusion code like this: >>> >>> eval( 'use Module;' ); die $@ if $@; >> >> Wrong! > > The problem was you can't see the reason of the inclusion-via-do() > parsing failure. You don't see the parsing failure because "do <file>;" functions like "eval", which traps exceptions. You will see consequences of parsing failure (like not defined subroutine). > But you may see it with "use warnings;" right? "use warnings;" pragma doesn't help, because of the 'trapping exceptions' part. That is why "require <file>" is recommended over "do <file>". >>> as always, require() can do the trick, not to mention usual >>> >>> use Module; >>> >>> This all will cause die() when it's necessary as only the application developer >>> knows how strict is the dependence on the Module. In some cases, application >>> can work without some Module but it's just better with it. >> >> First, both "use Module;" and "require Module;" (and "require '<file>';") >> do automatic error checking and raise an exception if there is problem. > > for web applications, half of exceptions or more are generated when the user > isn't the develioper. > Notifications() via the logs are just enough and more than it: should be the > prefered way of exceptions' notifications in a production. > Why worry about return code then? Checking $@ after "do <file>" would cover the situation where there were parsing errors, but wouldn't cover situation where file was not found, or there was error in executing code (but parsing was O.K.). >> Second, "use Module <LIST>;" is equivalent to >> BEGIN { require Module; import Module <LIST>; } >> and therefore it doesn't make sense to use it for conditional inclusion. > > eval() is used there. It's the fact that "use Module" uses BEGIN block that is incompatibile with *conditional* using it from eval. >>>> PSGI is interface, Plack is reference implementation. You can run PSGI >>>> app on any supported web server; this includes running PSGI apps on >>>> FastCGI. >>> >>> Existing problem FCGI::Spawn for is not the PSGI applications to be run as a >>> FastCGI, but the bunch of existing CGI.pm applications (even gitorious) need >>> to be more effective with the widest-spread protocol FastCGI. Best without any >>> patching of the application, deployed the same simple way as with apache's cgi >>> implementation. >> >> Gitorious is in Ruby, therefore is not a CGI.pm application, as it is >> not even in Perl. > > It was Girocco you mentioned earlier Girocco is shell scripts, not Perl either, see http://repo.or.cz/w/girocco.git/tree >> By using Plack::App::CGIBin you can load CGI scripts from a directory >> and convert them into a <persistent> PSGI application. You can use > > Such a conversion is more than a compilation? Does it mean converted CGI app > should be stored before to become a persistent application? This convertion is a.) compiling CGI file into subroutine (taking care of things like DATA filehandle) using CGI::Compile b.) converting between CGI interface and PSGI interface, using CGI::Emulate::PSGI CGI::Compile manpage includes this example: use CGI::Emulate::PSGI; use CGI::Compile; my $cgi_script = "/path/to/foo.cgi"; my $sub = CGI::Compile->compile($cgi_script); my $app = CGI::Emulate::PSGI->handler($sub); # $app is a PSGI application >> Plack::App::WrapCGI to convert single CGI script into PSGI application. >> You can use Plack::Buuilder's domain specific language to join (map) >> together a bunch of PSGI applications (in different paths) in a single >> app (via Plack::App::URLMap). > > And can the same process of that application server run for the several > applications depending on the FastCGI request? Yes, it can. Depending on request it would run appropriate CGI-converted-to-PSGI application. I am not sure how Plack::App::CGIBin works internally; it migh cimpile all CGI applications upfront; but it might not. >> You can then run PSGI application (for example the PSGI app which loads >> CGI apps via Plack::App::CGIBin) on any supported web server, which >> includes FCGI (FastCGI). -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-14 17:58 ` Jakub Narebski @ 2010-05-14 18:43 ` Jakub Narebski 2010-05-15 10:06 ` Peter Vereshagin 1 sibling, 0 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-14 18:43 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Fri, 14 May 2010, Jakub Narebski wrote: > Girocco is shell scripts, not Perl either, see > http://repo.or.cz/w/girocco.git/tree I'm sorry, I stand corrected: the CGI scripts in Girocco are in Perl. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-14 17:58 ` Jakub Narebski 2010-05-14 18:43 ` Jakub Narebski @ 2010-05-15 10:06 ` Peter Vereshagin 2010-05-15 13:58 ` Jakub Narebski 1 sibling, 1 reply; 30+ messages in thread From: Peter Vereshagin @ 2010-05-15 10:06 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen You're face to face with man who sold the world, Jakub! 2010/05/14 19:58:15 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : JN> You don't see the parsing failure because "do <file>;" functions like JN> "eval", which traps exceptions. You will see consequences of parsing JN> failure (like not defined subroutine). JN> JN> > But you may see it with "use warnings;" right? JN> JN> "use warnings;" pragma doesn't help, because of the 'trapping JN> exceptions' part. That is why "require <file>" is recommended over JN> "do <file>". JN> Checking $@ after "do <file>" would cover the situation where there were JN> parsing errors, but wouldn't cover situation where file was not found, JN> or there was error in executing code (but parsing was O.K.). I just use it like many others, here are the examples of the code http://www.jmarshall.com/tools/cgiproxy/ nph-proxy.cgi: === if ($scheme eq 'https') { eval { require Net::SSLeay } ; # don't check during compilation &no_SSL_warning($URL) if $@ ; === http://webgui.org lib/WebGUI/HTML.pm: === } elsif ($type eq "thumb-if-form-thumb") { eval "use Image::Magick;"; if ($@){ WebGUI::ErrorHandler::warn("Image::Magick not loaded: ".$@); === are those lemmings wrong? By far, people don't use to want the application should be trapped as inclusion fails and they are just sure to deal with the consequences. This is where the php is successful to offer include/include_once as well as its require* counterparters to offer such a choice to a developer. Are those consequences any danger anyway for applications like a gitweb? Whatever, I almost forgot to ask you again about your mysterious 'The subroutine was defined, but there was a bug in parsing included file'. Does Perl parser has a bug ( about 'bug in parsing' )? file was not included but the sub from it was successfully defined? file was about to include inside a sub but Perl reported the 'sub undefined' instead of 'file has failed to be included by the sub'? All of those seem just incredible to me ;-) JN> >> Second, "use Module <LIST>;" is equivalent to JN> >> BEGIN { require Module; import Module <LIST>; } JN> >> and therefore it doesn't make sense to use it for conditional inclusion. JN> > JN> > eval() is used there. JN> JN> It's the fact that "use Module" uses BEGIN block that is incompatibile JN> with *conditional* using it from eval. it works conditionally on those excerpts above. At the moment of the compilation, Perl doesn't know in general case what code should be eval()'d as its argument may vary at the runtime. Therefore Perl do not parse eval() string argument even if it is a constant. And thus it doesn't appear at the BEGIN{} execution moment. This is e.g., how the FCGI::Spawn works with CGI::Fast that defines the socket in its BEGIN{}. You may define your socket communications preference, the FCGI_SOCKET_PATH, on a shell before to start perl, or in the perl, before to eval "use CGI::Fast;" or eval "use FCGI::Spawn"; Both work just fine. JN> This convertion is JN> a.) compiling CGI file into subroutine (taking care of things like DATA JN> filehandle) using CGI::Compile JN> b.) converting between CGI interface and PSGI interface, using JN> CGI::Emulate::PSGI Sounds to me like all of that can happen in-memory. Great! JN> Yes, it can. Depending on request it would run appropriate JN> CGI-converted-to-PSGI application. JN> I am not sure how Plack::App::CGIBin works internally; it migh cimpile JN> all CGI applications upfront; but it might not. Will challenge. 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-15 10:06 ` Peter Vereshagin @ 2010-05-15 13:58 ` Jakub Narebski 2010-05-16 10:15 ` Peter Vereshagin 2010-05-16 10:26 ` Petr Baudis 0 siblings, 2 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-15 13:58 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Sat, 15 May 2010, Peter Vereshagin wrote: > 2010/05/14 19:58:15 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > > > > You don't see the parsing failure because "do <file>;" functions like > > "eval", which traps exceptions. You will see consequences of parsing > > failure (like not defined subroutine). > > > > > But you may see it with "use warnings;" right? > > > > "use warnings;" pragma doesn't help, because of the 'trapping > > exceptions' part. That is why "require <file>" is recommended over > > "do <file>". > > > > Checking $@ after "do <file>" would cover the situation where there were > > parsing errors, but wouldn't cover situation where file was not found, > > or there was error in executing code (but parsing was O.K.). > > I just use it like many others, here are the examples of the code > http://www.jmarshall.com/tools/cgiproxy/ nph-proxy.cgi: > === > if ($scheme eq 'https') { > eval { require Net::SSLeay } ; # don't check during compilation > &no_SSL_warning($URL) if $@ ; > === > http://webgui.org lib/WebGUI/HTML.pm: > === > } elsif ($type eq "thumb-if-form-thumb") { > eval "use Image::Magick;"; > if ($@){ > WebGUI::ErrorHandler::warn("Image::Magick not loaded: ".$@); > === > > are those lemmings wrong? No they are not. But there are two things. First, there is a difference between 'eval EXPR' and 'eval BLOCK' form, in that 'eval EXPR' is parsed (at execution time) and executed (and is slightly slower), while 'eval BLOCK' form is parsed only once, at the time code surrounding eval is parsed (and is slightly faster). This means that while 'use' in conditional 'eval EXPR' as below if (<condition>) { eval "use Image::Magick;" ... } would work as expected, I think that 'use' in conditional 'eval BLOCK' would not. if (<condition>) { eval { use Image::Magick; } ... } So if you want to use 'eval BLOCK' form, you need to use 'require' and not 'use': if (<condition>) { eval { require Image::Magick; import Image::Magick; } ... } Second, if you are not interested in error condition, and only whether require'ing some module failed or not, then instead of eval { require Net::SSLeay }; no_SSL_warning($URL) if $@; you can use the 'eval { <sth>; 1 };' idiom, i.e. eval { require Net::SSLeay; 1; } or no_SSL_warning($URL); [...] > Whatever, I almost forgot to ask you again about your mysterious 'The > subroutine was defined, but there was a bug in parsing included file'. > Does Perl parser has a bug (about 'bug in parsing')? File was not > included but the sub from it was successfully defined? File was about to > include inside a sub but Perl reported the 'sub undefined' instead of > 'file has failed to be included by the sub'? All of those seem just > incredible to me ;-) The situation looked like this. The included file (via 'do') had a few subroutines in it, looking roughly like this: use strict; use warnings; sub foo { # here was a syntax error } sub bar { # ... } 1; # last statement in file The main file used 'do $file;' and then tried to use 'bar' subroutine, looking like this: use strict; use warnings; do $file; # no checking for $@ ... bar(); And there Perl gives the following error: 'Undefined subroutine &bar called' This is caused by the fact that there was a syntax error before definition of foo(), and Perl didn't make it to defining foo(). When I added checking for $@ in the form of 'die $@ if $@', the error that Perl shown was the syntax error in the foo() subroutine in $file file. [...] > > This convertion is > > a.) compiling CGI file into subroutine (taking care of things like DATA > > filehandle) using CGI::Compile > > b.) converting between CGI interface and PSGI interface, using > > CGI::Emulate::PSGI > > Sounds to me like all of that can happen in-memory. Great! > > > Yes, it can. Depending on request it would run appropriate > > CGI-converted-to-PSGI application. > > I am not sure how Plack::App::CGIBin works internally; it migh cimpile > > all CGI applications upfront; but it might not. > > Will challenge. I don't know if it would be complete replacement for FCGI::Spawn, but from your description of it, using Plack::App::CGIBin middleware (+ plackup + Plack::Handler::FCGI wrapper) could be a valid alternative to it.. P.S. About Girocco: instead of writing it as set of separate CGI scripts, it could have been instead written as single app, loading its modules ('use lib' would help). -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-15 13:58 ` Jakub Narebski @ 2010-05-16 10:15 ` Peter Vereshagin 2010-05-18 1:06 ` Jakub Narebski 2010-05-16 10:26 ` Petr Baudis 1 sibling, 1 reply; 30+ messages in thread From: Peter Vereshagin @ 2010-05-16 10:15 UTC (permalink / raw) To: Jakub Narebski Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen Be sure to wear flowers on your hat, Jakub! 2010/05/15 15:58:11 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : === JN> > eval "use Image::Magick;"; JN> > if ($@){ JN> > === JN> > JN> > are those lemmings wrong? JN> JN> No they are not. so that code is just right, and this: === eval( 'use Module;' ); die $@ if $@; === is 'Wrong!'. And what is the difference? JN> would work as expected, I think that 'use' in conditional 'eval BLOCK' would JN> not. I think so too as I did never meant about eval BLOCK; JN> if (<condition>) { JN> eval { use Image::Magick; } JN> ... JN> } JN> JN> So if you want to use 'eval BLOCK' form, you need to use 'require' and not JN> 'use': JN> JN> if (<condition>) { JN> eval { require Image::Magick; import Image::Magick; } JN> ... JN> } JN> JN> JN> Second, if you are not interested in error condition, and only whether JN> require'ing some module failed or not, then instead of JN> JN> eval { require Net::SSLeay }; JN> no_SSL_warning($URL) if $@; JN> JN> you can use the 'eval { <sth>; 1 };' idiom, i.e. JN> JN> eval { require Net::SSLeay; 1; } JN> or no_SSL_warning($URL); 'eval BLOCK' versus 'eval EXPR' it's just better, but not a tabu. 'eval EXPR' with $@ checking causes no any errors on the same runtime with the code to be executed later. For most cases the modules are used, the read/parsing error can be the only error possible as no run-time code happens out there but only the symbols declaration. Therefore checking $@ is just fine. JN> When I added checking for $@ in the form of 'die $@ if $@', the error that JN> Perl shown was the syntax error in the foo() subroutine in $file file. and this is where the $@ was sufficient, too. JN> I don't know if it would be complete replacement for FCGI::Spawn, but from JN> your description of it, using Plack::App::CGIBin middleware (+ plackup + JN> Plack::Handler::FCGI wrapper) could be a valid alternative to it.. There are some more features those are on by default in FCGI::Spawn if they are to be replaced, not sure if I will find them inside that framework. JN> P.S. About Girocco: instead of writing it as set of separate CGI scripts, it JN> could have been instead written as single app, loading its modules ('use JN> lib' would help). ... and sharing them with gitweb, right. ;-) 73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627) -- http://vereshagin.org ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-16 10:15 ` Peter Vereshagin @ 2010-05-18 1:06 ` Jakub Narebski 0 siblings, 0 replies; 30+ messages in thread From: Jakub Narebski @ 2010-05-18 1:06 UTC (permalink / raw) To: Peter Vereshagin Cc: Petr Baudis, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Sun, 16 May 2010, Peter Vereshagin wrote: > 2010/05/15 15:58:11 +0200 Jakub Narebski <jnareb@gmail.com> => To Peter Vereshagin : > === > > > eval "use Image::Magick;"; > > > if ($@){ > > > === > > > > > > are those lemmings wrong? > > > > No they are not. > > so that code is just right, and this: > === > eval( 'use Module;' ); die $@ if $@; > === > > is 'Wrong!'. And what is the difference? Why use eval('use Module;'); die $@ if $@; instead of simply use Module; or, if it is inside conditional, require Module; import Module; or perhaps use if ($enable_module) Module; if you 'die', like default, anyway? > > I don't know if it would be complete replacement for FCGI::Spawn, but from > > your description of it, using Plack::App::CGIBin middleware (+ plackup + > > Plack::Handler::FCGI wrapper) could be a valid alternative to it.. > > There are some more features those are on by default in FCGI::Spawn if they are > to be replaced, not sure if I will find them inside that framework. Note that with Plack::Middleware::Static you can serve static files, like stylesheets and images, too. See Plack::Handler::FCGI manpage for details on how to configure FastCGI backend for a PSGI application. > > P.S. About Girocco: instead of writing it as set of separate CGI scripts, it > > could have been instead written as single app, loading its modules ('use > > lib' would help). > > ... and sharing them with gitweb, right. ;-) Well, no. I'd rather the Gitweb::Admin / Girocco to remain separate... perhaps with gitweb / git as submodule. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-15 13:58 ` Jakub Narebski 2010-05-16 10:15 ` Peter Vereshagin @ 2010-05-16 10:26 ` Petr Baudis 1 sibling, 0 replies; 30+ messages in thread From: Petr Baudis @ 2010-05-16 10:26 UTC (permalink / raw) To: Jakub Narebski Cc: Peter Vereshagin, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Sat, May 15, 2010 at 03:58:11PM +0200, Jakub Narebski wrote: > P.S. About Girocco: instead of writing it as set of separate CGI scripts, it > could have been instead written as single app, loading its modules ('use > lib' would help). That would be a relatively trivial change given how the simple CGI scripts are designed. The scripts share a common model and parts of view already anyway. Kind regards, Petr "Pasky" Baudis ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script 2010-05-14 10:53 ` Jakub Narebski 2010-05-14 15:36 ` Peter Vereshagin @ 2010-05-15 11:51 ` Petr Baudis 1 sibling, 0 replies; 30+ messages in thread From: Petr Baudis @ 2010-05-15 11:51 UTC (permalink / raw) To: Jakub Narebski Cc: Peter Vereshagin, Eric Wong, git, Sam Vilain, Juan Jose Comellas, John Goerzen On Fri, May 14, 2010 at 12:53:42PM +0200, Jakub Narebski wrote: > Note also that Dijkstra wrote in seminal article "Go To Statement > Considered Harmful" that the problem with abused 'goto' is that it > compilcates and muddles control flow of program. But there are > legitimate uses of 'goto' that make the program simpler to understand, > and not harder,... among those is handling exceptions. Also, Dijkstra is well-known for statements that were intentionally very radical to stir a real debate in the sleepy academic circles and probably even Dijkstra was not as radical as people would think based on some of his statements. For another side of the goto debate, I really recommend reading the somewhat dated, but still interesting paper [Donald Knuth, "Structured programming with goto statements," Computing Surveys, December 1974]. -- Petr "Pasky" Baudis When I feel like exercising, I just lie down until the feeling goes away. -- xed_over ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2010-05-18 1:05 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-07 12:54 [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski 2010-05-07 12:54 ` [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine Jakub Narebski 2010-05-07 12:54 ` [RFC/PATCH 2/2] gitweb: Add support for FastCGI, using CGI::Fast Jakub Narebski 2010-05-08 7:59 ` [RFC/PATCHv2 " Jakub Narebski 2010-05-08 22:41 ` [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski 2010-05-09 9:31 ` Eric Wong 2010-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2010-05-09 12:39 ` Jakub Narebski 2010-05-09 16:47 ` Peter Vereshagin 2010-05-09 18:18 ` Jakub Narebski 2010-05-10 7:13 ` Peter Vereshagin 2010-05-10 15:29 ` Jakub Narebski 2010-05-11 6:24 ` Peter Vereshagin 2010-05-11 8:35 ` Petr Baudis 2010-05-11 10:58 ` Jakub Narebski 2010-05-11 12:09 ` Peter Vereshagin 2010-05-11 13:51 ` Jakub Narebski 2010-05-13 13:10 ` Peter Vereshagin 2010-05-13 17:13 ` Ævar Arnfjörð Bjarmason 2010-05-14 15:58 ` Peter Vereshagin 2010-05-14 10:53 ` Jakub Narebski 2010-05-14 15:36 ` Peter Vereshagin 2010-05-14 17:58 ` Jakub Narebski 2010-05-14 18:43 ` Jakub Narebski 2010-05-15 10:06 ` Peter Vereshagin 2010-05-15 13:58 ` Jakub Narebski 2010-05-16 10:15 ` Peter Vereshagin 2010-05-18 1:06 ` Jakub Narebski 2010-05-16 10:26 ` Petr Baudis 2010-05-15 11:51 ` Petr Baudis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).