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

[exploratory] Module system types.record, types.fix #257511

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ let
mapAttrs' mapAttrsToList concatMapAttrs mapAttrsRecursive mapAttrsRecursiveCond
genAttrs isDerivation toDerivation optionalAttrs
zipAttrsWithNames zipAttrsWith zipAttrs recursiveUpdateUntil
recursiveUpdate matchAttrs overrideExisting showAttrPath getOutput getBin
recursiveUpdate removeAttrs matchAttrs overrideExisting showAttrPath getOutput getBin
getLib getDev getMan chooseDevOutputs zipWithNames zip
recurseIntoAttrs dontRecurseIntoAttrs cartesianProductOfSets
updateManyAttrsByPath;
Expand Down
29 changes: 29 additions & 0 deletions lib/tests/modules.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,33 @@ checkConfigError() {
fi
}

# record field
checkConfigOutput '^"Alice"$' config.people.alice.name ./declare-record.nix ./define-record-alice.nix ./define-record-bob.nix
checkConfigOutput '^2019$' config.people.bob.nixerSince ./declare-record.nix ./define-record-alice.nix ./define-record-bob.nix

# record field type error
checkConfigError 'A definition for option .people.mallory.nixerSince. is not of type .signed integer.. Definition values' config.people.mallory.nixerSince ./declare-record.nix ./define-record-mallory.nix
checkConfigError 'define-record-mallory.nix.: "beginning of time"' config.people.mallory.nixerSince ./declare-record.nix ./define-record-mallory.nix

# record field default
checkConfigOutput '^true$' config.people.bob.isCool ./declare-record.nix ./define-record-alice.nix ./define-record-bob.nix

# record field bad default definition
checkConfigError 'In .the default value of option people.mallory.: "yeah"' config.people.mallory.isCool ./declare-record-bad-default.nix ./define-record-mallory.nix
checkConfigError 'A definition for option .people.mallory.isCool. is not of type .boolean.. Definition values:' config.people.mallory.isCool ./declare-record-bad-default.nix ./define-record-mallory.nix

# record field works in presence of wildcard
checkConfigOutput '^2016$' config.people.alice.nixerSince ./declare-record-wildcard.nix ./define-record-alice-prefs.nix

# record wildcard field
checkConfigOutput '^true$' config.people.alice.mechKeyboard ./declare-record-wildcard.nix ./define-record-alice-prefs.nix


# fix
checkConfigOutput '"bobby"' config.people.lilbob.name ./fixpoint.nix

if false; then

# Shorthand meta attribute does not duplicate the config
checkConfigOutput '^"one two"$' config.result ./shorthand-meta.nix

Expand Down Expand Up @@ -462,6 +489,8 @@ checkConfigOutput '^34|23$' options.submoduleLine34.declarationPositions.1.line
# nested options work
checkConfigOutput '^30$' options.nested.nestedLine30.declarationPositions.0.line ./declaration-positions.nix

fi

cat <<EOF
====== module tests ======
$pass Pass
Expand Down
25 changes: 25 additions & 0 deletions lib/tests/modules/declare-record-bad-default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{ lib, ... }:

let
inherit (lib) mkOption types;

person = types.record {
fields = {
nixerSince = mkOption {
type = types.int;
};
name = mkOption {
type = types.str;
};
isCool = mkOption {
type = types.bool;
default = "yeah";
};
};
};

in {
options.people = mkOption {
type = types.attrsOf person;
};
}
24 changes: 24 additions & 0 deletions lib/tests/modules/declare-record-wildcard.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{ lib, ... }:

let
inherit (lib) mkOption types;

person = types.record {
fields = {
nixerSince = mkOption {
type = types.int;
};
name = mkOption {
type = types.str;
};
};
wildcard = mkOption {
type = types.bool;
};
};

in {
options.people = mkOption {
type = types.attrsOf person;
};
}
25 changes: 25 additions & 0 deletions lib/tests/modules/declare-record.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{ lib, ... }:

let
inherit (lib) mkOption types;

person = types.record {
fields = {
nixerSince = mkOption {
type = types.int;
};
name = mkOption {
type = types.str;
};
isCool = mkOption {
type = types.bool;
default = true;
};
};
};

in {
options.people = mkOption {
type = types.attrsOf person;
};
}
9 changes: 9 additions & 0 deletions lib/tests/modules/define-record-alice-prefs.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{ lib, ... }: {
people.alice = {
nixerSince = 2016;
name = "Alice";
hiRes = true;
mechKeyboard = true;
spiders = false;
};
}
6 changes: 6 additions & 0 deletions lib/tests/modules/define-record-alice.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
people.alice = {
nixerSince = 2016;
name = "Alice";
};
}
6 changes: 6 additions & 0 deletions lib/tests/modules/define-record-bob.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
people.bob = {
nixerSince = 2019;
name = "Bob";
};
}
6 changes: 6 additions & 0 deletions lib/tests/modules/define-record-mallory.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
people.mallory = {
nixerSince = "beginning of time";
name = "Bobby";
};
}
27 changes: 27 additions & 0 deletions lib/tests/modules/fixpoint.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{ lib, ... }:
let
inherit (lib) types mkOption;

person = types.record {
fields = {
name = mkOption { type = lib.types.str; };
age = mkOption { type = lib.types.int; };
};
};

bobber = { value }: {
name = if value.age < 10 then "bobby" else "bob";
};
in
{
options = {
people = mkOption {
type = types.attrsOf (types.fix person);
default = {};
};
};
config = {
people.bob = lib.mkMerge [ { age = 23; } bobber ];
people.lilbob = lib.mkMerge [ { age = 3; } bobber ];
};
}
4 changes: 3 additions & 1 deletion lib/types.nix
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,9 @@ rec {
# Augment the given type with an additional type check function.
addCheck = elemType: check: elemType // { check = x: elemType.check x && check x; };

};
}
// import ./types/fix.nix { inherit lib; }
// import ./types/record.nix { inherit lib; };
};

in outer_types // outer_types.types
29 changes: 29 additions & 0 deletions lib/types/fix.nix
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like fix is rather useless in a world where we still have (more capable) submodules to handle this aspect.
We could first, or only, make record useful on its own, for data modeling use cases, whereas submodule (or usually just evalModules) can continue to fill a more programming-like role.

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{ lib }:

let
inherit (lib) toFunction mkOptionType
optionDescriptionPhrase isFunction mergeDefinitions defaultFunctor;

# TODO: should this be a type or a "property"? (i.e. one of `mkIf`, `mkMerge`, etc)
fix = t: mkOptionType {
name = "fixpoint of ${t.name}";
description = t.description;
descriptionClass = "composite";
check = v: isFunction v || t.check v;
merge = loc: defs:
let
# TODO: facilities elsewhere for wiring `options` and/or other "meta" info into this.
args = { value = r; };
r = (mergeDefinitions loc t (map (fn: { inherit (fn) file; value = toFunction fn.value args; }) defs)).mergedValue;
in r;
getSubOptions = prefix: t.getSubOptions prefix;
getSubModules = t.getSubModules;
substSubModules = m: fix (t.substSubModules m);
functor = defaultFunctor "fix" // { inherit t; };
nestedTypes.t = t;
};

in
{
inherit fix;
}
86 changes: 86 additions & 0 deletions lib/types/record.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
{ lib }:

let
inherit (lib)
mapAttrs zipAttrsWith removeAttrs attrNames showOption
mergeDefinitions mkOptionType isAttrs
;

record = args@{ fields, wildcard ? null }:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's missing is something like optionalFields.
It's best not to mix required fields and optional fields, because

  • required fields can be returned without looking at their declarations, making it a bit more lazy, which is good for performance and robustness
  • if we don't have optional or wildcard fields, we can return all the fields without even looking at the definitions
  • it avoids having to partition the required and optional fields at runtime, which we'd then have to do; it's more efficient this way

let
fields =
mapAttrs
(name: value:
if value._type or null != "option"
then throw "Record field `${lib.escapeNixIdentifier name}` must be declared with `mkOption`."
else if value?apply then throw "In field ${lib.escapeNixIdentifier name} records do not support options with `apply`"
else if value?readOnly then throw "In field ${lib.escapeNixIdentifier name} records do not support options with `readOnly`"
else value)
args.fields;
wildcard =
if args.wildcard or null == null
then null
else if args.wildcard._type or null != "option"
then throw "Record wildcard must be declared with `mkOption`."
else if args.wildcard?apply
then throw "Records do not support options with `apply`, but wildcard has `apply`."
else args.wildcard;
in
mkOptionType {
name = "record";
description = if wildcard == null then "record" else "open record of ${wildcard.description}";
descriptionClass = if wildcard == null then "noun" else "composite";
check = isAttrs;
merge = loc: defs:
let
data = zipAttrsWith
(name: values: values)
(map
(def: mapAttrs (_fieldName: fieldValue: { inherit (def) file; value = fieldValue; }) def.value)
defs);
requiredFieldValues =
mapAttrs
(fieldName: fieldOption:
builtins.addErrorContext ("while evaluating the field `${fieldName}' of option `${showOption loc}'") (
(mergeDefinitions
(loc ++ [fieldName])
fieldOption.type
(data.${fieldName} or [] ++
(if fieldOption?default
then [{ value = fieldOption.default; file = "the default value of option ${showOption loc}"; }]
else []))
).mergedValue
)
)
fields;
extraData = removeAttrs data (attrNames fields);
in
if wildcard == null
then
if extraData == {}
then requiredFieldValues
else throw "A definition for option `${showOption loc}' has an unknown field."
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which one? Print at least one attribute name.

else
requiredFieldValues //
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe flip the //? I believe their mututally exclusive, so it doesn't really matter, but if @infinisil's lazy attrset update is implemented, I think we'll want to have the more predictable set on the right.

mapAttrs
(fieldName: fieldDefs:
builtins.addErrorContext ("while evaluating the wildcard field `${fieldName}' of option `${showOption loc}'") (
(mergeDefinitions
(loc ++ [fieldName])
wildcard.type
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
wildcard.type
optionalFields.${fieldName} or wildcard.type
  • move all of this outside the wildcard == null conditional above.

fieldDefs
).mergedValue
)
)
extraData;
nestedTypes = fields // {
# potential collision with `_wildcard` field
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unimportant, but I guess we could rename the original fields of that satisfy regex _*wildcard to get an extra underscore. (The word wildcard looks weird in a regex, but it's just the literal word)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be an error for nested types to have a wildcard option?

_wildcard = wildcard;
};
};

in
# public
{
inherit record;
}
Loading