Skip to content

Add dag library to config.lib#160

Merged
rycee merged 1 commit intomasterfrom
config-lib-dag
Dec 26, 2017
Merged

Add dag library to config.lib#160
rycee merged 1 commit intomasterfrom
config-lib-dag

Conversation

@rycee
Copy link
Copy Markdown
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
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

rycee commented Dec 5, 2017

Force-pushed updated code.

@infinisil
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants