From 3b7e15c58f3b4a26fbcdab24d376dc1fa37bbb63 Mon Sep 17 00:00:00 2001 From: "enne@chromium.org" Date: Tue, 21 Jan 2014 17:44:47 +0000 Subject: [PATCH] Add a canned clang-format presubmit check For simplicity, the canned presubmit check just calls git cl format directly with a new option --dry-run which does not change the files on disk. Because some users may not have git or clang-format might somehow have errors, this canned check is a warning. Additionally, if git cl format fails with an error then the presubmit check will silently report success to avoid spamming misconfigured users during upload. BUG=none Review URL: https://codereview.chromium.org/141493002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@246066 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 28 +++++++++++++++++++++------- presubmit_canned_checks.py | 12 ++++++++++++ tests/presubmit_unittest.py | 1 + 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/git_cl.py b/git_cl.py index f6c1ae7d7..d9e2d9803 100755 --- a/git_cl.py +++ b/git_cl.py @@ -80,15 +80,20 @@ def RunGit(args, **kwargs): return RunCommand(['git'] + args, **kwargs) -def RunGitWithCode(args): +def RunGitWithCode(args, suppress_stderr=False): """Returns return code and stdout.""" try: env = os.environ.copy() # 'cat' is a magical git string that disables pagers on all platforms. env['GIT_PAGER'] = 'cat' + if suppress_stderr: + stderr = subprocess2.VOID + else: + stderr = sys.stderr out, code = subprocess2.communicate(['git'] + args, env=env, - stdout=subprocess2.PIPE) + stdout=subprocess2.PIPE, + stderr=stderr) return code, out[0] except ValueError: # When the subprocess fails, it returns None. That triggers a ValueError @@ -2301,7 +2306,10 @@ def CMDowners(parser, args): def CMDformat(parser, args): """Runs clang-format on the diff.""" CLANG_EXTS = ['.cc', '.cpp', '.h'] - parser.add_option('--full', action='store_true', default=False) + parser.add_option('--full', action='store_true', + help='Reformat the full content of all touched files') + parser.add_option('--dry-run', action='store_true', + help='Don\'t modify any file on disk.') opts, args = parser.parse_args(args) if args: parser.error('Unrecognized args: %s' % ' '.join(args)) @@ -2358,8 +2366,10 @@ def CMDformat(parser, args): if not files: print "Nothing to format." return 0 - RunCommand([clang_format_tool, '-i'] + files, - cwd=top_dir) + cmd = [clang_format_tool] + if not opts.dry_run: + cmd.append('-i') + stdout = RunCommand(cmd + files, cwd=top_dir) else: env = os.environ.copy() env['PATH'] = os.path.dirname(clang_format_tool) @@ -2370,9 +2380,13 @@ def CMDformat(parser, args): except clang_format.NotFoundError, e: DieWithError(e) - cmd = [sys.executable, script, '-p0', '-i'] + cmd = [sys.executable, script, '-p0'] + if not opts.dry_run: + cmd.append('-i') - RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env) + stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env) + if opts.dry_run and len(stdout) > 0: + return 2 return 0 diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index b11102a94..d52ad9582 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1041,3 +1041,15 @@ def PanProjectChecks(input_api, output_api, input_api, output_api)) snapshot("done") return results + + +def CheckPatchFormatted(input_api, output_api): + import git_cl + code, _ = git_cl.RunGitWithCode(['cl', 'format', '--dry-run'], + suppress_stderr=True) + if code == 2: + return [output_api.PresubmitPromptWarning( + 'Your patch is not formatted, please run git cl format.')] + # As this is just a warning, ignore all other errors if the user + # happens to have a broken clang-format, doesn't use git, etc etc. + return [] diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index d0d3ab4e3..28bf5bcec 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1752,6 +1752,7 @@ class CannedChecksUnittest(PresubmitTestsBase): 'CheckLongLines', 'CheckTreeIsOpen', 'PanProjectChecks', 'CheckLicense', 'CheckOwners', + 'CheckPatchFormatted', 'CheckRietveldTryJobExecution', 'CheckSingletonInHeaders', 'CheckSvnModifiedDirectories',