Skip to content

Naming overlays arguments something other than final and prev should not be a hard failure #5146

@deliciouslytyped

Description

@deliciouslytyped

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:

  1. lambdas should not semantically care about what their arguments are called (edit: yes, technically, builtins.functionArgs does exist, but normally you aren't deliberately trying to break people's code. It's apparently used a lot, for example to handle callPackage arguments, but that uses attrset arguments and not asd: asd style functions, for which functionArgs doesn't even work properly)
  2. the error message is extremely misleading and suggests some manner of evaluation error
  3. stylistic/linting issues should not cause a hard failure
  4. linting issues causing hard failure prevents the checks from proceeding further

See also #4416

The responsible code is at

nix/src/nix/flake.cc

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions