Allow for directories in module-list

[Pacman99]

fixes #221
building on #222 this PR improves the lib functions pathsToImportedAttrs and pathsIn. First to add support for directories. This does not support actually passing a file in a directory, so ./matrix/default.nix won't work but ./matrix will - I should probably document this somewhere.
Also I moved the filtering for nix files to pathsIn, since its only necessary for auto-import. We can assume that users would pass proper files in module-list.nix.

[nrdxp]

It might just be a translation issue, but I'm still not clear on exactly what the issue is in #221 or how this addresses it.

[Pacman99]

Ohh so currently when you try to add a directory with a default.nix in module-list.nix, pathsToImportedAttrs filters it out. Because it only accepts files with the '.nix' extension. So the issue in #221 is when you pass a folder, it doesn't get included.
In this PR I change it so that the name is just the file if it doesn't have a '.nix' extension and if it does then it strips the '.nix'. And along with that I removed any filtering.
But then that breaks the overlays folder since now the folder's README will try to be imported. So I then add filtering in pathsIn, which I think logically makes sense, since thats where the auto-import is done.

[GTrunSec]

when you try to add a directory with a default.nix in module-list.nix, pathsToImportedAttrs filters it out. Because it only accepts files with the '.nix' extension. So the issue in #221 is when you pass a folder, it doesn't get included.

Exactly, thanks for you clarifying my issue.

[nrdxp]

Okay great.

Wouldn't a more minimal fix for pathsToImportedAttrs be:

{
  pathsToImportedAttrs = paths:
    let
      paths' = lib.filter
        (path: lib.hasSuffix ".nix" path
          # simply add another filter condition
          || lib.pathExists "${path}/default.nix")
        paths;
    in
    genAttrs' paths' (path: {
      name = lib.removeSuffix ".nix"
        # Safe as long this is just used as a name
        (builtins.unsafeDiscardStringContext (baseNameOf path));
      value = import path;
    });
}

I'll take a look at pathsIn to get a better understanding of the issue there.

[Pacman99]

Ohh yeah that would work well. I don't mind switching to that.
But I still don't like that pathsToImportedAttrs does any filtering in the first place. I feel like that should be done in functions which are directly related to auto-importing. It doesn't make sense to try and filter things that a user explicitly passes in their module-list.

[nrdxp]

I hear what your saying but I kinda feel the opposite. The final operation is an import, and if this is called on anything besides a valid nix file it will fail, so filtering is performed so invalid files cannot be imported.

I reciprocate your feeling, but about pathsIn instead. Users may want to collect paths of all files, not just nix files, which is fine since they might want them for something other than import, so no filtering should be done there.

[Pacman99]

I hear what your saying but I kinda feel the opposite. The final operation is an import, and if this is called on anything besides a valid nix file it will fail, so filtering is performed so invalid files cannot be imported.

I reciprocate your feeling, but about pathsIn instead. Users may want to collect paths of all files, not just nix files, which is fine since they might want them for something other than import, so no filtering should be done there.

I see what your saying. Thanks for explaining the reasoning. I'll update this PR.

[Pacman99]

updated, but I also updated teh relevant test and thats failing. I'll debug tomorrow.

[blaggacao]

Is this also how nixpkgs module system works?

[nrdxp]

Is this also how nixpkgs module system works?

In what respect?

[Pacman99]

I'm lost, this fails nix flake check. But when I run the exact expression in nix repl, this the output I get.

[nrdxp]

The following will fix it:

        "${self}/tests/testPathsToImportedAttrs/dir"
        "${self}/tests/testPathsToImportedAttrs/foo.nix"
        "${self}/tests/testPathsToImportedAttrs/bar.nix"
        "${self}/tests/testPathsToImportedAttrs/t.nix"
        "${self}/tests/testPathsToImportedAttrs/f.nix"

[Pacman99]

fixed, good find, I never thought that this would be necessary

[Pacman99]

Is this also how nixpkgs module system works?

For the most part yeah, you can pass files or directories. The difference here is that we want to export modules as an attribute, so we need some way to get a name from a file. Whereas nixpkgs, doesn't need to do that, since it instead lumps them all into nixosSystem.

[nrdxp]

bors r+

[bors]

Build succeeded:

• ci/hercules/derivations
• ci/hercules/evaluation