WIP: feat: Namespaced includes #3

Closed
austreelis wants to merge 6 commits from austreelis/feat/namespaced-includes into main AGit
2 changed files with 158 additions and 2 deletions
Showing only changes of commit a6ad4027a1 - Show all commits

View file

@ -168,12 +168,69 @@ lib: {
let
invalid = builtins.removeAttrs module lib.modules.VALID_KEYS;
invalidKeys = builtins.concatStringsSep ", " (builtins.attrNames invalid);
__key__ = builtins.toString module.__key__ or key;
normalizeIncludes =
includes:
let
flattened = builtins.concatMap (
include:
if
builtins.isAttrs include
austreelis marked this conversation as resolved
Review

There may be issues with this approach if we end up doing shorthand syntax for all modules. I don't think that we will, but submodules do support it for convenience. I wonder if we can have a different kind of object used for namespaces includes. What about something like:

includes = [
  { namespace = "my-namespace"; module = ./mod.nix }
];

At the very least we can switch on whether include ? namespace is a thing.

There may be issues with this approach if we end up doing shorthand syntax for all modules. I don't _think_ that we will, but submodules do support it for convenience. I wonder if we can have a different kind of object used for namespaces includes. What about something like: ```nix includes = [ { namespace = "my-namespace"; module = ./mod.nix } ]; ``` At the very least we can switch on whether `include ? namespace` is a thing.
&& include != { }
&& !builtins.any (n: builtins.elem n lib.modules.VALID_KEYS) (builtins.attrNames include)
then
lib.attrs.mapToList (namespace: module: {
inherit namespace;
include = module;
}) include
else
[
{
inherit include;
namespace = null;
}
]
) (lib.lists.when (includes != { }) includes);
throwOnConflict =
let
filter =
namespaces:
{ namespace, include }:
if namespace != null && builtins.elem namespace namespaces then
builtins.throw "Module ${key} declares several includes under the same namespace '${namespace}'"
else
namespaces ++ [ namespace ];
in
builtins.foldl' filter [ ] flattened;
austreelis marked this conversation as resolved
Review

nit: maybe a name like createNamespacedModule would be informative here?

nit: maybe a name like `createNamespacedModule` would be informative here?
doNamespace =
{ namespace, include }:
if namespace == null then
include
else
{
__key__ = "${__key__}:include-${namespace}";
options.${namespace} = lib.options.create {
description = "options and configuration included from ${namespace}";
default.value = { };
type = lib.types.submodules.of {
modules = [ include ];
description = "include ${namespace}";
};
austreelis marked this conversation as resolved
Review

Is this comment needed?

Is this comment needed?
Review

No, I thought I left it out when cleaning up my patch.

No, I thought I left it out when cleaning up my patch.
};
# config.${namespace} = lib.modules.override 1000 {};
};
austreelis marked this conversation as resolved
Review

Can we extract the builtins.map doNamespace flattened bit to a variable so that this value is easier to read? Something like builtins.seq throwOnConflict namespacedModules might make it easier to understand.

Can we extract the `builtins.map doNamespace flattened` bit to a variable so that this value is easier to read? Something like `builtins.seq throwOnConflict namespacedModules` might make it easier to understand.
in
builtins.seq throwOnConflict builtins.map doNamespace flattened;
in
if lib.modules.validate.keys module then
{
inherit __key__;
austreelis marked this conversation as resolved
Review

Can we wrap usage of or in parens here? It can be a bit confusing otherwise.

Can we wrap usage of `or` in parens here? It can be a bit confusing otherwise.
__file__ = builtins.toString module.__file__ or file;
__key__ = builtins.toString module.__key__ or key;
includes = module.includes or [ ];
includes = normalizeIncludes module.includes or [ ];
excludes = module.excludes or [ ];
options = module.options or { };
config =

View file

@ -236,6 +236,81 @@ in
};
in
actual == expected;
"includes" = {
"handles an empty list" =
let
expected = [ ];
actual = (lib.modules.normalize "/aux/example.nix" "example" { includes = [ ]; }).includes;
in
expected == actual;
"handles an empty set" =
let
expected = [ ];
actual = (lib.modules.normalize "/aux/example.nix" "example" { includes = { }; }).includes;
in
expected == actual;
"handles a mixed list" =
let
expected = [
{ }
{ a = null; }
];
actual =
# Because includes leverage submodules, we can't match the actual
# included namespaced submodule under "a". So we just assert the
# namespace was gotten right and do not evaluate the included value.
builtins.map (include: builtins.mapAttrs (_: _: null) include.options or include)
(lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ }
{ a = null; }
];
}).includes;
in
expected == actual;
"rejects conflicting namespaces" =
let
normalized = lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ a = { }; }
{ a = { }; }
];
};
in
!(builtins.tryEval normalized.includes).success;
"allows multiple without namespace" =
let
normalized = lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ }
{ }
];
};
in
(builtins.tryEval normalized.includes).success;
"handles multiple without namespace" =
let
expected = [
{ }
{ }
];
actual =
(lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ }
{ }
];
}).includes;
in
expected == actual;
};
};
"resolve" = {
@ -699,5 +774,29 @@ in
};
in
(evaluated.config.aux.message == evaluated.config.aux.message2) && evaluated.config.aux.exists;
"namespaced includes" =
let
expected = {
msg = "hello";
myinclude.msg = "hello";
};
evaluated = lib.modules.run {
modules = [
{
includes = [
{
myinclude = {
options.msg = lib.options.create { default.value = "hello"; };
};
}
{ options.msg = lib.options.create { default.value = "hello"; }; }
];
}
];
};
actual = evaluated.config;
in
expected == actual;
};
}