From 4c26f960592bf42c63d66b873dd5781bdd6c80b2 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon Date: Sun, 13 Apr 2025 17:25:26 +0100 Subject: [PATCH 1/2] lib/tests/modules: test all `mkPackageOption` cases There were several test case options declared in `declare-mkPackageOption.nix` that were not actually tested in `modules.sh`. --- lib/tests/modules.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index d0f25c283dcb..66ce2df9fe87 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -315,12 +315,21 @@ checkConfigOutput '^false$' config.enableAlias ./alias-with-priority-can-overrid checkConfigOutput '^"hello"$' config.package.pname ./declare-mkPackageOption.nix checkConfigOutput '^"hello"$' config.namedPackage.pname ./declare-mkPackageOption.nix checkConfigOutput '^".*Hello.*"$' options.namedPackage.description ./declare-mkPackageOption.nix +checkConfigOutput '^"literalExpression"$' options.namedPackage.defaultText._type ./declare-mkPackageOption.nix +checkConfigOutput '^"pkgs\.hello"$' options.namedPackage.defaultText.text ./declare-mkPackageOption.nix +checkConfigOutput '^"hello"$' config.namedPackageSingletonDefault.pname ./declare-mkPackageOption.nix +checkConfigOutput '^".*Hello.*"$' options.namedPackageSingletonDefault.description ./declare-mkPackageOption.nix +checkConfigOutput '^"pkgs\.hello"$' options.namedPackageSingletonDefault.defaultText.text ./declare-mkPackageOption.nix checkConfigOutput '^"hello"$' config.pathPackage.pname ./declare-mkPackageOption.nix +checkConfigOutput '^"literalExpression"$' options.packageWithExample.example._type ./declare-mkPackageOption.nix checkConfigOutput '^"pkgs\.hello\.override \{ stdenv = pkgs\.clangStdenv; \}"$' options.packageWithExample.example.text ./declare-mkPackageOption.nix +checkConfigOutput '^"literalExpression"$' options.packageWithPathExample.example._type ./declare-mkPackageOption.nix +checkConfigOutput '^"pkgs\.hello"$' options.packageWithPathExample.example.text ./declare-mkPackageOption.nix checkConfigOutput '^".*Example extra description\..*"$' options.packageWithExtraDescription.description ./declare-mkPackageOption.nix checkConfigError 'The option .undefinedPackage. was accessed but has no value defined. Try setting the option.' config.undefinedPackage ./declare-mkPackageOption.nix checkConfigOutput '^null$' config.nullablePackage ./declare-mkPackageOption.nix -checkConfigOutput '^"null or package"$' options.nullablePackageWithDefault.type.description ./declare-mkPackageOption.nix +checkConfigOutput '^"null or package"$' options.nullablePackage.type.description ./declare-mkPackageOption.nix +checkConfigOutput '^"hello"$' config.nullablePackageWithDefault.pname ./declare-mkPackageOption.nix checkConfigOutput '^"myPkgs\.hello"$' options.packageWithPkgsText.defaultText.text ./declare-mkPackageOption.nix checkConfigOutput '^"hello-other"$' options.packageFromOtherSet.default.pname ./declare-mkPackageOption.nix From 6107d48bcbbb5fa15fd0fbbddff3c55652c91308 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon Date: Sat, 12 Apr 2025 05:56:30 +0100 Subject: [PATCH 2/2] lib.options.mkPackageOption: use lib.showAttrPath Make use of `lib.showAttrPath` instead of manually doing `concatStringsSep "."`. This means edge-cases such as the attr-path including names that are not valid nix identifiers will be handled better. See: - https://nix.dev/manual/nix/2.26/language/identifiers - https://nixos.org/manual/nixpkgs/unstable/#function-library-lib.attrsets.showAttrPath --- lib/options.nix | 10 ++++++---- lib/tests/modules.sh | 3 +++ lib/tests/modules/declare-mkPackageOption.nix | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/options.nix b/lib/options.nix index 85ce6ca77e92..007a14f15b67 100644 --- a/lib/options.nix +++ b/lib/options.nix @@ -30,6 +30,7 @@ let inherit (lib.attrsets) attrByPath optionalAttrs + showAttrPath ; inherit (lib.strings) concatMapStrings @@ -40,6 +41,7 @@ let ; inherit (lib.lists) last + toList ; prioritySuggestion = '' Use `lib.mkForce value` or `lib.mkDefault value` to change the priority on any of these definitions. @@ -310,14 +312,14 @@ rec { }: let name' = if isList name then last name else name; - default' = if isList default then default else [ default ]; - defaultText = concatStringsSep "." default'; + default' = toList default; + defaultText = showAttrPath default'; defaultValue = attrByPath default' (throw "${defaultText} cannot be found in ${pkgsText}") pkgs; defaults = if default != null then { default = defaultValue; - defaultText = literalExpression ("${pkgsText}." + defaultText); + defaultText = literalExpression "${pkgsText}.${defaultText}"; } else optionalAttrs nullable { @@ -333,7 +335,7 @@ rec { } // optionalAttrs (example != null) { example = literalExpression ( - if isList example then "${pkgsText}." + concatStringsSep "." example else example + if isList example then "${pkgsText}.${showAttrPath example}" else example ); } ); diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 66ce2df9fe87..3b19c8c63f26 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -332,6 +332,9 @@ checkConfigOutput '^"null or package"$' options.nullablePackage.type.description checkConfigOutput '^"hello"$' config.nullablePackageWithDefault.pname ./declare-mkPackageOption.nix checkConfigOutput '^"myPkgs\.hello"$' options.packageWithPkgsText.defaultText.text ./declare-mkPackageOption.nix checkConfigOutput '^"hello-other"$' options.packageFromOtherSet.default.pname ./declare-mkPackageOption.nix +checkConfigOutput '^"hello"$' config.packageInvalidIdentifier.pname ./declare-mkPackageOption.nix +checkConfigOutput '^"pkgs\.\\"123\\"\.\\"with\\\\\\"quote\\"\.hello"$' options.packageInvalidIdentifier.defaultText.text ./declare-mkPackageOption.nix +checkConfigOutput '^"pkgs\.\\"123\\"\.\\"with\\\\\\"quote\\"\.hello"$' options.packageInvalidIdentifierExample.example.text ./declare-mkPackageOption.nix # submoduleWith diff --git a/lib/tests/modules/declare-mkPackageOption.nix b/lib/tests/modules/declare-mkPackageOption.nix index 058c0addc0c3..3f4f91d7497e 100644 --- a/lib/tests/modules/declare-mkPackageOption.nix +++ b/lib/tests/modules/declare-mkPackageOption.nix @@ -57,5 +57,19 @@ in }; in lib.mkPackageOption myPkgs "hello" { }; + + packageInvalidIdentifier = + let + myPkgs."123"."with\"quote" = { inherit (pkgs) hello; }; + in + lib.mkPackageOption myPkgs [ "123" "with\"quote" "hello" ] { }; + + packageInvalidIdentifierExample = lib.mkPackageOption pkgs "hello" { + example = [ + "123" + "with\"quote" + "hello" + ]; + }; }; }