Changeset 50547 in webkit

Timestamp:

Nov 4, 2009 11:07:00 PM (14 years ago)

Author:

eric@webkit.org

Message:

2009-11-04 Eric Seidel <​eric@webkit.org>

Reviewed by David Kilzer.

svn-apply's fixChangeLogPatch function seems broken
​https://bugs.webkit.org/show_bug.cgi?id=30683

Update fixChangeLogPatch to be able to handle patches which
don't start at line 1.
Add unit tests for svn-apply to scm_unittest.py.

• Scripts/VCSUtils.pm:
• Scripts/modules/scm_unittest.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/VCSUtils.pm (modified) (1 diff)
• Scripts/modules/scm_unittest.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-11-04  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by David Kilzer.
+
+        svn-apply's fixChangeLogPatch function seems broken
+        https://bugs.webkit.org/show_bug.cgi?id=30683
+
+        Update fixChangeLogPatch to be able to handle patches which
+        don't start at line 1.
+        Add unit tests for svn-apply to scm_unittest.py.
+
+        * Scripts/VCSUtils.pm:
+        * Scripts/modules/scm_unittest.py:
+
 2009-11-04  Chris Fleizach  <cfleizach@apple.com>
 

trunk/WebKitTools/Scripts/VCSUtils.pm

@@ -356 +356 @@
 }
 
+# The diff(1) command is greedy when matching lines, so a new ChangeLog entry will
+# have lines of context at the top of a patch when the existing entry has the same
+# date and author as the new entry.  Alter the ChangeLog patch so
+# that the added lines ("+") in the patch always start at the beginning of the
+# patch and there are no initial lines of context.
 sub fixChangeLogPatch($)
 {
-    my $patch = shift;
-    my $contextLineCount = 3;
-
-    return $patch if $patch !~ /\n@@ -1,(\d+) \+1,(\d+) @@\n( .*\n)+(\+.*\n)+( .*\n){$contextLineCount}$/m;
-    my ($oldLineCount, $newLineCount) = ($1, $2);
-    return $patch if $oldLineCount <= $contextLineCount;
-
-    # The diff(1) command is greedy when matching lines, so a new ChangeLog entry will
-    # have lines of context at the top of a patch when the existing entry has the same
-    # date and author as the new entry.  This nifty loop alters a ChangeLog patch so
-    # that the added lines ("+") in the patch always start at the beginning of the
-    # patch and there are no initial lines of context.
-    my $newPatch;
-    my $lineCountInState = 0;
-    my $oldContentLineCountReduction = $oldLineCount - $contextLineCount;
-    my $newContentLineCountWithoutContext = $newLineCount - $oldLineCount - $oldContentLineCountReduction;
-    my ($stateHeader, $statePreContext, $stateNewChanges, $statePostContext) = (1..4);
-    my $state = $stateHeader;
-    foreach my $line (split(/\n/, $patch)) {
-        $lineCountInState++;
-        if ($state == $stateHeader && $line =~ /^@@ -1,$oldLineCount \+1,$newLineCount @\@$/) {
-            $line = "@@ -1,$contextLineCount +1," . ($newLineCount - $oldContentLineCountReduction) . " @@";
-            $lineCountInState = 0;
-            $state = $statePreContext;
-        } elsif ($state == $statePreContext && substr($line, 0, 1) eq " ") {
-            $line = "+" . substr($line, 1);
-            if ($lineCountInState == $oldContentLineCountReduction) {
-                $lineCountInState = 0;
-                $state = $stateNewChanges;
+    my $patch = shift; # $patch will only contain patch fragments for ChangeLog.
+
+    $patch =~ /(\r?\n)/;
+    my $lineEnding = $1;
+    my @patchLines = split(/$lineEnding/, $patch);
+
+    # e.g. 2009-06-03  Eric Seidel  <eric@webkit.org>
+    my $dateLineRegexpString = '^\+(\d{4}-\d{2}-\d{2})' # Consume the leading '+' and the date.
+                             . '\s+(.+)\s+' # Consume the name.
+                             . '<([^<>]+)>$'; # And finally the email address.
+
+    # Figure out where the patch contents start and stop.
+    my $patchHeaderIndex;
+    my $firstContentIndex;
+    my $trailingContextIndex;
+    my $dateIndex;
+    my $patchEndIndex = scalar(@patchLines);
+    for (my $index = 0; $index < @patchLines; ++$index) {
+        my $line = $patchLines[$index];
+        if ($line =~ /^\@\@ -\d+,\d+ \+\d+,\d+ \@\@$/) { # e.g. @@ -1,5 +1,18 @@
+            if ($patchHeaderIndex) {
+                $patchEndIndex = $index; # We only bother to fix up the first patch fragment.
+                last;
             }
-        } elsif ($state == $stateNewChanges && substr($line, 0, 1) eq "+") {
-            # No changes to these lines
-            if ($lineCountInState == $newContentLineCountWithoutContext) {
-                $lineCountInState = 0;
-                $state = $statePostContext;
-            }
-        } elsif ($state == $statePostContext) {
-            if (substr($line, 0, 1) eq "+" && $lineCountInState <= $oldContentLineCountReduction) {
-                $line = " " . substr($line, 1);
-            } elsif ($lineCountInState > $contextLineCount && substr($line, 0, 1) eq " ") {
-                next; # Discard
-            }
+            $patchHeaderIndex = $index;
         }
-        $newPatch .= $line . "\n";
-    }
-
-    return $newPatch;
-}
-
+        $firstContentIndex = $index if ($patchHeaderIndex && !$firstContentIndex && $line =~ /^\+[^+]/); # Only match after finding patchHeaderIndex, otherwise we'd match "+++".
+        $dateIndex = $index if ($line =~ /$dateLineRegexpString/);
+        $trailingContextIndex = $index if ($firstContentIndex && !$trailingContextIndex && $line =~ /^ /);
+    }
+    my $contentLineCount = $trailingContextIndex - $firstContentIndex;
+    my $trailingContextLineCount = $patchEndIndex - $trailingContextIndex;
+
+    # If we didn't find a date line in the content then this is not a patch we should try and fix.
+    return $patch if (!$dateIndex);
+
+    # We only need to do anything if the date line is not the first content line.
+    return $patch if ($dateIndex == $firstContentIndex);
+
+    # Write the new patch.
+    my $totalNewContentLines = $contentLineCount + $trailingContextLineCount;
+    $patchLines[$patchHeaderIndex] = "@@ -1,$trailingContextLineCount +1,$totalNewContentLines @@"; # Write a new header.
+    my @repeatedLines = splice(@patchLines, $dateIndex, $trailingContextIndex - $dateIndex); # The date line and all the content after it that diff saw as repeated.
+    splice(@patchLines, $firstContentIndex, 0, @repeatedLines); # Move the repeated content to the top.
+    foreach my $line (@repeatedLines) {
+        $line =~ s/^\+/ /;
+    }
+    splice(@patchLines, $trailingContextIndex, $patchEndIndex, @repeatedLines); # Replace trailing context with the repeated content.
+    splice(@patchLines, $patchHeaderIndex + 1, $firstContentIndex - $patchHeaderIndex - 1); # Remove any leading context.
+
+    return join($lineEnding, @patchLines) . "\n"; # patch(1) expects an extra trailing newline.
+}
 
 1;

trunk/WebKitTools/Scripts/modules/scm_unittest.py

@@ -36 +36 @@
 import unittest
 import urllib
+
+from datetime import date
 from modules.scm import detect_scm_system, SCM, ScriptError, CheckoutNeedsUpdate, ignore_error, commit_error_handler
 
@@ -205 +207 @@
 class SVNTest(SCMTest):
 
+    @staticmethod
+    def _set_date_and_reviewer(changelog_entry):
+        # Joe Cool matches the reviewer set in SCMTest._create_patch
+        changelog_entry = changelog_entry.replace('REVIEWER_HERE', 'Joe Cool')
+        # svn-apply will update ChangeLog entries with today's date.
+        return changelog_entry.replace('DATE_HERE', date.today().isoformat())
+
+    def test_svn_apply(self):
+        first_entry = """2009-10-26  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Foo Bar.
+
+        Most awesome change ever.
+
+        * scm_unittest.py:
+"""
+        intermediate_entry = """2009-10-27  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Baz Bar.
+
+        A more awesomer change yet!
+
+        * scm_unittest.py:
+"""
+        one_line_overlap_patch = """Index: ChangeLog
+===================================================================
+--- ChangeLog   (revision 5)
++++ ChangeLog   (working copy)
+@@ -1,5 +1,13 @@
+ 2009-10-26  Eric Seidel  <eric@webkit.org>
+
++        Reviewed by NOBODY (OOPS!).
++
++        Second most awsome change ever.
++
++        * scm_unittest.py:
++
++2009-10-26  Eric Seidel  <eric@webkit.org>
++
+         Reviewed by Foo Bar.
+
+         Most awesome change ever.
+"""
+        one_line_overlap_entry = """DATE_HERE  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by REVIEWER_HERE.
+
+        Second most awsome change ever.
+
+        * scm_unittest.py:
+"""
+        two_line_overlap_patch = """Index: ChangeLog
+===================================================================
+--- ChangeLog   (revision 5)
++++ ChangeLog   (working copy)
+@@ -2,6 +2,14 @@
+
+         Reviewed by Foo Bar.
+
++        Second most awsome change ever.
++
++        * scm_unittest.py:
++
++2009-10-26  Eric Seidel  <eric@webkit.org>
++
++        Reviewed by Foo Bar.
++
+         Most awesome change ever.
+
+         * scm_unittest.py:
+"""
+        two_line_overlap_entry = """DATE_HERE  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Foo Bar.
+
+        Second most awsome change ever.
+
+        * scm_unittest.py:
+"""
+        write_into_file_at_path('ChangeLog', first_entry)
+        run(['svn', 'add', 'ChangeLog'])
+        run(['svn', 'commit', '--quiet', '--message', 'ChangeLog commit'])
+
+        # Patch files were created against just 'first_entry'.
+        # Add a second commit to make svn-apply have to apply the patches with fuzz.
+        changelog_contents = "%s\n%s" % (intermediate_entry, first_entry)
+        write_into_file_at_path('ChangeLog', changelog_contents)
+        run(['svn', 'commit', '--quiet', '--message', 'Intermediate commit'])
+
+        self._setup_webkittools_scripts_symlink(self.scm)
+        self.scm.apply_patch(self._create_patch(one_line_overlap_patch))
+        expected_changelog_contents = "%s\n%s" % (self._set_date_and_reviewer(one_line_overlap_entry), changelog_contents)
+        self.assertEquals(read_from_path('ChangeLog'), expected_changelog_contents)
+
+        self.scm.revert_files(['ChangeLog'])
+        self.scm.apply_patch(self._create_patch(two_line_overlap_patch))
+        expected_changelog_contents = "%s\n%s" % (self._set_date_and_reviewer(two_line_overlap_entry), changelog_contents)
+        self.assertEquals(read_from_path('ChangeLog'), expected_changelog_contents)
+
     def setUp(self):
         SVNTestRepository.setup(self)