Re: [PATCH 16/18] gitweb: When changing output (STDOUT) change STDERR as well
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:50:13
"John 'Warthog9' Hawley" [off-list ref] writes:
This sets up a trap for STDERR as well as STDOUT. This should prevent any transient error messages from git itself percolating up to gitweb and outputting errant information before the HTTP header has been sent.
Hmm... anuthing that happens after 'use CGI::Carp;' is parsed should
have STDERR redirected to web server logs, see CGI::Carp manpage
[...]
use CGI::Carp
And the standard warn(), die (), croak(), confess() and carp() calls will
automagically be replaced with functions that write out nicely time-stamped
messages to the HTTP server error log.
[...]
REDIRECTING ERROR MESSAGES
By default, error messages are sent to STDERR. Most HTTPD servers direct
STDERR to the server's error log.
[...]
Especially the second part.
Could you give us example which causes described misbehaviour?
I have nothing against this patch: if you have to have it, then you
have to have it. I oly try to understand what might be core cause
behind the issue that this patch is to solve...
quoted hunk ↗ jump to hunk
Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net> --- gitweb/gitweb.perl | 22 +++++++++++++++++++++- gitweb/lib/cache.pl | 22 ---------------------- 2 files changed, 21 insertions(+), 23 deletions(-)diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 7f8292e..d39982a 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -1214,6 +1214,10 @@ sub evaluate_argv { sub change_output { our $output; + # + # STDOUT + # + # Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such open(STDOUT_REAL,">&STDOUT") or die "Unable to capture STDOUT $!\n"; print STDOUT_REAL "";@@ -1223,12 +1227,28 @@ sub change_output { # Trap STDOUT to the $output variable, which is what I was using in the original # patch anyway. - open(STDOUT,">", \$output) || die "Unable to open STDOUT: $!"; #open STDOUT handle to use $var + open(STDOUT,">", \$output) || die "Unable to open STDOUT: $!"; #open STDOUT handle to use $output + + # + # STDERR + # + + # Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such + open(STDERR_REAL,">&STDERR") or die "Unable to capture STDERR $!\n"; + print STDERR_REAL "";
'print STDERR_REAL "";' nicely solves the spurious warning problem. Nice.
+ + # Close STDOUT, so that it isn't being used anymore. + close STDERR; + + # Trap STDOUT to the $output variable, which is what I was using in the original + # patch anyway. + open(STDERR,">", \$output_err) || die "Unable to open STDERR: $!"; #open STDERR handle to use $output_err
Err... where $output_err is defined?
quoted hunk ↗ jump to hunk
} sub reset_output { # This basically takes STDOUT_REAL and puts it back as STDOUT open(STDOUT,">&STDOUT_REAL"); + open(STDERR,">&STDERR_REAL"); } sub run {diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl index 28e4240..a8c902d 100644 --- a/gitweb/lib/cache.pl +++ b/gitweb/lib/cache.pl@@ -380,28 +380,6 @@ EOF return; } -sub cacheDisplayErr { - - return if ( ! -e "$fullhashpath.err" ); - - open($cacheFileErr, '<:utf8', "$fullhashpath.err"); - $lockStatus = flock($cacheFileErr,LOCK_SH|LOCK_NB); - - if (! $lockStatus ){ - show_warning( - "<p>". - "<strong>*** Warning ***:</strong> Locking error when trying to lock error cache page, file $fullhashpath.err<br/>/\n". - "This is about as screwed up as it gets folks - see your systems administrator for more help with this.". - "<p>" - ); - } - - while( <$cacheFileErr> ){ - print $_; - } - exit(0); -}
Errr... in which patch it was added? -- Jakub Narebski Poland ShadeHawk on #git