diff options
| -rw-r--r-- | lib/spack/docs/contribution_guide.rst | 31 | ||||
| -rwxr-xr-x | lib/spack/spack/cmd/flake8.py | 144 | ||||
| -rwxr-xr-x | share/spack/qa/run-flake8-tests | 66 |
3 files changed, 163 insertions, 78 deletions
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 <https://github.com/LLNL/spack/blob/develop/.flake8>`_. @@ -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) diff --git a/share/spack/qa/run-flake8-tests b/share/spack/qa/run-flake8-tests index 6fe97160e3..83469eeb9d 100755 --- a/share/spack/qa/run-flake8-tests +++ b/share/spack/qa/run-flake8-tests @@ -23,67 +23,7 @@ deps=( # Check for dependencies "$QA_DIR/check_dependencies" "${deps[@]}" || exit 1 -# Gather array of changed files -changed=($("$QA_DIR/changed_files" "*.py")) +# Add Spack to the PATH. +export PATH="$SPACK_ROOT/bin:$PATH" -# Exit if no Python files were modified -if [[ ! "${changed[@]}" ]]; then - echo "No Python files were modified." - exit 0 -fi - -# Move to root directory of Spack -# Allows script to be run from anywhere -cd "$SPACK_ROOT" - -function cleanup { - # Restore original package files after modifying them. - for file in "${changed[@]}"; do - if [[ -e "${file}.sbak~" ]]; then - mv "${file}.sbak~" "${file}" - fi - done -} - -# Cleanup temporary files upon exit or when script is killed -trap cleanup EXIT SIGINT SIGTERM - -# Add approved style exemptions to the changed packages. -for file in "${changed[@]}"; do - # Make a backup to restore later - cp "$file" "$file.sbak~" - - # - # Exemptions for package.py files - # - if [[ $file = *package.py ]]; then - # Exempt lines with urls and descriptions from overlong line errors. - perl -i -pe 's/^(\s*homepage\s*=.*)$/\1 # NOQA: ignore=E501/' "$file" - perl -i -pe 's/^(\s*url\s*=.*)$/\1 # NOQA: ignore=E501/' "$file" - perl -i -pe 's/^(\s*version\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file" - perl -i -pe 's/^(\s*variant\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file" - perl -i -pe 's/^(\s*depends_on\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file" - perl -i -pe 's/^(\s*extends\(.*\).*)$/\1 # NOQA: ignore=E501/' "$file" - - # Exempt '@when' decorated functions from redefinition errors. - perl -i -pe 's/^(\s*\@when\(.*\).*)$/\1 # NOQA: ignore=F811/' "$file" - fi - - # - # Exemptions for all files - # - perl -i -pe 's/^(.*(https?|file)\:.*)$/\1 # NOQA: ignore=E501/' $file -done - -echo ======================================================= -echo flake8: running flake8 code checks on spack. -echo -echo Modified files: -echo "${changed[@]}" | perl -pe 's/^/ /;s/ +/\n /g' -echo ======================================================= -if flake8 --format pylint "${changed[@]}"; then - echo "Flake8 checks were clean." -else - echo "Flake8 found errors." - exit 1 -fi +exec spack flake8 |