[Nix-dev] Re: [Nix-commits] SVN commit: nix - 15553 - eelco - nix/trunk/src/libexpr

[Nicolas Pierron]

On Tue, May 12, 2009 at 13:25, Eelco Dolstra <e.dolstra at tudelft.nl> wrote:
> Nicolas Pierron wrote:
>
>> I just want to warn you that NixOS should not compile anymore because
>> it relies on:
>> - attribute set comparison.
>> - function comparison.
>
> I've now made the check conditional on an environment variable
> (NIX_NO_UNSAFE_EQ).  It's disabled by default (i.e. everything continues to
> work).  This can be removed once we've fixed all attrset equality tests.

Thanks, for this temporary feature.

>> This type of comparison is used in NixOS to avoid circular imports of
>> option sets.  It is hidden inside the "uniqFlatten" function which
>> call the "elem" function.
>
> This is for "require", right?  There are a few solutions:

Right.

> - (Quick solution)  Get rid of require, and just make sure that every NixOS
> module is included somewhere, say in system/options.nix.

Unfortunately, doing this will avoid any extension without modifying
NixOS sources.  This point is extremely important because it could be
used to implement multiple CD-configurations without adding an extra
file listing all imported files.

> - Make "require" a list of filenames rather than a list of (usually) imports.
> Then the option handling code knows which modules it has already processed.

Currently, "require" attribute can expect functions and attribute
sets. Most of the imported module use the syntax:

require = [
  (import ./1.nix)
  (import ./2.nix)
];

which will be replace by:

require = [
  ./1.nix
  ./2.nix
];

Unfortunately, most of the module also declare a set of options which
are made available via the "require" attribute.  Option declarations
are often too small to be put in an extra file.  So we would have to
keep this import without being able to compare it to anything else.

require = [
  options // an attribute set
  ./1.nix
  ./2.nix
];

Internally defined attribute sets are not accesible from external
modules.  So if the current file is imported, the related option
should also be imported once.

Thus, this solution may be practical if the comparisons on attribute
set is removed.  This leads to a weird behaviour where users have to
care to include things only once inside each module, but they don't
have to care about any cycle between multiple files.  Even with the
last remark, this is acceptable because such problem can easily be
detected inside a module.

> - Give NixOS modules an explicit name, i.e. a module name:
>
>  {pkgs, config, ...}:
>
>  { name = "cron";
>    requires =
>      [ (import ./upstart.nix)
>        ...
>      ];
>  };
>
>  The name allows the option handling code to know which modules it has already
> processed.

This solution implies that each option set provides a name attribute.
This is not practical because this duplicates the name of the file
inside the file.

> Really, the fundamental problem is that we're doing a graph traversal here (over
> the graph of option "requires"), and in a graph traversal you need a notion of
> node identity to know which nodes you've already visited.  In purely functional
> languages you can't use pointer equality for this, so you need to give nodes an
> explicit identity.

Right, and I think your second solution is the most practical as it
fits our usage of NixOS by removing extra keywords and it may also
avoid useless comparisons.

I think I will patch this soon.

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