From e0a7c5d4af8227dab04e530a41ac2c024779028c Mon Sep 17 00:00:00 2001 From: "erg@chromium.org" Date: Mon, 23 Feb 2015 20:30:08 +0000 Subject: [PATCH] Run dartfmt when invoking git cl format. If the repository has third_party/dart-sdk/ unpacked, use that to format dart files modified in the current patch. BUG=459376 R=maruel@chromium.org, zra@google.com Review URL: https://codereview.chromium.org/933383002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294179 0039d316-1c4b-4281-b951-d872f2087c98 --- dart_format.py | 58 +++++++++++++++++++++++ gclient_utils.py | 28 +++++++---- git_cl.py | 96 ++++++++++++++++++++++++-------------- presubmit_canned_checks.py | 2 +- 4 files changed, 139 insertions(+), 45 deletions(-) create mode 100755 dart_format.py diff --git a/dart_format.py b/dart_format.py new file mode 100755 index 000000000..ee07efe2e --- /dev/null +++ b/dart_format.py @@ -0,0 +1,58 @@ +#!/usr/bin/python +# Copyright 2015 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +"""Redirects to the version of dartfmt checked into a gclient repo. + +dartfmt binaries are pulled down during gclient sync in the mojo repo. + +This tool is named dart_format.py instead of dartfmt to parallel +clang_format.py, which is in this same repository.""" + +import os +import subprocess +import sys + +import gclient_utils + +class NotFoundError(Exception): + """A file could not be found.""" + def __init__(self, e): + Exception.__init__(self, + 'Problem while looking for dartfmt in Chromium source tree:\n' + ' %s' % e) + + +def FindDartFmtToolInChromiumTree(): + """Return a path to the dartfmt executable, or die trying.""" + primary_solution_path = gclient_utils.GetPrimarySolutionPath() + if not primary_solution_path: + raise NotFoundError( + 'Could not find checkout in any parent of the current path.') + + dartfmt_path = os.path.join(primary_solution_path, 'third_party', 'dart-sdk', + 'dart-sdk', 'bin', 'dartfmt') + if not os.path.exists(dartfmt_path): + raise NotFoundError('File does not exist: %s' % dartfmt_path) + return dartfmt_path + + +def main(args): + try: + tool = FindDartFmtToolInChromiumTree() + except NotFoundError, e: + print >> sys.stderr, e + sys.exit(1) + + # Add some visibility to --help showing where the tool lives, since this + # redirection can be a little opaque. + help_syntax = ('-h', '--help', '-help', '-help-list', '--help-list') + if any(match in args for match in help_syntax): + print '\nDepot tools redirects you to the dartfmt at:\n %s\n' % tool + + return subprocess.call([tool] + sys.argv[1:]) + + +if __name__ == '__main__': + sys.exit(main(sys.argv)) diff --git a/gclient_utils.py b/gclient_utils.py index 96989a99e..f80cd78e4 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -662,15 +662,8 @@ def GetMacWinOrLinux(): raise Error('Unknown platform: ' + sys.platform) -def GetBuildtoolsPath(): - """Returns the full path to the buildtools directory. - This is based on the root of the checkout containing the current directory.""" - - # Overriding the build tools path by environment is highly unsupported and may - # break without warning. Do not rely on this for anything important. - override = os.environ.get('CHROMIUM_BUILDTOOLS_PATH') - if override is not None: - return override +def GetPrimarySolutionPath(): + """Returns the full path to the primary solution. (gclient_root + src)""" gclient_root = FindGclientRoot(os.getcwd()) if not gclient_root: @@ -691,9 +684,24 @@ def GetBuildtoolsPath(): # Some projects' top directory is not named 'src'. source_dir_name = GetGClientPrimarySolutionName(gclient_root) or 'src' - buildtools_path = os.path.join(gclient_root, source_dir_name, 'buildtools') + return os.path.join(gclient_root, source_dir_name) + + +def GetBuildtoolsPath(): + """Returns the full path to the buildtools directory. + This is based on the root of the checkout containing the current directory.""" + + # Overriding the build tools path by environment is highly unsupported and may + # break without warning. Do not rely on this for anything important. + override = os.environ.get('CHROMIUM_BUILDTOOLS_PATH') + if override is not None: + return override + + primary_solution = GetPrimarySolutionPath() + buildtools_path = os.path.join(primary_solution, 'buildtools') if not os.path.exists(buildtools_path): # Buildtools may be in the gclient root. + gclient_root = FindGclientRoot(os.getcwd()) buildtools_path = os.path.join(gclient_root, 'buildtools') return buildtools_path diff --git a/git_cl.py b/git_cl.py index 1b375e856..728bae7ab 100755 --- a/git_cl.py +++ b/git_cl.py @@ -36,6 +36,7 @@ from third_party import colorama from third_party import upload import breakpad # pylint: disable=W0611 import clang_format +import dart_format import fix_encoding import gclient_utils import git_common @@ -2894,6 +2895,26 @@ def CMDowners(parser, args): disable_color=options.no_color).run() +def BuildGitDiffCmd(diff_type, upstream_commit, args, extensions): + """Generates a diff command.""" + # Generate diff for the current branch's changes. + diff_cmd = ['diff', '--no-ext-diff', '--no-prefix', diff_type, + upstream_commit, '--' ] + + if args: + for arg in args: + if os.path.isdir(arg): + diff_cmd.extend(os.path.join(arg, '*' + ext) for ext in extensions) + elif os.path.isfile(arg): + diff_cmd.append(arg) + else: + DieWithError('Argument "%s" is not a file or a directory' % arg) + else: + diff_cmd.extend('*' + ext for ext in extensions) + + return diff_cmd + + @subcommand.usage('[files or directories to diff]') def CMDformat(parser, args): """Runs clang-format on the diff.""" @@ -2912,15 +2933,6 @@ def CMDformat(parser, args): if rel_base_path: os.chdir(rel_base_path) - # Generate diff for the current branch's changes. - diff_cmd = ['diff', '--no-ext-diff', '--no-prefix'] - if opts.full: - # Only list the names of modified files. - diff_cmd.append('--name-only') - else: - # Only generate context-less patches. - diff_cmd.append('-U0') - # Grab the merge-base commit, i.e. the upstream commit of the current # branch when it was created or the last time it was rebased. This is # to cover the case where the user may have called "git fetch origin", @@ -2936,20 +2948,14 @@ def CMDformat(parser, args): DieWithError('Could not find base commit for this branch. ' 'Are you in detached state?') - diff_cmd.append(upstream_commit) - - # Handle source file filtering. - diff_cmd.append('--') - if args: - for arg in args: - if os.path.isdir(arg): - diff_cmd += [os.path.join(arg, '*' + ext) for ext in CLANG_EXTS] - elif os.path.isfile(arg): - diff_cmd.append(arg) - else: - DieWithError('Argument "%s" is not a file or a directory' % arg) + if opts.full: + # Only list the names of modified files. + clang_diff_type = '--name-only' else: - diff_cmd += ['*' + ext for ext in CLANG_EXTS] + # Only generate context-less patches. + clang_diff_type = '-U0' + + diff_cmd = BuildGitDiffCmd(clang_diff_type, upstream_commit, args, CLANG_EXTS) diff_output = RunGit(diff_cmd) top_dir = os.path.normpath( @@ -2961,18 +2967,20 @@ def CMDformat(parser, args): except clang_format.NotFoundError, e: DieWithError(e) + # Set to 2 to signal to CheckPatchFormatted() that this patch isn't + # formatted. This is used to block during the presubmit. + return_value = 0 + if opts.full: # diff_output is a list of files to send to clang-format. files = diff_output.splitlines() - if not files: - print "Nothing to format." - return 0 - cmd = [clang_format_tool] - if not opts.dry_run and not opts.diff: - cmd.append('-i') - stdout = RunCommand(cmd + files, cwd=top_dir) - if opts.diff: - sys.stdout.write(stdout) + if files: + cmd = [clang_format_tool] + if not opts.dry_run and not opts.diff: + cmd.append('-i') + stdout = RunCommand(cmd + files, cwd=top_dir) + if opts.diff: + sys.stdout.write(stdout) else: env = os.environ.copy() env['PATH'] = str(os.path.dirname(clang_format_tool)) @@ -2991,9 +2999,29 @@ def CMDformat(parser, args): if opts.diff: sys.stdout.write(stdout) if opts.dry_run and len(stdout) > 0: - return 2 - - return 0 + return_value = 2 + + # Build a diff command that only operates on dart files. dart's formatter + # does not have the nice property of only operating on modified chunks, so + # hard code full. + dart_diff_cmd = BuildGitDiffCmd('--name-only', upstream_commit, + args, ['.dart']) + dart_diff_output = RunGit(dart_diff_cmd) + if dart_diff_output: + try: + command = [dart_format.FindDartFmtToolInChromiumTree()] + if not opts.dry_run and not opts.diff: + command.append('-w') + command.extend(dart_diff_output.splitlines()) + + stdout = RunCommand(command, cwd=top_dir, env=env) + if opts.dry_run and stdout: + return_value = 2 + except dart_format.NotFoundError as e: + print ('Unable to check dart code formatting. Dart SDK is not in ' + + 'this checkout.') + + return return_value def CMDlol(parser, args): diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 4e55d5087..1e2d1ab62 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1097,7 +1097,7 @@ def CheckPatchFormatted(input_api, output_api): code, _ = git_cl.RunGitWithCode(cmd, suppress_stderr=True) if code == 2: return [output_api.PresubmitPromptWarning( - 'The %s directory requires clang-formatting. ' + 'The %s directory requires source formatting. ' 'Please run git cl format %s' % (input_api.basename(input_api.PresubmitLocalPath()), input_api.basename(input_api.PresubmitLocalPath())))]