-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Describe the bug
I'm not familiar with the internals and intentions of nix flake check, however I was helping @bryanhonof figure out what was wrong when he got overlay does not take an argument named 'final'.
There are several issues here:
- lambdas should not semantically care about what their arguments are called (edit: yes, technically,
builtins.functionArgsdoes exist, but normally you aren't deliberately trying to break people's code. It's apparently used a lot, for example to handlecallPackagearguments, but that uses attrset arguments and notasd: asdstyle functions, for which functionArgs doesn't even work properly) - the error message is extremely misleading and suggests some manner of evaluation error
- stylistic/linting issues should not cause a hard failure
- linting issues causing hard failure prevents the checks from proceeding further
See also #4416
The responsible code is at
Lines 333 to 347 in c000cec
| auto checkOverlay = [&](const std::string & attrPath, Value & v, const Pos & pos) { | |
| try { | |
| state->forceValue(v, pos); | |
| if (!v.isLambda() || v.lambda.fun->matchAttrs || std::string(v.lambda.fun->arg) != "final") | |
| throw Error("overlay does not take an argument named 'final'"); | |
| auto body = dynamic_cast<ExprLambda *>(v.lambda.fun->body); | |
| if (!body || body->matchAttrs || std::string(body->arg) != "prev") | |
| throw Error("overlay does not take an argument named 'prev'"); | |
| // FIXME: if we have a 'nixpkgs' input, use it to | |
| // evaluate the overlay. | |
| } catch (Error & e) { | |
| e.addTrace(pos, hintfmt("while checking the overlay '%s'", attrPath)); | |
| reportError(e); | |
| } | |
| }; |
Steps To Reproduce
@bryanhonof can you give a repro for this?
Expected behavior
We will try to alleviate 2. a bit, by PR-ing a better error message.
We also took a short look at fixing the hard-failure problem but the changes looked not-quite-trivial - or at least we didn't offhand see how we should deal with the test suite, etc. I also don't know nix flake check how nix flake check is expected to behave, but standard things should be included, like being able to override checks if you decide to maintain everything as a hard failure. - otherwise, the solution is to separate linter issues, and give them as warnings.
nix-env --version output
nix-env (Nix) 2.4pre20210802_47e96bb