[Nix-dev] Re: [Nix-commits] SVN commit: nix - 15723 - eelco - nixos/branches/modular-nixos/system

[Nicolas Pierron]

On Tue, May 26, 2009 at 13:42, Eelco Dolstra <e.dolstra at tudelft.nl> wrote:
> Hi,
>
> Nicolas Pierron wrote:
>
>> I would even say that you don't need a require stament in this case.
>> You can just in-line the options instead of the " require = [ options
>> ]; ".
>>
>> -{
>> -  require = [
>> -    options
>> -  ];
>> -}
>> +options
>>
>> A set of option declarations is a correct modules.
>
> Yes.  However, it still strikes me as somewhat odd that options declarations and
> definitions (i.e. config values) are declared in the same set.  What is the
> motivation for having them in the same set?  I would be inclined to separate
> them, i.e.

The reason of having them in the same set is that the same process
occur for merging declarations and definitions.  Keeping them in two
different sets implies that you are doing the same traversal of two
sets at the same time.  Having declarations and definitions living at
the same level avoid this difference which is nothing concrete.

Declarations and definitions are identical when seen from the
traversal process.  So why not factoring them to into one level.  And
when you will write your file with options and config you will end-up
writing the same things.  So why not make option living at the same
state of the configuration?  Why not factor the code which handle the
evaluation?

>  {config, pkgs, ...}:
>
>  {
>    # Dependencies (just a list of filenames).
>    require =
>      [ ./filename.nix
>      ];
>
>    # Option declarations.
>    options = {
>      foo.bar.enable = mkOption { ... };
>    };
>
>    # Option definitions.
>    config = {
>      xyzzy.fnord = 123;
>    };
>
>    # And maybe a module could have some other attributes, like...
>    name = "bla";
>    description = "...";
>  }

A second point comes from the merge and apply attributes.  These
attributes, as said in a previous mail on this mailing list, are more
parts of the implementation than parts of the interface.  I made a
patch recently which allow you to have multiple declarations of an
option to complete its content in another part.

My feeling about the current system and your proposal: They are too
much indentation levels (but this is now solved with the a.b.c = {};
syntax). When I have build this system I've tried to avoid hard coded
behavior, which means less special cases like the require attribute.

> This should also make the config processing in options.nix a bit more readable
> (compared to fixOptionsSetsFun) by untangling the chasing of the "require"
> dependencies, combining the option declarations, and computing the final
> configuration.  Something like:

fixOptionsSetsFun has many purpose which are:
- solve the require attribute
- handle the multiple kind of imported module (filename, with
argument, attribute set)
- call the merge function which merge the declarations and the definitions.

>  # Given: the set of top-level modules.
>  modules = [./foo.nix ./bar.nix ...];
>
>  # The closure of modules under the "require" relation.  I.e. for each module
>  # m, add m.require to the list.  It should be possible to do this efficiently
>  # using the genericClosure primop.
>  modulesClosed = builtins.genericClosure { startSet = module; ... };

This part is only the job done by fixOptionsSetsFun (except that it
calls the merge function)m So if I understand your pointm you just
want to replace the current uniqFlatten function by a call to
genericClosure and remove the call of the merge function from
fixOptionsSetsFun and name it moduleClosure, right ?

This is entirely unrelated to the module organization that you are suggesting.

>  # The set of options is the union of the "options" attributes of each module.
>  options = recursiveUnion (map (m: m.options) modulesClosed);
>
>  # Compute the final configuration by traversing the options and the
>  # corresponding values in each module's "config", merging them using the
>  # "merge" attributes of each option, checking the type of values, etc.
>  config = completeConfig options (map (m: m.config) modulesClosed);

Ok, just have a look at mergeOptionSets & filterOptionSets. The two
function that you have called completeConfig & recursiveUnion
correspond to (fixOptionSets mergeOptionSets pkgs) and to
(fixOptionSets filterOptionSets pkgs)

So If I understand your proposal you have made two orthogonal points:
1/ You want to organize modules differently
2/ You want to clean-up pkgs/lib/options.nix

1) The first time you mention this, it was for scoping options to only
verify that every module import correctly its dependencies.  Since you
have introduce in modular-nixos a layout which entirely removes the
dependencies because this cause a relocation problem.  Now you
introduce modules meta-data which can be helpful for automated
filtering I would have say no before your commit about setting
attribute in deepeer attribute sets because this could have introduce
a highly depper hierarchy.

2) This can be done in a few steps.
I've a patch for that.

-- 
Nicolas Pierron
http://www.linkedin.com/in/nicolasbpierron
- If you are doing something twice then you should try to do it once.