From 67d27841ae796f02bce3efa465d20211f5f3f084 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 5 Jul 2021 21:43:52 -0700 Subject: black: configuration This adds necessary configuration for flake8 and black to work together. This also sets the line length to 99, per the data here: * https://github.com/spack/spack/pull/24718#issuecomment-876933636 Given the data and the spirit of black's 88-character limit, we set the limit to 99 characters for all of Spack, because: * 99 is one less than 100, a nice round number, and all lines will fit in a 100-character wide terminal (even when the text editor puts a \ at EOL). * 99 is just past the knee the file size curve for packages, and it means that packages remain readable and not significantly longer than they are now. * It doesn't seem to hurt core -- files in core might change length by a few percent but seem like they'll be mostly the same as before -- just a bit more roomy. - [x] set line length to 99 - [x] remove most exceptions from `.flake8` and add the ones black cares about - [x] add `[tool.black]` to `pyproject.toml` - [x] make `black` run if available in `spack style --fix` Co-Authored-By: Tom Scogland --- .flake8 | 42 +++------- .github/workflows/unit_tests.yaml | 2 +- .github/workflows/windows_python.yml | 4 +- lib/spack/spack/bootstrap.py | 6 +- lib/spack/spack/cmd/style.py | 89 +++++++++++++--------- pyproject.toml | 15 ++++ share/spack/spack-completion.bash | 2 +- .../repos/builtin/packages/py-black/package.py | 11 ++- 8 files changed, 96 insertions(+), 75 deletions(-) diff --git a/.flake8 b/.flake8 index a4c8aa2fbc..226e694894 100644 --- a/.flake8 +++ b/.flake8 @@ -1,43 +1,25 @@ # -*- conf -*- -# flake8 settings for Spack core files. +# flake8 settings for Spack. # # These exceptions are for Spack core files. We're slightly more lenient # with packages. See .flake8_packages for that. # -# E1: Indentation -# - E129: visually indented line with same indent as next logical line -# -# E2: Whitespace -# - E221: multiple spaces before operator -# - E241: multiple spaces after ',' -# - E272: multiple spaces before keyword -# -# E7: Statement +# This is the only flake8 rule Spack violates somewhat flagrantly # - E731: do not assign a lambda expression, use a def # -# W5: Line break warning -# - W503: line break before binary operator -# - W504: line break after binary operator -# -# These are required to get the package.py files to test clean: -# - F999: syntax error in doctest -# -# N8: PEP8-naming -# - N801: class names should use CapWords convention -# - N813: camelcase imported as lowercase -# - N814: camelcase imported as constant +# This is the only flake8 exception needed when using Black. +# - E203: white space around slice operators can be required, ignore : warn # -# F4: pyflakes import checks, these are now checked by mypy more precisely -# - F403: from module import * +# We still allow these in packages (Would like to get rid of them or rely on mypy +# in the future) +# - F403: from/import * used; unable to detect undefined names # - F405: undefined name or from * -# -# Black ignores, these are incompatible with black style and do not follow PEP-8 -# - E203: white space around slice operators can be required, ignore : warn -# - W503: see above, already ignored for line-breaks +# - F821: undefined name (needed with from/import *) # [flake8] -ignore = E129,E221,E241,E272,E731,W503,W504,F999,N801,N813,N814,F403,F405 -max-line-length = 88 +#ignore = E129,,W503,W504,F999,N801,N813,N814,F403,F405,E203 +extend-ignore = E731,E203 +max-line-length = 99 # F4: Import # - F405: `name` may be undefined, or undefined from star imports: `module` @@ -46,7 +28,7 @@ max-line-length = 88 # - F821: undefined name `name` # per-file-ignores = - var/spack/repos/*/package.py:F405,F821 + var/spack/repos/*/package.py:F403,F405,F821 # exclude things we usually do not want linting for. # These still get linted when passed explicitly, as when spack flake8 passes diff --git a/.github/workflows/unit_tests.yaml b/.github/workflows/unit_tests.yaml index 1aa07bb937..d6699dfacb 100644 --- a/.github/workflows/unit_tests.yaml +++ b/.github/workflows/unit_tests.yaml @@ -130,7 +130,7 @@ jobs: # ensure style checks are not skipped in unit tests for python >= 3.6 # note that true/false (i.e., 1/0) are opposite in conditions in python and bash if python -c 'import sys; sys.exit(not sys.version_info >= (3, 6))'; then - pip install --upgrade flake8 isort>=4.3.5 mypy>=0.900 black + pip install --upgrade flake8 "isort>=4.3.5" "mypy>=0.900" "click==8.0.4" "black<=21.12b0" fi - name: Pin pathlib for Python 2.7 if: ${{ matrix.python-version == 2.7 }} diff --git a/.github/workflows/windows_python.yml b/.github/workflows/windows_python.yml index d33c624376..44da541ca1 100644 --- a/.github/workflows/windows_python.yml +++ b/.github/workflows/windows_python.yml @@ -46,7 +46,7 @@ jobs: python-version: 3.9 - name: Install Python packages run: | - python -m pip install --upgrade pip six setuptools flake8 isort>=4.3.5 mypy>=0.800 black pywin32 types-python-dateutil + python -m pip install --upgrade pip six setuptools flake8 "isort>=4.3.5" "mypy>=0.800" "click==8.0.4" "black<=21.12b0" pywin32 types-python-dateutil - name: Create local develop run: | .\spack\.github\workflows\setup_git.ps1 @@ -190,4 +190,4 @@ jobs: $proc = Start-Process ${{ env.spack_installer }}\spack.msi "/quiet" -Passthru $handle = $proc.Handle # cache proc.Handle $proc.WaitForExit(); - $LASTEXITCODE \ No newline at end of file + $LASTEXITCODE diff --git a/lib/spack/spack/bootstrap.py b/lib/spack/spack/bootstrap.py index b4b705e57c..3313c475a2 100644 --- a/lib/spack/spack/bootstrap.py +++ b/lib/spack/spack/bootstrap.py @@ -845,11 +845,13 @@ def ensure_mypy_in_path_or_raise(): def black_root_spec(): - return _root_spec('py-black') + # black v21 is the last version to support Python 2.7. + # Upgrade when we no longer support Python 2.7 + return _root_spec('py-black@:21') def ensure_black_in_path_or_raise(): - """Ensure that isort is in the PATH or raise.""" + """Ensure that black is in the PATH or raise.""" executable, root_spec = 'black', black_root_spec() return ensure_executables_in_path_or_raise([executable], abstract_spec=root_spec) diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py index 85d7afa2ce..252e9f7b1c 100644 --- a/lib/spack/spack/cmd/style.py +++ b/lib/spack/spack/cmd/style.py @@ -65,7 +65,7 @@ def is_package(f): packages, since we allow `from spack import *` and poking globals into packages. """ - return f.startswith("var/spack/repos/") and f.endswith('package.py') + return f.startswith("var/spack/repos/") and f.endswith("package.py") #: decorator for adding tools to the list @@ -94,13 +94,14 @@ def changed_files(base="develop", untracked=True, all_files=False, root=None): git = which("git", required=True) # ensure base is in the repo - base_sha = git("rev-parse", "--quiet", "--verify", "--revs-only", base, - fail_on_error=False, output=str) + base_sha = git( + "rev-parse", "--quiet", "--verify", "--revs-only", base, fail_on_error=False, output=str + ) if git.returncode != 0: tty.die( "This repository does not have a '%s' revision." % base, "spack style needs this branch to determine which files changed.", - "Ensure that '%s' exists, or specify files to check explicitly." % base + "Ensure that '%s' exists, or specify files to check explicitly." % base, ) range = "{0}...".format(base_sha.strip()) @@ -122,10 +123,7 @@ def changed_files(base="develop", untracked=True, all_files=False, root=None): if all_files: git_args.append(["ls-files", "--exclude-standard"]) - excludes = [ - os.path.realpath(os.path.join(root, f)) - for f in exclude_directories - ] + excludes = [os.path.realpath(os.path.join(root, f)) for f in exclude_directories] changed = set() for arg_list in git_args: @@ -200,10 +198,10 @@ def setup_parser(subparser): help="do not run mypy (default: run mypy if available)", ) subparser.add_argument( - "--black", + "--no-black", dest="black", - action="store_true", - help="run black if available (default: skip black)", + action="store_false", + help="run black if available (default: run black if available)", ) subparser.add_argument( "--root", @@ -211,9 +209,7 @@ def setup_parser(subparser): default=None, help="style check a different spack instance", ) - subparser.add_argument( - "files", nargs=argparse.REMAINDER, help="specific files to check" - ) + subparser.add_argument("files", nargs=argparse.REMAINDER, help="specific files to check") def cwd_relative(path, args): @@ -227,9 +223,7 @@ def rewrite_and_print_output( """rewrite ouput with :: format to respect path args""" # print results relative to current working directory def translate(match): - return replacement.format( - cwd_relative(match.group(1), args), *list(match.groups()[1:]) - ) + return replacement.format(cwd_relative(match.group(1), args), *list(match.groups()[1:])) for line in output.split("\n"): if not line: @@ -291,18 +285,29 @@ def run_flake8(flake8_cmd, file_list, args): def run_mypy(mypy_cmd, file_list, args): # always run with config from running spack prefix common_mypy_args = [ - "--config-file", os.path.join(spack.paths.prefix, "pyproject.toml"), + "--config-file", + os.path.join(spack.paths.prefix, "pyproject.toml"), "--show-error-codes", ] - mypy_arg_sets = [common_mypy_args + [ - "--package", "spack", - "--package", "llnl", - ]] - if 'SPACK_MYPY_CHECK_PACKAGES' in os.environ: - mypy_arg_sets.append(common_mypy_args + [ - '--package', 'packages', - '--disable-error-code', 'no-redef', - ]) + mypy_arg_sets = [ + common_mypy_args + + [ + "--package", + "spack", + "--package", + "llnl", + ] + ] + if "SPACK_MYPY_CHECK_PACKAGES" in os.environ: + mypy_arg_sets.append( + common_mypy_args + + [ + "--package", + "packages", + "--disable-error-code", + "no-redef", + ] + ) returncode = 0 for mypy_args in mypy_arg_sets: @@ -334,16 +339,22 @@ def run_isort(isort_cmd, file_list, args): rewrite_and_print_output(output, args, pat, replacement) - packages_isort_args = ('--rm', 'spack', '--rm', 'spack.pkgkit', '--rm', - 'spack.package_defs', '-a', 'from spack.package import *') + packages_isort_args = ( + "--rm", + "spack", + "--rm", + "spack.pkgkit", + "--rm", + "spack.package_defs", + "-a", + "from spack.package import *", + ) packages_isort_args = packages_isort_args + isort_args # packages - process_files(filter(is_package, file_list), - packages_isort_args) + process_files(filter(is_package, file_list), packages_isort_args) # non-packages - process_files(filter(lambda f: not is_package(f), file_list), - isort_args) + process_files(filter(lambda f: not is_package(f), file_list), isort_args) print_tool_result("isort", returncode[0]) return returncode[0] @@ -369,6 +380,13 @@ def run_black(black_cmd, file_list, args): output = black_cmd(*packed_args, fail_on_error=False, output=str, error=str) returncode |= black_cmd.returncode + # ignore Python 2.7 deprecation error because we already know it's deprecated. + output = "\n".join( + line + for line in output.split("\n") + if not "DEPRECATION: Python 2 support will be removed" in line + ) + rewrite_and_print_output(output, args, pat, replacement) print_tool_result("black", returncode) @@ -393,10 +411,7 @@ def style(parser, args): args.root = os.path.realpath(args.root) if args.root else spack.paths.prefix spack_script = os.path.join(args.root, "bin", "spack") if not os.path.exists(spack_script): - tty.die( - "This does not look like a valid spack root.", - "No such file: '%s'" % spack_script - ) + tty.die("This does not look like a valid spack root.", "No such file: '%s'" % spack_script) file_list = args.files if file_list: diff --git a/pyproject.toml b/pyproject.toml index ffa28cfdba..446c7891f2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,18 @@ +[tool.black] +line-length = 99 +target-version = ['py27', 'py35', 'py36', 'py37', 'py38', 'py39', 'py310'] +include = ''' + \.pyi?$ +''' +extend-exclude = ''' +/( + \.git + | \.mypy_cache + | ^lib/spack/external/ + | ^opt/ +)/ +''' + [tool.isort] profile = "black" sections = [ diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 6bdefc6754..1b2cfac2de 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -1697,7 +1697,7 @@ _spack_stage() { _spack_style() { if $list_options then - SPACK_COMPREPLY="-h --help -b --base -a --all -r --root-relative -U --no-untracked -f --fix --no-isort --no-flake8 --no-mypy --black --root" + SPACK_COMPREPLY="-h --help -b --base -a --all -r --root-relative -U --no-untracked -f --fix --no-isort --no-flake8 --no-mypy --no-black --root" else SPACK_COMPREPLY="" fi diff --git a/var/spack/repos/builtin/packages/py-black/package.py b/var/spack/repos/builtin/packages/py-black/package.py index b679f88065..dd3b1d0b7f 100644 --- a/var/spack/repos/builtin/packages/py-black/package.py +++ b/var/spack/repos/builtin/packages/py-black/package.py @@ -21,6 +21,9 @@ class PyBlack(PythonPackage): version('22.3.0', sha256='35020b8886c022ced9282b51b5a875b6d1ab0c387b31a065b84db7c33085ca79') version('22.1.0', sha256='a7c0192d35635f6fc1174be575cb7915e92e5dd629ee79fdaf0dcfa41a80afb5') + # This is the last v21 release, and it's needed to format for Python 2.7 + version('21.12b0', sha256='77b80f693a569e2e527958459634f18df9b0ba2625ba4e0c2d5da5be42e6f2b3') + variant('d', default=False, description='enable blackd HTTP server') variant('colorama', default=False, description='enable colorama support') variant('uvloop', default=False, description='enable uvloop support') @@ -32,10 +35,14 @@ class PyBlack(PythonPackage): # setup.py depends_on('python@3.6.2:', type=('build', 'run')) + depends_on('py-click@8:', type=('build', 'run')) + # see: https://github.com/psf/black/issues/2964 + # note that pip doesn't know this constraint. + depends_on("py-click@:8.0", when="@:22.2", type=("build", "run")) + depends_on('py-platformdirs@2:', type=('build', 'run')) - depends_on('py-tomli@1.1:', when='@22.3: ^python@:3.10', type=('build', 'run')) - depends_on('py-tomli@1.1:', when='@22.1', type=('build', 'run')) + depends_on('py-tomli@1.1:', when='@21.7:', type=('build', 'run')) depends_on('py-typed-ast@1.4.2:', when='^python@:3.7', type=('build', 'run')) depends_on('py-pathspec@0.9:', type=('build', 'run')) depends_on('py-dataclasses@0.6:', when='^python@:3.6', type=('build', 'run')) -- cgit v1.2.3-60-g2f50