Skip to content

lib.types.submodule: deprecate the submodule family#377718

Open
hsjobeki wants to merge 3 commits intoNixOS:masterfrom
hsjobeki:types/submodule
Open

lib.types.submodule: deprecate the submodule family#377718
hsjobeki wants to merge 3 commits intoNixOS:masterfrom
hsjobeki:types/submodule

Conversation

@hsjobeki
Copy link
Contributor

@hsjobeki hsjobeki commented Jan 29, 2025

This PR adds a graceful trace to types.submoduie and promotes migrating to the alternative lib.types.module that is introduced here as more well behaved.

Problem: as described in #377575
The following code behaves inconsistently.

options.foo = lib.mkOption {
  # With this type modules in `default` or via `config` can't declare any `imports`
  type = lib.types.submodule {
    options.bar = mkOption {
      type = types.str;
    };            
   };
   default = {
   # Error
    imports = [
      {
        bar = "bar";
       }
     ];
    };
};
config.foo = {
  # Same Error as above
  imports = [ ... ];
}

While using the exact same code submoduleWith resolves the imports.

options.foo = lib.mkOption {
  # With this type the imports do work
   type = lib.types.submoduleWith {
      modules = [
          {
             options.bar = mkOption {
               type = types.str;
            };
         }
       ];
    };
    default = {
     # Works as expected
      imports = [
         {
           bar = "bar";
          }
        ];
    };
};
# Works as expected
config.foo = {
  imports = [ ... ];
}

Alternatives considered:

Cons:

Pros:

  • Migration to types.module is graceful
  • Migration is trivial with search and replace. (Excluding those who use config.imports and config.options via shorthand, must double check)
  • The name is shorter
  • The name more closely captures the actual type.
    Compare the type module vs submodule. Why should the type of a module be different that the submodule? In fact all module are submodules. So why the name of the type suggests something else?
    type Module :: { imports :: ListOf Module; config :: ...; options :: ...; }
    and
    evalModules :: { module :: ListOf Module; } -> Configuration (not sure about the result. but the point about the name should be clear)

I am also very divided on this topic. And seeking for advice how to actively improve the situation.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

builtins.warn does abort the evaluation when NIX_ABORT_ON_WARN is set
this leads to failing in documentation rendering where nixosOptionsDoc is called with warningsAreErrors=true (default)
bigger renames are therefore impossible with lib.warn / builtins.warn

The introduction allows for soft deprecation of types without breaking NixOS or downstream documentation tooling
@github-actions github-actions bot added 6.topic: module system About "NixOS" module system internals 6.topic: lib The Nixpkgs function library labels Jan 29, 2025
@hsjobeki hsjobeki added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 1.severity: significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. and removed 6.topic: module system About "NixOS" module system internals labels Jan 29, 2025
@hsjobeki hsjobeki requested review from infinisil and roberth and removed request for infinisil January 29, 2025 10:14
@github-actions github-actions bot added 6.topic: module system About "NixOS" module system internals 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Jan 29, 2025
@roberth
Copy link
Member

roberth commented Jan 29, 2025

I am aware of the problem, and part of me wants to do it, but I believe doing this just isn't worth the human cost of annoying messages and making all the changes.

Maybe this could be done in a future where all "non-programming"¹ submodules have become types.record, but that's probably not realistic either.

I would definitely block it on #334680 (records) anyway, because it provides a slightly better alternative for many submodules.

¹ I would call use of imports "programming" because of its similarity to function application (scopes, variable substitution of sorts). record is "merely" types or schemas, but it's more capable in that domain thanks to optional fields. Indeed types.submodule is the worst of both worlds.

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

See my previous comment, as I believe we should consider this best to be

  • Blocked on large scale adoption of #334680

@roberth
Copy link
Member

roberth commented Jan 30, 2025

Subconfiguration

We have a naming problem where it's not clear which things are "functions" and which are data

Functions:

  • module
  • types.deferredModule
  • configuration.nix

Value-like, no parameters:

  • the result of evalModules
    • _type = "configuration";
    • config, options
  • submodule

Arguably a submodule is not a module but a configuration, so I would suggest subconfiguration if we were to rename it.

(Also fwiw evalModules could be renamed to applyModules or even configure, but that doesn't seem productive unless we have a real reason to break the evalModules interface that isn't args and check)

@MattSturgeon
Copy link
Contributor

Problem: as described in #377575
The following code behaves inconsistently.
[...]
While using the exact same code submoduleWith resolves the imports.

So, essentially we're just changing submodule to use shorthandOnlyDefinesConfig's default (false) instead of arbitrarily setting it to true.

And renaming the option-type to a) draw attention to the semantic change and b) take the opportunity to improve its name.

Quoting the manual:

Enabling this only has a benefit when the submodule defines an option named config or options.

So it always felt strange to me that it was enabled in lib.submodule and disabled by default in lib.submoduleWith...

I would definitely block it on #334680 (records) anyway, because it provides a slightly better alternative for many submodules.

While they may solve similar problems in many cases, records and submodules (subconfigurations) are very different tools in the toolbox, IMO. I don't know if I agree that records should block improving how submodules/configurations are dealt with?

Arguably a submodule is not a module but a configuration, so I would suggest subconfiguration if we were to rename it.

Agreed, if a little verbose.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 2, 2025
@hsjobeki
Copy link
Contributor Author

Arguably a submodule is not a module but a configuration, so I would suggest subconfiguration if we were to rename it.

Agreed, if a little verbose.

I don't agree. Is this idea representing the implementation or is it what the user interacts with?

What is the difference between a module and a submodule for a user?

# A module
{
   imports = [];
   config = {};
   options  = {};
}

vs a submodule

# A submodule
{
   imports = [];
   config = {};
   options  = {};
}

But maybe i understand this wrong, what is the difference?

@nixpkgs-ci nixpkgs-ci bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 25, 2025
@MattSturgeon
Copy link
Contributor

MattSturgeon commented Sep 25, 2025

Submodules are fundamentally a bit of a weird type.

  • The definitions are modules.
  • The merge imports them into a configuration (evalModules or extendModules).
  • The final value is not a module; it's the result of evaluating the modules as a configuration (and taking its config).

...So the appropriate name depends on which aspect you wish to describe: Should the name describe the definitions? Or should it describe the final value? Or should it represent the whole thing end-to-end?

In my mind, "submodule" represents the definitions well, but it obfuscates what the final value is.

I think "sub-configuration" better represents the whole type end-to-end, but I still worry it could be overly verbose or too much jargon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.severity: significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. 2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: lib The Nixpkgs function library 6.topic: module system About "NixOS" module system internals 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants