What to pay attention to while reviewing:

• types in config.nix, especially the ones for *packageOverrides, I think they are correct, but I might have missed something,
• do commit messages make sense?

Btw, changes in metrics of nix-env -f . -qaP --drv-path --out-path between the parent of 570aed4 and this HEAD:

Before:

time 47.205 s
maxresident 4264000 KiB
allocations 6007093832 B
values 80800261

After:

time 47.047 s
maxresident 4241936 KiB
allocations 5956525992 B
values 79612951

I wouldn't believe the time metric, it's too random, but the rest are most certainly valid.

So, despite adding type checking to nixpkgs.config I improved all the things :]

Hm, also 1d1ebf5 changes the semantics of haskellPackageOverrides a bit as config.haskellPackageOverrides is never undefined now. @peti, should I change that? If so then how? The interaction between config.haskellPackageOverrides and haskell.packageOverrides seems nontrivial.

haskellPackageOverrides

As for haskellPackageOverrides, it is better to comment it. Maybe it is deprecated by overlays? It will also remove the need for @peti to review this PR.

functionTo

Do we need generic "function to"? I'm biased here, as I like my "dependently typed selector" variant (#44601 (comment)). listSelector, attrsSelector and overlay are the only required function types. ... I'm not sure overlays are worth to encode in types.

_file -> file

I've used _file for my custom modules already. There has to be backwards compat. But I also wonder why was it named like that. Maybe, to discourage usage? In that case I have a valid usecase:

let
  module1 = {
     key = "module1"; _file = "this_file.nix";
     config = { error.option = true; };
  };
  module2 = {
     key = "module2"; _file = "this_file.nix";
  };
in {
  imports = [ module1 module2 ];
}

# without _file
nix-repl> evalModules { modules = [ (import ../simple-2-modules.nix) ]; }
{ config = «error: The option `error' defined in `<unknown-file>' does not exist.»; options = { ... }; }

# with _file
nix-repl> evalModules { modules = [ (import ../simple-2-modules.nix) ]; }
{ config = «error: The option `error' defined in `this_file.nix' does not exist.»; options = { ... }; }

At least I know file name.

make unifyModuleSyntax idempotent

The previous patch made it accept its own output, this also makes it
idempotent.

I don't understand the comment. Which previous patch? As for changes, looks like you've removed mkMerge in cases when there is no meta, which is nice 👍

pushDownProperties

👍 I've also discovered that inside module system sometimes code is duplicated to not introduce extra variables, which take memory. Can you verify that it isn't the case here?

opaque type

This looks like introspection thing. It would generate questions - "what is this?". I guess a "module system manual" is needed now. I'm also not a фанат of introducing concepts without usages - maybe combine into one commit with next one?

move defaults

I'm okayish here, just a note that it makes you an author in git blame. @7c6f434c do we care about git blame when moving large pieces of code?

the last commit

looks very strange, have yet to review it.

Module configuration.nix:anon-1' has an unsupported attribute _file'

that is because it is a top-level module parameter. You can't place it into random config, you have to massage module first.

You don't have a filename in your example because you evalModules on a nix expression, try to supply a path instead

You're right. But in my case I did exactly import. Another example:

# simple-module-generator.nix
{
    module1 = {
        error = true;
    };
    module2 = {
    };
}

# simple-2-modules.nix
with import ./simple-module-generator.nix;
{
  imports = [ module1 module2 ];
}

nix-repl> evalModules { modules = [ ../simple-2-modules.nix ]; }
{ config = «error: The option `error' defined in `/home/danbst/dev/simple-2-modules.nix' does not exist.»; options = { ... }; }

Error is not in simple-2-modules.nix file.

@oxij,

1d1ebf5 changes the semantics of haskellPackageOverrides a bit as config.haskellPackageOverrides is never undefined now. @peti, should I change that? If so then how? The interaction between config.haskellPackageOverrides and haskell.packageOverrides seems nontrivial.

I removed haskellPackageOverrides entirely in ee1f34e. Does that help?

Well, sure, but it works the same on master and I don't see how this can be fixed using the current NixOS modules infra, if you don't specify file nor import the module via a path so that evalModules would set file for you it will be a "best guess", so to say.

{
    module1 = {
        _file = "module at ${__curPos.file}:${toString __curPos.line}";
        config = {
            error = true;
        };
    };

    module2 = {
        _file = "module at ${__curPos.file}:${toString __curPos.line}";
    };
}

nix-repl> evalModules { modules = [ (import ../simple-2-modules.nix) ]; }
{ config = «error: The option `error' defined in `module at /home/danbst/dev/simple-module-generator.nix:3' does not exist.»; options = { ... }; }

But I see now that key and _file are not public module interface, nowhere in manual is said that you can use anonymous modules and should set key/_file explicitly. I'd vote for at least release notes then.

But in the meantime I was thinking that maybe it would make sense to make haskellPackageOverrides into the default for haskell.packageOverrides. E.g. suppose you want to override several ghc versions at the same time.

I am sorry, but I don't understand what you mean.

haskellPackageOverrides
See the new patch. I made your ee1f34e not as violent with the HEAD of this branch. Does that look ok?

No, please don't re-introduce that option. I'm glad it is gone.

I also notice there's an rPackageOverrides now which I don't want either (or did I misread the patch?).

@oxij Your thoughts about overlays in config are very interesting!

So, I'm reading the finalizing commit... I have two questions:

1. Maybe it would be cleaner to support imports directive in config? Think of:

{
  imports = [ ./my-custom-config-modules.nix ];

  allowBroken = true;  
}

Then a) it would be possible to add use-flags thing without amending NixOS code, only extraScope needed; b) it would remove the need to pass list of configs, just pass config = { imports = cfg.config; }

As for implementation, I think it would require some massaging of config, but should be doable.

2. It also not very cool that pkgs/top-level/default.nix includes hacky "check only known options". Yes, I agree this is sole usage, but it may be harder to sell to others. The unknowns hack is because module system doesn't support partial checks, right?

A problem with this is that this doesn't give you the full power of overlays. I think with how it's defined right now, it's impossible to do multiple changes to the same attribute in different packageOverrides, which standard overlays support. However it might be possible to weave the different layers of the overlays through the merge function, so this can maybe even gotten to work.

But a big problem with this I'm just seeing is that this will be very unnatural and maybe problematic for other package sets. E.g. suppose you want to override some python package in an overlay-style fashion and you decide to use this new way of defining pythonPackageOverrides in config.nix. What is self going to be then? Is it python.pkgs/pythonPackages or perhaps python3Packages because a different overlay changed the default or so? And maybe some program even wants a program-specific overlay, which doesn't get propagated to the pkgs the module receives, then the package set from pkgs would be very wrong.

I guess the thing I'm getting at here is that pkgs is a like a global variable, it's shared accross everything and it would be the only way to access the result of the pkgs fixed-point. Overlays in comparison have a locally scoped self, which allows it to be much more flexible. And the self: super: structure lends itself well to uses outside of the module system too, see lib.makeExtensible and co.

In fact I think the python example doesn't actually have a correct answer, because the overlay would get used for both python2 and python3

The way you showed there, using isPy3k to decide where to get self from, is really not very good, because it's easy to make mistakes and get a bad override. Having to point out that everybody needs to remember to add some isPy3k boilerplate to their overrides otherwise they won't work seems like a step down. And what about package sets with 10 different versions? This doesn't scale well at all.

I'd really like for this "making packageOverrides work as overlays" functionality to be moved into a separate PR, it's the only change here that's rather controversial, and it can be separated cleanly from here.

Also pinging @nbp who probably has an opinion on this.

A 5 seconds reply, before a larger one coming over the weekend.

_file -> file

I've used _file for my custom modules already. There has to be backwards compat. But I also wonder why was it named like that. Maybe, to discourage usage?

I named it _file, because we have to carry the source information. The problem of naming it file is that file is very likely to be a valid attribute set or option name.

What you do not see is that this attribute is propagated at all the level of the evaluation, and will cause issues as soon as a single option is named file. Either you will accidentally define the option, or you will take the value defined as argument as a file location.

_file is meant, as the underscore suggests, to be an artifact of the module system, not a valid option name.

@nbp is the longer review still coming?

My #75031 includes making unifyModuleSyntax idempotent the other way around, with file -> _file (where necessary)

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
2. Ask on the NixOS Discourse.
3. Ask on the #nixos channel on irc.freenode.net.