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

Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:

> Daniel Shahaf wrote on Thu, Apr 17, 2014 at 11:12:15 +0000:
>> The fix would be to change the $parno accounting as follows: before
>> $entry{parno} is used, subtract from it the number of entries already
>> merged in this run.
>
> Untested patch:
>
> [[[
> Index: tools/dist/backport.pl
> ===================================================================
> --- tools/dist/backport.pl (revision 1588205)
> +++ tools/dist/backport.pl (working copy)
> @@ -80,6 +80,8 @@ $ENV{LC_ALL} = "C"; # since we parse 'svn info' o
>
> # Globals.
> my %ERRORS = ();
> +# TODO: can $MERGED_SOMETHING be removed and references to it replaced by scalar(@MERGES_TODAY) ?
> +my @MERGES_TODAY;
> my $MERGED_SOMETHING = 0;
> my $SVNq;
>
> @@ -255,6 +257,7 @@ sub my_tempfile {
> 
> sub merge {
> my %entry = @_;
> + my $parno = $entry{parno} - scalar grep { $_ < $entry{parno} } @MERGES_TODAY;
>
> my ($logmsg_fh, $logmsg_filename) = my_tempfile();
> my (@mergeargs);
> @@ -312,7 +315,7 @@ 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 ':0normal! $parno\x{7d}kdap' -c wq $STATUS
> $VIM -e -s -n -N -i NONE -u NONE -c '\$d' -c wq $STATUS
> $SVNq commit -F $logmsg_filename
> elif ! $sh[$YES]; then
> @@ -325,6 +328,11 @@ elif ! $sh[$YES]; then
> fi
> EOF
>
> + if ($MAY_COMMIT) {
> + # STATUS has been edited and the change has been committed
> + push @MERGES_TODAY, $entry{parno} if $MAY_COMMIT;
> + }
> +
> $script .= <<"EOF" if $entry{branch};
> reinteg_rev=\`$SVN info $STATUS | sed -ne 's/Last Changed Rev: //p'\`
> if $sh[$MAY_COMMIT]; then
> @@ -354,6 +362,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 $?;

I created the attached repository to test this:

There are three changes to be merged:

 Approved changes:
 =================

  * r4
    g import
    Votes:
      +1: pm
 
  * r8
    i import
    Votes:
      +1: pm
 
  * r5
    h import
    Votes:
      +1: pm

and the patched script is better but still not right. The unpatched
script only gets the first merge (r4) right. 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:

------------------------------------------------------------------------
r12 | pm | 2014-04-24 23:48:05 +0100 (Thu, 24 Apr 2014) | 8 lines

Merge r5 from trunk:

 * r5
   h import
   Votes:
     +1: pm
sentinel

Index: subversion/branches/1.8.x/A/h
===================================================================
--- subversion/branches/1.8.x/A/h (nonexistent)
+++ subversion/branches/1.8.x/A/h (revision 12)
@@ -0,0 +1 @@
+5
Index: subversion/branches/1.8.x/STATUS
===================================================================
--- subversion/branches/1.8.x/STATUS (revision 11)
+++ subversion/branches/1.8.x/STATUS (revision 12)
@@ -7,9 +7,3 @@
 =====================
 
 Approved changes:
-=================
-
- * r5
- h import
- Votes:
- +1: pm
Index: subversion/branches/1.8.x
===================================================================
--- subversion/branches/1.8.x (revision 11)
+++ subversion/branches/1.8.x (revision 12)

Property changes on: subversion/branches/1.8.x
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /subversion/trunk:r5

------------------------------------------------------------------------

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*