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.

Sign up for GitHub

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

Optionally request "publisher" in modules #50

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Optionally request "publisher" in modules #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions docs/folder-structure.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ For the following folder names, we also map them to the following outputs:

These and other unrecognized types also make to `modules.<type>.<name>`.

Each module can optionally be wrapped in a function that takes a single named
argument called `publisher`. If used, that function will be called with `publisher`
set to the current, defining flake before exposing that module.
This can be used to reference the defining flake when the module
is imported in another flake and the module argument `flake` points the consumer.

### `package.nix`, `formatter.nix`, `packages/<pname>(.nix|/default.nix)`

This `packages/` folder contains all your packages.
Expand Down
16 changes: 15 additions & 1 deletion lib/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,18 @@ let
) (lib.attrsToList hosts)
);

# Check if the given module is wrapped in a function expecting "publisher" as a named argument,
# and - if so - call that function with the current flake. This enables us to reference
# the publishing flake when re-using exported modules in consuming, downstream flakes.
withPublisher =
module:
let
module' = if builtins.isString module || builtins.isPath module then import module else module;
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@phaer phaer Dec 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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"

wantsPublisher =
builtins.isFunction module' && builtins.hasAttr "publisher" (builtins.functionArgs module');
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@phaer phaer Dec 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not entirely sure, I understand this proposal correctly:

  • If you mean changing the wrapping functions arguments to
    flake, inputs, perSystem instead of publisher. 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 args flake, inputs, perSystem to 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?

in
if wantsPublisher then lib.modules.importApply module { publisher = flake; } else module;

modules =
let
path = src + "/modules";
Expand All @@ -212,7 +224,9 @@ let
);
in
lib.optionalAttrs (builtins.pathExists path) (
lib.genAttrs moduleDirs (name: importDir (path + "/${name}") entriesPath)
lib.genAttrs moduleDirs (
name: lib.mapAttrs (_name: module: withPublisher module) (importDir (path + "/${name}") entriesPath)
)
);
in
# FIXME: maybe there are two layers to this. The blueprint, and then the mapping to flake outputs.
Expand Down