summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2021-07-04 01:53:02 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2021-07-07 17:27:31 -0700
commit24a4d81097c2a930b34d55e25400d14b74ec4112 (patch)
treedeb21e1d2706371226e846a2a5b84cdccfdece55 /lib
parentb5d2c30d26060d4a55c88c1a665b6e9d95ba1644 (diff)
downloadspack-24a4d81097c2a930b34d55e25400d14b74ec4112.tar.gz
spack-24a4d81097c2a930b34d55e25400d14b74ec4112.tar.bz2
spack-24a4d81097c2a930b34d55e25400d14b74ec4112.tar.xz
spack-24a4d81097c2a930b34d55e25400d14b74ec4112.zip
style: clean up and restructure `spack style` command
This consolidates code across tools in `spack style` so that each `run_<tool>` function can be called indirecty through a dictionary of handlers, and os that checks like finding the executable for the tool can be shared across commands. - [x] rework `spack style` to use decorators to register tools - [x] define tool order in one place in `spack style` - [x] fix python 2/3 issues to Get `isort` checks working - [x] make isort error regex more robust across versions - [x] remove unused output option - [x] change vestigial `TRAVIS_BRANCH` to `GITHUB_BASE_REF` - [x] update completion
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/cmd/style.py170
1 files changed, 77 insertions, 93 deletions
diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py
index 23fc3e37ae..dec8cbad4e 100644
--- a/lib/spack/spack/cmd/style.py
+++ b/lib/spack/spack/cmd/style.py
@@ -11,8 +11,10 @@ import re
import sys
import llnl.util.tty as tty
-import spack.paths
+import llnl.util.tty.color as color
from llnl.util.filesystem import working_dir
+
+import spack.paths
from spack.util.executable import which
if sys.version_info < (3, 0):
@@ -39,8 +41,12 @@ def grouper(iterable, n, fillvalue=None):
#: List of directories to exclude from checks.
exclude_directories = [spack.paths.external_path]
-#: max line length we're enforcing (note: this duplicates what's in .flake8)
-max_line_length = 79
+#: order in which tools should be run. flake8 is last so that it can
+#: double-check the results of other tools (if, e.g., --fix was provided)
+tool_order = ["isort", "mypy", "black", "flake8"]
+
+#: tools we run in spack style
+tools = {}
def is_package(f):
@@ -53,13 +59,25 @@ def is_package(f):
return f.startswith("var/spack/repos/") or "docs/tutorial/examples" in f
+#: decorator for adding tools to the list
+class tool(object):
+ def __init__(self, name, required=False):
+ self.name = name
+ self.required = required
+
+ def __call__(self, fun):
+ tools[self.name] = (fun, self.required)
+ return fun
+
+
def changed_files(base=None, untracked=True, all_files=False):
"""Get list of changed files in the Spack repository."""
git = which("git", required=True)
+ # GITHUB_BASE_REF is set to the base branch for pull request actions
if base is None:
- base = os.environ.get("TRAVIS_BRANCH", "develop")
+ base = os.environ.get("GITHUB_BASE_REF", "develop")
range = "{0}...".format(base)
@@ -115,12 +133,6 @@ def setup_parser(subparser):
help="check all files, not just changed files",
)
subparser.add_argument(
- "-o",
- "--output",
- action="store_true",
- help="send filtered files to stdout as well as temp files",
- )
- subparser.add_argument(
"-r",
"--root-relative",
action="store_true",
@@ -140,31 +152,31 @@ def setup_parser(subparser):
"--fix",
action="store_true",
default=False,
- help="If the tool supports automatically editing file contents, do that.",
+ help="format automatically if possible (e.g., with isort, black)",
)
subparser.add_argument(
"--no-isort",
dest="isort",
action="store_false",
- help="Do not run isort, default is run isort if available",
+ help="do not run isort (default: run isort if available)",
)
subparser.add_argument(
"--no-flake8",
dest="flake8",
action="store_false",
- help="Do not run flake8, default is run flake8",
+ help="do not run flake8 (default: run flake8 or fail)",
)
subparser.add_argument(
"--no-mypy",
dest="mypy",
action="store_false",
- help="Do not run mypy, default is run mypy if available",
+ help="do not run mypy (default: run mypy if available)",
)
subparser.add_argument(
"--black",
dest="black",
action="store_true",
- help="Run black checks, default is skip",
+ help="run black if available (default: skip black)",
)
subparser.add_argument(
"files", nargs=argparse.REMAINDER, help="specific files to check"
@@ -177,7 +189,8 @@ def rewrite_and_print_output(
"""rewrite ouput with <file>:<line>: format to respect path args"""
if args.root_relative or re_obj is None:
# print results relative to repo root.
- print(output)
+ for line in output.split("\n"):
+ print(" " + output)
else:
# print results relative to current working directory
def cwd_relative(path):
@@ -191,34 +204,32 @@ def rewrite_and_print_output(
for line in output.split("\n"):
if not line:
continue
- print(re_obj.sub(cwd_relative, line))
+ print(" " + re_obj.sub(cwd_relative, line))
def print_style_header(file_list, args):
- tty.msg("style: running code checks on spack.")
- tools = []
- if args.flake8:
- tools.append("flake8")
- if args.mypy:
- tools.append("mypy")
- if args.black:
- tools.append("black")
- tty.msg("style: tools selected: " + ", ".join(tools))
+ tools = [tool for tool in tool_order if getattr(args, tool)]
+ tty.msg("Running style checks on spack:", "selected: " + ", ".join(tools))
tty.msg("Modified files:", *[filename.strip() for filename in file_list])
sys.stdout.flush()
def print_tool_header(tool):
sys.stdout.flush()
- tty.msg("style: running %s checks on spack." % tool)
+ tty.msg("Running %s checks" % tool)
sys.stdout.flush()
-def run_flake8(file_list, args):
- returncode = 0
- print_tool_header("flake8")
- flake8_cmd = which("flake8", required=True)
+def print_tool_result(tool, returncode):
+ if returncode == 0:
+ color.cprint(" @g{%s checks were clean}" % tool)
+ else:
+ color.cprint(" @r{%s found errors}" % tool)
+
+@tool("flake8", required=True)
+def run_flake8(flake8_cmd, file_list, args):
+ returncode = 0
output = ""
# run in chunks of 100 at a time to avoid line length limit
# filename parameter in config *does not work* for this reliably
@@ -237,20 +248,12 @@ def run_flake8(file_list, args):
rewrite_and_print_output(output, args)
- if returncode == 0:
- tty.msg("Flake8 style checks were clean")
- else:
- tty.error("Flake8 style checks found errors")
+ print_tool_result("flake8", returncode)
return returncode
-def run_mypy(file_list, args):
- mypy_cmd = which("mypy")
- if mypy_cmd is None:
- tty.error("style: mypy is not available in path, skipping")
- return 1
-
- print_tool_header("mypy")
+@tool("mypy")
+def run_mypy(mypy_cmd, file_list, args):
mpy_args = ["--package", "spack", "--package", "llnl", "--show-error-codes"]
# not yet, need other updates to enable this
# if any([is_package(f) for f in file_list]):
@@ -261,49 +264,30 @@ def run_mypy(file_list, args):
rewrite_and_print_output(output, args)
- if returncode == 0:
- tty.msg("mypy checks were clean")
- else:
- tty.error("mypy checks found errors")
+ print_tool_result("mypy", returncode)
return returncode
-def run_isort(file_list, args):
- isort_cmd = which("isort")
- if isort_cmd is None:
- tty.error("style: isort is not available in path, skipping")
- return 1
-
- print_tool_header("isort")
- # TODO: isort apparently decides to avoid writing colored output at all unless the
- # output is a tty, even when --color is provided.
- check_fix_args = () if args.fix else ("--check", "--diff", "--color")
+@tool("isort")
+def run_isort(isort_cmd, file_list, args):
+ check_fix_args = () if args.fix else ("--check", "--diff")
- pat = re.compile("ERROR: (.*) Imports are incorrectly sorted and/or formatted")
- replacement = "ERROR: {0} Imports are incorrectly sorted and/or formatted"
+ pat = re.compile("ERROR: (.*) Imports are incorrectly sorted")
+ replacement = "ERROR: {0} Imports are incorrectly sorted"
returncode = 0
for chunk in grouper(file_list, 100):
- # TODO: py2 does not support multiple * unpacking expressions in a single call.
- packed_args = check_fix_args + chunk
+ packed_args = check_fix_args + tuple(chunk)
output = isort_cmd(*packed_args, fail_on_error=False, output=str, error=str)
returncode |= isort_cmd.returncode
rewrite_and_print_output(output, args, pat, replacement)
- if returncode == 0:
- tty.msg("isort style checks were clean")
- else:
- tty.error("isort checks found errors")
+ print_tool_result("isort", returncode)
return returncode
-def run_black(file_list, args):
- black_cmd = which("black")
- if black_cmd is None:
- tty.error("style: black is not available in path, skipping")
- return 1
-
- print_tool_header("black")
+@tool("black")
+def run_black(black_cmd, file_list, args):
check_fix_args = () if args.fix else ("--check", "--diff", "--color")
pat = re.compile("would reformat +(.*)")
@@ -313,17 +297,13 @@ def run_black(file_list, args):
# run in chunks of 100 at a time to avoid line length limit
# filename parameter in config *does not work* for this reliably
for chunk in grouper(file_list, 100):
- # TODO: py2 does not support multiple * unpacking expressions in a single call.
- packed_args = check_fix_args + chunk
+ packed_args = check_fix_args + tuple(chunk)
output = black_cmd(*packed_args, fail_on_error=False, output=str, error=str)
returncode |= black_cmd.returncode
rewrite_and_print_output(output, args, pat, replacement)
- if returncode == 0:
- tty.msg("black style checks were clean")
- else:
- tty.error("black checks found errors")
+ print_tool_result("black", returncode)
return returncode
@@ -343,20 +323,24 @@ def style(parser, args):
if not file_list:
file_list = changed_files(args.base, args.untracked, args.all)
print_style_header(file_list, args)
- if args.isort:
- # We run isort before flake8, assuming that #23947 with flake8-import-order
- # is also in, to allow flake8 to double-check the result of executing isort,
- # if --fix was provided.
- returncode = run_isort(file_list, args)
- if args.flake8:
- returncode |= run_flake8(file_list, args)
- if args.mypy:
- returncode |= run_mypy(file_list, args)
- if args.black:
- returncode |= run_black(file_list, args)
-
- if returncode != 0:
- print("spack style found errors.")
- sys.exit(1)
+
+ # run tools in order defined in tool_order
+ returncode = 0
+ for tool_name in tool_order:
+ if getattr(args, tool_name):
+ run_function, required = tools[tool_name]
+ print_tool_header(tool_name)
+
+ cmd = which(tool_name, required=required)
+ if not cmd:
+ color.cprint(" @y{%s not in PATH, skipped}" % tool_name)
+ continue
+
+ returncode |= run_function(cmd, file_list, args)
+
+ if returncode == 0:
+ tty.msg(color.colorize("@*{spack style checks were clean}"))
else:
- print("spack style checks were clean.")
+ tty.error(color.colorize("@*{spack style found errors}"))
+
+ return returncode