Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
06a5646
Add style check for easyconfigs
wpoely86 Feb 17, 2016
dd95739
Rename function
wpoely86 Feb 17, 2016
276f17a
Add test for style
wpoely86 Feb 17, 2016
9e92dab
Update test easyconfig to pass style check
wpoely86 Feb 18, 2016
d482ff2
Fix remarks
wpoely86 Feb 18, 2016
adac638
Make function private
wpoely86 Feb 18, 2016
9056d23
Fix checksums for tests
wpoely86 Feb 25, 2016
fbac45a
Merge branch 'develop' into style
wpoely86 Feb 25, 2016
6e62431
Fix style
wpoely86 Feb 25, 2016
613fdcf
Fix remarks
wpoely86 Feb 26, 2016
286c0e9
Typo
wpoely86 Feb 26, 2016
4604134
Fix style of test easyconfigs
wpoely86 Feb 26, 2016
04548c9
Fix remarks for tests
wpoely86 Feb 26, 2016
d434485
Print message for custom style checks
wpoely86 Feb 26, 2016
fce593f
Add documentation
wpoely86 Feb 26, 2016
215f7ad
Fix checksums
wpoely86 Feb 26, 2016
34315fa
Fix again...
wpoely86 Feb 26, 2016
74a8cf4
Final checksum fix.
wpoely86 Feb 26, 2016
41651bf
Mutiline description in yeb file
wpoely86 Feb 26, 2016
529af5c
Revert line change in test easyconfigs.
wpoely86 Feb 29, 2016
a7991b4
Merge branch 'develop' into style
wpoely86 Nov 24, 2016
210cc17
Fix author tag
wpoely86 Nov 24, 2016
5a51dee
Add pep8 to travis.yml
wpoely86 Nov 24, 2016
e1bf239
Some style changes
wpoely86 Nov 24, 2016
854f1dc
Fix remarks
wpoely86 Nov 29, 2016
5e047ca
Fix imports and order
wpoely86 Nov 29, 2016
d92bb09
Fix more remarks
wpoely86 Nov 29, 2016
a12c72c
Use constants and refactor
wpoely86 Nov 29, 2016
795048f
Drop unneeded comment
wpoely86 Nov 29, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ before_install:
# pydot (dep for python-graph-dot) 1.2.0 and more recent doesn't work with Python 2.6
- if [ "x$TRAVIS_PYTHON_VERSION" == 'x2.6' ]; then pip install pydot==1.1.0; else pip install pydot; fi
# optional Python packages for EasyBuild
- pip install autopep8 GC3Pie python-graph-dot python-hglib PyYAML
- pip install autopep8 GC3Pie pep8 python-graph-dot python-hglib PyYAML
# git config is required to make actual git commits (cfr. tests for GitRepository)
- git config --global user.name "Travis CI"
- git config --global user.email "[email protected]"
Expand Down
126 changes: 126 additions & 0 deletions easybuild/framework/easyconfig/style.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
##
# Copyright 2016-2016 Ghent University
#
# This file is part of EasyBuild,
# originally created by the HPC team of Ghent University (http://ugent.be/hpc/en),
# with support of Ghent University (http://ugent.be/hpc),
# the Flemish Supercomputer Centre (VSC) (https://www.vscentrum.be),
# Flemish Research Foundation (FWO) (http://www.fwo.be/en)
# and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en).
#
# http://github.com/hpcugent/easybuild
#
# EasyBuild is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation v2.
#
# EasyBuild 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
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with EasyBuild. If not, see <http://www.gnu.org/licenses/>.
# #

"""
Style tests for easyconfig files using pep8.

:author: Ward Poelmans (Ghent University)
"""

import re
from vsc.utils import fancylogger

from easybuild.tools.utilities import only_if_module_is_available

try:
import pep8
except ImportError:
pass

_log = fancylogger.getLogger('easyconfig.style', fname=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why add a logger if you're not using it?

(don't remove it, use it below :))


EB_CHECK = '_eb_check_'


# any function starting with _eb_check_ (see EB_CHECK variable) will be
# added to the tests if the test number is added to the select list.
# The test number is definied as WXXX and EXXX (for warnings and errors)
# where XXX is a 3 digit number.
#
# It should be mentioned in the docstring as a single word.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on a separate line?

# Read the pep8 docs to understand the arguments of these functions:
# https://pep8.readthedocs.org or more specifically:
# https://pep8.readthedocs.org/en/latest/developer.html#contribute
def _eb_check_trailing_whitespace(physical_line, lines, line_number, total_lines): # pylint:disable=unused-argument
"""
W299
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 299?

shouldn't be define our own range like 9xy (or whatever other range that is not used by PEP8)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there is a structure in the codes: https://pep8.readthedocs.org/en/latest/intro.html#error-codes

Warn about trailing whitespace, except for the description and comments.
This differs from the standard trailing whitespace check as that
will warn for any trailing whitespace.
The arguments are explained at
https://pep8.readthedocs.org/en/latest/developer.html#contribute
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc args?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can copy & paste the pep8 docs but I prefer not to. The names are already quite clear and the precise meaning should be looked up in the url above

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please refer to the right place in the pep8 docs in the docstring; the comments won't show up in the auto-generated API documentation for EB we'll have at easybuild.readthedocs.org someday...

comment_re = re.compile(r'^\s*#')
if comment_re.match(physical_line):
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a big fan of inline returns (we rarely do this in the current codebase, I'd prefer sticking to that)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's much more readable in this way. Inline returns are not bad.


result = pep8.trailing_whitespace(physical_line)
if result:
result = (result[0], result[1].replace("W291", "W299"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally, we only have the "W299" string in a single place

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be in the docs as well, so it's gonna be here twice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we string template a docstring? :P

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just tested this, templating on a docstring doesn't really work...


# if the warning is about the multiline string of description
# we will not issue a warning
keys_re = re.compile(r"^(?P<key>[a-z_]+)\s*=\s*")

# starting from the current line and going to the top,
# check that if the first `key = value` that is found, has
# key == description, then let the test pass, else return
# the result of the general pep8 check.
for line in reversed(lines[:line_number]):
res = keys_re.match(line)
if res and res.group("key") == "description":
result = None
break

return result


@only_if_module_is_available('pep8')
def check_easyconfigs_style(easyconfigs, verbose=False):
"""
Check the given list of easyconfigs for style
:param: easyconfigs list of file paths to easyconfigs
:param: verbose print our statistics and be verbose about the errors and warning
:return: the number of warnings and errors
"""
# importing autopep8 changes some pep8 functions.
# We reload it to be sure to get the real pep8 functions.
reload(pep8)

# register the extra checks before using pep8:
# any function in this module starting with `_eb_check_` will be used.
cands = globals()
for check_function in sorted([cands[f] for f in cands if callable(cands[f]) and f.startswith(EB_CHECK)]):
_log.debug("Adding custom style check %s", check_function)
pep8.register_check(check_function)

pep8style = pep8.StyleGuide(quiet=False, config_file=None)
options = pep8style.options
# we deviate from standard pep8 and allow 120 chars
# on a line: the default of 79 is too narrow.
options.max_line_length = 120
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment above to explain why (and what the default is)

# we ignore some tests
# note that W291 has be replaced by our custom W299
options.ignore = (
'W291', # replaced by W299
)
options.verbose = int(verbose)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just make the default value 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is 0?


result = pep8style.check_files(easyconfigs)

if verbose:
result.print_statistics()

return result.total_errors
43 changes: 24 additions & 19 deletions test/framework/easyconfigs/test_ecs/g/GCC/GCC-4.6.3.eb
Original file line number Diff line number Diff line change
@@ -1,29 +1,34 @@
# should be EB_GCC, but OK for testing purposes
easyblock = 'EB_toy'

name="GCC"
version='4.6.3'
name = "GCC"
version = '4.6.3'

homepage='http://gcc.gnu.org/'
description="The GNU Compiler Collection includes front ends for C, C++, Objective-C, Fortran, Java, and Ada, as well as libraries for these languages (libstdc++, libgcj,...)."
homepage = 'http://gcc.gnu.org/'
description = """The GNU Compiler Collection includes front ends for C, C++, Objective-C, Fortran, Java, and Ada,
as well as libraries for these languages (libstdc++, libgcj,...)."""

toolchain={'name': 'dummy','version': 'dummy'}
toolchain = {'name': 'dummy', 'version': 'dummy'}

sources=['%s-%s.tar.gz'%(name.lower(),version),
'gmp-5.0.4.tar.bz2',
'mpfr-3.0.1.tar.gz',
'mpc-0.9.tar.gz',
]
source_urls=['http://ftpmirror.gnu.org/%(name)s/%(name)s-%(version)s' % {'name':name.lower(), 'version':version}, # GCC auto-resolving HTTP mirror
'http://ftpmirror.gnu.org/gmp', # idem for GMP
'http://ftpmirror.gnu.org/mpfr', # idem for MPFR
'http://www.multiprecision.org/mpc/download', # MPC official
]
sources = [
'%s-%s.tar.gz' % (name.lower(), version),
'gmp-5.0.4.tar.bz2',
'mpfr-3.0.1.tar.gz',
'mpc-0.9.tar.gz',
]

languages=['c', 'c++', 'fortran', 'lto']
source_urls = [
# GCC auto-resolving HTTP mirror
'http://ftpmirror.gnu.org/%(name)s/%(name)s-%(version)s' % {'name': name.lower(), 'version': version},
'http://ftpmirror.gnu.org/gmp', # idem for GMP
'http://ftpmirror.gnu.org/mpfr', # idem for MPFR
'http://www.multiprecision.org/mpc/download', # MPC official
]

## compiler class
moduleclass='compiler'
languages = ['c', 'c++', 'fortran', 'lto']

# compiler class
moduleclass = 'compiler'

# building GCC sometimes fails if make parallelism is too high, so let's limit it
maxparallel=4
maxparallel = 4
8 changes: 4 additions & 4 deletions test/framework/easyconfigs/test_ecs/g/GCC/GCC-4.7.2.eb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ description = """The GNU Compiler Collection includes front ends for C, C++, Obj
toolchain = {'name': 'dummy', 'version': 'dummy'}

source_urls = [
'http://ftpmirror.gnu.org/%(namelower)s/%(namelower)s-%(version)s', # GCC auto-resolving HTTP mirror
'http://ftpmirror.gnu.org/gmp', # idem for GMP
'http://ftpmirror.gnu.org/mpfr', # idem for MPFR
'http://www.multiprecision.org/mpc/download', # MPC official
'http://ftpmirror.gnu.org/%(namelower)s/%(namelower)s-%(version)s', # GCC auto-resolving HTTP mirror
'http://ftpmirror.gnu.org/gmp', # idem for GMP
'http://ftpmirror.gnu.org/mpfr', # idem for MPFR
'http://www.multiprecision.org/mpc/download', # MPC official
]
sources = [
SOURCELOWER_TAR_GZ,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ homepage = "http://www.gzip.org/"
description = "gzip (GNU zip) is a popular data compression program as a replacement for compress"

# test toolchain specification
toolchain = {'name': 'GCC','version': '4.6.3'}
toolchain = {'name': 'GCC', 'version': '4.6.3'}

# source tarball filename
sources = ['%s-%s.tar.gz'%(name,version)]
sources = ['%(name)s-%(version)s.tar.gz']

# download location for source files
source_urls = [GNU_SOURCE]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ homepage = "http://www.gzip.org/"
description = "gzip (GNU zip) is a popular data compression program as a replacement for compress"

# test toolchain specification
toolchain = {'name':'dummy','version':'dummy'}
toolchain = {'name': 'dummy', 'version': 'dummy'}

# source tarball filename
sources = [SOURCE_TAR_GZ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
easyblock = 'ConfigureMake'

name = 'gzip'
version = '1.6' # FIXME bug: quotes are required here to make sure this is parsed a a string, not a floating point value
version = '1.6'

homepage = 'http://www.gnu.org/software/gzip/'
description = "gzip (GNU zip) is a popular data compression program as a replacement for compress"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ description = """The Portable Hardware Locality (hwloc) software package provide
information about modern computing hardware so as to exploit it accordingly and efficiently."""

toolchain = {'name': 'GCC', 'version': '4.6.4'}

source_urls = ['http://www.open-mpi.org/software/hwloc/v%(version_major_minor)s/downloads/']
sources = [SOURCE_TAR_GZ]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ description = """The Portable Hardware Locality (hwloc) software package provide
information about modern computing hardware so as to exploit it accordingly and efficiently."""

toolchain = {'name': 'GCC', 'version': '4.7.2'}

source_urls = ['http://www.open-mpi.org/software/hwloc/v%(version_major_minor)s/downloads/']
sources = [SOURCE_TAR_GZ]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ versionsuffix = '-GCC-%s-%s' % (gccver, binutilsver)

dependencies = [
('GCCcore', gccver),
# ('binutils', binutilsver, '', ('GCCcore', gccver)),
# ('binutils', binutilsver, '', ('GCCcore', gccver)),
]

# full list of components can be obtained from pset/mediaconfig.xml in unpacked sources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ version = '2016.1.150'
versionsuffix = '-GCC-4.9.3-2.25'

homepage = 'http://software.intel.com/en-us/intel-cluster-toolkit-compiler/'
description = """Intel Cluster Toolkit Compiler Edition provides Intel C,C++ and fortran compilers, Intel MPI and Intel MKL"""
description = """Intel Cluster Toolkit Compiler Edition provides Intel C,C++ and fortran compilers,
Intel MPI and Intel MKL"""

toolchain = {'name': 'dummy', 'version': 'dummy'}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ versionsuffix = '-GCC-%s-%s' % (gccver, binutilsver)

dependencies = [
('GCCcore', gccver),
# ('binutils', binutilsver, '', ('GCCcore', gccver)),
# ('binutils', binutilsver, '', ('GCCcore', gccver)),
]

# full list of components can be obtained from pset/mediaconfig.xml in unpacked sources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ buildopts = 'BINARY=64 ' + threading + ' CC="$CC" FC="$F77"'
installopts = threading + " PREFIX=%(installdir)s"

# extensive testing can be enabled by uncommenting the line below
#runtest = 'PATH=.:$PATH lapack-timing'
# runtest = 'PATH=.:$PATH lapack-timing'

sanity_check_paths = {
'files': ['include/cblas.h', 'include/f77blas.h', 'include/lapacke_config.h', 'include/lapacke.h',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ else:
sanity_check_paths = {
'files': ["bin/%s" % binfile for binfile in ["ompi_info", "opal_wrapper", "orterun"]] +
["lib/lib%s.%s" % (libfile, SHLIB_EXT) for libfile in ["mpi_cxx", "mpi_f77", "mpi_f90",
"mpi", "ompitrace", "open-pal",
"open-rte", "vt", "vt-hyb",
"vt-mpi", "vt-mpi-unify"]] +
"mpi", "ompitrace", "open-pal",
"open-rte", "vt", "vt-hyb",
"vt-mpi", "vt-mpi-unify"]] +
["include/%s.h" % x for x in ["mpi-ext", "mpif-common", "mpif-config", "mpif",
"mpif-mpi-io", "mpi", "mpi_portable_platform"]],
'dirs': ["include/openmpi/ompi/mpi/cxx"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ else:
sanity_check_paths = {
'files': ["bin/%s" % binfile for binfile in ["ompi_info", "opal_wrapper", "orterun"]] +
["lib/lib%s.%s" % (libfile, SHLIB_EXT) for libfile in ["mpi_cxx", "mpi_f77", "mpi_f90",
"mpi", "ompitrace", "open-pal",
"open-rte", "vt", "vt-hyb",
"vt-mpi", "vt-mpi-unify"]] +
"mpi", "ompitrace", "open-pal",
"open-rte", "vt", "vt-hyb",
"vt-mpi", "vt-mpi-unify"]] +
["include/%s.h" % x for x in ["mpi-ext", "mpif-common", "mpif-config", "mpif",
"mpif-mpi-io", "mpi", "mpi_portable_platform"]],
'dirs': ["include/openmpi/ompi/mpi/cxx"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ sources = [SOURCE_TGZ]
# python needs bzip2 to build the bz2 package
# commented out for testing to avoid having to add them all - dependencies are tested in other files
dependencies = [
# ('bzip2', '1.0.6'),
# ('zlib', '1.2.8'),
# ('libreadline', '6.3'),
# ('ncurses', '5.9'),
# ('SQLite', '3.8.10.2'),
# ('Tk', '8.6.4', '-no-X11'),
# ('OpenSSL', '1.0.1m'), # OS dependency should be preferred if the os version is more recent then this version, it's
# nice to have an up to date openssl for security reasons
# ('bzip2', '1.0.6'),
# ('zlib', '1.2.8'),
# ('libreadline', '6.3'),
# ('ncurses', '5.9'),
# ('SQLite', '3.8.10.2'),
# ('Tk', '8.6.4', '-no-X11'),
# ('OpenSSL', '1.0.1m'), # OS dependency should be preferred if the os version is more recent then this version,
# it's nice to have an up to date openssl for security reasons
]

# zlib is only included here for the sake of testing parsing of .yeb easyconfigs!
Expand All @@ -47,7 +47,7 @@ exts_list = [
['numpy', numpyversion, {
'source_urls': [['http://sourceforge.net/projects/numpy/files/NumPy/%s' % numpyversion, 'download']],
'patches': [
'numpy-1.8.0-mkl.patch', # % numpyversion,
'numpy-1.8.0-mkl.patch', # % numpyversion,
],
}],
['scipy', scipyversion, {
Expand Down Expand Up @@ -123,4 +123,3 @@ exts_list = [
]

moduleclass = 'lang'

Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ sources = ['sqlite-autoconf-%s.tar.gz' % version_str]

# commented out for testing to avoid having to add them all - dependencies are tested in other files
dependencies = [
# ('libreadline', '6.3'),
# ('Tcl', '8.6.4'),
# ('libreadline', '6.3'),
# ('Tcl', '8.6.4'),
]

parallel = 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ sources = ['sqlite-autoconf-%s.tar.gz' % version_str]

# commented out for testing to avoid having to add them all - dependencies are tested in other files
dependencies = [
# ('libreadline', '6.3'),
# ('Tcl', '8.6.4'),
# ('libreadline', '6.3'),
# ('Tcl', '8.6.4'),
]

parallel = 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ sources = ['sqlite-autoconf-%s.tar.gz' % version_str]

# commented out for testing to avoid having to add them all - dependencies are tested in other files
dependencies = [
# ('libreadline', '6.3'),
# ('Tcl', '8.6.4'),
# ('libreadline', '6.3'),
# ('Tcl', '8.6.4'),
]

parallel = 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ versionsuffix = "-%s-%s%s" % (blaslib, blasver, blassuff)

dependencies = [(blaslib, blasver, blassuff)]

## parallel build tends to fail, so disabling it
# parallel build tends to fail, so disabling it
parallel = 1

moduleclass = 'numlib'
Loading