flake-parts icon indicating copy to clipboard operation
flake-parts copied to clipboard

Exposing a flakeModule that depends on other flake modules without defining in the same file

Open terlar opened this issue 1 year ago • 19 comments

What is the best practice to expose a flakeModule that depends on other flake modules?

I have something like this: flake.nix:

...
  outputs = inputs:
    inputs.flake-parts.lib.mkFlake {inherit inputs;} {
      systems = ["x86_64-linux" "x86_64-darwin" "aarch64-darwin"];
      imports = [./modules.nix];
    };
...

modules.nix:

{inputs, ...}:

{
  imports = [inputs.flake-parts.flakeModules.flakeModules];
  flake.flakeModules.default = import ./my-module.nix;
}

my-module.nix:

{inputs, ...}: {
  imports = [input.flake-dep1.flakeModule input.flake-dep2.flakeModule];
}

I currently have 4 solutions, none of them feel optimal.

  1. Define the module in the same file, so I can access the outer scope.
  2. Pass a top-level argument to the module (then the module can't be imported directly via imports, but one needs to do (import ./modules.nix { inherit inputs; }) for example)
  3. Have a wrapper for the module file inside the modules.nix, { imports = [...inputs, ./my-module.nix]; }
  4. Split out the module as a module within the current flake that then defines the module.

I guess there are no _module.args.inputs or something that could be utilized?

What is the best practices for this kind of thing?

terlar avatar Feb 27 '23 13:02 terlar

I currently require the user to manually import all the dependencies, in https://github.com/Platonic-Systems/mission-control for example

image

But I guess what we probably want is some sort of module dependency system.

srid avatar Feb 27 '23 15:02 srid

I currently require the user to manually import all the dependencies

This touches on a deeper discussion about how we want to manage module dependencies, besides the syntax we use to import them:

  • #104

Let's purpose this thread for the more "syntax-level" details.

roberth avatar Feb 27 '23 16:02 roberth

{inputs, ...}:

{
  imports = [inputs.flake-parts.flakeModules.flakeModules];

This is making an assumption about the inputs of the final flake, which we should try to avoid. We can't know for sure that inputs.flake-parts exists. Of course this one is quite common, but even this one isn't great because it doesn't allow flake-parts to be wrapped into some other flake. Maybe someone uses inputs.companyflake.mkFlake with some pre-included modules or very restricted syntax, or whatever they think helps their company flakes. A more realistic example is the possibility of bundling modules. In that case, probably only one or two direct inputs are about flake-parts modules, and the rest are transitive dependencies. There's also the idea that when flakes allows arbitrary names for dependencies, flake-parts should probably allow that too.

  flake.flakeModules.default = import ./my-module.nix;

It's better not to use import. import turns a path-based module reference into an anonymous module (just a function) without remembering the path. This affects error messages and deduplication. flake.flakeModules.default = ./my-module.nix; will preserve the file name.

  1. Define the module in the same file, so I can access the outer scope.

This can be combined with a separate file.

2. Pass a top-level argument to the module (then the module can't be imported directly via imports, but one needs to do (import ./modules.nix { inherit inputs; }) for example)

This creates the input name dependency that we should try to avoid.

What is the best practices for this kind of thing?

I don't have a definitive answer yet, but these are competing solutions:

importApply

Use this library function to fix the forgotten path problem when defining modules, while still making things from the lexical scope available to the module.

      # Pass arguments to a module inside a function inside a file, while preserving
      # the file name behavior of the module system.
      importApply =
        modulePath: staticArgs:
          lib.setDefaultModuleLocation modulePath (import modulePath staticArgs);

This should perhaps be added to Nixpkgs. Here's a usage example with NixOS instead of flake-parts.

Separation of technical concerns

Here's an example of a module that always uses the exact same package in its flake-parts module, regardless of what the end-user pkgs ends up being.

# foologic/flake.nix
foologic@{ getSystem, ... }:
{
  flake.flakeModules.foologic = {
    imports = [
      # foologic.nix defines perSystem.foologic.package among other things
      ./foologic.nix
    ];
    config = {
      # reach into all the options that need values from the foologic flake
      perSystem = { system, ... }: {
        foologic.package = (foologic.getSystem system).packages.default;
      };
    };
  };
}

Separation of concerns is good, but I don't like having to separate things for technical reasons. How to divide up the domain should be up to the module author, not arbitrary technical requirements, ideally.

This doesn't work perfectly yet, but this should fix it

  • #111

roberth avatar Feb 27 '23 16:02 roberth

To clarify this was not part of the exposed module, this was the flake itself and how it defined the module, so I guess that should be fine:

{inputs, ...}:

{
  imports = [inputs.flake-parts.flakeModules.flakeModules];

I managed to get it to work with the importApply solution. As well as fixing all my usage to not use import where unnecessary (thank you for that hint, somehow I thought it would be weird to expose the module file itself). Is it also okay to expose an attribute-set as the module? or is that bad practice?

For example:

  flake.flakeModules = {
    default = {
      imports = [
        config.flake.flakeModules.moduleA
        config.flake.flakeModules.moduleB
      ];
    };

terlar avatar Mar 01 '23 12:03 terlar

Yeah flakeModules.flakeModules should also take care of giving identifiers to your modules, even if they were anonymous like the attrset module syntax in your example. So your example is fine.

(just to add) importApply seems like a good practice regardless of what flakeModules.flakeModules manages to fix up, because solving the problem completely is nicer than having to worry about whether the problem was solved sufficiently.

roberth avatar Mar 01 '23 13:03 roberth

Adding importApply:

  • https://github.com/NixOS/nixpkgs/pull/230588

This solves the location part. The "key" aspect (dedup and disabledModules) can not reasonably be covered by it, so this will be the responsibility of the flake.*Modules options.

roberth avatar May 07 '23 22:05 roberth

As mentioned in https://github.com/hercules-ci/flake-parts/pull/146#issuecomment-1538653513 , importApply does not solve all the problems.

I think the easier solution is to turn flakeModulesOption into a submodule instead of a lazyAttrsOf, because lazyAttrsOf is not really lazy.

flakeModulesOption = mkOption {
  apply = builtins.mapAttr (name: module: rec { imports = [ module ]; key = "${inputs.self}#flakeModules.${name}" }; _file = key; });
  type = types.submoduleOf {
    modules = [
      {
        # Making it be a freeformType for backward compatibility only
        config._module.freeformType = types.lazyAttrsOf types.deferredModule;
      }
    ];
  };
};

Then a flake module can be defined as an option to reference other flake modules:

# myModule1.nix

# Note that the inputs are from the lexical scope, not the user's root inputs, solving https://github.com/hercules-ci/flake-parts/issues/104
{flake-parts-lib, inputs, ...}:{
  option.flake = flake-parts-lib.mkSubmoduleOption {
    flakeModules = flake-parts-lib.mkSubmoduleOption {
      myModule1 = flake-parts-lib.mkDeferredModuleOption {
        options.myOption = mkOption { default = "my default value"; };
      };
    };
  };
}
# myModule2.nix
{flake-parts-lib, inputs, ...}:{
  imports = [ ./myModule1.nix ];
  option.flake = flake-parts-lib.mkSubmoduleOption {
    flakeModules = flake-parts-lib.mkSubmoduleOption (flakeModules: {
      myModule2 = flake-parts-lib.mkDeferredModuleOption {
        imports = [ flakeModules.config.myModule1 ];
        config.myOption = "my overridden value";
      };
    });
  };
}

If mapAttr does not work, we can provideflake-parts-lib.mkDeduplicableModuleOption to create an options of deferred module with an apply function to add the key to the module, equivalent to:

options.myModule1 = mkOption {
  apply = ...
  type = types.deferredModuleWith {
    default = {};
    staticModules = [
      {
        options.myOption = mkOption { default = "my default value"; };
      }
    ];
  };
};

Atry avatar May 08 '23 18:05 Atry

By turning flakeModulesOption into a submodule, we will not need importApply. Instead, we can provide the current inputs from the lexical scope by default

Atry avatar May 08 '23 18:05 Atry

We might want to move apply from flakeModulesOption to user defined modules' apply, because mapAttrs destroys laziness. Providing a flake-parts-lib.mkDeduplicableModuleOption could help user to create an option with the apply function that adds key to the module.

Atry avatar May 08 '23 19:05 Atry

How would mkDeduplicableModuleOption work? Why would it have to be in options instead of config?

mapAttrs destroys laziness.

You might be thinking of mapAttrs' instead. mapAttrs is as lazy as can be. The attrNames are taken from the second argument without calling the function, and the function is only called individually for each attribute. What else could you want? (without changing the attrset data type; strict names, lazy values)

roberth avatar May 08 '23 20:05 roberth

How would mkDeduplicableModuleOption work?

Updated https://github.com/hercules-ci/flake-parts/issues/121#issuecomment-1538846854 to describe mkDeduplicableModuleOption

Why would it have to be in options instead of config?

Because I want to avoid lazyAttrsOf, which would result in infinite loop

Atry avatar May 08 '23 20:05 Atry

Also we can call mkFlake twice to use a flake's own flake module, to address the comment. The first mkFlake call renders the flake module, the second mkFlake call creates its output from the rendered flake module.

Atry avatar May 08 '23 20:05 Atry

If mapAttrs works, we might not need mkDeduplicableModuleOption. Just use mkDeferredModuleOption to create the module and let mapAttrs add keys to them.

Atry avatar May 08 '23 20:05 Atry

I want to avoid lazyAttrsOf, which would result in infinite loop

We have tests for lazyAttrsOf, so you can paint me skeptical. Would love to have a reproducer for whatever would be wrong with it, but I find it unlikely that lazyAttrsOf would be in the critical path. If anything, I would expect the problem to be with config.

I can't verify any of this right now, and these discussions have already taken up a lot of time. I'll have to resume this later, but feel free to try things in the meantime.

Also we can call mkFlake twice to use a flake's own flake module,

Doing it twice smells a lot like we have a need for fixpoint iteration then, and that would not be feasible. I'd prefer to have a very restricted step that only processes flakeModules without the incomplete context of the first step. I would suppose that the incomplete context is unnecessary anyway. But yes, the logic I was proposing for localFlakeModules and all that resembles the flake-parts.flakeModules.flakeModules module a lot. Implementing it with the module system seems like a good idea, but I wouldn't reuse mkFlake for it. (I can sort of imagine perhaps some other "dependency management" options to end up in that little module context, although those are probably not very helpful ones without additions to Nix's flakes feature. Ideally flakes itself would just bear such responsibilities. But let's not get ahead of ourselves on this)

roberth avatar May 08 '23 21:05 roberth

Do you mean we can already reference other modules under the same repository via imports = [topLevel.config.flake.flakeModules.myOtherModule]?

If the claim is true, we should directly close this issue, because referencing a flake module from flake.nix is another problem, which can be solved by calling mkFlake twice.

Atry avatar May 08 '23 22:05 Atry

Doing it twice smells a lot like we have a need for fixpoint iteration then, and that would not be feasible.

Why do you think it's not feasible? Sbt supports recursively solving sbt plugins for sbt plugins according to this document.

Atry avatar May 08 '23 22:05 Atry

I think we don't have to change mkFlake to introduce the fixed point, because the trick on the user end would be good enough. I created #155 to update the document.

Atry avatar May 08 '23 22:05 Atry

It turned out that what resulted in infinite loop is self.outPath, not lazyAttrsOf.

On Mon, May 8, 2023 at 6:08 PM Robert Hensing @.***> wrote:

I want to avoid lazyAttrsOf, which would result in infinite loop

We have tests for lazyAttrsOf, so you can paint me skeptical. Would love to have a reproducer for whatever would be wrong with it, but I find it unlikely that lazyAttrsOf would be in the critical path. If anything, I would expect the problem to be with config.

I can't verify any of this right now, and these discussions have already taken up a lot of time. I'll have to resume this later, but feel free to try things in the meantime.

Also we can call mkFlake twice to use a flake's own flake module,

Doing it twice smells a lot like we have a need for fixpoint iteration then, and that would not be feasible. I'd prefer to have a very restricted step that only processes flakeModules without the incomplete context of the first step. I would suppose that the incomplete context is unnecessary anyway. But yes, the logic I was proposing for localFlakeModules and all that resembles the flake-parts.flakeModules.flakeModules module a lot. Implementing it with the module system seems like a good idea, but I wouldn't reuse mkFlake for it. (I can sort of imagine perhaps some other "dependency management" options to end up in that little module context, although those are probably not very helpful ones without additions to Nix's flakes feature. Ideally flakes itself would just bear such responsibilities. But let's not get ahead of ourselves on this)

— Reply to this email directly, view it on GitHub https://github.com/hercules-ci/flake-parts/issues/121#issuecomment-1539064253, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAES3OSAN2HW6GNCQJRHBIDXFGKJFANCNFSM6AAAAAAVJNYJAI . You are receiving this because you commented.Message ID: @.***>

Atry avatar May 09 '23 03:05 Atry

I updated #197 to utilize partitions to solve this issue. I think this issue can be considered fixed.

Atry avatar Sep 12 '24 17:09 Atry