[presubmit] Patches should use --default-prefix and similar opts (#36157)

The patching machinery in Brave usually creates patches with the
equivalent of:

```
git -C .. diff --src-prefix=a/ --dst-prefix=b/ --default-prefix --full-index --ignore-space-at-eol browser/foo.cc
```

This is not enforced officially, and every now and then someone
generates patches manually and gets them merged with some other type of
header, which ends up causing issues when syncing, or using other types
of tooling that assume certain expectations from these patches. This PR
prevents that.

Another check being added is to validate that a patch added to this path
always have one file only in it, as this is an invariant of our patching
system.

Resolves https://github.com/brave/brave-browser/issues/55231

patches/PRESUBMIT.py

@@ -5,8 +5,13 @@
 
 import chromium_presubmit_overrides
 import os
+import re
+
+# Full 40-char SHA hashes as produced by `git diff --full-index`.
+_FULL_INDEX_RE = re.compile(r'^index ([0-9a-f]{40})\.\.([0-9a-f]{40})( \d+)?$')
 
 PRESUBMIT_VERSION = '2.0.0'
+TEST_FILE_PATTERN = [r'.+_test\.py$']
 
 
 # Adds support for chromium_presubmit_config.json5 and some helpers.
@@ -15,6 +20,11 @@ def CheckToModifyInputApi(input_api, _output_api):
     return []
 
 
+def CheckTests(input_api, output_api):
+    return input_api.canned_checks.RunUnitTestsInDirectory(
+        input_api, output_api, '.', files_to_check=TEST_FILE_PATTERN)
+
+
 def CheckPatchFile(input_api, output_api):
     files_to_check = (r'.+\.patch$', )
     files_to_skip = ()
@@ -98,3 +108,91 @@ def CheckPatchFileSource(input_api, output_api):
         output_api.PresubmitError(
             'Patch filename does not match source file in diff header', items)
     ]
+
+
+def CheckPatchFileIndexHeader(input_api, output_api):
+    """Checks that patch files use full SHA hashes in the index line.
+
+    Patches must be generated with `git diff --full-index` so that index lines
+    contain full 40-character SHA hashes rather than abbreviated ones.
+
+    Expected header format:
+        diff --git a/path/to/file.cc b/path/to/file.cc
+        index a1b2c3d4e5f6...(40 hex chars)..f6e5d4c3b2a1 100644
+        --- a/path/to/file.cc
+        +++ b/path/to/file.cc
+    """
+    files_to_check = (r'.+\.patch$', )
+    files_to_skip = ()
+
+    file_filter = lambda f: input_api.FilterSourceFile(
+        f, files_to_check=files_to_check, files_to_skip=files_to_skip)
+
+    items = []
+    for f in input_api.AffectedSourceFiles(file_filter):
+        contents = list(f.NewContents())
+        if not contents:
+            continue
+
+        # The index line should appear within the first few header lines.
+        index_line = None
+        for i, line in enumerate(contents[:5]):
+            if line.startswith('index '):
+                index_line = (i + 1, line)
+                break
+
+        if index_line is None:
+            items.append(
+                f'{f.LocalPath()}: missing index line in patch header')
+            continue
+
+        lineno, line = index_line
+        if not _FULL_INDEX_RE.match(line):
+            items.append(f'{f.LocalPath()}:{lineno}: index line must use full'
+                         ' 40-character SHA hashes (generate patches with'
+                         ' `git diff --full-index`).\n'
+                         f'  Found: {line}')
+
+    if not items:
+        return []
+
+    return [
+        output_api.PresubmitError(
+            'Patch index line does not use full SHA hashes', items)
+    ]
+
+
+def CheckPatchFileSingleSource(input_api, output_api):
+    """Checks that each patch file patches exactly one source file.
+
+    Each .patch file must contain a single `diff --git` section. Patching
+    multiple files in one .patch file is not allowed; use one patch file per
+    source file instead.
+    """
+    files_to_check = (r'.+\.patch$', )
+    files_to_skip = ()
+
+    file_filter = lambda f: input_api.FilterSourceFile(
+        f, files_to_check=files_to_check, files_to_skip=files_to_skip)
+
+    items = []
+    for f in input_api.AffectedSourceFiles(file_filter):
+        contents = list(f.NewContents())
+
+        diff_headers = [(i + 1, line) for i, line in enumerate(contents)
+                        if line.startswith('diff --git ')]
+
+        if len(diff_headers) != 1:
+            count = len(diff_headers)
+            detail = (f'  Found {count} diff --git headers'
+                      if count > 1 else '  No diff --git header found')
+            items.append(f'{f.LocalPath()}: patch must contain exactly one'
+                         f' source file.\n{detail}')
+
+    if not items:
+        return []
+
+    return [
+        output_api.PresubmitError(
+            'Patch file must patch exactly one source file', items)
+    ]

patches/PRESUBMIT_test.py

@@ -10,7 +10,7 @@ import brave_chromium_utils
 import PRESUBMIT
 
 with brave_chromium_utils.sys_path("//"):
-    from PRESUBMIT_test_mocks import MockFile, MockAffectedFile
+    from PRESUBMIT_test_mocks import MockAffectedFile
     from PRESUBMIT_test_mocks import MockInputApi, MockOutputApi
 
 
@@ -134,8 +134,200 @@ class PatchFileTest(unittest.TestCase):
                 actual_errors = len(results)
                 self.assertEqual(
                     case['expected_errors'], actual_errors,
-                    f"Expected {case['expected_errors']} errors, got {actual_errors}"
+                    f"Expected {case['expected_errors']} errors,"
-                )
+                    f" got {actual_errors}")
+
+
+class PatchFileSourceTest(unittest.TestCase):
+
+    def _run(self, contents, filename):
+        affected_file = MockAffectedFile(filename, contents)
+        input_api = MockInputApi()
+        input_api.files = [affected_file]
+        return PRESUBMIT.CheckPatchFileSource(input_api, MockOutputApi())
+
+    def testFilenameMatchesConventionPasses(self):
+        # base/metrics/foo.h  ->  base-metrics-foo.h.patch
+        results = self._run(
+            contents=['diff --git a/base/metrics/foo.h b/base/metrics/foo.h'],
+            filename='base-metrics-foo.h.patch')
+        self.assertEqual(0, len(results))
+
+    def testFilenameDeepPathPasses(self):
+        # chrome/browser/ui/views/frame/bar.cc
+        #   -> chrome-browser-ui-views-frame-bar.cc.patch
+        results = self._run(
+            contents=[
+                'diff --git a/chrome/browser/ui/views/frame/bar.cc'
+                ' b/chrome/browser/ui/views/frame/bar.cc'
+            ],
+            filename='chrome-browser-ui-views-frame-bar.cc.patch')
+        self.assertEqual(0, len(results))
+
+    def testWrongFilenameFails(self):
+        results = self._run(
+            contents=['diff --git a/base/metrics/foo.h b/base/metrics/foo.h'],
+            filename='wrong-name.patch')
+        self.assertEqual(1, len(results))
+
+    def testMissingDirectoryInFilenameFails(self):
+        # Header says base/metrics/foo.h but filename omits the directory level.
+        results = self._run(
+            contents=['diff --git a/base/metrics/foo.h b/base/metrics/foo.h'],
+            filename='base-foo.h.patch')
+        self.assertEqual(1, len(results))
+
+    def testEmptyPatchFails(self):
+        results = self._run(contents=[], filename='base-foo.h.patch')
+        self.assertEqual(1, len(results))
+
+    def testMissingDiffHeaderFails(self):
+        results = self._run(contents=['--- a/base/foo.h', '+++ b/base/foo.h'],
+                            filename='base-foo.h.patch')
+        self.assertEqual(1, len(results))
+
+    def testNonPatchFileIgnored(self):
+        results = self._run(contents=['diff --git a/base/foo.h b/base/foo.h'],
+                            filename='base-foo.h.txt')
+        self.assertEqual(0, len(results))
+
+
+_FULL_SHA = 'a' * 40
+_FULL_SHA2 = 'b' * 40
+_SHORT_SHA = 'abc1234'
+_SHORT_SHA2 = 'def5678'
+
+
+class PatchFileIndexHeaderTest(unittest.TestCase):
+
+    def _run(self, contents, filename='foo-bar.patch'):
+        affected_file = MockAffectedFile(filename, contents)
+        input_api = MockInputApi()
+        input_api.files = [affected_file]
+        return PRESUBMIT.CheckPatchFileIndexHeader(input_api, MockOutputApi())
+
+    def testValidFullIndexHeader(self):
+        results = self._run([
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            f'index {_FULL_SHA}..{_FULL_SHA2} 100644',
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+            '@@ -1 +1 @@',
+            '+// brave',
+        ])
+        self.assertEqual(0, len(results))
+
+    def testAbbreviatedShaFails(self):
+        results = self._run([
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            f'index {_SHORT_SHA}..{_SHORT_SHA2} 100644',
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+        ])
+        self.assertEqual(1, len(results))
+
+    def testMissingIndexLineFails(self):
+        results = self._run([
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+        ])
+        self.assertEqual(1, len(results))
+
+    def testEmptyPatchSkipped(self):
+        results = self._run([])
+        self.assertEqual(0, len(results))
+
+    def testExecutableModeInIndexLine(self):
+        results = self._run([
+            'diff --git a/foo/bar.py b/foo/bar.py',
+            f'index {_FULL_SHA}..{_FULL_SHA2} 100755',
+            '--- a/foo/bar.py',
+            '+++ b/foo/bar.py',
+        ])
+        self.assertEqual(0, len(results))
+
+    def testIndexLineWithoutMode(self):
+        # Some git diff outputs omit the mode when it doesn't change.
+        results = self._run([
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            f'index {_FULL_SHA}..{_FULL_SHA2}',
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+        ])
+        self.assertEqual(0, len(results))
+
+    def testNonPatchFileIgnored(self):
+        affected_file = MockAffectedFile('foo.txt', [
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            f'index {_SHORT_SHA}..{_SHORT_SHA2} 100644',
+        ])
+        input_api = MockInputApi()
+        input_api.files = [affected_file]
+        results = PRESUBMIT.CheckPatchFileIndexHeader(input_api,
+                                                      MockOutputApi())
+        self.assertEqual(0, len(results))
+
+
+class PatchFileSingleSourceTest(unittest.TestCase):
+
+    def _run(self, contents, filename='foo-bar.patch'):
+        affected_file = MockAffectedFile(filename, contents)
+        input_api = MockInputApi()
+        input_api.files = [affected_file]
+        return PRESUBMIT.CheckPatchFileSingleSource(input_api, MockOutputApi())
+
+    def testSingleSourcePasses(self):
+        results = self._run([
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            f'index {_FULL_SHA}..{_FULL_SHA2} 100644',
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+            '@@ -1 +1 @@',
+            '+// brave',
+        ])
+        self.assertEqual(0, len(results))
+
+    def testMultipleSourcesFails(self):
+        results = self._run([
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            f'index {_FULL_SHA}..{_FULL_SHA2} 100644',
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+            '@@ -1 +1 @@',
+            '+// brave',
+            'diff --git a/foo/baz.cc b/foo/baz.cc',
+            f'index {_FULL_SHA}..{_FULL_SHA2} 100644',
+            '--- a/foo/baz.cc',
+            '+++ b/foo/baz.cc',
+            '@@ -1 +1 @@',
+            '+// also brave',
+        ])
+        self.assertEqual(1, len(results))
+
+    def testNoDiffHeaderFails(self):
+        results = self._run([
+            '--- a/foo/bar.cc',
+            '+++ b/foo/bar.cc',
+            '@@ -1 +1 @@',
+            '+// brave',
+        ])
+        self.assertEqual(1, len(results))
+
+    def testEmptyPatchFails(self):
+        results = self._run([])
+        self.assertEqual(1, len(results))
+
+    def testNonPatchFileIgnored(self):
+        affected_file = MockAffectedFile('foo.txt', [
+            'diff --git a/foo/bar.cc b/foo/bar.cc',
+            'diff --git a/foo/baz.cc b/foo/baz.cc',
+        ])
+        input_api = MockInputApi()
+        input_api.files = [affected_file]
+        results = PRESUBMIT.CheckPatchFileSingleSource(input_api,
+                                                       MockOutputApi())
+        self.assertEqual(0, len(results))
 
 
 if __name__ == '__main__':