auxolotl/labs
22
8
Fork 4
Code Issues 1 Pull requests 3 Actions Packages Projects Releases Wiki Activity

WIP: feat: Namespaced includes #3

Closed
austreelis wants to merge 6 commits from austreelis/feat/namespaced-includes into main AGit
pull from: austreelis/feat/namespaced-includes
merge into: auxolotl:main
Conversation 2 Commits 6 Files changed 2 +86 -46
2 changed files with 86 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit feb9987300 - Show all commits

80
lib/src/modules/default.nix
View file

@ -142,6 +142,18 @@ lib: {
invalid = builtins.removeAttrs module lib.modules.VALID_KEYS;
in
invalid == { };
## Check that an include object has the required attributes and otherwise
## only specifies supported attributes.
##
## @type (Attrs | String | Path | Function) -> Bool
include =
include:
let
invalidKeys = builtins.removeAttrs include lib.modules.VALID_INCLUDE_KEYS;
namespaceIsString = include ? namespace -> builtins.isString include.namespace;
in
builtins.isAttrs include -> (include ? module && namespaceIsString && invalidKeys == { });
};
## Modules only support certain keys at the root level. This list determines
@ -159,6 +171,15 @@ lib: {
"meta"
];
## Include objects only support certain keys. This list determines the
## valid attributes that users can supply.
##
## @type List String
VALID_INCLUDE_KEYS = [
"module"
"namespace"
austreelis marked this conversation as resolved
jakehamilton commented 2024-06-18 16:35:41 +00:00
Review
Copy link

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.
👍 1
];
## Normalize a module to a standard structure. All other information will be
## lost in the conversion.
##
@ -168,48 +189,59 @@ lib: {
let
invalid = builtins.removeAttrs module lib.modules.VALID_KEYS;
invalidKeys = builtins.concatStringsSep ", " (builtins.attrNames invalid);
throwError = msg: builtins.throw ("Module ${key} (${file}) " + msg);
__key__ = builtins.toString module.__key__ or key;
normalizeIncludes =
includes:
let
flattened = builtins.concatMap (
include:
if
builtins.isAttrs include
&& 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
[
normalized = lib.lists.mapWithIndex1 (
n: include:
let
invalid = builtins.removeAttrs module lib.modules.VALID_INCLUDE_KEYS;
invalidKeys = builtins.concatStringsSep ", " (builtins.attrNames invalid);
hasInvalidKeys = invalid != { };
throwError' = msg: throwError ("has invalid include at position ${builtins.toString n}" + msg);
in
if lib.modules.validate.include include then
austreelis marked this conversation as resolved
jakehamilton commented 2024-06-18 16:30:30 +00:00
Review
Copy link

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

nit: maybe a name like `createNamespacedModule` would be informative here?
👍 1
if builtins.isAttrs include then
{
inherit include;
inherit (include) module;
namespace = include.namespace or null;
}
else
{
module = include;
namespace = null;
}
]
) (lib.lists.when (includes != { }) includes);
else if !include ? module then
{
module = include;
namespace = null;
}
austreelis marked this conversation as resolved
jakehamilton commented 2024-06-18 16:29:34 +00:00
Review
Copy link

Is this comment needed?

Is this comment needed?
austreelis commented 2024-06-19 12:09:05 +00:00
Review
Copy link

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

No, I thought I left it out when cleaning up my patch.
else if hasInvalidKeys then
throwError' " with unsupported attribute(s): ${invalidKeys}"
else
austreelis marked this conversation as resolved
jakehamilton commented 2024-06-18 16:31:57 +00:00
Review
Copy link

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.
👍 1
throwError' ": namespace is not a string"
) includes;
throwOnConflict =
let
filter =
austreelis marked this conversation as resolved
jakehamilton commented 2024-06-18 16:29:16 +00:00
Review
Copy link

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.
👍 1
namespaces:
{ namespace, include }:
{ namespace, ... }:
if namespace != null && builtins.elem namespace namespaces then
builtins.throw "Module ${key} declares several includes under the same namespace '${namespace}'"
throwError "declares several includes under the same namespace '${namespace}'"
else
namespaces ++ [ namespace ];
in
builtins.foldl' filter [ ] flattened;
builtins.foldl' filter [ ] normalized;
createNamespacedModule =
{ namespace, include }:
{ namespace, module, ... }:
if namespace == null then
include
module
else
{
__key__ = "${__key__}:include-${namespace}";
@ -217,13 +249,13 @@ lib: {
description = "options and configuration included from ${namespace}";
default.value = { };
type = lib.types.submodules.of {
modules = [ include ];
modules = [ module ];
description = "include ${namespace}";
};
};
};
namespacedModules = builtins.map createNamespacedModule flattened;
namespacedModules = builtins.map createNamespacedModule normalized;
in
builtins.seq throwOnConflict namespacedModules;
in
@ -259,7 +291,7 @@ lib: {
withFreeform (withMeta base);
}
else
builtins.throw "Module `${key}` (${file}) has unsupported attribute(s): ${invalidKeys}";
throwError "has unsupported attribute(s): ${invalidKeys}";
## Convert a module that is either a function or an attribute set into
## a resolved attribute set. If the module was a function then it will

52
lib/src/modules/default.test.nix
View file

@ -246,28 +246,31 @@ in
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 = [
{ }
null
{ 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)
builtins.map
(
include:
if builtins.isAttrs include then
builtins.mapAttrs (_: _: null) include.options or include
else
include
)
(lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ }
{ a = null; }
null
{
module = null;
namespace = "a";
}
];
}).includes;
in
@ -277,8 +280,14 @@ in
let
normalized = lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ a = { }; }
{ a = { }; }
{
module = null;
namespace = "a";
}
{
module = null;
namespace = "a";
}
];
};
in
@ -288,8 +297,8 @@ in
let
normalized = lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ }
{ }
null
null
];
};
in
@ -298,14 +307,14 @@ in
"handles multiple without namespace" =
let
expected = [
{ }
{ }
null
null
];
actual =
(lib.modules.normalize "/aux/example.nix" "example" {
includes = [
{ }
{ }
null
null
];
}).includes;
in
@ -786,9 +795,8 @@ in
{
includes = [
{
myinclude = {
options.msg = lib.options.create { default.value = "hello"; };
};
namespace = "myinclude";
module.options.msg = lib.options.create { default.value = "hello"; };
}
{ options.msg = lib.options.create { default.value = "hello"; }; }
];