Fixing the diff_parser to correctly identify git diffs even with leading comments.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Jan 2013 05:06:31 +0000 (05:06 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Jan 2013 05:06:31 +0000 (05:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=107871

Patch by Tim 'mithro' Ansell <mithro@mithis.com> on 2013-01-24
Reviewed by Eric Seidel.

* Scripts/webkitpy/common/checkout/diff_parser.py:
* Scripts/webkitpy/common/checkout/diff_parser_unittest.py:
* Scripts/webkitpy/tool/steps/haslanded.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140775 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/checkout/diff_parser.py
Tools/Scripts/webkitpy/common/checkout/diff_parser_unittest.py
Tools/Scripts/webkitpy/tool/steps/haslanded.py

index db4bf4c..8acca4e 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-24  Tim 'mithro' Ansell  <mithro@mithis.com>
+
+        Fixing the diff_parser to correctly identify git diffs even with leading comments.
+        https://bugs.webkit.org/show_bug.cgi?id=107871
+
+        Reviewed by Eric Seidel.
+
+        * Scripts/webkitpy/common/checkout/diff_parser.py:
+        * Scripts/webkitpy/common/checkout/diff_parser_unittest.py:
+        * Scripts/webkitpy/tool/steps/haslanded.py:
+
 2013-01-24  James Robinson  <jamesr@chromium.org>
 
         [chromium] Remove dead transitional code from WebViewImpl
index 3c5b45b..3a9ea92 100644 (file)
@@ -69,19 +69,27 @@ def git_diff_to_svn_diff(line):
     return line
 
 
+# This function exists so we can unittest get_diff_converter function
+def svn_diff_to_svn_diff(line):
+    return line
+
+
 # FIXME: This method belongs on DiffParser
-def get_diff_converter(first_diff_line):
+def get_diff_converter(lines):
     """Gets a converter function of diff lines.
 
     Args:
-      first_diff_line: The first filename line of a diff file.
-                       If this line is git formatted, we'll return a
-                       converter from git to SVN.
+      lines: The lines of a diff file.
+             If this line is git formatted, we'll return a
+             converter from git to SVN.
     """
-    if match(r"^diff --git \w/", first_diff_line):
-        return git_diff_to_svn_diff
-    return lambda input: input
-
+    for i, line in enumerate(lines[:-1]):
+        # Stop when we find the first patch
+        if line[:3] == "+++" and lines[i + 1] == "---":
+            break
+        if match(r"^diff --git \w/", line):
+            return git_diff_to_svn_diff
+    return svn_diff_to_svn_diff
 
 _INITIAL_STATE = 1
 _DECLARED_FILE_PATH = 2
@@ -142,10 +150,9 @@ class DiffParser(object):
         current_file = None
         old_diff_line = None
         new_diff_line = None
+        transform_line = get_diff_converter(diff_input)
         for line in diff_input:
             line = line.rstrip("\n")
-            if state == _INITIAL_STATE:
-                transform_line = get_diff_converter(line)
             line = transform_line(line)
 
             file_declaration = match(r"^Index: (?P<FilePath>.+)", line)
index 832adad..78dab26 100644 (file)
@@ -78,6 +78,42 @@ class DiffParserTest(unittest.TestCase):
         self.assertEqual(1, len(diff.lines))
         self.assertEqual((0, 1), diff.lines[0][0:2])
 
+    def test_diff_converter(self):
+        comment_lines = [
+            "Hey guys,\n",
+            "\n",
+            "See my awesome patch below!\n",
+            "\n",
+            " - Cool Hacker\n",
+            "\n",
+            ]
+
+        revision_lines = [
+            "Subversion Revision 289799\n",
+            ]
+
+        svn_diff_lines = [
+            "Index: Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
+            "===================================================================\n",
+            "--- Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
+            "+++ Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
+            "@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):\n",
+            ]
+        self.assertEqual(diff_parser.get_diff_converter(svn_diff_lines), diff_parser.svn_diff_to_svn_diff)
+        self.assertEqual(diff_parser.get_diff_converter(comment_lines + svn_diff_lines), diff_parser.svn_diff_to_svn_diff)
+        self.assertEqual(diff_parser.get_diff_converter(revision_lines + svn_diff_lines), diff_parser.svn_diff_to_svn_diff)
+
+        git_diff_lines = [
+            "diff --git a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
+            "index 3c5b45b..0197ead 100644\n",
+            "--- a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
+            "+++ b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
+            "@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):\n",
+            ]
+        self.assertEqual(diff_parser.get_diff_converter(git_diff_lines), diff_parser.git_diff_to_svn_diff)
+        self.assertEqual(diff_parser.get_diff_converter(comment_lines + git_diff_lines), diff_parser.git_diff_to_svn_diff)
+        self.assertEqual(diff_parser.get_diff_converter(revision_lines + git_diff_lines), diff_parser.git_diff_to_svn_diff)
+
     def test_git_mnemonicprefix(self):
         p = re.compile(r' ([a|b])/')
 
index 116a340..b0692b3 100644 (file)
@@ -46,7 +46,7 @@ class HasLanded(confirmdiff.ConfirmDiff):
     @classmethod
     def convert_to_svn(cls, diff):
         lines = StringIO.StringIO(diff).readlines()
-        convert = diff_parser.get_diff_converter(lines[0])
+        convert = diff_parser.get_diff_converter(lines)
         return "".join(convert(x) for x in lines)
 
     @classmethod