Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flake overlays cannot use __functor #7349

Open
arcnmx opened this issue Nov 25, 2022 · 6 comments
Open

flake overlays cannot use __functor #7349

arcnmx opened this issue Nov 25, 2022 · 6 comments

Comments

@arcnmx
Copy link
Member

arcnmx commented Nov 25, 2022

Describe the bug

Using callable attrsets as overlays does not pass flake check:

# flake.nix
{
  outputs = { self, nixpkgs, ... }: {
    overlays.default = {
      __functor = self: final: prev: { };
    };
  };
}

Steps To Reproduce

  1. nix flake check
  2. observe error: overlay does not take an argument named 'final'

Expected behavior

I expect that it would check the __functor self instead, and stop complaining!

nix-env --version output

nix-env (Nix) 2.11.0

Additional context

These values are valid as far as nixpkgs is concerned, since it typechecks overlays via lib.isFunction (which also accepts functors).

@arcnmx arcnmx added the bug label Nov 25, 2022
@edolstra
Copy link
Member

This is intended behaviour, see #6404.

@edolstra edolstra removed the bug label Nov 25, 2022
@arcnmx
Copy link
Member Author

arcnmx commented Nov 25, 2022

Sorry, I'm not sure how that's related? I'm not saying that nix should do anything special with evaluation or auto-call anything.

The point is that functor attrsets themselves (self.overlays.default in this case) are valid overlays (explicitly so in nixpkgs, and presumably in anything else that may use them unless they specifically guard against it using builtins.isFunction), and it seems like a mismatch between nix and nixpkgs that flake check doesn't explicitly check for and allow them (the code very explicitly only allows lambdas through), which I'd argue is incomplete logic (or at the very least, overly restrictive).

Plus the error message states overlay does not take an argument named 'final', which is a lie - the overlay is callable with two arguments and behaves like any other overlay 😛

@aameen-tulip
Copy link
Contributor

While I agree with the existing implementation's decision to reject functors; this should get a footnote in the manual explaining why functors are rejected.

@arcnmx
Copy link
Member Author

arcnmx commented Nov 29, 2022

Hm, perhaps the problem really is that flake check attempts to be a "one size fits all" check, while not actually matching any particular implementation or use-case. nix's idea of an overlay and nixosModule does not actually match up with nixpkgs (though it allows a subset of valid overlays/modules through, so that just makes it overly restrictive, but not entirely incorrect). #6866 is another instance of this issue for nixosModules.
Offtopic, but it feels like the only thing it should actually check is checks.x, which could do their own type-checking on self if desired. I assume there's already an issue tracking the fact that you can't turn particular checks off? Or maybe the expectation is you just make a custom checker invoked via nix build .#check or something.

But yes, if this is intentionally over-restrictive, the error messages should be improved at the very least.

@aakropotkin
Copy link
Contributor

aakropotkin commented Nov 29, 2022

Checking that overlays are strictly defined as final: prev: enforces standardization in an already difficult to navigate area, so I do support it's behavior.

If you want to use a functor the fix isn't painful :

overlays.foo = final: prev: let
  funk = { 
    junk = x: x.hello or null;
    __functor = self: f: p: {
      pkg = if self.junk f == null
            then self.junk p
            else self.junk f;
    };
  };
in funk final prev;

@aameen-tulip
Copy link
Contributor

aameen-tulip commented Nov 30, 2022

Another noteworthy quality of a "standard" overlay is that it is strictly a thunk from the perspective of the checker when evaluating builtins.isFunction overlays.foo {} which is not necessarily the case for a functor or overlays that use formal arguments ( { foo ? builtins.deepSeq nixpkgs.x86_64-linux true, ... } @ final: if foo then { bar, ...} @ prev: {...} else prev: {...}; ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants