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`.

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