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

[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

[bryanhonof]

Steps To Reproduce
@bryanhonof can you give a repro for this?

Any flake where the overlay attribute is a lambda that takes anything else than final: prev: should reproduce the error. E.g. the flake template from poetry2nix is able to reproduce this error. Since the nixpkgs.lib.composeManyExtensions returns a lambda with self: super:.

1. cd $(mktemp -d)
2. nix flake init --template github:nix-community/poetry2nix
3. nix flake check
   output: error: overlay does not take an argument named 'final'

[deliciouslytyped]

You can use hack = f: final: prev: f final prev as a workaround.

[bryanhonof]

Someone has fixed the lib.compose{Many,}Extensions in NixPkgs. So this is no longer reproducible with the poetry2nix flake template. Instead try the following flake:

#flake.nix
{
  description = "Overlay error message flake";

  inputs.flake-utils.url = "github:numtide/flake-utils";
  inputs.nixpkgs.url = "github:NixOS/nixpkgs";

  outputs = { self, nixpkgs, flake-utils }:
    {
      # Use `self: super:` to make Nix generate the error message
      overlay = (self: super: {
        myapp = derivation;
      });
    } // (flake-utils.lib.eachDefaultSystem (system:
      let
        pkgs = import nixpkgs {
          inherit system;
          overlays = [ self.overlay ];
        };
      in rec {
        apps = { myapp = pkgs.myapp; };

        defaultApp = apps.myapp;
      }));
}

[stale]

I marked this as stale due to inactivity. → More info

[SuperSandro2000]

Bump

---

@oxapentane @revol-xut @astro

[astro]

In deadnix I still have to implement a workaround so that these lambda args cannot get renamed to _ which would cause this issue. I'd rather not.