Author: danielsh
Date: Fri Feb 27 17:35:03 2015
New Revision: 1662760
URL: http://svn.apache.org/r1662760
Log:
backport.pl conflicts mode: Fix a bug whereby a two-pass merge where the second
pass tried to touch a file conflicted in the first pass, in an entry marked as
Depends:, would redden the bot.
* tools/dist/backport.pl
(IO::Select, IPC::Open3): New use()s.
(run_in_shell): New helper.
(merge):
Change signature. Take new optional parameter EXPECTED_ERROR_P, and use it
to conditionalize treating a non-zero exit code as an error. Call
run_in_shell(). Correct the error handling of subshell failures.
(handle_entry): Update calls to merge().
In conflicts mode, ignore SVN_ERR_WC_FOUND_CONFLICT for Depends:-ed entries.
* tools/dist/backport_tests.py
(backport_double_conflict): Extend test. Expect PASS.
Modified:
subversion/trunk/tools/dist/backport.pl
subversion/trunk/tools/dist/backport_tests.py
Modified: subversion/trunk/tools/dist/backport.pl
URL:
http://svn.apache.org/viewvc/subversion/trunk/tools/dist/backport.pl?rev=1662760&r1=1662759&r2=1662760&view=diff
==============================================================================
--- subversion/trunk/tools/dist/backport.pl (original)
+++ subversion/trunk/tools/dist/backport.pl Fri Feb 27 17:35:03 2015
@@ -28,6 +28,8 @@ use Term::ReadKey qw/ReadMode ReadKey/;
use File::Basename qw/basename dirname/;
use File::Copy qw/copy move/;
use File::Temp qw/tempfile/;
+use IO::Select ();
+use IPC::Open3 qw/open3/;
use POSIX qw/ctermid strftime isprint isspace/;
use Text::Wrap qw/wrap/;
use Tie::File ();
@@ -318,9 +320,46 @@ sub my_tempfile {
return ($fh, $fn);
}
+# The first argument is a shell script. Run it and return the shell's
+# exit code, and stdout and stderr as references to arrays of lines.
+sub run_in_shell($) {
+ my $script = shift;
+ my $pid = open3 \*SHELL_IN, \*SHELL_OUT, \*SHELL_ERR, qw#/bin/sh#;
+ # open3 raises exception when it fails; no need to error check
+
+ print SHELL_IN $script;
+ close SHELL_IN;
+
+ # Read loop: tee stdout,stderr to arrays.
+ my $select = IO::Select->new(\*SHELL_OUT, \*SHELL_ERR);
+ my (@readable, $outlines, $errlines);
+ while (@readable = $select->can_read) {
+ for my $fh (@readable) {
+ my $line = <$fh>;
+ $select->remove($fh) if eof $fh or not defined $line;
+ next unless defined $line;
+
+ if ($fh == \*SHELL_OUT) {
+ push @$outlines, $line;
+ print STDOUT $line;
+ }
+ if ($fh == \*SHELL_ERR) {
+ push @$errlines, $line;
+ print STDERR $line;
+ }
+ }
+ }
+ waitpid $pid, 0; # sets $?
+ return $?, $outlines, $errlines;
+}
+
+# EXPECTED_ERROR_P is subref called with EXIT_CODE, OUTLINES, ERRLINES,
+# expected to return TRUE if the error should be considered fatal (cause
+# backport.pl to exit non-zero) or not. It may be undef for default behaviour.
sub merge {
- my %entry = @_;
+ my %entry = %{ +shift };
+ my $expected_error_p = shift // sub { 0 }; # by default, errors are
unexpected
my $parno = $entry{parno} - scalar grep { $_->{parno} < $entry{parno} }
@MERGES_TODAY;
my ($logmsg_fh, $logmsg_filename) = my_tempfile();
@@ -434,15 +473,26 @@ EOF
revert(verbose => 0, discard_STATUS => $MAY_COMMIT);
$MERGED_SOMETHING++;
- open SHELL, '|-', qw#/bin/sh# or die "$! (in '$entry{header}')";
- print SHELL $script;
- close SHELL or do {
- warn "$0: sh($?): $! (in '$entry{header}')";
- die "Lost track of paragraph numbers; aborting" if $MAY_COMMIT
- };
- $ERRORS{$entry{id}} = [\%entry, "sh($?): $!"] if $?;
+ my ($exit_code, $outlines, $errlines) = run_in_shell $script;
+ unless ($! == 0) {
+ die "system() failed to spawn subshell ($!); aborting";
+ }
+ unless ($exit_code == 0) {
+ warn "$0: subshell exited with code $exit_code (in '$entry{header}') "
+ ."(maybe due to 'set -e'?)";
+
+ # If we're committing, don't attempt to guess the problem and gracefully
+ # continue; just abort.
+ if ($MAY_COMMIT) {
+ die "Lost track of paragraph numbers; aborting";
+ }
+
+ # Record the error, unless the caller wants not to.
+ $ERRORS{$entry{id}} = [\%entry, "subshell exited with code $exit_code"]
+ unless $expected_error_p->($exit_code, $outlines, $errlines);
+ }
- unlink $logmsg_filename unless $? or $!;
+ unlink $logmsg_filename unless $exit_code;
}
# Input formats:
@@ -881,7 +931,7 @@ sub handle_entry {
unless (@vetoes) {
if ($MAY_COMMIT and $in_approved) {
# svn-role mode
- merge %entry if validate_branch_contains_named_revisions %entry;
+ merge \%entry if validate_branch_contains_named_revisions %entry;
} elsif (!$MAY_COMMIT) {
# Scan-for-conflicts mode
@@ -890,7 +940,14 @@ sub handle_entry {
# block.
validate_branch_contains_named_revisions %entry;
- merge %entry;
+ # E155015 is SVN_ERR_WC_FOUND_CONFLICT
+ my $expected_error_p = sub {
+ my ($exit_code, $outlines, $errlines) = @_;
+ ($exit_code == 0)
+ or
+ (grep /svn: E155015:/, @$errlines)
+ };
+ merge \%entry, ($entry{depends} ? $expected_error_p : undef);
my $output = `$SVN status`;
@@ -951,7 +1008,7 @@ sub handle_entry {
verbose => 1, extra => qr/[+-]/) {
when (/^y/i) {
#validate_branch_contains_named_revisions %entry;
- merge %entry;
+ merge \%entry;
while (1) {
given (prompt "Shall I open a subshell? [ydN] ", verbose => 1) {
when (/^y/i) {
Modified: subversion/trunk/tools/dist/backport_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/tools/dist/backport_tests.py?rev=1662760&r1=1662759&r2=1662760&view=diff
==============================================================================
--- subversion/trunk/tools/dist/backport_tests.py (original)
+++ subversion/trunk/tools/dist/backport_tests.py Fri Feb 27 17:35:03 2015
@@ -480,7 +480,6 @@ def backport_branch_contains(sbox):
#----------------------------------------------------------------------
-@XFail()
@BackportTest(None) # would be 000000000008
def backport_double_conflict(sbox):
"two-revisioned entry with two conflicts"
@@ -531,6 +530,25 @@ def backport_double_conflict(sbox):
svntest.verify.verify_outputs(None, output, errput,
expected_output, expected_errput)
svntest.verify.verify_exit_code(None, exit_code, 0)
+ if any("Warning summary" in line for line in errput):
+ raise svntest.verify.SVNUnexpectedStderr(errput)
+
+ ## Now, let's ensure this does get detected if not silenced.
+ # r9: Re-nominate
+ approved_entries = [
+ make_entry([4,7]) # no depends=
+ ]
+ sbox.simple_append(STATUS, serialize_STATUS(approved_entries), truncate=True)
+ sbox.simple_commit(message='Re-nominate the r4 group')
+
+ exit_code, output, errput = run_backport(sbox, True, ["MAY_COMMIT=0"])
+
+ # [1-9]\d+ matches non-zero exit codes
+ expected_errput = r'r4 .*: subshell exited with code (?:[1-9]\d+)'
+ svntest.verify.verify_exit_code(None, exit_code, 1)
+ svntest.verify.verify_outputs(None, output, errput,
+ svntest.verify.AnyOutput, expected_errput)
+
#----------------------------------------------------------------------
