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

Add dag library to config.lib #160

Merged
merged 1 commit into from Dec 26, 2017
Merged

Add dag library to config.lib #160

merged 1 commit into from Dec 26, 2017

Conversation

rycee
Copy link
Member

@rycee rycee commented Dec 2, 2017

Also replace all imports of dag.nix by the entry in config.lib.

@rycee rycee requested a review from infinisil December 2, 2017 18:17
@@ -74,6 +74,7 @@ let
config._module.args.baseModules = modules;
config._module.args.pkgs = lib.mkDefault pkgs;
config._module.check = check;
config.lib.dag = import lib/dag.nix { inherit lib; };
Copy link
Collaborator

@infinisil infinisil Dec 2, 2017

Choose a reason for hiding this comment

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

I think we could use this opportunity to make it possible to add more general purpose functions to the library. Adding a lib/default.nix with

{ lib }: {
  dag = import ./dag { inherit lib; };
  ...
}

Then config.lib = import ./lib { inherit lib; }; (or config.lib.home = ...)

This also calls for a DAG library refactor, such that you can use config.lib.dag.entryAfter instead of config.lib.dag.dagEntryAfter, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about the function renaming. Unfortunately it would break the configuration of anybody who uses the dag functions directly.

I'm now thinking the config.lib.dag entries actually should be different from the dag.nix file. I.e., to have config.lib.dag = { …; entryAfter = dagEntryAfter; … }; etc. The old dag.nix could be deprecated and removed in the future.

Also agree on the use of lib/default.nix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea regarding the slow deprecation

@rycee
Copy link
Member Author

rycee commented Dec 5, 2017

Force-pushed updated code.

@infinisil
Copy link
Collaborator

infinisil commented Dec 6, 2017

Apparently it also works to just use actual modules, for some reason I don't get recursion when I do this: infinisil@efc486b

This also enables having the file-types.nix in config.lib and seems generally nicer

@rycee
Copy link
Member Author

rycee commented Dec 7, 2017

Hmm, when I switch

with config.lib;
with lib;

to

with lib;
with config.lib;

e.g. in systemd.nix I get

error: infinite recursion encountered, at /home/rycee/devel/nixpkgs-17.09/lib/modules.nix:62:71

with or without your changes. I tried to track this down but unfortunately couldn't find the cause.

Also replace all imports of `dag.nix` by the entry in `config.lib`.
@rycee
Copy link
Member Author

rycee commented Dec 26, 2017

Replaced all use of with config.lib to instead explicitly bind dag = config.lib.dag in the let. This seems to resolve all recursion problems.

@rycee rycee merged commit f0d207f into master Dec 26, 2017
@rycee rycee deleted the config-lib-dag branch December 26, 2017 16:48
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.

None yet

2 participants