From 51fc4043965b9fa90da17b8a53e722409ba476f2 Mon Sep 17 00:00:00 2001 From: Andi-Bogdan Postelnicu Date: Wed, 31 Jan 2018 11:02:23 +0200 Subject: [PATCH] Bug 1405554 - Merge clang-format with clang-tidy under the same package from toolchains. r=gps MozReview-Commit-ID: 1XokTUVmVPL --HG-- extra : rebase_source : 13cab9c90d98da10dcafbe788c4e8b67fcd56803 --- python/mozbuild/mozbuild/mach_commands.py | 236 +++++++++++++++++++--- tools/mach_commands.py | 218 -------------------- 2 files changed, 203 insertions(+), 251 deletions(-) diff --git a/python/mozbuild/mozbuild/mach_commands.py b/python/mozbuild/mozbuild/mach_commands.py index ad8c84056d01..d50af0431908 100644 --- a/python/mozbuild/mozbuild/mach_commands.py +++ b/python/mozbuild/mozbuild/mach_commands.py @@ -11,6 +11,7 @@ import json import logging import operator import os +import re import subprocess import sys @@ -1565,7 +1566,12 @@ class StaticAnalysisMonitor(object): @CommandProvider class StaticAnalysis(MachCommandBase): - """Utilities for running C++ static analysis checks.""" + """Utilities for running C++ static analysis checks and format.""" + + # List of file extension to consider (should start with dot) + _format_include_extensions = ('.cpp', '.c', '.h') + # File contaning all paths to exclude from formatting + _format_ignore_file = '.clang-format-ignore' @Command('static-analysis', category='testing', description='Run C++ static analysis checks') @@ -1585,10 +1591,10 @@ class StaticAnalysis(MachCommandBase): 'in the patch streamed through stdin. This is called ' 'the diff mode.') @CommandArgument('--checks', '-c', default='-*', metavar='checks', - help='Static analysis checks to enable. By default, this enables only ' - 'custom Mozilla checks, but can be any clang-tidy checks syntax.') + help='Static analysis checks to enable. By default, this enables only ' + 'custom Mozilla checks, but can be any clang-tidy checks syntax.') @CommandArgument('--jobs', '-j', default='0', metavar='jobs', type=int, - help='Number of concurrent jobs to run. Default is the number of CPUs.') + help='Number of concurrent jobs to run. Default is the number of CPUs.') @CommandArgument('--strip', '-p', default='1', metavar='NUM', help='Strip NUM leading components from file names in diff mode.') @CommandArgument('--fix', '-f', default=False, action='store_true', @@ -1615,7 +1621,7 @@ class StaticAnalysis(MachCommandBase): if rc != 0: return rc - rc = self._get_clang_tidy(verbose=verbose) + rc = self._get_clang_tools(verbose=verbose) if rc != 0: return rc @@ -1668,9 +1674,9 @@ class StaticAnalysis(MachCommandBase): @StaticAnalysisSubCommand('static-analysis', 'install', 'Install the static analysis helper tool') @CommandArgument('source', nargs='?', type=str, - help='Where to fetch a local archive containing the clang-tidy helper tool.' - 'It will be installed in ~/.mozbuild/clang-tidy/.' - 'Can be omitted, in which case the latest clang-tidy ' + help='Where to fetch a local archive containing the static-analysis and format helper tool.' + 'It will be installed in ~/.mozbuild/clang-tools/.' + 'Can be omitted, in which case the latest clang-tools ' ' helper for the platform would be automatically ' 'detected and installed.') @CommandArgument('--skip-cache', action='store_true', @@ -1678,16 +1684,16 @@ class StaticAnalysis(MachCommandBase): default=False) def install(self, source=None, skip_cache=False, verbose=False): self._set_log_level(verbose) - rc = self._get_clang_tidy(force=True, skip_cache=skip_cache, - source=source, verbose=verbose) + rc = self._get_clang_tools(force=True, skip_cache=skip_cache, + source=source, verbose=verbose) return rc @StaticAnalysisSubCommand('static-analysis', 'clear-cache', 'Delete local helpers and reset static analysis helper tool cache') def clear_cache(self, verbose=False): self._set_log_level(verbose) - rc = self._get_clang_tidy(force=True, download_if_needed=False, - verbose=verbose) + rc = self._get_clang_tools(force=True, download_if_needed=False, + verbose=verbose) if rc != 0: return rc @@ -1697,12 +1703,34 @@ class StaticAnalysis(MachCommandBase): 'Print a list of the static analysis checks performed by default') def print_checks(self, verbose=False): self._set_log_level(verbose) - rc = self._get_clang_tidy(verbose=verbose) + rc = self._get_clang_tools(verbose=verbose) if rc != 0: return rc args = [self._clang_tidy_path, '-list-checks', '-checks=-*,mozilla-*'] return self._run_command_in_objdir(args=args, pass_thru=True) + @Command('clang-format', category='misc', description='Run clang-format on current changes') + @CommandArgument('--show', '-s', action='store_true', default=False, + help='Show diff output on instead of applying changes') + @CommandArgument('--path', '-p', nargs='+', default=None, + help='Specify the path(s) to reformat') + def clang_format(self, show, path, verbose=False): + # Run clang-format or clang-format-diff on the local changes + # or files/directories + if path is not None: + path = self._conv_to_abspath(path) + + os.chdir(self.topsrcdir) + + rc = self._get_clang_tools(verbose=verbose) + if rc != 0: + return rc + + if path is None: + return self._run_clang_format_diff(self._clang_format_diff, show) + else: + return self._run_clang_format_path(self._clang_format_path, show, path) + def _get_checks(self): checks = '-*' import yaml @@ -1780,24 +1808,33 @@ class StaticAnalysis(MachCommandBase): line_handler=None, silent=not verbose, num_jobs=jobs) - def _get_clang_tidy(self, force=False, skip_cache=False, - source=None, download_if_needed=True, - verbose=False): + def _conv_to_abspath(self, paths): + # Converts all the paths to absolute pathnames + tmp_path = [] + for f in paths: + tmp_path.append(os.path.abspath(f)) + return tmp_path + + def _get_clang_tools(self, force=False, skip_cache=False, + source=None, download_if_needed=True, + verbose=False): rc, config, _ = self._get_config_environment() if rc != 0: return rc - clang_tidy_path = mozpath.join(self._mach_context.state_dir, - "clang-tidy") - self._clang_tidy_path = mozpath.join(clang_tidy_path, "clang", "bin", + clang_tools_path = mozpath.join(self._mach_context.state_dir, + "clang-tools") + self._clang_tidy_path = mozpath.join(clang_tools_path, "clang", "bin", "clang-tidy" + config.substs.get('BIN_SUFFIX', '')) - self._clang_format_path = mozpath.join(clang_tidy_path, "clang", "bin", - "clang-format" + config.substs.get('BIN_SUFFIX', '')) - self._clang_apply_replacements = mozpath.join(clang_tidy_path, "clang", "bin", + self._clang_format_path = mozpath.join(clang_tools_path, "clang", "bin", + "clang-format" + config.substs.get('BIN_SUFFIX', '')) + self._clang_apply_replacements = mozpath.join(clang_tools_path, "clang", "bin", "clang-apply-replacements" + config.substs.get('BIN_SUFFIX', '')) - self._run_clang_tidy_path = mozpath.join(clang_tidy_path, "clang", "share", + self._run_clang_tidy_path = mozpath.join(clang_tools_path, "clang", "share", "clang", "run-clang-tidy.py") + self._clang_format_diff = mozpath.join(clang_tools_path, "clang", "share", + "clang", "clang-format-diff.py") if os.path.exists(self._clang_tidy_path) and \ os.path.exists(self._clang_format_path) and \ @@ -1806,20 +1843,20 @@ class StaticAnalysis(MachCommandBase): not force: return 0 else: - if os.path.isdir(clang_tidy_path) and download_if_needed: + if os.path.isdir(clang_tools_path) and download_if_needed: # The directory exists, perhaps it's corrupted? Delete it # and start from scratch. import shutil - shutil.rmtree(clang_tidy_path) - return self._get_clang_tidy(force=force, skip_cache=skip_cache, - source=source, verbose=verbose, - download_if_needed=download_if_needed) + shutil.rmtree(clang_tools_path) + return self._get_clang_tools(force=force, skip_cache=skip_cache, + source=source, verbose=verbose, + download_if_needed=download_if_needed) # Create base directory where we store clang binary - os.mkdir(clang_tidy_path) + os.mkdir(clang_tools_path) if source: - return self._get_clang_tidy_from_source(source) + return self._get_clang_tools_from_source(source) self._artifact_manager = PackageFrontend(self._mach_context) @@ -1837,7 +1874,7 @@ class StaticAnalysis(MachCommandBase): # We want to unpack data in the clang-tidy mozbuild folder currentWorkingDir = os.getcwd() - os.chdir(clang_tidy_path) + os.chdir(clang_tools_path) rc = self._artifact_manager.artifact_toolchain(verbose=verbose, skip_cache=skip_cache, from_build=[job], @@ -1848,10 +1885,10 @@ class StaticAnalysis(MachCommandBase): return rc - def _get_clang_tidy_from_source(self, filename): + def _get_clang_tools_from_source(self, filename): from mozbuild.action.tooltool import unpack_file clang_tidy_path = mozpath.join(self._mach_context.state_dir, - "clang-tidy") + "clang-tools") currentWorkingDir = os.getcwd() os.chdir(clang_tidy_path) @@ -1873,6 +1910,139 @@ class StaticAnalysis(MachCommandBase): assert os.path.exists(self._run_clang_tidy_path) return 0 + def _get_clang_format_diff_command(self): + if self.repository.name == 'hg': + args = ["hg", "diff", "-U0", "-r" ".^"] + for dot_extension in self._format_include_extensions: + args += ['--include', 'glob:**{0}'.format(dot_extension)] + args += ['--exclude', 'listfile:{0}'.format(self._format_ignore_file)] + else: + args = ["git", "diff", "--no-color", "-U0", "HEAD", "--"] + for dot_extension in self._format_include_extensions: + args += ['*{0}'.format(dot_extension)] + # git-diff doesn't support an 'exclude-from-files' param, but + # allow to add individual exclude pattern since v1.9, see + # https://git-scm.com/docs/gitglossary#gitglossary-aiddefpathspecapathspec + with open(self._format_ignore_file, 'rb') as exclude_pattern_file: + for pattern in exclude_pattern_file.readlines(): + pattern = pattern.rstrip() + pattern = pattern.replace('.*', '**') + if not pattern or pattern.startswith('#'): + continue # empty or comment + magics = ['exclude'] + if pattern.startswith('^'): + magics += ['top'] + pattern = pattern[1:] + args += [':({0}){1}'.format(','.join(magics), pattern)] + return args + + def _run_clang_format_diff(self, clang_format_diff, show): + # Run clang-format on the diff + # Note that this will potentially miss a lot things + from subprocess import Popen, PIPE, check_output, CalledProcessError + + diff_process = Popen(self._get_clang_format_diff_command(), stdout=PIPE) + args = [sys.executable, clang_format_diff, "-p1"] + if not show: + args.append("-i") + try: + output = check_output(args, stdin=diff_process.stdout) + if show: + # We want to print the diffs + print(output) + return 0 + except CalledProcessError as e: + # Something wrong happend + print("clang-format: An error occured while running clang-format-diff.") + return e.returncode + + def _is_ignored_path(self, ignored_dir_re, f): + # Remove upto topsrcdir in pathname and match + if f.startswith(self.topsrcdir + '/'): + match_f = f[len(self.topsrcdir + '/'):] + else: + match_f = f + return re.match(ignored_dir_re, match_f) + + def _generate_path_list(self, paths): + path_to_third_party = os.path.join(self.topsrcdir, self._format_ignore_file) + ignored_dir = [] + with open(path_to_third_party, 'r') as fh: + for line in fh: + # Remove comments and empty lines + if line.startswith('#') or len(line.strip()) == 0: + continue + # The regexp is to make sure we are managing relative paths + ignored_dir.append(r"^[\./]*" + line.rstrip()) + + # Generates the list of regexp + ignored_dir_re = '(%s)' % '|'.join(ignored_dir) + extensions = self._format_include_extensions + + path_list = [] + for f in paths: + if self._is_ignored_path(ignored_dir_re, f): + # Early exit if we have provided an ignored directory + print("clang-format: Ignored third party code '{0}'".format(f)) + continue + + if os.path.isdir(f): + # Processing a directory, generate the file list + for folder, subs, files in os.walk(f): + subs.sort() + for filename in sorted(files): + f_in_dir = os.path.join(folder, filename) + if (f_in_dir.endswith(extensions) + and not self._is_ignored_path(ignored_dir_re, f_in_dir)): + # Supported extension and accepted path + path_list.append(f_in_dir) + else: + if f.endswith(extensions): + path_list.append(f) + + return path_list + + def _run_clang_format_path(self, clang_format, show, paths): + # Run clang-format on files or directories directly + from subprocess import check_output, CalledProcessError + + args = [clang_format, "-i"] + + path_list = self._generate_path_list(paths) + + if path_list == []: + return + + print("Processing %d file(s)..." % len(path_list)) + + batchsize = 200 + + for i in range(0, len(path_list), batchsize): + l = path_list[i: (i + batchsize)] + # Run clang-format on the list + try: + check_output(args + l) + except CalledProcessError as e: + # Something wrong happend + print("clang-format: An error occured while running clang-format.") + return e.returncode + + if show: + # show the diff + if self.repository.name == 'hg': + diff_command = ["hg", "diff"] + paths + else: + assert self.repository.name == 'git' + diff_command = ["git", "diff"] + paths + try: + output = check_output(diff_command) + print(output) + except CalledProcessError as e: + # Something wrong happend + print("clang-format: Unable to run the diff command.") + return e.returncode + return 0 + @CommandProvider class Vendor(MachCommandBase): """Vendor third-party dependencies into the source repository.""" diff --git a/tools/mach_commands.py b/tools/mach_commands.py index 16adb2bcf119..1a78b45b12d0 100644 --- a/tools/mach_commands.py +++ b/tools/mach_commands.py @@ -164,224 +164,6 @@ class PastebinProvider(object): return 1 return 0 - -@CommandProvider -class FormatProvider(MachCommandBase): - @Command('clang-format', category='misc', - description='Run clang-format on current changes') - @CommandArgument('--show', '-s', action='store_true', default=False, - help='Show diff output on instead of applying changes') - @CommandArgument('--path', '-p', nargs='+', default=None, - help='Specify the path(s) to reformat') - def clang_format(self, show, path): - # Run clang-format or clang-format-diff on the local changes - # or files/directories - import urllib2 - - plat = platform.system() - fmt = plat.lower() + "/clang-format-5.0" - fmt_diff = "clang-format-diff-5.0" - - if plat == "Windows": - fmt += ".exe" - else: - arch = os.uname()[4] - if (plat != "Linux" and plat != "Darwin") or arch != 'x86_64': - print("Unsupported platform " + plat + "/" + arch + - ". Supported platforms are Windows/*, Linux/x86_64 and Darwin/x86_64") - return 1 - - if path is not None: - path = self.conv_to_abspath(path) - - os.chdir(self.topsrcdir) - self.prompt = True - - try: - clang_format = self.locate_or_fetch(fmt) - if not clang_format: - return 1 - clang_format_diff = self.locate_or_fetch(fmt_diff, python_script=True) - if not clang_format_diff: - return 1 - - except urllib2.HTTPError as e: - print("HTTP error {0}: {1}".format(e.code, e.reason)) - return 1 - - if path is None: - return self.run_clang_format_diff(clang_format_diff, show) - else: - return self.run_clang_format_path(clang_format, show, path) - - def conv_to_abspath(self, paths): - # Converts all the paths to absolute pathnames - tmp_path = [] - for f in paths: - tmp_path.append(os.path.abspath(f)) - return tmp_path - - def locate_or_fetch(self, root, python_script=False): - # Download the clang-format binary & python clang-format-diff if doesn't - # exists - import urllib2 - import hashlib - bin_sha = { - "Windows": "5b6a236425abde1a04ff09e74d8fd0fee1d49e5a35e228b24d77366cab03e1141b8073eec1b36c43e265a80bee707baaa7f96856b4820cbb02069775e58a3f9d", # noqa: E501 - "Linux": "64444efd9b6895447359a9f70d6781251e74d7881f993b5d81a19f8e6a8503f798d42506061fb9eb48729b7327c42a9d273c80dde18816a350fdbc020ebfa783", # noqa: E501 - "Darwin": "d9b08e21c233426628e39dd49bbb9b4e43cccb9aeb78d043dec2bdf6b1eacafddd13488558d38dfa0a0d39946b03b72c58933f1f79d638c045353cf3f4ae0fa4", # noqa: E501 - "python_script": "051b8c8932085616a775ef8b7b1384687db8f37660938f94e9389bf6dba6f6e244d2dc63d23e1d2bf8ab96c9bd5244faefc5218a1f90d5ec692698f0094a3238", # noqa: E501 - } - - target = os.path.join(self._mach_context.state_dir, os.path.basename(root)) - - if not os.path.exists(target): - tooltool_url = "https://tooltool.mozilla-releng.net/sha512/" - if self.prompt and raw_input("Download clang-format executables from {0} (yN)? ".format(tooltool_url)).lower() != 'y': # noqa: E501,F821 - print("Download aborted.") - return None - self.prompt = False - plat = platform.system() - if python_script: - # We want to download the python script (clang-format-diff) - dl = bin_sha["python_script"] - else: - dl = bin_sha[plat] - u = tooltool_url + dl - print("Downloading {0} to {1}".format(u, target)) - data = urllib2.urlopen(url=u).read() - temp = target + ".tmp" - # Check that the checksum of the downloaded data matches the hash - # of the file - sha512Hash = hashlib.sha512(data).hexdigest() - if sha512Hash != dl: - print("Checksum verification for {0} failed: {1} found instead of {2} ".format(target, sha512Hash, dl)) # noqa: E501 - return 1 - with open(temp, "wb") as fh: - fh.write(data) - fh.close() - os.chmod(temp, os.stat(temp).st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) - os.rename(temp, target) - return target - - # List of file extension to consider (should start with dot) - _format_include_extensions = ('.cpp', '.c', '.h') - # File contaning all paths to exclude from formatting - _format_ignore_file = '.clang-format-ignore' - - def _get_clang_format_diff_command(self): - if self.repository.name == 'hg': - args = ["hg", "diff", "-U0", "-r" ".^"] - for dot_extension in self._format_include_extensions: - args += ['--include', 'glob:**{0}'.format(dot_extension)] - args += ['--exclude', 'listfile:{0}'.format(self._format_ignore_file)] - else: - args = ["git", "diff", "--no-color", "-U0", "HEAD", "--"] - for dot_extension in self._format_include_extensions: - args += ['*{0}'.format(dot_extension)] - # git-diff doesn't support an 'exclude-from-files' param, but - # allow to add individual exclude pattern since v1.9, see - # https://git-scm.com/docs/gitglossary#gitglossary-aiddefpathspecapathspec - with open(self._format_ignore_file, 'rb') as exclude_pattern_file: - for pattern in exclude_pattern_file.readlines(): - pattern = pattern.rstrip() - pattern = pattern.replace('.*', '**') - if not pattern or pattern.startswith('#'): - continue # empty or comment - magics = ['exclude'] - if pattern.startswith('^'): - magics += ['top'] - pattern = pattern[1:] - args += [':({0}){1}'.format(','.join(magics), pattern)] - return args - - def run_clang_format_diff(self, clang_format_diff, show): - # Run clang-format on the diff - # Note that this will potentially miss a lot things - from subprocess import Popen, PIPE - - diff_process = Popen(self._get_clang_format_diff_command(), stdout=PIPE) - args = [sys.executable, clang_format_diff, "-p1"] - if not show: - args.append("-i") - cf_process = Popen(args, stdin=diff_process.stdout) - return cf_process.communicate()[0] - - def is_ignored_path(self, ignored_dir_re, f): - # Remove upto topsrcdir in pathname and match - match_f = f.split(self.topsrcdir + '/', 1) - match_f = match_f[1] if len(match_f) == 2 else match_f[0] - return re.match(ignored_dir_re, match_f) - - def generate_path_list(self, paths): - pathToThirdparty = os.path.join(self.topsrcdir, self._format_ignore_file) - ignored_dir = [] - for line in open(pathToThirdparty): - # Remove comments and empty lines - if line.startswith('#') or len(line.strip()) == 0: - continue - # The regexp is to make sure we are managing relative paths - ignored_dir.append("^[\./]*" + line.rstrip()) - - # Generates the list of regexp - ignored_dir_re = '(%s)' % '|'.join(ignored_dir) - extensions = self._format_include_extensions - - path_list = [] - for f in paths: - if self.is_ignored_path(ignored_dir_re, f): - # Early exit if we have provided an ignored directory - print("clang-format: Ignored third party code '{0}'".format(f)) - continue - - if os.path.isdir(f): - # Processing a directory, generate the file list - for folder, subs, files in os.walk(f): - subs.sort() - for filename in sorted(files): - f_in_dir = os.path.join(folder, filename) - if (f_in_dir.endswith(extensions) - and not self.is_ignored_path(ignored_dir_re, f_in_dir)): - # Supported extension and accepted path - path_list.append(f_in_dir) - else: - if f.endswith(extensions): - path_list.append(f) - - return path_list - - def run_clang_format_path(self, clang_format, show, paths): - # Run clang-format on files or directories directly - from subprocess import Popen - - args = [clang_format, "-i"] - - path_list = self.generate_path_list(paths) - - if path_list == []: - return - - print("Processing %d file(s)..." % len(path_list)) - - batchsize = 200 - - for i in range(0, len(path_list), batchsize): - l = path_list[i: (i + batchsize)] - # Run clang-format on the list - cf_process = Popen(args + l) - # Wait until the process is over - cf_process.communicate()[0] - - if show: - # show the diff - if self.repository.name == 'hg': - cf_process = Popen(["hg", "diff"] + paths) - else: - assert self.repository.name == 'git' - cf_process = Popen(["git", "diff"] + paths) - cf_process.communicate()[0] - - def mozregression_import(): # Lazy loading of mozregression. # Note that only the mach_interface module should be used from this file.