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.

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

Can't merge hierarchical options into submodule (or freeform option) #146882

Closed
roberth opened this issue Nov 21, 2021 · 6 comments · Fixed by #156533
Closed

Can't merge hierarchical options into submodule (or freeform option) #146882

roberth opened this issue Nov 21, 2021 · 6 comments · Fixed by #156533
Labels
0.kind: bug Something is broken 0.kind: enhancement Add something new 6.topic: module system About "NixOS" module system internals

Comments

@roberth
Copy link
Member

roberth commented Nov 21, 2021

Describe the bug

Part of an option tree may be free-form.

Extending the free-form part in a separate module is hard and prevents certain use cases and migrations.

Steps To Reproduce

let
  lib = import ./lib;
  inherit (lib) types mkOption;
in
(lib.evalModules {
  modules = [
    {
      options.free = mkOption {
        type = lib.types.submoduleWith {
          modules = [
            # UPDATE: A freeformType is not necessarily part of the problem.
            # { freeformType = lib.types.anything; }
          ];
        };
        default = {};
      };
    }
    {
      options.free = mkOption {
        type = lib.types.submoduleWith {
          modules = [
            {
              options.complicated = mkOption {
                type = types.int;
                default = 1;
              };
            }
          ];
        };
      };
    }
    {
      options.free.specific = mkOption {
        type = lib.types.int;
        default = 2;
      };
    }
  ];
}).config
$ nix-instantiate test.nix  --eval --strict
error: The option `free' in `<unknown-file>' is a prefix of options in `<unknown-file>'.
(use '--show-trace' to show detailed location information)

The submodule only exists in order to provide a freeform type for that subtree, but it is conceptually still the same tree.

Expected behavior

$ nix-instantiate test.nix  --eval --strict
{ free = { complicated = 1; specific = 2; }; }

Additional context

A semi-workaround is to make writing the unnecessary module boundary easier.

    mkSubmoduleOptions =
      options:
      mkOption {
        type = types.submoduleWith {
          modules = [ { inherit options; } ];
        };
      };

However, this is needlessly complicated and it does not help in situations where a freeform type is introduced in a decentralized scenario.

Notify maintainers

@infinisil

Metadata

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute:
  - lib.evalModules
  - lib.mkOption
# a list of nixos modules affected by the problem
module:
@roberth roberth added 0.kind: bug Something is broken 6.topic: module system About "NixOS" module system internals labels Nov 21, 2021
@roberth roberth changed the title Can't merge hierarchical options into freeform option Can't merge hierarchical options into submodule (or freeform option) Nov 21, 2021
@infinisil
Copy link
Member

I don't think this is a bug, but it's very much something we should make easier. If we implement this, we should also implement the same functionality for submodules nested in attrsOf's, because currently you need to do

{
  # Adding a submodule to `options.foo`, which is an `attrsOf (submodule ...)`
  options.foo = mkOption {
    type = attrsOf (submodule ...);
  };
}

It would be nice if something like

{
  options.foo._ = ...;
  options.foo."*" = ...;
  options.foo."<name>" = ...;
}

was supported. This could then of course also be used to declare the option in the first place.

@roberth roberth added the 0.kind: enhancement Add something new label Nov 22, 2021
@roberth
Copy link
Member Author

roberth commented Nov 22, 2021

This could then of course also be used to declare the option in the first place.

I would prefer for options.foo."*".x to just create x in the submodule instance where name is "*". Besides being more predictable, it doesn't lock people into thinking about submodules the way many already do; merely as a way to create a data structure. Modules are suitable for organizing logic as well. For instance, the systemd modules are quite messy. At least some of its logic has a natural place in the module tree and refactoring it that way could clean up the mess of utils libraries.

if we implement this, we should also implement the same functionality for submodules nested in attrsOf's

We could. We'll be removing restrictions and it's ok to do one at a time. We should certainly consider it though.

@infinisil
Copy link
Member

I would prefer for options.foo."*".x to just create x in the submodule instance where name is "*"

Isn't that the same behavior as assigning config.foo."*".x? Or do you mean something different

@roberth
Copy link
Member Author

roberth commented Nov 22, 2021

Here x is an option declaration, not just a config definition.

I actually know a use case for this, which I've encountered in the pre-commit-hooks.nix project, where you can generally do everything in the part of the tree that builds the config file, but some hooks have interesting options that you want to model. These are currently unintuitive to write and I don't know if they'd even end up in the docs. It's like config.systemd.services vs config.services, except the balance is such that you rarely need services. Here's an example in terms of systemd, so you don't need to dive into pre-commit-hooks.nix. I don't think we want this for systemd, because we do want to abstract over its details. Anyway, it goes like this:

{
  config.systemd.services.foo = { ... }: {
    options.fooSpecific = lib.mkOption {};
  };
}

whereas more natural would be

{
  options.systemd.services.foo.fooSpecific = lib.mkOption {};
}

@roberth
Copy link
Member Author

roberth commented Jan 20, 2022

@infinisil I forgot about this limitation today. We should be able to do this: #155883

@roberth
Copy link
Member Author

roberth commented Jan 25, 2022

Possible fix in #156533

roberth added a commit to hercules-ci/nixpkgs that referenced this issue Mar 2, 2022
... where a bare submodule is an option that has a type like
`submoduleWith x`, as opposed to `attrsOf (submoduleWith x)`.

This makes migration unnecessary when introducing a freeform type
in an existing option tree.

Closes NixOS#146882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken 0.kind: enhancement Add something new 6.topic: module system About "NixOS" module system internals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants