From 7de0c651b7f50d66138f405c883ebc66aef6b9db Mon Sep 17 00:00:00 2001 From: DavHau Date: Sun, 10 Dec 2023 20:07:51 +0700 Subject: [PATCH 1/4] raise error when flake attribute doesn't exist Currently when trying to access packages from input flakes for a system that isn't supported by that flake, the error is not very conclusive. It costed me at least an hour to grasp this. Now putting in this error message to prevent others from falling into the same trap. --- modules/transposition.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/transposition.nix b/modules/transposition.nix index 7e985ba..94470c6 100644 --- a/modules/transposition.nix +++ b/modules/transposition.nix @@ -67,7 +67,11 @@ in perInput = system: flake: mapAttrs - (attrName: attrConfig: flake.${attrName}.${system}) + (attrName: attrConfig: + flake.${attrName}.${system} or (throw '' + Attemt to access non existent attribute ${attrName}.${system} of flake ${flake}. + '') + ) config.transposition; perSystem = { From ac5d0b2d4d51a53a1cd4a4a10d22f4a12c3fe652 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Jan 2024 12:16:42 +0100 Subject: [PATCH 2/4] transposition: Improve perInput error message --- modules/transposition.nix | 48 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/modules/transposition.nix b/modules/transposition.nix index 94470c6..244b736 100644 --- a/modules/transposition.nix +++ b/modules/transposition.nix @@ -7,6 +7,9 @@ let mkOption types ; + inherit (lib.strings) + escapeNixIdentifier + ; transpositionModule = { options = { @@ -24,6 +27,45 @@ let }; }; + perInputAttributeError = { flake, attrName, system, attrConfig }: + # This uses flake.outPath for lack of a better identifier. + # Consider adding a perInput variation that has a normally-redundant argument for the input name. + # Tested manually with + # perSystem = { inputs', ... }: { + # packages.extra = inputs'.nixpkgs.extra; + # packages.default = inputs'.nixpkgs.packages.default; + # packages.veryWrong = (top.config.perInput "x86_64-linux" inputs'.nixpkgs.legacyPackages.hello).packages.default; + # }; + # transposition.extra = {}; + let + attrPath = "${escapeNixIdentifier attrName}.${escapeNixIdentifier system}"; + flakeIdentifier = + if flake._type or null != "flake" + then + throw "An attempt was made to access attribute ${attrPath} on a value that's supposed to be a flake, but may not be a proper flake." + else + builtins.addErrorContext "while trying to find out how to describe what is supposedly a flake, whose attribute ${attrPath} was accessed but does not exist" ( + toString flake.outPath + ); + # This ought to be generalized by extending attrConfig, but this is the only known and common mistake for now. + alternateAttrNameHint = + if attrName == "packages" && flake?legacyPackages + then # Unfortunately we can't just switch them out, because that will put packages *sets* where single packages are expected in user code, resulting in potentially much worse and more confusing errors down the line. + "\nIt does define legacyPackages; try that instead?" + else ""; + in + if flake?${attrName} + then + throw '' + Attempt to access ${attrPath} of flake ${flakeIdentifier}, but it does not have it. + It does have attribute ${escapeNixIdentifier attrName}, so it appears that it does not support system type ${escapeNixIdentifier system}. + '' + else + throw '' + Attempt to access ${attrPath} of flake ${flakeIdentifier}, but it does not have attribute ${escapeNixIdentifier attrName}.${alternateAttrNameHint} + ''; + + in { options = { @@ -68,9 +110,9 @@ in system: flake: mapAttrs (attrName: attrConfig: - flake.${attrName}.${system} or (throw '' - Attemt to access non existent attribute ${attrName}.${system} of flake ${flake}. - '') + flake.${attrName}.${system} or ( + throw (perInputAttributeError { inherit system flake attrName attrConfig; }) + ) ) config.transposition; From 3a0408e3acf785354d8a9b4074b9d4dbb0a12852 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Jan 2024 12:19:58 +0100 Subject: [PATCH 3/4] perSystem: Check that inputs'. is a flake, for better error message The alternative would have been to try and make this not an evaluation error, but that would cause problems. Idea: use filterAttrs to construct an attrset "inputFlakes". This would be bad because it requires that we evaluate all inputs, just to see whether they're flakes or not. If Nix were to provide this metadata independently, which it could do very efficiently, we still ought to wonder what's the purpose of eliding those attributes. The only ways you'll encounter this exception in `inputs'.` is by either - making a mistake, in which case the error message is great - or traversing all inputs, which nobody should do. See https://flake.parts/best-practices-for-module-writing#do-not-traverse-inputs Other idea: return *something empty*. This avoids the strictness problem above, but still optimizes for the wrong use case. Ultimately though, attribute transposition should not be needed at all, but this project is largely dependent on the implicit schema imposed by Nix. --- modules/perSystem.nix | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/modules/perSystem.nix b/modules/perSystem.nix index 851b30a..499858a 100644 --- a/modules/perSystem.nix +++ b/modules/perSystem.nix @@ -6,6 +6,9 @@ let mkOption types ; + inherit (lib.strings) + escapeNixIdentifier + ; inherit (flake-parts-lib) mkPerSystemType ; @@ -92,8 +95,21 @@ in type = mkPerSystemType ({ config, system, ... }: { _file = ./perSystem.nix; config = { - _module.args.inputs' = mapAttrs (k: rootConfig.perInput system) self.inputs; - _module.args.self' = rootConfig.perInput system self; + _module.args.inputs' = + mapAttrs + (inputName: input: + builtins.addErrorContext "while retrieving system-dependent attributes for input ${escapeNixIdentifier inputName}" ( + if input._type or null == "flake" + then rootConfig.perInput system input + else + throw "Trying to retrieve system-dependent attributes for input ${escapeNixIdentifier inputName}, but this input is not a flake. Perhaps flake = false was added to the input declarations by mistake, or you meant to use a different input, or you meant to use plain old inputs, not inputs'." + ) + ) + self.inputs; + _module.args.self' = + builtins.addErrorContext "while retrieving system-dependent attributes for a flake's own outputs" ( + rootConfig.perInput system self + ); # Custom error messages _module.args.self = throwAliasError' "self"; From 528a7d0cb9668fb633d0ce39fb2255c94e4f8250 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Jan 2024 12:59:10 +0100 Subject: [PATCH 4/4] transposition: Explain incomplete usage of transposition Abort because this is a clear programming error, that should never be caught. --- modules/transposition.nix | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/transposition.nix b/modules/transposition.nix index 244b736..d532e76 100644 --- a/modules/transposition.nix +++ b/modules/transposition.nix @@ -101,7 +101,10 @@ in lib.mapAttrs (attrName: attrConfig: mapAttrs - (system: v: v.${attrName}) + (system: v: v.${attrName} or ( + abort '' + Could not find option ${attrName} in the perSystem module. It is required to declare such an option whenever transposition. is defined (and in this instance is ${attrName}). + '')) config.allSystems ) config.transposition;