Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@subversion.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: svn commit: r1661252 - in /subversion/trunk/tools/dist: backport.pl
 backport_tests.py
Date: Sat, 21 Feb 2015 00:09:36 -0000
To: commits@subversion.apache.org
From: danielsh@apache.org
Message-Id: <20150221000936.F2389AC0154@hades.apache.org>

Author: danielsh
Date: Sat Feb 21 00:09:36 2015
New Revision: 1661252

URL: http://svn.apache.org/r1661252
Log:
backport.pl: Ensure the "Branch:" header, if present, contains a superset of
the given revisions.  This fixes the problem demonstrated in r1640665 and
explained in r1643849.

The check is asymmetric: all revisions nominated must have been merged to the
branch, but it is (presently) not an error if the branch contains operative
revisions beyond those declared in the nomination.

* tools/dist/backport.pl
  ($TRUNK): New constant.
  (validate_branch_contains_named_revisions): New function.
  (handle_entry): Call validate_branch_contains_named_revisions() prior to
    calling merge() in headless modes.
  (backport_usage): Document new validation.

* tools/dist/backport_tests.py
  (backport_branch_contains): New test.
  (backport_branches): Add cross-reference.

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=1661252&r1=1661251&r2=1661252&view=diff
==============================================================================
--- subversion/trunk/tools/dist/backport.pl (original)
+++ subversion/trunk/tools/dist/backport.pl Sat Feb 21 00:09:36 2015
@@ -99,6 +99,7 @@ unless (defined $AVAILID) {
 my $STATUS = './STATUS';
 my $STATEFILE = './.backports1';
 my $BRANCHES = '^/subversion/branches';
+my $TRUNK = '^/subversion/trunk';
 $ENV{LC_ALL} = "C";  # since we parse 'svn info' output and use isprint()
 
 # Globals.
@@ -193,6 +194,13 @@ undefined.  In this mode, the script wil
 (including unapproved and vetoed ones), and complain to stderr if the merge
 failed due to a conflict.  This mode never commits anything.
 
+Both batch modes also perform a basic sanity-check on entries that declare
+backport branches (via the "Branch:" header): if a backport branch is used, but
+at least one of the revisions enumerated in the entry title had not been merged
+from $TRUNK to the branch root, the hourly bot will turn red and 
+nightly bot will skip the entry and email its admins.  (The nightly bot does
+not email the list on failure, since it doesn't use buildbot.)
+
 The 'svn' binary defined by the environment variable \$SVN, or otherwise the
 'svn' found in \$PATH, will be used to manage the working copy.
 EOF
@@ -818,6 +826,37 @@ sub exit_stage_left {
   exit scalar keys %ERRORS;
 }
 
+# Given an ENTRY, check whether all ENTRY->{revisions} have been merged
+# into ENTRY->{branch}, if it has one.  If revisions are missing, record
+# a warning in $ERRORS.  Return TRUE If the entry passed the validation
+# and FALSE otherwise.
+sub validate_branch_contains_named_revisions {
+  my %entry = @_;
+  return 1 unless defined $entry{branch};
+  my %present;
+
+  return "Why are you running so old versions?" # true in boolean context
+    if $SVNvsn < 1_005_000;     # doesn't have the 'mergeinfo' subcommand
+
+  my $shell_escaped_branch = shell_escape($entry{branch});
+  %present = do {
+    my @present = `$SVN mergeinfo --show-revs=merged -- $TRUNK $BRANCHES/$shell_escaped_branch`;
+    chomp @present;
+    @present = map /(\d+)/g, @present;
+    map +($_ => 1), @present;
+  };
+
+  my @absent = grep { not exists $present{$_} } @{$entry{revisions}};
+
+  if (@absent) {
+    $ERRORS{$entry{id}} //= [\%entry,
+      sprintf("Revisions '%s' nominated but not included in branch",
+              (join ", ", map { "r$_" } @absent)),
+    ];
+  }
+  return @absent ? 0 : 1;
+}
+
 sub handle_entry {
   my $in_approved = shift;
   my $approved = shift;
@@ -837,9 +876,15 @@ sub handle_entry {
     unless (@vetoes) {
       if ($MAY_COMMIT and $in_approved) {
         # svn-role mode
-        merge %entry;
+        merge %entry if validate_branch_contains_named_revisions %entry;
       } elsif (!$MAY_COMMIT) {
         # Scan-for-conflicts mode
+
+        # First, sanity-check the entry.  We ignore the result; even if it
+        # failed, we do want to check for conflicts, in the remainder of this
+        # block.
+        validate_branch_contains_named_revisions %entry;
+
         merge %entry;
 
         my $output = `$SVN status`;
@@ -900,6 +945,7 @@ sub handle_entry {
     given (prompt 'Run a merge? [y,l,v,Â±1,Â±0,q,e,a, ,N] ',
                    verbose => 1, extra => qr/[+-]/) {
       when (/^y/i) {
+        #validate_branch_contains_named_revisions %entry;
         merge %entry;
         while (1) {
           given (prompt "Shall I open a subshell? [ydN] ", verbose => 1) {

Modified: subversion/trunk/tools/dist/backport_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/dist/backport_tests.py?rev=1661252&r1=1661251&r2=1661252&view=diff
==============================================================================
--- subversion/trunk/tools/dist/backport_tests.py (original)
+++ subversion/trunk/tools/dist/backport_tests.py Sat Feb 21 00:09:36 2015
@@ -342,6 +342,8 @@ def backport_branches(sbox):
   # Run it.
   run_backport(sbox)
 
+  # This also serves as the 'success mode' part of backport_branch_contains().
+
 
 #----------------------------------------------------------------------
 @BackportTest('76cee987-25c9-4d6c-ad40-000000000005')
@@ -399,6 +401,60 @@ def backport_conflicts_detection(sbox):
 
 
 #----------------------------------------------------------------------
+@BackportTest(None) # would be 000000000007
+def backport_branch_contains(sbox):
+  "branch must contain the revisions"
+
+  # r6: conflicting change on branch
+  sbox.simple_append('branch/iota', 'Conflicts with first change')
+  sbox.simple_commit(message="Conflicting change on iota")
+
+  # r7: backport branch
+  sbox.simple_update()
+  sbox.simple_copy('branch', 'subversion/branches/r4')
+  sbox.simple_commit(message='Create a backport branch')
+
+  # r8: merge into backport branch
+  sbox.simple_update()
+  svntest.main.run_svn(None, 'merge', '--record-only', '-c4',
+                       '^/subversion/trunk', sbox.ospath('subversion/branches/r4'))
+  sbox.simple_mkdir('subversion/branches/r4/A_resolved')
+  sbox.simple_append('subversion/branches/r4/iota', "resolved\n", truncate=1)
+  sbox.simple_commit(message='Conflict resolution via mkdir')
+
+  # r9: nominate r4,r5 with branch that contains not all of them
+  approved_entries = [
+    make_entry([4,5], branch="r4")
+  ]
+  sbox.simple_append(STATUS, serialize_STATUS(approved_entries))
+  sbox.simple_commit(message='Nominate r4')
+
+  # Run it.
+  exit_code, output, errput = run_backport(sbox, error_expected=True)
+
+  # Verify the error message.
+  expected_errput = svntest.verify.RegexOutput(
+    ".*Revisions 'r5' nominated but not included in branch",
+    match_all=False,
+  )
+  svntest.verify.verify_outputs(None, output, errput,
+                                [], expected_errput)
+  svntest.verify.verify_exit_code(None, exit_code, 1)
+
+  # Verify no commit occurred.
+  svntest.actions.run_and_verify_svnlook(["9\n"], [],
+                                         'youngest', sbox.repo_dir)
+
+  # Verify the working copy has been reverted.
+  svntest.actions.run_and_verify_svn([], [], 'status', '-q',
+                                     sbox.repo_dir)
+
+  # The sibling test backport_branches() verifies the success mode.
+
+
+
+
+#----------------------------------------------------------------------
 
 ########################################################################
 # Run the tests
@@ -411,6 +467,7 @@ test_list = [ None,
               backport_branches,
               backport_multirevisions,
               backport_conflicts_detection,
+              backport_branch_contains,
               # When adding a new test, include the test number in the last
               # 6 bytes of the UUID.
              ]