Add minimal presubmit patch check (#26807)

* Update presubmits a bit.

* Prevent unnecessary empty lines modification in patches.

PRESUBMIT.py

@@ -13,7 +13,6 @@ import brave_node
 import chromium_presubmit_overrides
 import override_utils
 
-USE_PYTHON3 = True
 PRESUBMIT_VERSION = '2.0.0'
 
 

chromium_presubmit_config.json5

@@ -30,6 +30,8 @@
       "CheckTreeIsOpen",
       // We run our own format check.
       "CheckPatchFormatted",
+      // We don't use Chromium LSC process for Brave repository.
+      "CheckLargeScaleChange",
     ],
   },
 

chromium_src/PRESUBMIT.py

@@ -6,7 +6,6 @@
 import brave_chromium_utils
 import chromium_presubmit_overrides
 
-USE_PYTHON3 = True
 PRESUBMIT_VERSION = '2.0.0'
 
 

patches/PRESUBMIT.py

@@ -0,0 +1,54 @@
+# Copyright (c) 2024 The Brave Authors. All rights reserved.
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at https://mozilla.org/MPL/2.0/.
+
+import chromium_presubmit_overrides
+
+PRESUBMIT_VERSION = '2.0.0'
+
+
+# Adds support for chromium_presubmit_config.json5 and some helpers.
+def CheckToModifyInputApi(input_api, _output_api):
+    chromium_presubmit_overrides.modify_input_api(input_api)
+    return []
+
+
+def CheckPatchFile(input_api, output_api):
+    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())
+        for lineno, line in enumerate(contents, 1):
+            # Prevent removing empty lines, it's a guaranteed git conflict.
+            if line == '-':
+                items.append(f'{f.LocalPath()}:{lineno} (removed empty line)')
+                continue
+
+            # Prevent adding empty lines at diff block boundaries. This doesn't
+            # affect git conflict resolution, but adds unnecessary noise. It's
+            # okay to have empty lines in the middle of a diff block.
+            if line != '+':
+                continue
+
+            # Look at previous and next lines to determine if this empty line is
+            # at diff block boundary.
+            prev_line = contents[lineno - 2] if lineno > 1 else ''
+            next_line = contents[lineno] if lineno < len(contents) else ''
+
+            # Empty line at block boundary or single-line block.
+            if not (prev_line.startswith('+') and next_line.startswith('+')):
+                items.append(f'{f.LocalPath()}:{lineno} (added empty line)')
+
+    if not items:
+        return []
+
+    return [
+        output_api.PresubmitError(
+            'Patch file should not add or remove empty lines', items)
+    ]

patches/PRESUBMIT_test.py

@@ -0,0 +1,119 @@
+# Copyright (c) 2024 The Brave Authors. All rights reserved.
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at https://mozilla.org/MPL/2.0/.
+
+import unittest
+import brave_chromium_utils
+
+# pylint: disable=import-error,no-member
+import PRESUBMIT
+
+with brave_chromium_utils.sys_path("//"):
+    from PRESUBMIT_test_mocks import MockFile, MockAffectedFile
+    from PRESUBMIT_test_mocks import MockInputApi, MockOutputApi
+
+
+class PatchFileTest(unittest.TestCase):
+
+    def testEmptyLinesInPatch(self):
+        cases = [
+            {
+                'name': 'valid patch without empty lines',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '+void func() {',
+                    '+  DoSomething();',
+                    '+}',
+                ],
+                'expected_errors': 0
+            },
+            {
+                'name': 'added single empty line',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '+',
+                ],
+                'expected_errors': 1
+            },
+            {
+                'name': 'added multiple empty lines',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '+',
+                    '+',
+                ],
+                'expected_errors': 1
+            },
+            {
+                'name': 'removed empty line',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '-',
+                ],
+                'expected_errors': 1
+            },
+            {
+                'name': 'empty line at start of block',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '+',
+                    '+void func() {',
+                    '+  DoSomething();',
+                    '+}',
+                ],
+                'expected_errors': 1
+            },
+            {
+                'name': 'empty line at end of block',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '+void func() {',
+                    '+  DoSomething();',
+                    '+}',
+                    '+',
+                ],
+                'expected_errors': 1
+            },
+            {
+                'name': 'empty line in middle is ok',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    '+void func() {',
+                    '+',
+                    '+  DoSomething();',
+                    '+}',
+                ],
+                'expected_errors': 0
+            },
+            {
+                'name': 'removed empty line',
+                'contents': [
+                    'diff --git a/file.cc b/file.cc',
+                    ' void func() {',
+                    '-',
+                    '   DoSomething();',
+                    ' }',
+                ],
+                'expected_errors': 1
+            },
+        ]
+
+        for case in cases:
+            with self.subTest(name=case['name']):
+                affected_file = MockAffectedFile('test.patch',
+                                                 case['contents'])
+                input_api = MockInputApi()
+                input_api.files = [affected_file]
+
+                results = PRESUBMIT.CheckPatchFile(input_api, MockOutputApi())
+
+                actual_errors = len(results)
+                self.assertEqual(
+                    case['expected_errors'], actual_errors,
+                    f"Expected {case['expected_errors']} errors, got {actual_errors}"
+                )
+
+
+if __name__ == '__main__':
+    unittest.main()

tools/perf/PRESUBMIT.py

@@ -7,7 +7,6 @@ import pathlib
 import platform
 import os
 
-USE_PYTHON3 = True
 PRESUBMIT_VERSION = '2.0.0'