diff options
| -rw-r--r-- | Bugzilla/Attachment/PatchReader.pm | 90 |
1 files changed, 63 insertions, 27 deletions
diff --git a/Bugzilla/Attachment/PatchReader.pm b/Bugzilla/Attachment/PatchReader.pm index b29240c5b..c2ef53a65 100644 --- a/Bugzilla/Attachment/PatchReader.pm +++ b/Bugzilla/Attachment/PatchReader.pm @@ -10,6 +10,8 @@ package Bugzilla::Attachment::PatchReader; use 5.10.1; use strict; +use Config; +use IO::Select; use IPC::Open3; use Symbol 'gensym'; @@ -17,6 +19,8 @@ use Bugzilla::Error; use Bugzilla::Attachment; use Bugzilla::Util; +use constant PERLIO_IS_ENABLED => $Config{useperlio}; + sub process_diff { my ($attachment, $format, $context) = @_; my $dbh = Bugzilla->dbh; @@ -30,8 +34,7 @@ sub process_diff { require PatchReader::DiffPrinter::raw; $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw()); # Actually print out the patch. - print $cgi->header(-type => 'text/plain', - -expires => '+3M'); + print $cgi->header(-type => 'text/plain'); disable_utf8(); $reader->iterate_string('Attachment ' . $attachment->id, $attachment->data); } @@ -102,9 +105,12 @@ sub process_interdiff { # Send through interdiff, send output directly to template. # Must hack path so that interdiff will work. - $ENV{'PATH'} = $lc->{diffpath}; + local $ENV{'PATH'} = $lc->{diffpath}; - my ($pid, $interdiff_stdout, $interdiff_stderr); + # Open the interdiff pipe, reading from both STDOUT and STDERR + # To avoid deadlocks, we have to read the entire output from all handles + my ($stdout, $stderr) = ('', ''); + my ($pid, $interdiff_stdout, $interdiff_stderr, $use_select); if ($ENV{MOD_PERL}) { require Apache2::RequestUtil; require Apache2::SubProcess; @@ -112,37 +118,73 @@ sub process_interdiff { (undef, $interdiff_stdout, $interdiff_stderr) = $request->spawn_proc_prog( $lc->{interdiffbin}, [$old_filename, $new_filename] ); + $use_select = !PERLIO_IS_ENABLED; } else { $interdiff_stderr = gensym; - my $pid = open3(gensym, $interdiff_stdout, $interdiff_stderr, + $pid = open3(gensym, $interdiff_stdout, $interdiff_stderr, $lc->{interdiffbin}, $old_filename, $new_filename); + $use_select = 1; } - binmode $interdiff_stdout; - # Check for errors - { - local $/ = undef; - my $error = <$interdiff_stderr>; - if ($error) { - warn($error); - $warning = 'interdiff3'; + if ($format ne 'raw' && Bugzilla->params->{'utf8'}) { + binmode $interdiff_stdout, ':utf8'; + binmode $interdiff_stderr, ':utf8'; + } else { + binmode $interdiff_stdout; + binmode $interdiff_stderr; + } + + if ($use_select) { + my $select = IO::Select->new(); + $select->add($interdiff_stdout, $interdiff_stderr); + while (my @handles = $select->can_read) { + foreach my $handle (@handles) { + my $line = <$handle>; + if (!defined $line) { + $select->remove($handle); + next; + } + if ($handle == $interdiff_stdout) { + $stdout .= $line; + } else { + $stderr .= $line; + } + } } + waitpid($pid, 0) if $pid; + + } else { + local $/ = undef; + $stdout = <$interdiff_stdout>; + $stdout //= ''; + $stderr = <$interdiff_stderr>; + $stderr //= ''; } - my ($reader, $last_reader) = setup_patch_readers("", $context); + close($interdiff_stdout), + close($interdiff_stderr); + + # Tidy up + unlink($old_filename) or warn "Could not unlink $old_filename: $!"; + unlink($new_filename) or warn "Could not unlink $new_filename: $!"; + + # Any output on STDERR means interdiff failed to full process the patches. + # Interdiff's error messages are generic and not useful to end users, so we + # show a generic failure message. + if ($stderr) { + warn($stderr); + $warning = 'interdiff3'; + } + my ($reader, $last_reader) = setup_patch_readers("", $context); if ($format eq 'raw') { require PatchReader::DiffPrinter::raw; $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw()); # Actually print out the patch. - print $cgi->header(-type => 'text/plain', - -expires => '+3M'); + print $cgi->header(-type => 'text/plain'); disable_utf8(); } else { - # In case the HTML page is displayed with the UTF-8 encoding. - binmode $interdiff_stdout, ':utf8' if Bugzilla->params->{'utf8'}; - $vars->{'warning'} = $warning if $warning; $vars->{'bugid'} = $new_attachment->bug_id; $vars->{'oldid'} = $old_attachment->id; @@ -152,14 +194,8 @@ sub process_interdiff { setup_template_patch_reader($last_reader, $format, $context, $vars); } - $reader->iterate_fh($interdiff_stdout, 'interdiff #' . $old_attachment->id . - ' #' . $new_attachment->id); - waitpid($pid, 0) if $pid; - $ENV{'PATH'} = ''; - - # Delete temporary files. - unlink($old_filename) or warn "Could not unlink $old_filename: $!"; - unlink($new_filename) or warn "Could not unlink $new_filename: $!"; + $reader->iterate_string('interdiff #' . $old_attachment->id . + ' #' . $new_attachment->id, $stdout); } ###################### |