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

sharedModules: add pkgs to module args #1879

Closed
wants to merge 1 commit into from

Conversation

nrdxp
Copy link

@nrdxp nrdxp commented Mar 25, 2021

Fixes #1878.

Description

See the linked issue for context. Opting for a minimal solution.

Todo

  • fix for NixOS module
  • consider removing useGlobalPackages altogether and always opt for NixOS systems pkgs.
  • check nix-darwin module for similar issue and fix if found

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

@nrdxp nrdxp requested a review from rycee as a code owner March 25, 2021 00:55
@nrdxp nrdxp force-pushed the sharedModules-fix branch from fc99de1 to 2a08899 Compare March 25, 2021 00:56
@berbiche
Copy link
Member

Looking at:

imports = import ../modules/modules.nix {
inherit pkgs;
lib = extendedLib;
useNixpkgsModule = !cfg.useGlobalPkgs;
};

I wonder whether we could rewrite it to imports = (import ....) ++ cfg.sharedModules;

@nrdxp
Copy link
Author

nrdxp commented Mar 25, 2021

That was my first attempt at a solution, unfortunately it didn't work.

@nrdxp nrdxp marked this pull request as draft March 25, 2021 01:13
@berbiche
Copy link
Member

berbiche commented Mar 25, 2021

Sorry, I talked too soon :(

@Pacman99
Copy link
Contributor

@berbiche I thought that would work(earlier comment) too. Looking at the submoduleWith implementation it seems like the error might come up, because the first set of modules gets evaluated first or at least collected(I think). But then again, most of the implementation is very cryptic to me. And considering that adding sharedModules to imports doesn't help, it seems like its something else.

@berbiche
Copy link
Member

@Pacman99 I think paths are treated differently from functions unfortunately.

If the sharedModule is a file then pkgs is available, otherwise it isn't and specialArgs needs to be set.

@berbiche
Copy link
Member

https://github.com/NixOS/nixpkgs/blob/d600f006643e074c2ef1d72e462e218b647a096c/lib/modules.nix#L284

Yup, functions are eagerly checked whereas files and paths are not.

@nrdxp
Copy link
Author

nrdxp commented Mar 25, 2021

Moreover, it looks like they need to be in the current implementation or infinite recursion is inevitable, so I now feel more confident that this is the proper and least invasive fix.

@nrdxp nrdxp marked this pull request as ready for review March 25, 2021 05:59
@Pacman99
Copy link
Contributor

No idea if this would work, but could a modules argument be added to modules/modules.nix and these modules can be passed to that.
Also would this PR fix the issue if useGlobalPkgs is false, or does the issue even exist then?

nrdxp added a commit to divnix/digga that referenced this pull request Mar 25, 2021
@nrdxp
Copy link
Author

nrdxp commented Mar 25, 2021

Since pkgs is generated based on the value of config.nixpkgs fixing it for useGlobalPackages = false may not even be possible without causing infinite recursion. Perhaps we should make a stronger move and always use system pkgs when using the home-manager NixOS module, removing useGlobalPackages as an option entirely. A similar problem may exist for the darwin module as well.

@Pacman99
Copy link
Contributor

I think this is an issue of using the wrong type for shared modules. See my comment on the issue.

`functionTo` type causes eager module eval before `_module.args` is
passed. Relax submodule type constraint to fix nix-community#1878.
Pacman99 pushed a commit to Pacman99/home-manager that referenced this pull request Mar 25, 2021
check for sharedModule type functionTo tries to evaluate functions too
quickly and prevents sharedModules from accessing pkgs argument.
Fixes nix-community#1879.
Pacman99 pushed a commit to Pacman99/home-manager that referenced this pull request Mar 25, 2021
check for sharedModule type functionTo tries to evaluate functions too
quickly and prevents sharedModules from accessing pkgs argument.
Fixes nix-community#1879.
@nrdxp nrdxp force-pushed the sharedModules-fix branch from 2a08899 to b27dc1e Compare March 25, 2021 21:43
@nrdxp
Copy link
Author

nrdxp commented Mar 25, 2021

Duplicate of #1880

@nrdxp nrdxp closed this Mar 25, 2021
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.

sharedModules are not passed pkgs arg
3 participants