Changeset 87641 in webkit

Timestamp:

May 29, 2011 12:38:46 PM (13 years ago)

Author:

dbates@webkit.org

Message:

2011-05-29 Daniel Bates <​dbates@rim.com>

Reviewed by David Kilzer.

REGRESSION (r86515): svn-apply ignores diffs that omit line count in chunk range
​https://bugs.webkit.org/show_bug.cgi?id=61162

Fixes an issue where svn-apply may ignore a diff that contains a chunk range line
that omits a line count. In particular, the chunk range regular expression does
not match a chunk range line that omits a line count. GNU diff(1) will omit the
line count in the chunk range if the line count is exactly 1. For example, appending
a new line to the end of an existing file F that contains exactly one line of text will
be represented in a diff with a chunk range line that omits the line count for F.

• Scripts/VCSUtils.pm: (parseChunkRange): Added.
• Scripts/webkitperl/VCSUtils_unittest/parseChunkRange.pl: Added.
• Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:
  • Added unit test "Git: Append new line to the end of an existing file".

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/VCSUtils.pm (modified) (6 diffs)
• Scripts/webkitperl/VCSUtils_unittest/parseChunkRange.pl (added)
• Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2011-05-29  Daniel Bates  <dbates@rim.com>
+
+        Reviewed by David Kilzer.
+
+        REGRESSION (r86515): svn-apply ignores diffs that omit line count in chunk range
+        https://bugs.webkit.org/show_bug.cgi?id=61162
+
+        Fixes an issue where svn-apply may ignore a diff that contains a chunk range line
+        that omits a line count. In particular, the chunk range regular expression does
+        not match a chunk range line that omits a line count. GNU diff(1) will omit the
+        line count in the chunk range if the line count is exactly 1. For example, appending
+        a new line to the end of an existing file F that contains exactly one line of text will
+        be represented in a diff with a chunk range line that omits the line count for F.
+
+        * Scripts/VCSUtils.pm:
+          (parseChunkRange): Added.
+        * Scripts/webkitperl/VCSUtils_unittest/parseChunkRange.pl: Added.
+        * Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:
+          - Added unit test "Git: Append new line to the end of an existing file".
+
 2011-05-28  Adam Barth  <abarth@webkit.org>
 

trunk/Tools/Scripts/VCSUtils.pm

@@ -69 +69 @@
         &mergeChangeLogs
         &normalizePath
+        &parseChunkRange
         &parseFirstEOL
         &parsePatch
@@ -99 +100 @@
 my $changeLogTimeZone = "PST8PDT";
 
-my $chunkRangeRegEx = qr#^\@\@ -(\d+),(\d+) \+\d+,(\d+) \@\@#; # e.g. "@@ -2,6 +2,18 @@" or "@@ -2,6 +2,18 @@ foo()"
 my $gitDiffStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#;
 my $svnDiffStartRegEx = qr#^Index: ([^\r\n]+)#;
@@ -484 +484 @@
     }
     return $eol;
+}
+
+# Parses a chunk range line into its components.
+#
+# A chunk range line has the form: @@ -L_1,N_1 +L_2,N_2 @@, where the pairs (L_1, N_1),
+# (L_2, N_2) are ranges that represent the starting line number and line count in the
+# original file and new file, respectively.
+#
+# Note, some versions of GNU diff may omit the comma and trailing line count (e.g. N_1),
+# in which case the omitted line count defaults to 1. For example, GNU diff may output
+# @@ -1 +1 @@, which is equivalent to @@ -1,1 +1,1 @@.
+#
+# This subroutine returns undef if given an invalid or malformed chunk range.
+#
+# Args:
+#   $line: the line to parse.
+#
+# Returns $chunkRangeHashRef
+#   $chunkRangeHashRef: a hash reference representing the parts of a chunk range, as follows--
+#     startingLine: the starting line in the original file.
+#     lineCount: the line count in the original file.
+#     newStartingLine: the new starting line in the new file.
+#     newLineCount: the new line count in the new file.
+sub parseChunkRange($)
+{
+    my ($line) = @_;
+    my $chunkRangeRegEx = qr#^\@\@ -(\d+)(,(\d+))? \+(\d+)(,(\d+))? \@\@#;
+    if ($line !~ /$chunkRangeRegEx/) {
+        return;
+    }
+    my %chunkRange;
+    $chunkRange{startingLine} = $1;
+    $chunkRange{lineCount} = defined($2) ? $3 : 1;
+    $chunkRange{newStartingLine} = $4;
+    $chunkRange{newLineCount} = defined($5) ? $6 : 1;
+    return \%chunkRange;
 }
 
@@ -906 +942 @@
         if ($line !~ $headerStartRegEx) {
             # Then we are in the body of the diff.
-            $numTextChunks += $line =~ /$chunkRangeRegEx/;
-            if ($indexPathEOL && $line !~ /$chunkRangeRegEx/) {
+            my $isChunkRange = defined(parseChunkRange($line));
+            $numTextChunks += 1 if $isChunkRange;
+            if ($indexPathEOL && !$isChunkRange) {
                 # The chunk range is part of the body of the diff, but its line endings should't be
                 # modified or patch(1) will complain. So, we only modify non-chunk range lines.
@@ -1501 +1538 @@
 
     # Update the initial chunk range.
-    if ($lines[$chunkStartIndex - 1] !~ /$chunkRangeRegEx/) {
+    my $chunkRangeHashRef = parseChunkRange($lines[$chunkStartIndex - 1]);
+    if (!$chunkRangeHashRef) {
         # FIXME: Handle errors differently from ChangeLog files that
         # are okay but should not be altered. That way we can find out
@@ -1508 +1546 @@
         return \%changeLogHashRef;
     }
-    my $oldSourceLineCount = $2;
-    my $oldTargetLineCount = $3;
+    my $oldSourceLineCount = $chunkRangeHashRef->{lineCount};
+    my $oldTargetLineCount = $chunkRangeHashRef->{newLineCount};
 
     my $sourceLineCount = $oldSourceLineCount + @overlappingLines - $deletedLineCount;

trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl

@@ -995 +995 @@
     expectedNextLine => undef,
 },
+{
+    # New test
+    diffName => "Git: Append new line to the end of an existing file",
+    inputText => <<'END',
+diff --git a/foo b/foo
+index 863339f..db418b2 100644
+--- a/foo
++++ b/foo
+@@ -1 +1,2 @@
+ Passed
++
+END
+    expectedReturn => [
+[{
+    svnConvertedText =>  <<'END',
+Index: foo
+index 863339f..db418b2 100644
+--- foo
++++ foo
+@@ -1 +1,2 @@
+ Passed
++
+END
+    indexPath => "foo",
+    isGit => 1,
+    numTextChunks => 1,
+}],
+undef],
+    expectedNextLine => undef,
+},
 {   # New test
     diffName => "Git: new file",