Optionally request "publisher" in modules #50
Conversation
| withPublisher = | ||
| module: | ||
| let | ||
| module' = if builtins.isString module || builtins.isPath module then import module else module; |
There was a problem hiding this comment.
One downside with importing a module manually is that its result doesn't get annotated with its "_file" attribute. Which is used by the module system to display errors in the right location.
Fixing this is a bit tricky as it happens during the module evaluation.
There was a problem hiding this comment.
Yes, this took me a while. But the way it works here is that we just import module' (prime) to check whether that particular module is wrapped or not.
The actual import of module (no prime) happens in the lib.modules.importApply call which handles _file for us. i.e.:
nix-repl> nixosModules.common._file
"/nix/store/7ds7wj847q654jqc5gs29zyfa8gzph08-source/modules/nixos/common.nix"
| let | ||
| module' = if builtins.isString module || builtins.isPath module then import module else module; | ||
| wantsPublisher = | ||
| builtins.isFunction module' && builtins.hasAttr "publisher" (builtins.functionArgs module'); |
There was a problem hiding this comment.
What do you think of applying the same logic for all the specialArgs? flake, inputs, perSystem. As a module author, I don't think I ever wanted to get access to inputs that are not part of the current flake.
There was a problem hiding this comment.
Not entirely sure, I understand this proposal correctly:
-
If you mean changing the wrapping functions arguments to
flake,inputs,perSysteminstead ofpublisher. Could do that but the current design was meant to a) keep the api surface small and clearly named and b) still allow people to use the module argsflake,inputs,perSystemto refer to the current/consuming flake (without shadowing/need for aliases) -
If you mean not as an addition, but moving those args to the wrapping function all together instead of module arguments: That would make it difficult to access i.e. packages from the current/consum flake, no?
Closes #23