Properly ignore flake8 F811 redefinition errors (#3932)
* Properly ignore flake8 F811 redefinition errors * Add unit tests for flake8 command * Allow spack flake8 to work on systems with older git * Skip flake8 unit tests for Python 2.6 and 3.3
This commit is contained in:
Adam J. Stewart
Todd Gamblin
parent
827ebe280d
commit
58f2a947db
8 changed files with 324 additions and 41 deletions
4
.flake8
4
.flake8
|
|
@ -11,12 +11,12 @@
|
|||
# - E272: multiple spaces before keyword
|
||||
#
|
||||
# Let people use terse Python features:
|
||||
# - E731 : lambda expressions
|
||||
# - E731: lambda expressions
|
||||
#
|
||||
# Spack allows wildcard imports:
|
||||
# - F403: disable wildcard import
|
||||
#
|
||||
# These are required to get the package.py files to test clean.
|
||||
# These are required to get the package.py files to test clean:
|
||||
# - F405: `name` may be undefined, or undefined from star imports: `module`
|
||||
# - F821: undefined name `name` (needed for cmake, configure, etc.)
|
||||
# - F999: syntax error in doctest
|
||||
|
|
|
|||
|
|
@ -37,8 +37,6 @@
|
|||
from spack.util.executable import *
|
||||
|
||||
description = "runs source code style checks on Spack. requires flake8"
|
||||
flake8 = None
|
||||
include_untracked = True
|
||||
|
||||
"""List of directories to exclude from checks."""
|
||||
exclude_directories = [spack.external_path]
|
||||
|
|
@ -53,24 +51,34 @@
|
|||
# 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*=',
|
||||
'E501': [
|
||||
r'^\s*homepage\s*=',
|
||||
r'^\s*url\s*=',
|
||||
r'^\s*git\s*=',
|
||||
r'^\s*svn\s*=',
|
||||
r'^\s*hg\s*=',
|
||||
r'^\s*list_url\s*=',
|
||||
r'^\s*version\(.*\)',
|
||||
r'^\s*variant\(.*\)',
|
||||
r'^\s*depends_on\(.*\)',
|
||||
r'^\s*provides\(.*\)',
|
||||
r'^\s*extends\(.*\)',
|
||||
r'^\s*patch\(.*\)'],
|
||||
r'^\s*depends_on\(.*\)',
|
||||
r'^\s*conflicts\(.*\)',
|
||||
r'^\s*resource\(.*\)',
|
||||
r'^\s*patch\(.*\)',
|
||||
],
|
||||
# Exempt '@when' decorated functions from redefinition errors.
|
||||
811: [r'^\s*\@when\(.*\)'],
|
||||
'F811': [
|
||||
r'^\s*@when\(.*\)',
|
||||
],
|
||||
},
|
||||
|
||||
# exemptions applied to all files.
|
||||
r'.py$': {
|
||||
# Exempt lines with URLs from overlong line errors.
|
||||
501: [r'(https?|ftp|file)\:']
|
||||
'E501': [
|
||||
r'(https?|ftp|file)\:',
|
||||
]
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -81,7 +89,7 @@
|
|||
for file_pattern, error_dict in exemptions.items())
|
||||
|
||||
|
||||
def changed_files():
|
||||
def changed_files(args):
|
||||
"""Get list of changed files in the Spack repository."""
|
||||
|
||||
git = which('git', required=True)
|
||||
|
|
@ -92,23 +100,30 @@ def changed_files():
|
|||
# Add changed files that have been staged but not yet committed
|
||||
['diff', '--name-only', '--diff-filter=ACMR', '--cached'],
|
||||
# Add changed files that are unstaged
|
||||
['diff', '--name-only', '--diff-filter=ACMR']]
|
||||
['diff', '--name-only', '--diff-filter=ACMR'],
|
||||
]
|
||||
|
||||
# Add new files that are untracked
|
||||
if include_untracked:
|
||||
if args.untracked:
|
||||
git_args.append(['ls-files', '--exclude-standard', '--other'])
|
||||
|
||||
excludes = [os.path.realpath(f) for f in exclude_directories]
|
||||
changed = set()
|
||||
for git_arg_list in git_args:
|
||||
arg_list = git_arg_list + ['--', '*.py']
|
||||
|
||||
files = [f for f in git(*arg_list, output=str).split('\n') if f]
|
||||
for arg_list in git_args:
|
||||
files = git(*arg_list, output=str).split('\n')
|
||||
|
||||
for f in files:
|
||||
# don't look at files that are in the exclude locations
|
||||
# Ignore non-Python files
|
||||
if not f.endswith('.py'):
|
||||
continue
|
||||
|
||||
# Ignore files in the exclude locations
|
||||
if any(os.path.realpath(f).startswith(e) for e in excludes):
|
||||
continue
|
||||
|
||||
changed.add(f)
|
||||
|
||||
return sorted(changed)
|
||||
|
||||
|
||||
|
|
@ -120,7 +135,17 @@ def filter_file(source, dest, output=False):
|
|||
|
||||
with open(dest, 'w') as outfile:
|
||||
for line in infile:
|
||||
line = line.rstrip()
|
||||
# Only strip newline characters
|
||||
# We still want to catch trailing whitespace warnings
|
||||
line = line.rstrip('\n')
|
||||
|
||||
if line == '# flake8: noqa':
|
||||
# Entire file is ignored
|
||||
break
|
||||
|
||||
if line.endswith('# noqa'):
|
||||
# Line is already ignored
|
||||
continue
|
||||
|
||||
for file_pattern, errors in exemptions.items():
|
||||
if not file_pattern.search(source):
|
||||
|
|
@ -129,7 +154,10 @@ def filter_file(source, dest, output=False):
|
|||
for code, patterns in errors.items():
|
||||
for pattern in patterns:
|
||||
if pattern.search(line):
|
||||
line += (" # NOQA: E%d" % code)
|
||||
if '# noqa: ' in line:
|
||||
line += ',{0}'.format(code)
|
||||
else:
|
||||
line += ' # noqa: {0}'.format(code)
|
||||
break
|
||||
|
||||
oline = line + '\n'
|
||||
|
|
@ -157,10 +185,7 @@ def setup_parser(subparser):
|
|||
|
||||
|
||||
def flake8(parser, args):
|
||||
# Just use this to check for flake8 -- we actually execute it with Popen.
|
||||
global flake8, include_untracked
|
||||
flake8 = which('flake8', required=True)
|
||||
include_untracked = args.untracked
|
||||
|
||||
temp = tempfile.mkdtemp()
|
||||
try:
|
||||
|
|
@ -174,7 +199,7 @@ def prefix_relative(path):
|
|||
|
||||
with working_dir(spack.prefix):
|
||||
if not file_list:
|
||||
file_list = changed_files()
|
||||
file_list = changed_files(args)
|
||||
shutil.copy('.flake8', os.path.join(temp, '.flake8'))
|
||||
|
||||
print('=======================================================')
|
||||
|
|
@ -182,7 +207,7 @@ def prefix_relative(path):
|
|||
print()
|
||||
print('Modified files:')
|
||||
for filename in file_list:
|
||||
print(" %s" % filename.strip())
|
||||
print(' {0}'.format(filename.strip()))
|
||||
print('=======================================================')
|
||||
|
||||
# filter files into a temporary directory with exemptions added.
|
||||
|
|
@ -202,20 +227,20 @@ def prefix_relative(path):
|
|||
else:
|
||||
# print results relative to current working directory
|
||||
def cwd_relative(path):
|
||||
return '%s: [' % os.path.relpath(
|
||||
os.path.join(spack.prefix, path.group(1)), os.getcwd())
|
||||
return '{0}: ['.format(os.path.relpath(
|
||||
os.path.join(spack.prefix, path.group(1)), os.getcwd()))
|
||||
|
||||
for line in output.split('\n'):
|
||||
print(re.sub(r'^(.*): \[', cwd_relative, line))
|
||||
|
||||
if flake8.returncode != 0:
|
||||
print("Flake8 found errors.")
|
||||
print('Flake8 found errors.')
|
||||
sys.exit(1)
|
||||
else:
|
||||
print("Flake8 checks were clean.")
|
||||
print('Flake8 checks were clean.')
|
||||
|
||||
finally:
|
||||
if args.keep_temp:
|
||||
print("temporary files are in ", temp)
|
||||
print('Temporary files are in: ', temp)
|
||||
else:
|
||||
shutil.rmtree(temp, ignore_errors=True)
|
||||
|
|
|
|||
95
lib/spack/spack/test/cmd/flake8.py
Normal file
95
lib/spack/spack/test/cmd/flake8.py
Normal file
|
|
@ -0,0 +1,95 @@
|
|||
##############################################################################
|
||||
# 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 argparse
|
||||
import os
|
||||
import pytest
|
||||
import sys
|
||||
|
||||
from llnl.util.filesystem import FileFilter
|
||||
|
||||
import spack
|
||||
from spack.cmd.flake8 import *
|
||||
from spack.repository import Repo
|
||||
|
||||
|
||||
@pytest.fixture(scope='module')
|
||||
def parser():
|
||||
"""Returns the parser for the ``flake8`` command"""
|
||||
parser = argparse.ArgumentParser()
|
||||
setup_parser(parser)
|
||||
return parser
|
||||
|
||||
|
||||
@pytest.fixture(scope='module')
|
||||
def flake8_package():
|
||||
"""Flake8 only checks files that have been modified.
|
||||
This fixture makes a small change to the ``flake8``
|
||||
mock package, yields the filename, then undoes the
|
||||
change on cleanup.
|
||||
"""
|
||||
repo = Repo(spack.mock_packages_path)
|
||||
filename = repo.filename_for_package_name('flake8')
|
||||
package = FileFilter(filename)
|
||||
|
||||
# Make the change
|
||||
package.filter('unmodified', 'modified')
|
||||
|
||||
yield filename
|
||||
|
||||
# Undo the change
|
||||
package.filter('modified', 'unmodified')
|
||||
|
||||
|
||||
def test_changed_files(parser, flake8_package):
|
||||
args = parser.parse_args()
|
||||
|
||||
# changed_files returns file paths relative to the root
|
||||
# directory of Spack. Convert to absolute file paths.
|
||||
files = changed_files(args)
|
||||
files = [os.path.join(spack.spack_root, path) for path in files]
|
||||
|
||||
# There will likely be other files that have changed
|
||||
# when these tests are run
|
||||
assert flake8_package in files
|
||||
|
||||
|
||||
# As of flake8 3.0.0, Python 2.6 and 3.3 are no longer supported
|
||||
# http://flake8.pycqa.org/en/latest/release-notes/3.0.0.html
|
||||
@pytest.mark.skipif(
|
||||
sys.version_info[:2] <= (2, 6) or
|
||||
(3, 0) <= sys.version_info[:2] <= (3, 3),
|
||||
reason='flake8 no longer supports Python 2.6 or 3.3 and older')
|
||||
def test_flake8(parser, flake8_package):
|
||||
# Only test the flake8_package that we modified
|
||||
# Otherwise, the unit tests would fail every time
|
||||
# the flake8 tests fail
|
||||
args = parser.parse_args([flake8_package])
|
||||
|
||||
flake8(parser, args)
|
||||
|
||||
# Get even more coverage
|
||||
args = parser.parse_args(['--output', '--root-relative', flake8_package])
|
||||
|
||||
flake8(parser, args)
|
||||
83
var/spack/repos/builtin.mock/packages/boost/package.py
Normal file
83
var/spack/repos/builtin.mock/packages/boost/package.py
Normal file
|
|
@ -0,0 +1,83 @@
|
|||
##############################################################################
|
||||
# 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
|
||||
##############################################################################
|
||||
from spack import *
|
||||
|
||||
|
||||
class Boost(Package):
|
||||
"""Fake boost package."""
|
||||
|
||||
homepage = "http://www.boost.org"
|
||||
url = "http://downloads.sourceforge.net/project/boost/boost/1.63.0/boost_1_63_0.tar.bz2"
|
||||
|
||||
version('1.63.0', '1c837ecd990bb022d07e7aab32b09847')
|
||||
|
||||
default_install_libs = set(['atomic',
|
||||
'chrono',
|
||||
'date_time',
|
||||
'filesystem',
|
||||
'graph',
|
||||
'iostreams',
|
||||
'locale',
|
||||
'log',
|
||||
'math',
|
||||
'program_options',
|
||||
'random',
|
||||
'regex',
|
||||
'serialization',
|
||||
'signals',
|
||||
'system',
|
||||
'test',
|
||||
'thread',
|
||||
'timer',
|
||||
'wave'])
|
||||
|
||||
# mpi/python are not installed by default because they pull in many
|
||||
# dependencies and/or because there is a great deal of customization
|
||||
# possible (and it would be difficult to choose sensible defaults)
|
||||
default_noinstall_libs = set(['mpi', 'python'])
|
||||
|
||||
all_libs = default_install_libs | default_noinstall_libs
|
||||
|
||||
for lib in all_libs:
|
||||
variant(lib, default=(lib not in default_noinstall_libs),
|
||||
description="Compile with {0} library".format(lib))
|
||||
|
||||
variant('debug', default=False,
|
||||
description='Switch to the debug version of Boost')
|
||||
variant('shared', default=True,
|
||||
description="Additionally build shared libraries")
|
||||
variant('multithreaded', default=True,
|
||||
description="Build multi-threaded versions of libraries")
|
||||
variant('singlethreaded', default=False,
|
||||
description="Build single-threaded versions of libraries")
|
||||
variant('icu', default=False,
|
||||
description="Build with Unicode and ICU suport")
|
||||
variant('graph', default=False,
|
||||
description="Build the Boost Graph library")
|
||||
variant('taggedlayout', default=False,
|
||||
description="Augment library names with build options")
|
||||
|
||||
def install(self, spec, prefix):
|
||||
pass
|
||||
|
|
@ -0,0 +1 @@
|
|||
Fake patch
|
||||
79
var/spack/repos/builtin.mock/packages/flake8/package.py
Normal file
79
var/spack/repos/builtin.mock/packages/flake8/package.py
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
##############################################################################
|
||||
# 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
|
||||
##############################################################################
|
||||
from spack import *
|
||||
|
||||
|
||||
class Flake8(Package):
|
||||
"""Package containing as many PEP 8 violations as possible.
|
||||
All of these violations are exceptions that we allow in
|
||||
package.py files."""
|
||||
|
||||
# Used to tell whether or not the package has been modified
|
||||
state = 'unmodified'
|
||||
|
||||
# Make sure pre-existing noqa is not interfered with
|
||||
blatant_violation = 'line-that-has-absolutely-no-execuse-for-being-over-79-characters' # noqa
|
||||
blatant_violation = 'line-that-has-absolutely-no-execuse-for-being-over-79-characters' # noqa: E501
|
||||
|
||||
# Keywords exempt from line-length checks
|
||||
homepage = '#####################################################################'
|
||||
url = '#####################################################################'
|
||||
git = '#####################################################################'
|
||||
svn = '#####################################################################'
|
||||
hg = '#####################################################################'
|
||||
list_url = '#####################################################################'
|
||||
|
||||
# URL strings exempt from line-length checks
|
||||
# http://########################################################################
|
||||
# https://#######################################################################
|
||||
# ftp://#########################################################################
|
||||
# file://########################################################################
|
||||
|
||||
# Directives exempt from line-length checks
|
||||
version('2.0', '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef')
|
||||
version('1.0', '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef')
|
||||
|
||||
variant('super-awesome-feature', default=True, description='Enable super awesome feature')
|
||||
variant('somewhat-awesome-feature', default=False, description='Enable somewhat awesome feature')
|
||||
|
||||
provides('lapack', when='@2.0+super-awesome-feature+somewhat-awesome-feature')
|
||||
|
||||
extends('python', ignore='bin/(why|does|every|package|that|depends|on|numpy|need|to|copy|f2py3?)')
|
||||
|
||||
depends_on('boost+atomic+chrono+date_time~debug+filesystem~graph~icu+iostreams+locale+log+math~mpi+multithreaded+program_options~python+random+regex+serialization+shared+signals~singlethreaded+system~taggedlayout+test+thread+timer+wave')
|
||||
|
||||
conflicts('+super-awesome-feature', when='%intel@16:17+somewhat-awesome-feature')
|
||||
|
||||
resource(name='Deez-Nuts', destination='White-House', placement='President', when='@2020', url='www.elect-deez-nuts.com')
|
||||
|
||||
patch('hyper-specific-patch-that-fixes-some-random-bug-that-probably-only-affects-one-user.patch', when='%gcc@3.2.2:3.2.3')
|
||||
|
||||
def install(self, spec, prefix):
|
||||
pass
|
||||
|
||||
# '@when' decorated functions are exempt from redefinition errors
|
||||
@when('@2.0')
|
||||
def install(self, spec, prefix):
|
||||
pass
|
||||
|
|
@ -84,7 +84,7 @@ def patch(self):
|
|||
filter_file('#define MAX_JBUFS 128', '#define MAX_JBUFS 24',
|
||||
join_path(source_path, 'GKlib', 'error.c'))
|
||||
|
||||
@when('@:4') # noqa: F811
|
||||
@when('@:4')
|
||||
def install(self, spec, prefix):
|
||||
# Process library spec and options
|
||||
if any('+{0}'.format(v) in spec for v in ['gdb', 'int64', 'real64']):
|
||||
|
|
@ -175,7 +175,7 @@ def install(self, spec, prefix):
|
|||
Executable(test_bin('mesh2dual'))(test_graph('metis.mesh'))
|
||||
"""
|
||||
|
||||
@when('@5:') # noqa: F811
|
||||
@when('@5:')
|
||||
def install(self, spec, prefix):
|
||||
source_directory = self.stage.source_path
|
||||
build_directory = join_path(source_directory, 'build')
|
||||
|
|
|
|||
|
|
@ -251,7 +251,7 @@ def common_config_args(self):
|
|||
# Don't disable all the database drivers, but should
|
||||
# really get them into spack at some point.
|
||||
|
||||
@when('@3') # noqa: F811
|
||||
@when('@3')
|
||||
def configure(self):
|
||||
# A user reported that this was necessary to link Qt3 on ubuntu.
|
||||
# However, if LD_LIBRARY_PATH is not set the qt build fails, check
|
||||
|
|
@ -268,7 +268,7 @@ def configure(self):
|
|||
'-release',
|
||||
'-fast')
|
||||
|
||||
@when('@4') # noqa: F811
|
||||
@when('@4')
|
||||
def configure(self):
|
||||
configure('-fast',
|
||||
'-{0}gtkstyle'.format('' if '+gtk' in self.spec else 'no-'),
|
||||
|
|
@ -276,7 +276,7 @@ def configure(self):
|
|||
'-arch', str(self.spec.architecture.target),
|
||||
*self.common_config_args)
|
||||
|
||||
@when('@5.0:5.6') # noqa: F811
|
||||
@when('@5.0:5.6')
|
||||
def configure(self):
|
||||
webkit_args = [] if '+webkit' in self.spec else ['-skip', 'qtwebkit']
|
||||
configure('-no-eglfs',
|
||||
|
|
@ -284,7 +284,7 @@ def configure(self):
|
|||
'-{0}gtkstyle'.format('' if '+gtk' in self.spec else 'no-'),
|
||||
*(webkit_args + self.common_config_args))
|
||||
|
||||
@when('@5.7:') # noqa: F811
|
||||
@when('@5.7:')
|
||||
def configure(self):
|
||||
config_args = self.common_config_args
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue