Merge pull request #929 from epfl-scitas/differentiate_framework_from_packages

QA : make qa checking less painful

8 files changed, 98 insertions, 26 deletions

diff --git a/.flake8 b/.flake8
new file mode 100644
index 0000000000..a1e2fcc1f8
--- /dev/null
+++ b/.flake8
@@ -0,0 +1,20 @@
+# -*- conf -*-
+# flake8 settings for Spack.
+#
+# Below we describe which flake8 checks Spack ignores and what the
+# rationale is.
+#
+# Let people line things up nicely:
+# - E221: multiple spaces before operator
+# - E241: multiple spaces after ‘,’
+#
+# Spack allows wildcard imports:
+# - F403: disable wildcard import
+#
+# These are required to get the package.py files to test clean.
+# - F821: undefined name (needed for cmake, configure, etc.)
+# - F999: name name be undefined or undefined from star imports.
+#
+[flake8]
+ignore = E221,E241,F403,F821,F999
+max-line-length = 79
diff --git a/.style.yapf b/.style.yapf
deleted file mode 100644
index a4b3f65252..0000000000
--- a/.style.yapf
+++ /dev/null
@@ -1,3 +0,0 @@
-[style]
-based_on_style = pep8
-column_limit = 79
diff --git a/.travis.yml b/.travis.yml
index 4ff4d5f483..904143a00f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -15,20 +15,22 @@ before_install:
   # Need this for the git tests to succeed.
   - git config --global user.email "spack@example.com"
   - git config --global user.name "Test User"
+
   # Need this to be able to compute the list of changed files
   - git fetch origin develop:develop
 
 script:
+  # Regular spack setup and tests
   - . share/spack/setup-env.sh
   - spack compilers
   - spack config get compilers
   - spack install -v libdwarf
+
   # Run unit tests with code coverage
   - coverage run bin/spack test
-  # Checks if the file that have been changed are flake8 conformant
-  - CHANGED_PYTHON_FILES=`git diff develop... --name-only | perl -ne 'print if /\.py$/'`
-  - if [[ ${CHANGED_PYTHON_FILES} ]] ; then flake8 --format pylint --config flake8.ini ${CHANGED_PYTHON_FILES} ; fi
 
+  # Run flake8 code style checks.
+  - share/spack/qa/run-flake8
 
 after_success:
   - coveralls
diff --git a/flake8.ini b/flake8.ini
deleted file mode 100644
index 32ced73dd7..0000000000
--- a/flake8.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[flake8]
-ignore = W391,F403,E221,F821
-max-line-length = 79
diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py
index 2701fab90c..62c25c8003 100644
--- a/lib/spack/spack/architecture.py
+++ b/lib/spack/spack/architecture.py
@@ -34,14 +34,14 @@ import spack.error as serr
 
 class InvalidSysTypeError(serr.SpackError):
     def __init__(self, sys_type):
-        super(InvalidSysTypeError, self).__init__(
-            "Invalid sys_type value for Spack: " + sys_type)
+        super(InvalidSysTypeError,
+              self).__init__("Invalid sys_type value for Spack: " + sys_type)
 
 
 class NoSysTypeError(serr.SpackError):
     def __init__(self):
-        super(NoSysTypeError, self).__init__(
-            "Could not determine sys_type for this machine.")
+        super(NoSysTypeError,
+              self).__init__("Could not determine sys_type for this machine.")
 
 
 def get_sys_type_from_spack_globals():
@@ -69,15 +69,15 @@ def get_sys_type_from_platform():
 @memoized
 def sys_type():
     """Returns a SysType for the current machine."""
-    methods = [get_sys_type_from_spack_globals,
-               get_sys_type_from_environment,
+    methods = [get_sys_type_from_spack_globals, get_sys_type_from_environment,
                get_sys_type_from_platform]
 
     # search for a method that doesn't return None
     sys_type = None
     for method in methods:
         sys_type = method()
-        if sys_type: break
+        if sys_type:
+            break
 
     # Couldn't determine the sys_type for this machine.
     if sys_type is None:
diff --git a/share/spack/qa/run-flake8 b/share/spack/qa/run-flake8
new file mode 100755
index 0000000000..722c7fcba6
--- /dev/null
+++ b/share/spack/qa/run-flake8
@@ -0,0 +1,55 @@
+#!/bin/bash
+#
+# This script runs source code style checks on Spack.
+#
+# It should be executed from the top-level directory of the repo,
+# e.g.:
+#
+#    share/spack/qa/run-flake8
+#
+# To run it, you'll need to have the Python flake8 installed locally.
+#
+PYTHONPATH=./lib/spack:$PYTHONPATH
+
+flake8="$(which flake8)"
+if [[ ! $flake8 ]]; then
+    echo "ERROR: flake8 is required to run this script."
+    exit 1
+fi
+
+# Check if changed files are flake8 conformant [framework]
+changed=$(git diff --name-only develop... | grep '.py$')
+
+# Exempt url lines in changed packages from overlong line errors.
+for file in $changed; do
+    if [[ $file = *package.py ]]; then
+        perl -i~ -pe 's/^(\s*url\s*=.*)$/\1  # NOQA: ignore=E501/' $file;
+    fi
+done
+
+return_code=0
+if [[ $changed ]]; then
+    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."
+        return_code=1
+    fi
+else
+    echo No core framework files modified.
+fi
+
+# Restore original package files after modifying them.
+for file in $changed; do
+    if [[ $file = *package.py ]]; then
+        mv "${file}~" "${file}"
+    fi
+done
+
+exit $return_code
diff --git a/var/spack/repos/builtin/packages/ImageMagick/package.py b/var/spack/repos/builtin/packages/ImageMagick/package.py
index 753ea80ca6..3a86d9fb7c 100644
--- a/var/spack/repos/builtin/packages/ImageMagick/package.py
+++ b/var/spack/repos/builtin/packages/ImageMagick/package.py
@@ -1,10 +1,11 @@
 from spack import *
 
+
 class Imagemagick(Package):
     """ImageMagick is a image processing library"""
     homepage = "http://www.imagemagic.org"
 
-    #-------------------------------------------------------------------------
+    # -------------------------------------------------------------------------
     # ImageMagick does not keep around anything but *-10 versions, so
     # this URL may change.  If you want the bleeding edge, you can
     # uncomment it and see if it works but you may need to try to
@@ -17,14 +18,16 @@ class Imagemagick(Package):
     # version('6.9.0-6', 'c1bce7396c22995b8bdb56b7797b4a1b',
     # url="http://www.imagemagick.org/download/ImageMagick-6.9.0-6.tar.bz2")
 
-    #-------------------------------------------------------------------------
+    # -------------------------------------------------------------------------
     # *-10 versions are archived, so these versions should fetch reliably.
     # -------------------------------------------------------------------------
-    version('6.8.9-10', 'aa050bf9785e571c956c111377bbf57c',
-            url="http://sourceforge.net/projects/imagemagick/files/old-sources/6.x/6.8/ImageMagick-6.8.9-10.tar.gz/download")
+    version(
+        '6.8.9-10',
+        'aa050bf9785e571c956c111377bbf57c',
+        url="http://sourceforge.net/projects/imagemagick/files/old-sources/6.x/6.8/ImageMagick-6.8.9-10.tar.gz/download")
 
-    depends_on('libtool')
     depends_on('jpeg')
+    depends_on('libtool')
     depends_on('libpng')
     depends_on('freetype')
     depends_on('fontconfig')
@@ -32,6 +35,5 @@ class Imagemagick(Package):
 
     def install(self, spec, prefix):
         configure("--prefix=%s" % prefix)
-
         make()
         make("install")
diff --git a/var/spack/repos/builtin/packages/Mitos/package.py b/var/spack/repos/builtin/packages/Mitos/package.py
index ea131872dd..ec1d56a5c7 100644
--- a/var/spack/repos/builtin/packages/Mitos/package.py
+++ b/var/spack/repos/builtin/packages/Mitos/package.py
@@ -1,19 +1,18 @@
 from spack import *
 
+
 class Mitos(Package):
     """Mitos is a library and a tool for collecting sampled memory
     performance data to view with MemAxes"""
 
     homepage = "https://github.com/llnl/Mitos"
-    url      = "https://github.com/llnl/Mitos"
+    url = "https://github.com/llnl/Mitos"
 
     version('0.9.2',
             git='https://github.com/llnl/Mitos.git',
             commit='8cb143a2e8c00353ff531a781a9ca0992b0aaa3d')
 
-    version('0.9.1',
-            git='https://github.com/llnl/Mitos.git',
-            tag='v0.9.1')
+    version('0.9.1', git='https://github.com/llnl/Mitos.git', tag='v0.9.1')
 
     depends_on('dyninst@8.2.1:')
     depends_on('hwloc')