Re: Tonight's svn-role merges - bug analysis

[Daniel Shahaf]

Philip Martin wrote on Thu, Apr 24, 2014 at 23:57:32 +0100:
> The patched script gets the first two merges right (r4 and r8) but the
> final merge (r5) is still wrong: the log message contains "sentinel"
> and the wrong lines are removed from STATUS:
> 
..
>  Approved changes:
> -=================
> -
> - * r5

Thanks for looking into this.  This is exactly the issue I mentioned in
my other email (<http://mid.gmane.org/20140417113242.GD1802%40tarsus.local2>).
The attached patch should fix it.

I'll commit it if I find time to test it, but feel free to beat me to it.

--- tools/dist/backport.pl
+++ tools/dist/backport.pl
@@ -80,6 +80,11 @@ $ENV{LC_ALL} = "C";  # since we parse 'svn info' output and use isprint()
 
 # Globals.
 my %ERRORS = ();
+# TODO: can $MERGED_SOMETHING be removed and references to it replaced by scalar(@MERGES_TODAY) ?
+#       alternately, does @MERGES_TODAY need to be purged whenever $MERGED_SOMETHING is reset?
+#       The scalar is only used in interactive runs, but the array is used in
+#       svn-role batch mode too.
+my @MERGES_TODAY;
 my $MERGED_SOMETHING = 0;
 my $SVNq;
 
@@ -255,6 +260,7 @@ sub my_tempfile {
 

 sub merge {
   my %entry = @_;
+  my $parno = $entry{parno} - scalar grep { $_->{parno} < $entry{parno} } @MERGES_TODAY;
 
   my ($logmsg_fh, $logmsg_filename) = my_tempfile();
   my (@mergeargs);
@@ -309,11 +315,13 @@ if [ "`$SVN status -q | wc -l`" -eq 1 ]; then
   fi
 fi
 if $sh[$MAY_COMMIT]; then
-  # Remove the approved entry.  The sentinel guarantees the right number of blank
-  # lines is removed, which prevents spurious '--renormalize' commits tomorrow.
-  echo "sentinel" >> $STATUS
-  $VIM -e -s -n -N -i NONE -u NONE -c ':0normal! $entry{parno}\x{7d}kdap' -c wq $STATUS
-  $VIM -e -s -n -N -i NONE -u NONE -c '\$d' -c wq $STATUS
+  # Remove the approved entry.  The sentinel is important when the entry being
+  # removed is the very last one in STATUS, and in that case it has two effects:
+  # (1) keeps STATUS from ending in a run of multiple empty lines;
+  # (2) makes the \x{7d}k motion behave the same as in all other cases.
+  (echo; echo; echo "sentinel paragraph") >> $STATUS
+  $VIM -e -s -n -N -i NONE -u NONE -c ':0normal! $parno\x{7d}kdap' -c wq $STATUS
+  $VIM -e -s -n -N -i NONE -u NONE -c '\$normal! dap' -c wq $STATUS
   $SVNq commit -F $logmsg_filename
 elif ! $sh[$YES]; then
   echo "Would have committed:"
@@ -325,6 +333,11 @@ elif ! $sh[$YES]; then
 fi
 EOF
 
+  if ($MAY_COMMIT) {
+    # STATUS has been edited and the change has been committed
+    push @MERGES_TODAY, \%entry;
+  }
+
   $script .= <<"EOF" if $entry{branch};
 reinteg_rev=\`$SVN info $STATUS | sed -ne 's/Last Changed Rev: //p'\`
 if $sh[$MAY_COMMIT]; then
@@ -354,6 +367,8 @@ EOF
   $MERGED_SOMETHING++;
   open SHELL, '|-', qw#/bin/sh# or die "$! (in '$entry{header}')";
   print SHELL $script;
+  # TODO: s/warn/die/ in the $MAY_COMMIT case?  (since we don't know
+  #       whether to update @MERGES_TODAY)
   close SHELL or warn "$0: sh($?): $! (in '$entry{header}')";
   $ERRORS{$entry{id}} = [\%entry, "sh($?): $!"] if $?;
 
@@ -1076,12 +1091,6 @@ sub nominate_main {
 }
 
 # Dispatch to the appropriate main().
-die 'svn-role mode is broken when there are >=2 entries in Approved; '.
-    'see <http://mail-archives.apache.org/mod_mbox/subversion-dev/201404.mbox/%3C20140417111215.GB1802@tarsus.local2%3E> '.
-    'and <http://mail-archives.apache.org/mod_mbox/subversion-dev/201404.mbox/%3c8738h36mwh....@ntlworld.com%3E> '.
-    'for details.'
-    if $MAY_COMMIT;
-
 given (basename($0)) {
   when (/^b$|backport/) {
     chdir dirname $0 or die "Can't chdir: $!" if /^b$/;