Add an audit system to Spack (#23053)

Add a new "spack audit" command. This command can check for issues
with configuration or with packages and is intended to help a
user debug a failed Spack build. 

In some cases the reported issues are always errors but are too
costly to check for (e.g. packages that specify missing variants on
dependencies). In other cases the issues may be legitimate but
uncommon usage of Spack and we want to be sure the user intended the
behavior (e.g. duplicate compiler definitions).

Audits are grouped by theme, and for now the two themes are packages
and configuration. For example you can run all available audits
on packages with "spack audit packages". It is intended that in
the future users will be able to define their own audits.

The package audits are good candidates for running in package_sanity
(i.e. they could catch bugs in user-submitted packages before they
are merged) but that is left for a later PR.
This commit is contained in:
Massimiliano Culpo 2021-06-18 15:52:08 +02:00 committed by GitHub
parent 8ad05d6a74
commit 32f1aa607c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 652 additions and 1 deletions

View File

@ -1730,6 +1730,39 @@ This issue typically manifests with the error below:
A nicer error message is TBD in future versions of Spack.
---------------
Troubleshooting
---------------
The ``spack audit`` command:
.. command-output:: spack audit -h
can be used to detect a number of configuration issues. This command detects
configuration settings which might not be strictly wrong but are not likely
to be useful outside of special cases.
It can also be used to detect dependency issues with packages - for example
cases where a package constrains a dependency with a variant that doesn't
exist (in this case Spack could report the problem ahead of time but
automatically performing the check would slow down most runs of Spack).
A detailed list of the checks currently implemented for each subcommand can be
printed with:
.. command-output:: spack -v audit list
Depending on the use case, users might run the appropriate subcommands to obtain
diagnostics. Issues, if found, are reported to stdout:
.. code-block:: console
% spack audit packages lammps
PKG-DIRECTIVES: 1 issue found
1. lammps: wrong variant in "conflicts" directive
the variant 'adios' does not exist
in /home/spack/spack/var/spack/repos/builtin/packages/lammps/package.py
------------
Getting Help

393
lib/spack/spack/audit.py Normal file
View File

@ -0,0 +1,393 @@
# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
"""Classes and functions to register audit checks for various parts of
Spack and run them on-demand.
To register a new class of sanity checks (e.g. sanity checks for
compilers.yaml), the first action required is to create a new AuditClass
object:
.. code-block:: python
audit_cfgcmp = AuditClass(
tag='CFG-COMPILER',
description='Sanity checks on compilers.yaml',
kwargs=()
)
This object is to be used as a decorator to register functions
that will perform each a single check:
.. code-block:: python
@audit_cfgcmp
def _search_duplicate_compilers(error_cls):
pass
These functions need to take as argument the keywords declared when
creating the decorator object plus an ``error_cls`` argument at the
end, acting as a factory to create Error objects. It should return a
(possibly empty) list of errors.
Calls to each of these functions are triggered by the ``run`` method of
the decorator object, that will forward the keyword arguments passed
as input.
"""
import collections
import itertools
try:
from collections.abc import Sequence # novm
except ImportError:
from collections import Sequence
#: Map an audit tag to a list of callables implementing checks
CALLBACKS = {}
#: Map a group of checks to the list of related audit tags
GROUPS = collections.defaultdict(list)
class Error(object):
"""Information on an error reported in a test."""
def __init__(self, summary, details):
self.summary = summary
self.details = tuple(details)
def __str__(self):
return self.summary + '\n' + '\n'.join([
' ' + detail for detail in self.details
])
def __eq__(self, other):
if self.summary != other.summary or self.details != other.details:
return False
return True
def __hash__(self):
value = (self.summary, self.details)
return hash(value)
class AuditClass(Sequence):
def __init__(self, group, tag, description, kwargs):
"""Return an object that acts as a decorator to register functions
associated with a specific class of sanity checks.
Args:
group (str): group in which this check is to be inserted
tag (str): tag uniquely identifying the class of sanity checks
description (str): description of the sanity checks performed
by this tag
kwargs (tuple of str): keyword arguments that each registered
function needs to accept
"""
if tag in CALLBACKS:
msg = 'audit class "{0}" already registered'
raise ValueError(msg.format(tag))
self.group = group
self.tag = tag
self.description = description
self.kwargs = kwargs
self.callbacks = []
# Init the list of hooks
CALLBACKS[self.tag] = self
# Update the list of tags in the group
GROUPS[self.group].append(self.tag)
def __call__(self, func):
self.callbacks.append(func)
def __getitem__(self, item):
return self.callbacks[item]
def __len__(self):
return len(self.callbacks)
def run(self, **kwargs):
msg = 'please pass "{0}" as keyword arguments'
msg = msg.format(', '.join(self.kwargs))
assert set(self.kwargs) == set(kwargs), msg
errors = []
kwargs['error_cls'] = Error
for fn in self.callbacks:
errors.extend(fn(**kwargs))
return errors
def run_group(group, **kwargs):
"""Run the checks that are part of the group passed as argument.
Args:
group (str): group of checks to be run
**kwargs: keyword arguments forwarded to the checks
Returns:
List of (tag, errors) that failed.
"""
reports = []
for check in GROUPS[group]:
errors = run_check(check, **kwargs)
reports.append((check, errors))
return reports
def run_check(tag, **kwargs):
"""Run the checks associated with a single tag.
Args:
tag (str): tag of the check
**kwargs: keyword arguments forwarded to the checks
Returns:
Errors occurred during the checks
"""
return CALLBACKS[tag].run(**kwargs)
# TODO: For the generic check to be useful for end users,
# TODO: we need to implement hooks like described in
# TODO: https://github.com/spack/spack/pull/23053/files#r630265011
#: Generic checks relying on global state
generic = AuditClass(
group='generic',
tag='GENERIC',
description='Generic checks relying on global variables',
kwargs=()
)
#: Sanity checks on compilers.yaml
config_compiler = AuditClass(
group='configs',
tag='CFG-COMPILER',
description='Sanity checks on compilers.yaml',
kwargs=()
)
@config_compiler
def _search_duplicate_compilers(error_cls):
"""Report compilers with the same spec and two different definitions"""
import spack.config
errors = []
compilers = list(sorted(
spack.config.get('compilers'), key=lambda x: x['compiler']['spec']
))
for spec, group in itertools.groupby(
compilers, key=lambda x: x['compiler']['spec']
):
group = list(group)
if len(group) == 1:
continue
error_msg = 'Compiler defined multiple times: {0}'
try:
details = [str(x._start_mark).strip() for x in group]
except Exception:
details = []
errors.append(error_cls(
summary=error_msg.format(spec), details=details
))
return errors
#: Sanity checks on packages.yaml
config_packages = AuditClass(
group='configs',
tag='CFG-PACKAGES',
description='Sanity checks on packages.yaml',
kwargs=()
)
@config_packages
def _search_duplicate_specs_in_externals(error_cls):
"""Search for duplicate specs declared as externals"""
import spack.config
errors, externals = [], collections.defaultdict(list)
packages_yaml = spack.config.get('packages')
for name, pkg_config in packages_yaml.items():
# No externals can be declared under all
if name == 'all' or 'externals' not in pkg_config:
continue
current_externals = pkg_config['externals']
for entry in current_externals:
# Ask for the string representation of the spec to normalize
# aspects of the spec that may be represented in multiple ways
# e.g. +foo or foo=true
key = str(spack.spec.Spec(entry['spec']))
externals[key].append(entry)
for spec, entries in sorted(externals.items()):
# If there's a single external for a spec we are fine
if len(entries) < 2:
continue
# Otherwise wwe need to report an error
error_msg = 'Multiple externals share the same spec: {0}'.format(spec)
try:
lines = [str(x._start_mark).strip() for x in entries]
details = [
'Please remove all but one of the following entries:'
] + lines + [
'as they might result in non-deterministic hashes'
]
except TypeError:
details = []
errors.append(error_cls(summary=error_msg, details=details))
return errors
#: Sanity checks on package directives
package_directives = AuditClass(
group='packages',
tag='PKG-DIRECTIVES',
description='Sanity checks on specs used in directives',
kwargs=('pkgs',)
)
@package_directives
def _unknown_variants_in_directives(pkgs, error_cls):
"""Report unknown or wrong variants in directives for this package"""
import llnl.util.lang
import spack.repo
import spack.spec
errors = []
for pkg_name in pkgs:
pkg = spack.repo.get(pkg_name)
# Check "conflicts" directive
for conflict, triggers in pkg.conflicts.items():
for trigger, _ in triggers:
vrn = spack.spec.Spec(conflict)
try:
vrn.constrain(trigger)
except Exception as e:
msg = 'Generic error in conflict for package "{0}": '
errors.append(error_cls(msg.format(pkg.name), [str(e)]))
continue
errors.extend(_analyze_variants_in_directive(
pkg, vrn, directive='conflicts', error_cls=error_cls
))
# Check "depends_on" directive
for _, triggers in pkg.dependencies.items():
triggers = list(triggers)
for trigger in list(triggers):
vrn = spack.spec.Spec(trigger)
errors.extend(_analyze_variants_in_directive(
pkg, vrn, directive='depends_on', error_cls=error_cls
))
# Check "patch" directive
for _, triggers in pkg.provided.items():
triggers = [spack.spec.Spec(x) for x in triggers]
for vrn in triggers:
errors.extend(_analyze_variants_in_directive(
pkg, vrn, directive='patch', error_cls=error_cls
))
# Check "resource" directive
for vrn in pkg.resources:
errors.extend(_analyze_variants_in_directive(
pkg, vrn, directive='resource', error_cls=error_cls
))
return llnl.util.lang.dedupe(errors)
@package_directives
def _unknown_variants_in_dependencies(pkgs, error_cls):
"""Report unknown dependencies and wrong variants for dependencies"""
import spack.repo
import spack.spec
errors = []
for pkg_name in pkgs:
pkg = spack.repo.get(pkg_name)
filename = spack.repo.path.filename_for_package_name(pkg_name)
for dependency_name, dependency_data in pkg.dependencies.items():
# No need to analyze virtual packages
if spack.repo.path.is_virtual(dependency_name):
continue
try:
dependency_pkg = spack.repo.get(dependency_name)
except spack.repo.UnknownPackageError:
# This dependency is completely missing, so report
# and continue the analysis
summary = (pkg_name + ": unknown package '{0}' in "
"'depends_on' directive".format(dependency_name))
details = [
" in " + filename
]
errors.append(error_cls(summary=summary, details=details))
continue
for _, dependency_edge in dependency_data.items():
dependency_variants = dependency_edge.spec.variants
for name, value in dependency_variants.items():
try:
dependency_pkg.variants[name].validate_or_raise(
value, pkg=dependency_pkg
)
except Exception as e:
summary = (pkg_name + ": wrong variant used for a "
"dependency in a 'depends_on' directive")
error_msg = str(e).strip()
if isinstance(e, KeyError):
error_msg = ('the variant {0} does not '
'exist'.format(error_msg))
error_msg += " in package '" + dependency_name + "'"
errors.append(error_cls(
summary=summary, details=[error_msg, 'in ' + filename]
))
return errors
def _analyze_variants_in_directive(pkg, constraint, directive, error_cls):
import spack.variant
variant_exceptions = (
spack.variant.InconsistentValidationError,
spack.variant.MultipleValuesInExclusiveVariantError,
spack.variant.InvalidVariantValueError,
KeyError
)
errors = []
for name, v in constraint.variants.items():
try:
pkg.variants[name].validate_or_raise(v, pkg=pkg)
except variant_exceptions as e:
summary = pkg.name + ': wrong variant in "{0}" directive'
summary = summary.format(directive)
filename = spack.repo.path.filename_for_package_name(pkg.name)
error_msg = str(e).strip()
if isinstance(e, KeyError):
error_msg = 'the variant {0} does not exist'.format(error_msg)
err = error_cls(summary=summary, details=[
error_msg, 'in ' + filename
])
errors.append(err)
return errors

View File

@ -0,0 +1,80 @@
# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import llnl.util.tty.color as cl
import spack.audit
import spack.repo
description = "audit configuration files, packages, etc."
section = "system"
level = "short"
def setup_parser(subparser):
# Top level flags, valid for every audit class
sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='subcommand')
# Audit configuration files
sp.add_parser('configs', help='audit configuration files')
# Audit package recipes
pkg_parser = sp.add_parser('packages', help='audit package recipes')
pkg_parser.add_argument(
'name', metavar='PKG', nargs='*',
help='package to be analyzed (if none all packages will be processed)',
)
# List all checks
sp.add_parser('list', help='list available checks and exits')
def configs(parser, args):
reports = spack.audit.run_group(args.subcommand)
_process_reports(reports)
def packages(parser, args):
pkgs = args.name or spack.repo.path.all_package_names()
reports = spack.audit.run_group(args.subcommand, pkgs=pkgs)
_process_reports(reports)
def list(parser, args):
for subcommand, check_tags in spack.audit.GROUPS.items():
print(cl.colorize('@*b{' + subcommand + '}:'))
for tag in check_tags:
audit_obj = spack.audit.CALLBACKS[tag]
print(' ' + audit_obj.description)
if args.verbose:
for idx, fn in enumerate(audit_obj.callbacks):
print(' {0}. '.format(idx + 1) + fn.__doc__)
print()
print()
def audit(parser, args):
subcommands = {
'configs': configs,
'packages': packages,
'list': list
}
subcommands[args.subcommand](parser, args)
def _process_reports(reports):
for check, errors in reports:
if errors:
msg = '{0}: {1} issue{2} found'.format(
check, len(errors), '' if len(errors) == 1 else 's'
)
header = '@*b{' + msg + '}'
print(cl.colorize(header))
for idx, error in enumerate(errors):
print(str(idx + 1) + '. ' + str(error))
raise SystemExit(1)
else:
msg = '{0}: 0 issues found.'.format(check)
header = '@*b{' + msg + '}'
print(cl.colorize(header))

View File

@ -0,0 +1,85 @@
# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import pytest
import spack.audit
import spack.config
@pytest.mark.parametrize('packages,failing_check', [
# A non existing variant is used in a conflict directive
(['wrong-variant-in-conflicts'], 'PKG-DIRECTIVES'),
# The package declares a non-existing dependency
(['missing-dependency'], 'PKG-DIRECTIVES'),
# The package use a non existing variant in a depends_on directive
(['wrong-variant-in-depends-on'], 'PKG-DIRECTIVES'),
# This package has no issues
(['mpileaks'], None)
])
def test_package_audits(packages, failing_check, mock_packages):
reports = spack.audit.run_group('packages', pkgs=packages)
for check, errors in reports:
# Check that we have errors only if there is an expected failure
# and that the tag matches our expectations
if bool(failing_check):
assert check == failing_check
assert errors
else:
assert not errors
# Data used in the test below to audit the double definition of a compiler
_double_compiler_definition = [
{'compiler': {
'spec': 'gcc@9.0.1',
'paths': {
'cc': '/usr/bin/gcc-9',
'cxx': '/usr/bin/g++-9',
'f77': '/usr/bin/gfortran-9',
'fc': '/usr/bin/gfortran-9'
},
'flags': {},
'operating_system': 'ubuntu18.04',
'target': 'x86_64',
'modules': [],
'environment': {},
'extra_rpaths': []
}},
{'compiler': {
'spec': 'gcc@9.0.1',
'paths': {
'cc': '/usr/bin/gcc-9',
'cxx': '/usr/bin/g++-9',
'f77': '/usr/bin/gfortran-9',
'fc': '/usr/bin/gfortran-9'
},
'flags': {"cflags": "-O3"},
'operating_system': 'ubuntu18.04',
'target': 'x86_64',
'modules': [],
'environment': {},
'extra_rpaths': []
}}
]
@pytest.mark.parametrize('config_section,data,failing_check', [
# Double compiler definitions in compilers.yaml
('compilers', _double_compiler_definition, 'CFG-COMPILER'),
# Multiple definitions of the same external spec in packages.yaml
('packages', {
"mpileaks": {"externals": [
{"spec": "mpileaks@1.0.0", "prefix": "/"},
{"spec": "mpileaks@1.0.0", "prefix": "/usr"},
]}
}, 'CFG-PACKAGES')
])
def test_config_audits(config_section, data, failing_check):
with spack.config.override(config_section, data):
reports = spack.audit.run_group('configs')
assert any(
(check == failing_check) and errors for check, errors in reports
)

View File

@ -0,0 +1,34 @@
# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import pytest
from spack.main import SpackCommand
audit = SpackCommand('audit')
@pytest.mark.parametrize('pkgs,expected_returncode', [
# A single package with issues, should exit 1
(['wrong-variant-in-conflicts'], 1),
# A "sane" package should exit 0
(['mpileaks'], 0),
# A package with issues and a package without should exit 1
(['wrong-variant-in-conflicts', 'mpileaks'], 1),
(['mpileaks', 'wrong-variant-in-conflicts'], 1),
])
def test_audit_packages(
pkgs, expected_returncode, mutable_config, mock_packages
):
"""Sanity check ``spack audit packages`` to make sure it works."""
audit('packages', *pkgs, fail_on_error=False)
assert audit.returncode == expected_returncode
def test_audit_configs(mutable_config, mock_packages):
"""Sanity check ``spack audit packages`` to make sure it works."""
audit('configs', fail_on_error=False)
# The mock configuration has duplicate definitions of some compilers
assert audit.returncode == 1

View File

@ -333,7 +333,7 @@ _spack() {
then
SPACK_COMPREPLY="-h --help -H --all-help --color -c --config -C --config-scope -d --debug --timestamp --pdb -e --env -D --env-dir -E --no-env --use-env-repo -k --insecure -l --enable-locks -L --disable-locks -m --mock -p --profile --sorted-profile --lines -v --verbose --stacktrace -V --version --print-shell-vars"
else
SPACK_COMPREPLY="activate add analyze arch blame build-env buildcache cd checksum ci clean clone commands compiler compilers concretize config containerize create deactivate debug dependencies dependents deprecate dev-build develop docs edit env extensions external fetch find flake8 gc gpg graph help info install license list load location log-parse maintainers mark mirror module monitor patch pkg providers pydoc python reindex remove rm repo resource restage solve spec stage style test test-env tutorial undevelop uninstall unit-test unload url verify versions view"
SPACK_COMPREPLY="activate add analyze arch audit blame build-env buildcache cd checksum ci clean clone commands compiler compilers concretize config containerize create deactivate debug dependencies dependents deprecate dev-build develop docs edit env extensions external fetch find flake8 gc gpg graph help info install license list load location log-parse maintainers mark mirror module monitor patch pkg providers pydoc python reindex remove rm repo resource restage solve spec stage style test test-env tutorial undevelop uninstall unit-test unload url verify versions view"
fi
}
@ -381,6 +381,32 @@ _spack_arch() {
SPACK_COMPREPLY="-h --help --known-targets -p --platform -o --operating-system -t --target -f --frontend -b --backend"
}
_spack_audit() {
if $list_options
then
SPACK_COMPREPLY="-h --help"
else
SPACK_COMPREPLY="configs packages list"
fi
}
_spack_audit_configs() {
SPACK_COMPREPLY="-h --help"
}
_spack_audit_packages() {
if $list_options
then
SPACK_COMPREPLY="-h --help"
else
SPACK_COMPREPLY=""
fi
}
_spack_audit_list() {
SPACK_COMPREPLY="-h --help"
}
_spack_blame() {
if $list_options
then