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

nixos: add key to shared module to allow disabling it #6017

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

Enzime
Copy link
Contributor

@Enzime Enzime commented Oct 29, 2024

Description

By adding key, this allows users to disable this shared module or they can choose to not disable this shared module (by filtering by key before disabling)

This means users can disable all shared modules if all modules are paths or attrsets with a key:

configuration.nix:

{ config, ... }:

{
  home-manager.users.enzime = { ... }: {
    disabledModules = config.home-manager.sharedModules;
  };
}

Or disabling just this module specifically:

{ ... }:

{
  home-manager.users.enzime = { ... }: {
    disabledModules = [ { key = "home-manager#nixos-shared-module"; } ];
  };
}

Or disabling all modules when you have modules you can't disable (like lambdas):

{ ... }:

{
  home-manager.users.enzime = { ... }: {
    disabledModules = lib.filter (v: lib.isString v || lib.isPath v || (lib.isAttrs v && v ? key)) config.home-manager.sharedModules;
  };
}

https://nixos.org/manual/nixos/unstable/#sec-replace-modules

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

Maintainer CC

cc @rycee

@roberth
Copy link
Contributor

roberth commented Oct 30, 2024

The key is good, but I don't understand why sharedModules is a list, and it probably doesn't need to be.
List indexes aren't good identifiers, so I would recommend to change it to coercedTo (listOf raw) (ms: { imports = ms; }) deferredModule.
Useful, stable identifiers for this module / these modules can be provided in the form of flake output attributes, whose values (modules) each have a unique key.

Copy link
Contributor

@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.

Good first step, and further improvements possible.

@Enzime
Copy link
Contributor Author

Enzime commented Oct 31, 2024

Changing sharedModules is a lot bigger of a refactor than the currently proposed changes, I think it would be best to do that in a different PR

@Enzime
Copy link
Contributor Author

Enzime commented Nov 6, 2024

cc @teto :)

@teto
Copy link
Collaborator

teto commented Nov 16, 2024

I dont know this aspect of the module system but the approval of roberth is good enough for me

@teto teto merged commit 192f123 into nix-community:master Nov 16, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants