Apply patches from Phabricator in a smarter way

bot/code_review_bot/mercurial.py

@@ -361,11 +361,42 @@ def get_author(commit):
             message += f"Differential Diff: {patch.phid}"
 
             logger.info("Applying patch", phid=patch.phid, message=message)
+            patches = io.BytesIO(patch.patch.encode("utf-8"))
             try:
                 self.repo.import_(
-                    patches=io.BytesIO(patch.patch.encode("utf-8")),
+                    patches=patches,
                     message=message.encode("utf-8"),
                     user=user.encode("utf-8"),
+                    similarity=95,
+                )
+            except hglib.error.CommandError as e:
+                logger.warning(
+                    (
+                        f"Mercurial command from hglib failed: {e}. "
+                        "Retrying with --config ui.patch=patch."
+                    ),
+                    phid=patch.phid,
+                    exc_info=True,
+                )
+                patches.seek(0)
+                # Same method as repo.import_() but with the extra argument "--config ui.patch=patch".
+                # https://repo.mercurial-scm.org/python-hglib/file/484b56ac4aec/hglib/client.py#l959
+                self.repo.rawcommand(
+                    hglib.util.cmdbuilder(
+                        b"import",
+                        message=message.encode("utf-8"),
+                        user=user.encode("utf-8"),
+                        similarity=95,
+                        config="ui.patch=patch",
+                        *patches,
+                    )
+                )
+                # When using an external patch util mercurial won't automatically handle add/remove/renames
+                self.repo.rawcommand(
+                    hglib.util.cmdbuilder(
+                        b"addremove",
+                        similarity=95,
+                    )
                 )
             except Exception as e:
                 logger.info(

bot/tests/conftest.py

@@ -69,6 +69,7 @@ def mock_config(mock_repositories):
         "test",
         ["dom/*", "tests/*.py", "test/*.c"],
         mock_repositories,
+        github_api_token="test_token",
     )
 
     return settings
@@ -1034,6 +1035,7 @@ def build_repository(tmpdir, name):
 
     # Mock push to avoid reaching try server
     repo.push = MagicMock(return_value=True)
+    repo.rawcommand = MagicMock(wraps=repo.rawcommand)
 
     return repo
 

bot/tests/test_analysis.py

@@ -187,6 +187,7 @@ def mock_hgrun(cmd):
             b" @@\n+Hello World\n",
             {
                 "message": b"Random commit message\nDifferential Diff: PHID-DIFF-testABcd12",
+                "similarity": 95,
                 "user": b"test <[email protected]>",
             },
         ),

bot/tests/test_mercurial.py

@@ -13,12 +13,6 @@
 
 from code_review_bot import mercurial
 
-MERCURIAL_FAILURE = """unable to find 'crash.txt' for patching
-(use '--prefix' to apply patch relative to the current directory)
-1 out of 1 hunks FAILED -- saving rejects to file crash.txt.rej
-abort: patch failed to apply
-"""
-
 
 class STDOutputMock:
     def fileno(self):
@@ -456,7 +450,7 @@ def boom(*args):
     assert tip.node == initial.node
 
 
-def test_failure_mercurial(PhabricatorMock, mock_mc):
+def test_failure_mercurial(PhabricatorMock, mock_config, mock_mc):
     """
     Run mercurial worker on a single diff
     and check the treeherder link publication as an artifact
@@ -495,9 +489,35 @@ def test_failure_mercurial(PhabricatorMock, mock_mc):
 
     # Check the treeherder link was queued
     assert mode == "fail:mercurial"
+    import_calls = mock_mc.repo.rawcommand.call_args_list[-2:]
+    assert len(import_calls) == 2
+    assert [c.args[0] for c in import_calls] == [
+        [
+            b"import",
+            b"--message=This patch has no real base and will crash libmozevent\nDifferential Diff: PHID-DIFF-666",
+            b"--user=code review bot <[email protected]>",
+            b"--similarity=95",
+            b"-",
+        ],
+        [
+            b"import",
+            b"--message=This patch has no real base and will crash libmozevent\nDifferential Diff: PHID-DIFF-666",
+            b"--user=code review bot <[email protected]>",
+            b"--similarity=95",
+            b"--config=ui.patch=patch",
+            b"--",
+            b"--- a/crash.txt Thu Jan 01 00:00:00 2019 +0000\n",
+            b"+++ b/crash.txt  Fri Feb 08 10:54:26 2019 +0000\n",
+            b"@@ -12,0 +12,1 @@\n",
+            b"+This cannot apply !\n",
+            b"\n",
+        ],
+    ]
     assert out_build == build
     assert details["duration"] > 0
-    assert details["message"] == MERCURIAL_FAILURE
+    assert details["message"] == (
+        "abort: No such file or directory: '--- a/crash.txt Thu Jan 01 00:00:00 2019 +0000\n'\n"
+    )
 
     # Clone should not be modified
     tip = mock_mc.repo.tip()