New category of checks: log checks

[rmcgibbo]

As discussed previously on IRC, it seems it would be nice to incorporate a class of checks that looks at build logs. Ideally these checks would be pretty simple to write -- maybe just grep?

The check binary receives the log from stdin, and is supposed to write its lint message to stdout and exit 1 if it triggers.

This is an initial draft of the idea. I haven't added any checks yet, although this is what I was testing with

#!/usr/bin/env bash
grep  'Ran 0 tests in 0.000s' >/dev/null
if [ $? = 0 ]; then
    echo '{"name": "hello", "msg": "message", "severity": "warning", "link": true}'
    exit 1
fi

I'm not sure if a better design would be just to provide the outPath of the drv to the binary as $1 and then ask it to call nix log itself.

[rmcgibbo]

Perhaps we should go through the list of checks of interest in #1 and the new ones that have been proposed, and figure out what kind of information they require. For ones that can't be done using the overlay technique, do they require logs? output paths? the .nix file containing the expression? Something else? If these can be grouped into a smaller number of categories, that would probably be preferable.

For example, if each binary were passed a JSON struct containing the name of the attr, the output path in the nix store if one exists, and the path to the .nix file in which it were defined then the current AST checks and these log checks could all be performed with the same mechanism.

[SuperSandro2000]

Please do not use the .drv to collect the logs but the output path. See Mic92/nixpkgs-review#170

[SuperSandro2000]

Things I currently grep in logs:

• Zero tests run in Python
  • rg --ignore-case -e 'Ran 0 tests in 0.000s'
• stale substituteInPlace
  • rg --ignore-case -e "substituteStream\(\): WARNING: pattern (.*?) doesn't match anything in file '(.*?)'"

[rmcgibbo]

I made a lot of progress on this. I still need to work on the tests, but the basic idea is to unify the AST- and log-based checks into a single category of "external checks". The "external checks" are passed substantially more information by the python tool than the AST checks were passed previously, and can decide how to work with it as they please (i.e. by looking at the log file, looking at the .nix file, or looking at some other information).

[rmcgibbo]

Hm, unfortunately I think the approach of having a single nix expression that evaluates the outPath and drvPath of each of the attrs is pretty incompatible with our test suite, because most of the tests don't fully evaluate to drvs for reason or another, and you can't tryEval lots of types of nix type errors.

[jtojnar]

I think we should fix the derivations. They were initially intended to build (hence the fixtures) but once we switched to the __reports and stopped actually building them they broke.

[rmcgibbo]

Okay, I'll take a look at what's possible. For some of the attribute-typo tests, it wasn't obvious to me that they could even be made to build.

[rmcgibbo]

Need to add explanation file

[rmcgibbo]

Need to add explanation file

[rmcgibbo]

I think this PR is roughly ready to be reviewed. There are two new checks: 'stale-substitute' and 'no-python-tests', as well as a new mechanism for passing data to the checks that are written in rust. Both required a bit of internal reorganization in the nixpkgs-hammer python code. I'll need to squash the commits and add explanation files.

[jtojnar]

This is quite hard to review. Could you please split the style changes and ast → rust rename into separate commits and rebase the PR onto them so that they do not overshadow the changes?

[rmcgibbo]

Will do.

[rmcgibbo]

Moving to a new PR, because I got confused with all the git stuff.