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

Apply disabledModules recursively #76857

Merged
merged 4 commits into from Jan 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/default.nix
Expand Up @@ -101,7 +101,7 @@ let
cleanSource sourceByRegex sourceFilesBySuffices
commitIdFromGitRepo cleanSourceWith pathHasContext
canCleanSource;
inherit (modules) evalModules closeModules unifyModuleSyntax
inherit (modules) evalModules unifyModuleSyntax
applyIfFunction mergeModules
mergeModules' mergeOptionDecls evalOptionValue mergeDefinitions
pushDownProperties dischargeProperties filterOverrides
Expand All @@ -110,7 +110,7 @@ let
mkFixStrictness mkOrder mkBefore mkAfter mkAliasDefinitions
mkAliasAndWrapDefinitions fixMergeModules mkRemovedOptionModule
mkRenamedOptionModule mkMergedOptionModule mkChangedOptionModule
mkAliasOptionModule doRename filterModules;
mkAliasOptionModule doRename;
inherit (options) isOption mkEnableOption mkSinkUndeclaredOptions
mergeDefaultOption mergeOneOption mergeEqualOption getValues
getFiles optionAttrSetToDocList optionAttrSetToDocList'
Expand Down
98 changes: 73 additions & 25 deletions lib/modules.nix
Expand Up @@ -59,9 +59,12 @@ rec {
};
};

closed = closeModules (modules ++ [ internalModule ]) ({ inherit config options lib; } // specialArgs);
collected = collectModules
(specialArgs.modulesPath or "")
(modules ++ [ internalModule ])
({ inherit config options lib; } // specialArgs);

options = mergeModules prefix (reverseList (filterModules (specialArgs.modulesPath or "") closed));
options = mergeModules prefix (reverseList collected);

# Traverse options and extract the option values into the final
# config set. At the same time, check whether all option
Expand All @@ -87,31 +90,76 @@ rec {
result = { inherit options config; };
in result;

# collectModules :: (modulesPath: String) -> (modules: [ Module ]) -> (args: Attrs) -> [ Module ]
#
# Collects all modules recursively through `import` statements, filtering out
# all modules in disabledModules.
collectModules = let

# Filter disabled modules. Modules can be disabled allowing
# their implementation to be replaced.
filterModules = modulesPath: modules:
let
moduleKey = m: if isString m then toString modulesPath + "/" + m else toString m;
disabledKeys = map moduleKey (concatMap (m: m.disabledModules) modules);
in
filter (m: !(elem m.key disabledKeys)) modules;
# Like unifyModuleSyntax, but also imports paths and calls functions if necessary
loadModule = args: fallbackFile: fallbackKey: m:
if isFunction m || isAttrs m then
unifyModuleSyntax fallbackFile fallbackKey (applyIfFunction fallbackKey m args)
else unifyModuleSyntax (toString m) (toString m) (applyIfFunction (toString m) (import m) args);

/* Close a set of modules under the ‘imports’ relation. */
closeModules = modules: args:
let
toClosureList = file: parentKey: imap1 (n: x:
if isAttrs x || isFunction x then
let key = "${parentKey}:anon-${toString n}"; in
unifyModuleSyntax file key (applyIfFunction key x args)
else
let file = toString x; key = toString x; in
unifyModuleSyntax file key (applyIfFunction key (import x) args));
in
builtins.genericClosure {
infinisil marked this conversation as resolved.
Show resolved Hide resolved
startSet = toClosureList unknownModule "" modules;
operator = m: toClosureList m._file m.key m.imports;
};
/*
Collects all modules recursively into the form

{
disabled = [ <list of disabled modules> ];
# All modules of the main module list
modules = [
{
key = <key1>;
module = <module for key1>;
# All modules imported by the module for key1
modules = [
{
key = <key1-1>;
module = <module for key1-1>;
# All modules imported by the module for key1-1
modules = [ ... ];
}
...
];
}
...
];
}
*/
collectStructuredModules =
let
collectResults = modules: {
disabled = concatLists (catAttrs "disabled" modules);
inherit modules;
};
in parentFile: parentKey: initialModules: args: collectResults (imap1 (n: x:
let
module = loadModule args parentFile "${parentKey}:anon-${toString n}" x;
collectedImports = collectStructuredModules module._file module.key module.imports args;
in {
key = module.key;
module = module;
modules = collectedImports.modules;
disabled = module.disabledModules ++ collectedImports.disabled;
}) initialModules);

# filterModules :: String -> { disabled, modules } -> [ Module ]
#
# Filters a structure as emitted by collectStructuredModules by removing all disabled
# modules recursively. It returns the final list of unique-by-key modules
filterModules = modulesPath: { disabled, modules }:
let
moduleKey = m: if isString m then toString modulesPath + "/" + m else toString m;
disabledKeys = listToAttrs (map (k: nameValuePair (moduleKey k) null) disabled);
keyFilter = filter (attrs: ! disabledKeys ? ${attrs.key});
Copy link
Member Author

Choose a reason for hiding this comment

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

@roberth Now doing this. Was able to confirm slight memory decrease with this approach for ~25 disabled modules

in map (attrs: attrs.module) (builtins.genericClosure {
startSet = keyFilter modules;
operator = attrs: keyFilter attrs.modules;
});

in modulesPath: initialModules: args:
filterModules modulesPath (collectStructuredModules unknownModule "" initialModules args);

/* Massage a module into canonical form, that is, a set consisting
of ‘options’, ‘config’ and ‘imports’ attributes. */
Expand Down
6 changes: 6 additions & 0 deletions lib/tests/modules.sh
Expand Up @@ -182,6 +182,12 @@ checkConfigOutput "true" config.submodule.outer ./declare-submoduleWith-modules.
## Paths should be allowed as values and work as expected
checkConfigOutput "true" config.submodule.enable ./declare-submoduleWith-path.nix

# Check that disabledModules works recursively and correctly
checkConfigOutput "true" config.enable ./disable-recursive/main.nix
checkConfigOutput "true" config.enable ./disable-recursive/{main.nix,disable-foo.nix}
checkConfigOutput "true" config.enable ./disable-recursive/{main.nix,disable-bar.nix}
checkConfigError 'The option .* defined in .* does not exist' config.enable ./disable-recursive/{main.nix,disable-foo.nix,disable-bar.nix}

cat <<EOF
====== module tests ======
$pass Pass
Expand Down
5 changes: 5 additions & 0 deletions lib/tests/modules/disable-recursive/bar.nix
@@ -0,0 +1,5 @@
{
imports = [
../declare-enable.nix
];
}
7 changes: 7 additions & 0 deletions lib/tests/modules/disable-recursive/disable-bar.nix
@@ -0,0 +1,7 @@
{

disabledModules = [
./bar.nix
];

}
7 changes: 7 additions & 0 deletions lib/tests/modules/disable-recursive/disable-foo.nix
@@ -0,0 +1,7 @@
{

disabledModules = [
./foo.nix
];

}
5 changes: 5 additions & 0 deletions lib/tests/modules/disable-recursive/foo.nix
@@ -0,0 +1,5 @@
{
imports = [
../declare-enable.nix
];
}
8 changes: 8 additions & 0 deletions lib/tests/modules/disable-recursive/main.nix
@@ -0,0 +1,8 @@
{
imports = [
./foo.nix
./bar.nix
];

enable = true;
}
4 changes: 2 additions & 2 deletions nixos/doc/manual/development/replace-modules.xml
Expand Up @@ -6,8 +6,8 @@
<title>Replace Modules</title>

<para>
Modules that are imported can also be disabled. The option declarations and
config implementation of a disabled module will be ignored, allowing another
Modules that are imported can also be disabled. The option declarations,
config implementation and the imports of a disabled module will be ignored, allowing another
to take it's place. This can be used to import a set of modules from another
channel while keeping the rest of the system on a stable release.
</para>
Expand Down
7 changes: 5 additions & 2 deletions nixos/modules/misc/documentation.nix
@@ -1,4 +1,4 @@
{ config, lib, pkgs, baseModules, extraModules, modules, ... }:
{ config, lib, pkgs, baseModules, extraModules, modules, modulesPath, ... }:

with lib;

Expand All @@ -22,7 +22,10 @@ let
scrubbedEval = evalModules {
modules = [ { nixpkgs.localSystem = config.nixpkgs.localSystem; } ] ++ manualModules;
args = (config._module.args) // { modules = [ ]; };
specialArgs = { pkgs = scrubDerivations "pkgs" pkgs; };
specialArgs = {
pkgs = scrubDerivations "pkgs" pkgs;
inherit modulesPath;
};
};
scrubDerivations = namePrefix: pkgSet: mapAttrs
(name: value:
Expand Down