Skip to content

Add an audit system to Spack#23053

Merged
alalazo merged 17 commits intospack:developfrom
alalazo:features/spack_audit
Jun 18, 2021
Merged

Add an audit system to Spack#23053
alalazo merged 17 commits intospack:developfrom
alalazo:features/spack_audit

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Apr 16, 2021 •
edited
Loading

This PR adds a way to check various parts of Spack from the command line. The motivation to start this has been seeing a few issues where errors / inconsistencies in package recipes where mistaken for bugs in the new concretizer, due to poor reporting, and a debug session with @jjellio in which after much struggle we understood that an issue was due to a compiler being defined multiple times in the configuration (which is not allowed but Spack was not clear at all on what was the problem).

The PR should introduce:

  • A way to declare new class of audits (e.g. audits for compiler configuration, audits for package directives etc.)
  • A way to register functions as part of those audits
  • A command to drive this from the terminal
  • Documentation of the new feature
  • Tests for the spack.audit module

As a first example, recreating a case like the one debugged with @jjellio we can obtain with this PR:

culpo@MacBook-Pro bootstrap % cat spack.yaml             
spack:
  specs:
  - zlib
  compilers:
  - compiler:
      spec: [email protected]
      paths:
        cc: /Users/culpo/PycharmProjects/spack/opt/spack/darwin-bigsur-cannonlake/apple-clang-12.0.0/gcc-10.2.0-zm6u6otbxbacpvsbx6lugzo6qeb2hd7v/bin/gcc
        cxx: /Users/culpo/PycharmProjects/spack/opt/spack/darwin-bigsur-cannonlake/apple-clang-12.0.0/gcc-10.2.0-zm6u6otbxbacpvsbx6lugzo6qeb2hd7v/bin/g++
        f77: /Users/culpo/PycharmProjects/spack/opt/spack/darwin-bigsur-cannonlake/apple-clang-12.0.0/gcc-10.2.0-zm6u6otbxbacpvsbx6lugzo6qeb2hd7v/bin/gfortran
        fc: /Users/culpo/PycharmProjects/spack/opt/spack/darwin-bigsur-cannonlake/apple-clang-12.0.0/gcc-10.2.0-zm6u6otbxbacpvsbx6lugzo6qeb2hd7v/bin/gfortran
      flags:
        cflags: '-O3'
      operating_system: bigsur
      target: x86_64
      modules: []
      environment: {}
      extra_rpaths: []
culpo@MacBook-Pro bootstrap % spack audit configuration     
culpo@MacBook-Pro bootstrap % spack -e . audit configuration
CFG-COMPILER: 1 issue found
1. Compiler defined multiple times: [email protected]
    in "/Users/culpo/spack/bootstrap/spack.yaml", line 5, column 5
    in "/Users/culpo/.spack/darwin/compilers.yaml", line 2, column 3

This was referenced Apr 19, 2021
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@alalazo alalazo force-pushed the features/spack_audit branch 3 times, most recently from 10549f9 to a4720ca Compare April 20, 2021 13:10
@alalazo alalazo marked this pull request as ready for review April 20, 2021 13:59
@scheibelp scheibelp self-assigned this Apr 20, 2021
@alalazo alalazo force-pushed the features/spack_audit branch 3 times, most recently from fbe1754 to 8137056 Compare May 6, 2021 15:51
scheibelp
scheibelp reviewed May 7, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@scheibelp scheibelp left a comment

Choose a reason for hiding this comment

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

I have a few preliminary comments but I think the most important discussion to address first is the one I marked as "high-level": I'm wondering about the necessity of the AuditClass class.

@alalazo alalazo force-pushed the features/spack_audit branch from 8137056 to 873860f Compare May 13, 2021 16:53
@alalazo alalazo force-pushed the features/spack_audit branch 2 times, most recently from bf6a81d to fd330cd Compare May 24, 2021 09:05
This was referenced Jun 1, 2021
Merged
Merged
Merged
Merged
@alalazo alalazo force-pushed the features/spack_audit branch from fd330cd to 6482eeb Compare June 1, 2021 16:26
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jun 1, 2021

@scheibelp Ready for a second review

@alalazo alalazo mentioned this pull request Jun 9, 2021
Closed
@scheibelp scheibelp reopened this Jun 15, 2021
alalazo added 17 commits June 16, 2021 07:36
@alalazo
Add an audit system to Spack
9c08726 
Create a new module that permits to:

- Define class of audits to be done on
  different parts of Spack
- Register functions to perform single
  checks

Also added a command to interface with users
@alalazo
Check variants in package directives
e8fc59f 
Add a check to validate variants used in package directives
@alalazo
Update command completion
206015a
@alalazo
Added a check on variants in depends_on directives
b0fdef4
@alalazo
Add unit tests for the command
6375435
@alalazo
Add documentation for spack audit
a7fcf85
@alalazo
Add a check for duplicate specs in packages.yaml
eb4e023
@alalazo
Added a brief explanation of why this command might be useful
ba4f6d8
@alalazo
Turn the list of packages into positional arguments
3273a2e
@alalazo
Make audit more robust to inconsistencies in conflicts
0e5ea27
@alalazo
Moved grouping into the spack.audit module
ee6a1db
@alalazo
Add a decorator for generic checks
7700249
@alalazo
Updated docs according to review
0c27575
@alalazo
Add a comment to make clear that we still need hooks for the generic …
2e317bb 
…checks
@alalazo
Add unit tests for each specific audit
0efe179
@alalazo
Make a parametrized test more readable
f39c8cc
@alalazo
Test the "packages" subcommand at a finer level
e0f8abc
@alalazo alalazo force-pushed the features/spack_audit branch from 29551c2 to e0f8abc Compare June 16, 2021 05:38
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jun 16, 2021

@scheibelp Pushed again to fix a conflict with command completion

@scheibelp
Copy link
Copy Markdown
Member

scheibelp commented Jun 18, 2021

Close/reopen to restart CI

@scheibelp scheibelp closed this Jun 18, 2021
@scheibelp scheibelp reopened this Jun 18, 2021
@alalazo alalazo enabled auto-merge (squash) June 18, 2021 13:51
@alalazo alalazo merged commit 32f1aa6 into spack:develop Jun 18, 2021
@alalazo alalazo deleted the features/spack_audit branch June 18, 2021 13:52
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jun 18, 2021

Thanks @scheibelp !

bollig pushed a commit to bollig/spack that referenced this pull request Jun 29, 2021
@alalazo @bollig
Add an audit system to Spack (spack#23053)
fb26dd8 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scheibelp scheibelp scheibelp approved these changes

Assignees

@scheibelp scheibelp

Labels

new-command

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alalazo @scheibelp