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

Option for hookModule #422

Merged
merged 2 commits into from
Apr 1, 2024
Merged

Conversation

roberth
Copy link
Contributor

@roberth roberth commented Apr 1, 2024

This removes a bit of duplication, and allows hookModule to be monkey patched.

@domenkozar domenkozar requested a review from sandydoo April 1, 2024 07:03
@roberth roberth mentioned this pull request Apr 1, 2024
17 tasks
@roberth roberth changed the title Option for hook module Option for hookModule Apr 1, 2024
config._module.args.default_stages = cfg.default_stages;
};
config._module.args.hookModule = config.hookModule;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This chunk could be moved out to a separate file if you like.

Copy link
Member

@sandydoo sandydoo left a comment

Choose a reason for hiding this comment

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

Oooooh! So _module.args are way more powerful than I initially thought. Great stuff!

@roberth
Copy link
Contributor Author

roberth commented Apr 1, 2024

_module.args are way more powerful than I initially thought.

You can't use it for imports in the same fixpoint (here: the root, or initial evalModules), but you can refer to it in submodules. Those are independent evalModules calls performed by the submodule types.

@sandydoo sandydoo merged commit 1674357 into cachix:master Apr 1, 2024
4 checks passed
@azahi
Copy link

azahi commented Apr 1, 2024

Looks like this commit broke something for me:

[azahi@eonwe:~/src/foo/bar]$ nix develop --show-trace
error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'nix-shell'
         whose name attribute is located at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/pkgs/stdenv/generic/make-derivation.nix:331:7

       … while evaluating attribute 'shellHook' of derivation 'nix-shell'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/pkgs/build-support/mkshell/default.nix:45:3:

           44|
           45|   shellHook = lib.concatStringsSep "\n" (lib.catAttrs "shellHook"
             |   ^
           46|     (lib.reverseList inputsFrom ++ [ attrs ]));

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1171:18:

         1170|         mapAttrs
         1171|           (name: value:
             |                  ^
         1172|             if isAttrs value && cond value

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1174:18:

         1173|             then recurse (path ++ [ name ]) value
         1174|             else f (path ++ [ name ]) value);
             |                  ^
         1175|     in

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:242:72:

          241|           # For definitions that have an associated option
          242|           declaredConfig = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options;
             |                                                                        ^
          243|

       … while evaluating the option `installationScript':

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:824:28:

          823|         # Process mkMerge and mkIf properties.
          824|         defs' = concatMap (m:
             |                            ^
          825|           map (value: { inherit (m) file; inherit value; }) (builtins.addErrorContext "while evaluating definitions from `${m.file}':" (dischargeProperties m.value))

       … while evaluating definitions from `/nix/store/sd6zsx33p93wjpf94ylmf3lfr62br40k-source/modules/pre-commit.nix':

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:825:137:

          824|         defs' = concatMap (m:
          825|           map (value: { inherit (m) file; inherit value; }) (builtins.addErrorContext "while evaluating definitions from `${m.file}':" (dischargeProperties m.value))
             |                                                                                                                                         ^
          826|         ) defs;

       … while calling 'dischargeProperties'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:896:25:

          895|   */
          896|   dischargeProperties = def:
             |                         ^
          897|     if def._type or "" == "merge" then

       … while evaluating derivation 'pre-commit-config.json'
         whose name attribute is located at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/pkgs/stdenv/generic/make-derivation.nix:331:7

       … while evaluating attribute 'rawJSON' of derivation 'pre-commit-config.json'

         at /nix/store/sd6zsx33p93wjpf94ylmf3lfr62br40k-source/modules/pre-commit.nix:41:11:

           40|           passAsFile = [ "rawJSON" ];
           41|           rawJSON = builtins.toJSON cfg.rawConfig;
             |           ^
           42|         } ''

       … from call site

         at /nix/store/sd6zsx33p93wjpf94ylmf3lfr62br40k-source/modules/pre-commit.nix:28:5:

           27|   processedHooks =
           28|     mapAttrsToList (id: value: value.raw // { inherit id; }) enabledHooks;
             |     ^
           29|

       … while calling 'mapAttrsToList'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1061:5:

         1060|     f:
         1061|     attrs:
             |     ^
         1062|     map (name: f name attrs.${name}) (attrNames attrs);

       … from call site

         at /nix/store/sd6zsx33p93wjpf94ylmf3lfr62br40k-source/modules/pre-commit.nix:26:18:

           25|
           26|   enabledHooks = filterAttrs (id: value: value.enable) cfg.hooks;
             |                  ^
           27|   processedHooks =

       … while calling 'filterAttrs'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:646:5:

          645|     pred:
          646|     set:
             |     ^
          647|     listToAttrs (concatMap (name: let v = set.${name}; in if pred name v then [(nameValuePair name v)] else []) (attrNames set));

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:647:29:

          646|     set:
          647|     listToAttrs (concatMap (name: let v = set.${name}; in if pred name v then [(nameValuePair name v)] else []) (attrNames set));
             |                             ^
          648|

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:647:62:

          646|     set:
          647|     listToAttrs (concatMap (name: let v = set.${name}; in if pred name v then [(nameValuePair name v)] else []) (attrNames set));
             |                                                              ^
          648|

       … while calling anonymous lambda

         at /nix/store/sd6zsx33p93wjpf94ylmf3lfr62br40k-source/modules/pre-commit.nix:26:35:

           25|
           26|   enabledHooks = filterAttrs (id: value: value.enable) cfg.hooks;
             |                                   ^
           27|   processedHooks =

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1537:24:

         1536|     let f = attrPath:
         1537|       zipAttrsWith (n: values:
             |                        ^
         1538|         let here = attrPath ++ [n]; in

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1171:18:

         1170|         mapAttrs
         1171|           (name: value:
             |                  ^
         1172|             if isAttrs value && cond value

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1174:18:

         1173|             then recurse (path ++ [ name ]) value
         1174|             else f (path ++ [ name ]) value);
             |                  ^
         1175|     in

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:242:72:

          241|           # For definitions that have an associated option
          242|           declaredConfig = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options;
             |                                                                        ^
          243|

       … while evaluating the option `hooks.no-commit-to-branch':

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:846:59:

          845|       if isDefined then
          846|         if all (def: type.check def.value) defsFinal then type.merge loc defsFinal
             |                                                           ^
          847|         else let allInvalid = filter (def: ! type.check def.value) defsFinal;

       … while calling 'merge'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/types.nix:779:22:

          778|         check = x: isAttrs x || isFunction x || path.check x;
          779|         merge = loc: defs:
             |                      ^
          780|           (base.extendModules {

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:242:28:

          241|           # For definitions that have an associated option
          242|           declaredConfig = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options;
             |                            ^
          243|

       … while calling 'mapAttrsRecursiveCond'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/attrsets.nix:1167:5:

         1166|     f:
         1167|     set:
             |     ^
         1168|     let

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:234:33:

          233|           ({ inherit lib options config specialArgs; } // specialArgs);
          234|         in mergeModules prefix (reverseList collected);
             |                                 ^
          235|

       … while calling 'reverseList'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/lists.nix:1068:17:

         1067|   */
         1068|   reverseList = xs:
             |                 ^
         1069|     let l = length xs; in genList (n: elemAt xs (l - n - 1)) l;

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:229:25:

          228|       merged =
          229|         let collected = collectModules
             |                         ^
          230|           class

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:445:37:

          444|
          445|     in modulesPath: initialModules: args:
             |                                     ^
          446|       filterModules modulesPath (collectStructuredModules unknownModule "" initialModules args);

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:446:7:

          445|     in modulesPath: initialModules: args:
          446|       filterModules modulesPath (collectStructuredModules unknownModule "" initialModules args);
             |       ^
          447|

       … while calling 'filterModules'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:413:36:

          412|       # modules recursively. It returns the final list of unique-by-key modules
          413|       filterModules = modulesPath: { disabled, modules }:
             |                                    ^
          414|         let

       … while calling anonymous lambda

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:439:31:

          438|           disabledKeys = concatMap ({ file, disabled }: map (moduleKey file) disabled) disabled;
          439|           keyFilter = filter (attrs: ! elem attrs.key disabledKeys);
             |                               ^
          440|         in map (attrs: attrs.module) (builtins.genericClosure {

       … from call site

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/modules.nix:398:73:

          397|           };
          398|         in parentFile: parentKey: initialModules: args: collectResults (imap1 (n: x:
             |                                                                         ^
          399|           let

       … while calling 'imap1'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/lib/lists.nix:334:14:

          333|   */
          334|   imap1 = f: list: genList (n: f (n + 1) (elemAt list n)) (length list);
             |              ^
          335|

       error: value is a set while a list was expected
error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'nix-shell'
         whose name attribute is located at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/pkgs/stdenv/generic/make-derivation.nix:331:7

       … while evaluating attribute 'shellHook' of derivation 'nix-shell'

         at /nix/store/63wn3078wdbrrax7vbcamzffaabmsvdl-source/pkgs/build-support/mkshell/default.nix:45:3:

           44|
           45|   shellHook = lib.concatStringsSep "\n" (lib.catAttrs "shellHook"
             |   ^
           46|     (lib.reverseList inputsFrom ++ [ attrs ]));

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: value is a set while a list was expected

Relevant part of the flake looks something like this:

        devShells.default = pkgs.mkShellNoCC { inherit (self.checks.${system}.preCommit) shellHook; };

        checks.preCommit = pre-commit-hooks.lib.${system}.run {
          src = ./.;
          hooks = {
          };
        };

P.S. The previous commit on the master branch works fine.

@sandydoo
Copy link
Member

sandydoo commented Apr 1, 2024

@azahi, fixed

@SebTM
Copy link

SebTM commented Apr 1, 2024

Thanks for the fast fix 🥇 Works fine again on 8ae5bb63f9bcd0472b67cf4ada414ae37f1f2bcc (latest master) with both options:

inherit (self.checks.${system}.pre-commit-check) shellHook;
buildInputs = self.checks.${system}.pre-commit-check.enabledPackages;

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.

4 participants