From 1b7f9e24f49990c6bee245916e080a1fc891162f Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 31 Oct 2016 11:40:20 -0700 Subject: Add `spack flake8` command. (#2186) - Ported old run-flake8-tests qa script to `spack flake8` command. - New command does not modify files in the source tree - Copies files to a temp stage modifies them there, and runs tests. - Updated docs and `run-flake8-tests` script to call `spack flake8`. --- lib/spack/docs/contribution_guide.rst | 31 ++++---- lib/spack/spack/cmd/flake8.py | 144 ++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 15 deletions(-) create mode 100755 lib/spack/spack/cmd/flake8.py (limited to 'lib') diff --git a/lib/spack/docs/contribution_guide.rst b/lib/spack/docs/contribution_guide.rst index ebfab854d5..49595fecf8 100644 --- a/lib/spack/docs/contribution_guide.rst +++ b/lib/spack/docs/contribution_guide.rst @@ -83,10 +83,11 @@ adding new unit tests or strengthening existing tests. .. note:: - There is also a ``run-unit-tests`` script in ``share/spack/qa`` that runs the - unit tests. Afterwards, it reports back to Coverage with the percentage of Spack - that is covered by unit tests. This script is designed for Travis CI. If you - want to run the unit tests yourself, we suggest you use ``spack test``. + There is also a ``run-unit-tests`` script in ``share/spack/qa`` that + runs the unit tests. Afterwards, it reports back to Coverage with the + percentage of Spack that is covered by unit tests. This script is + designed for Travis CI. If you want to run the unit tests yourself, we + suggest you use ``spack test``. ^^^^^^^^^^^^ Flake8 Tests @@ -99,24 +100,25 @@ from variable naming to indentation. In order to limit the number of PRs that were mostly style changes, we decided to enforce PEP 8 conformance. Your PR needs to comply with PEP 8 in order to be accepted. -Testing for PEP 8 compliance is easy. Simply add the quality assurance -directory to your ``PATH`` and run the flake8 script: +Testing for PEP 8 compliance is easy. Simply run the ``spack flake8`` +command: .. code-block:: console - $ export PATH+=":$SPACK_ROOT/share/spack/qa" - $ run-flake8-tests + $ spack flake8 -``run-flake8-tests`` has a couple advantages over running ``flake8`` by hand: +``spack flake8`` has a couple advantages over running ``flake8`` by hand: -#. It only tests files that you have modified since branching off of develop. +#. It only tests files that you have modified since branching off of + ``develop``. #. It works regardless of what directory you are in. -#. It automatically adds approved exemptions from the flake8 checks. For example, - URLs are often longer than 80 characters, so we exempt them from the line - length checks. We also exempt lines that start with "homepage", "url", "version", - "variant", "depends_on", and "extends" in the ``package.py`` files. +#. It automatically adds approved exemptions from the ``flake8`` + checks. For example, URLs are often longer than 80 characters, so we + exempt them from line length checks. We also exempt lines that start + with "homepage", "url", "version", "variant", "depends_on", and + "extends" in ``package.py`` files. More approved flake8 exemptions can be found `here `_. @@ -518,4 +520,3 @@ and create a PR: .. code-block:: console $ git push origin --set-upstream - diff --git a/lib/spack/spack/cmd/flake8.py b/lib/spack/spack/cmd/flake8.py new file mode 100755 index 0000000000..8648bc88d6 --- /dev/null +++ b/lib/spack/spack/cmd/flake8.py @@ -0,0 +1,144 @@ +############################################################################## +# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +import re +import os +import sys +import shutil +import tempfile + +from llnl.util.filesystem import * + +import spack +from spack.util.executable import * + +description = "Runs source code style checks on Spack. Requires flake8." + +changed_files_path = os.path.join(spack.share_path, 'qa', 'changed_files') +changed_files = Executable(changed_files_path) +flake8 = None + +# +# This is a dict that maps: +# filename pattern -> +# a flake8 exemption code -> +# list of patterns, for which matching lines should have codes applied. +# +exemptions = { + # exemptions applied only to package.py files. + r'package.py$': { + # Exempt lines with urls and descriptions from overlong line errors. + 501: [r'^\s*homepage\s*=', + r'^\s*url\s*=', + r'^\s*version\(.*\)', + r'^\s*variant\(.*\)', + r'^\s*depends_on\(.*\)', + r'^\s*extends\(.*\)'], + # Exempt '@when' decorated functions from redefinition errors. + 811: [r'^\s*\@when\(.*\)'], + }, + + # exemptions applied to all files. + r'.py$': { + # Exempt lines with URLs from overlong line errors. + 501: [r'^(https?|file)\:'] + }, +} + +# compile all regular expressions. +exemptions = dict((re.compile(file_pattern), + dict((code, [re.compile(p) for p in patterns]) + for code, patterns in error_dict.items())) + for file_pattern, error_dict in exemptions.items()) + + +def filter_file(source, dest): + """Filter a single file through all the patterns in exemptions.""" + for file_pattern, errors in exemptions.items(): + if not file_pattern.search(source): + continue + + with open(source) as infile: + parent = os.path.dirname(dest) + mkdirp(parent) + + with open(dest, 'w') as outfile: + for line in infile: + line = line.rstrip() + for code, patterns in errors.items(): + for pattern in patterns: + if pattern.search(line): + line += (" # NOQA: ignore=%d" % code) + break + outfile.write(line + '\n') + + +def setup_parser(subparser): + subparser.add_argument( + '-k', '--keep-temp', action='store_true', + help="Do not delete temporary directory where flake8 runs. " + "Use for debugging, to see filtered files.") + + +def flake8(parser, args): + # Just use this to check for flake8 -- we actually execute it with Popen. + global flake8 + flake8 = which('flake8', required=True) + + temp = tempfile.mkdtemp() + try: + with working_dir(spack.prefix): + changed = changed_files('*.py', output=str) + changed = [x for x in changed.split('\n') if x] + shutil.copy('.flake8', os.path.join(temp, '.flake8')) + + print '=======================================================' + print 'flake8: running flake8 code checks on spack.' + print + print 'Modified files:' + for filename in changed: + print " %s" % filename.strip() + print('=======================================================') + + # filter files into a temporary directory with exemptions added. + for filename in changed: + src_path = os.path.join(spack.prefix, filename) + dest_path = os.path.join(temp, filename) + filter_file(src_path, dest_path) + + # run flake8 on the temporary tree. + with working_dir(temp): + flake8('--format', 'pylint', *changed, fail_on_error=False) + + if flake8.returncode != 0: + print "Flake8 found errors." + sys.exit(1) + else: + print "Flake8 checks were clean." + + finally: + if args.keep_temp: + print "temporary files are in ", temp + else: + shutil.rmtree(temp, ignore_errors=True) -- cgit v1.2.3-70-g09d2