Don't check file-by-file in presubmit if there are more than 50..
..changed files. Otherwise changes have a max-size because on Windows
the command-line length is limiting. This allows large CLs such as
https://crrev.com/c/2720369/1 to land.
Bug: chromium:1166108
Change-Id: I87c871a77e46ee8b300aab0acf748755ea165f1a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2727708
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 5e50e04..1405171 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -37,9 +37,9 @@
import time
EXCLUSIVE_CHANGE_DIRECTORIES = [
- [ 'third_party', 'v8' ],
- [ 'node_modules' ],
- [ 'OWNERS' ],
+ ['third_party', 'v8'],
+ ['node_modules'],
+ ['OWNERS'],
]
AUTOROLL_ACCOUNT = "devtools-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com"
@@ -50,7 +50,9 @@
script_path = [input_api.python_executable, script_path]
start_time = time.time()
- process = input_api.subprocess.Popen(script_path + args, stdout=input_api.subprocess.PIPE, stderr=input_api.subprocess.STDOUT)
+ process = input_api.subprocess.Popen(script_path + args,
+ stdout=input_api.subprocess.PIPE,
+ stderr=input_api.subprocess.STDOUT)
out, _ = process.communicate()
end_time = time.time()
@@ -67,6 +69,7 @@
if input_api.change.DISABLE_THIRD_PARTY_CHECK != None:
return []
results = [output_api.PresubmitNotifyResult('Directory Exclusivity Check:')]
+
def IsParentDir(file, dir):
while file != '':
if file == dir:
@@ -125,7 +128,8 @@
def _CheckBuildGN(input_api, output_api):
results = [output_api.PresubmitNotifyResult('Running BUILD.GN check:')]
- script_path = input_api.os_path.join(input_api.PresubmitLocalPath(), 'scripts', 'check_gn.js')
+ script_path = input_api.os_path.join(input_api.PresubmitLocalPath(),
+ 'scripts', 'check_gn.js')
results.extend(_checkWithNodeScript(input_api, output_api, script_path))
return results
@@ -157,13 +161,17 @@
def _CheckJSON(input_api, output_api):
results = [output_api.PresubmitNotifyResult('Running JSON Validator:')]
- script_path = input_api.os_path.join(input_api.PresubmitLocalPath(), 'scripts', 'json_validator', 'validate_module_json.js')
+ script_path = input_api.os_path.join(input_api.PresubmitLocalPath(),
+ 'scripts', 'json_validator',
+ 'validate_module_json.js')
results.extend(_checkWithNodeScript(input_api, output_api, script_path))
return results
def _CheckFormat(input_api, output_api):
- node_modules_affected_files = _getAffectedFiles(input_api, [input_api.os_path.join(input_api.PresubmitLocalPath(), 'node_modules')], [], [])
+ node_modules_affected_files = _getAffectedFiles(input_api, [
+ input_api.os_path.join(input_api.PresubmitLocalPath(), 'node_modules')
+ ], [], [])
# TODO(crbug.com/1068198): Remove once `git cl format --js` can handle large CLs.
if (len(node_modules_affected_files) > 0):
@@ -176,7 +184,8 @@
def _CheckDevtoolsLocalization(input_api, output_api, check_all_files=False): # pylint: disable=invalid-name
devtools_root = input_api.PresubmitLocalPath()
- script_path = input_api.os_path.join(devtools_root, 'scripts', 'test', 'run_localization_check.py')
+ script_path = input_api.os_path.join(devtools_root, 'scripts', 'test',
+ 'run_localization_check.py')
if check_all_files == True:
# Scan all files and fix any errors
args = ['--autofix', '--all']
@@ -296,18 +305,27 @@
input_api, output_api, lint_config_files, default_linted_directories,
['.css'], results)
- if not css_should_bail_out:
- script_args = ["--files"] + css_files_to_lint
- results.extend(
- _checkWithNodeScript(input_api, output_api, lint_path,
- script_args))
-
ts_should_bail_out, ts_files_to_lint = _getFilesToLint(
input_api, output_api, lint_config_files, default_linted_directories,
['.ts'], results)
+ # If there are more than 50 files to check, don't bother and check
+ # everything, so as to not run into command line length limits on Windows.
+ if not css_should_bail_out:
+ if len(css_files_to_lint) < 50:
+ script_args = ["--files"] + css_files_to_lint
+ else:
+ script_args = [] # The defaults check all CSS files.
+ results.extend(
+ _checkWithNodeScript(input_api, output_api, lint_path,
+ script_args))
+
if not ts_should_bail_out:
- script_args = ["--syntax", "html", "--files"] + ts_files_to_lint
+ script_args = ["--syntax", "html"]
+ if len(ts_files_to_lint) < 50:
+ script_args += ["--files"] + ts_files_to_lint
+ else:
+ script_args += ["--glob", "front_end/**/*.ts"]
results.extend(
_checkWithNodeScript(input_api, output_api, lint_path,
script_args))