Work on Values instead of Exprs

This prevents some duplicate evaluation in nix-env and
nix-instantiate.

Also, when traversing ~/.nix-defexpr, only read regular files with the
extension .nix.  Previously it was reading files like
.../channels/binary-caches/<name>.  The only reason this didn't cause
problems is pure luck (namely, <name> shadows an actual Nix
expression, the binary-caches files happen to be syntactically valid
Nix expressions, and we iterate over the directory contents in just
the right order).

1 files changed, 38 insertions, 29 deletions

diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index 1cb1ec4833..1e1e4d00d5 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -94,18 +94,14 @@ static bool parseInstallSourceOptions(Globals & globals,
 }
 
 
-static bool isNixExpr(const Path & path)
+static bool isNixExpr(const Path & path, struct stat & st)
 {
-    struct stat st;
-    if (stat(path.c_str(), &st) == -1)
-        throw SysError(format("getting information about `%1%'") % path);
-
-    return !S_ISDIR(st.st_mode) || pathExists(path + "/default.nix");
+    return S_ISREG(st.st_mode) || (S_ISDIR(st.st_mode) && pathExists(path + "/default.nix"));
 }
 
 
 static void getAllExprs(EvalState & state,
-    const Path & path, ExprAttrs & attrs)
+    const Path & path, Value & v)
 {
     Strings names = readDirectory(path);
     StringSet namesSorted(names.begin(), names.end());
@@ -122,7 +118,7 @@ static void getAllExprs(EvalState & state,
         if (stat(path2.c_str(), &st) == -1)
             continue; // ignore dangling symlinks in ~/.nix-defexpr
 
-        if (isNixExpr(path2)) {
+        if (isNixExpr(path2, st) && (!S_ISREG(st.st_mode) || hasSuffix(path2, ".nix"))) {
             /* Strip off the `.nix' filename suffix (if applicable),
                otherwise the attribute cannot be selected with the
                `-A' option.  Useful if you want to stick a Nix
@@ -130,20 +126,28 @@ static void getAllExprs(EvalState & state,
             string attrName = *i;
             if (hasSuffix(attrName, ".nix"))
                 attrName = string(attrName, 0, attrName.size() - 4);
-            attrs.attrs[state.symbols.create(attrName)] =
-                ExprAttrs::AttrDef(state.parseExprFromFile(resolveExprPath(absPath(path2))), noPos);
+            // FIXME: make loading lazy.
+            Value & v2(*state.allocAttr(v, state.symbols.create(attrName)));
+            state.evalFile(path2, v2);
         }
-        else
+        else if (S_ISDIR(st.st_mode))
             /* `path2' is a directory (with no default.nix in it);
                recurse into it. */
-            getAllExprs(state, path2, attrs);
+            getAllExprs(state, path2, v);
     }
 }
 
 
-static Expr * loadSourceExpr(EvalState & state, const Path & path)
+static void loadSourceExpr(EvalState & state, const Path & path, Value & v)
 {
-    if (isNixExpr(path)) return state.parseExprFromFile(resolveExprPath(absPath(path)));
+    struct stat st;
+    if (stat(path.c_str(), &st) == -1)
+        throw SysError(format("getting information about `%1%'") % path);
+
+    if (isNixExpr(path, st)) {
+        state.evalFile(path, v);
+        return;
+    }
 
     /* The path is a directory.  Put the Nix expressions in the
        directory in an attribute set, with the file name of each
@@ -151,11 +155,12 @@ static Expr * loadSourceExpr(EvalState & state, const Path & path)
        (but keep the attribute set flat, not nested, to make it easier
        for a user to have a ~/.nix-defexpr directory that includes
        some system-wide directory). */
-    ExprAttrs * attrs = new ExprAttrs;
-    attrs->attrs[state.symbols.create("_combineChannels")] =
-        ExprAttrs::AttrDef(new ExprList(), noPos);
-    getAllExprs(state, path, *attrs);
-    return attrs;
+    if (S_ISDIR(st.st_mode)) {
+        state.mkAttrs(v, 16);
+        state.mkList(*state.allocAttr(v, state.symbols.create("_combineChannels")), 0);
+        getAllExprs(state, path, v);
+        v.attrs->sort();
+    }
 }
 
 
@@ -163,8 +168,10 @@ static void loadDerivations(EvalState & state, Path nixExprPath,
     string systemFilter, Bindings & autoArgs,
     const string & pathPrefix, DrvInfos & elems)
 {
-    Value v;
-    findAlongAttrPath(state, pathPrefix, autoArgs, loadSourceExpr(state, nixExprPath), v);
+    Value vRoot;
+    loadSourceExpr(state, nixExprPath, vRoot);
+
+    Value & v(*findAlongAttrPath(state, pathPrefix, autoArgs, vRoot));
 
     getDerivations(state, v, pathPrefix, autoArgs, elems, true);
 
@@ -356,13 +363,15 @@ static void queryInstSources(EvalState & state,
            (import ./foo.nix)' = `(import ./foo.nix).bar'. */
         case srcNixExprs: {
 
-            Expr * e1 = loadSourceExpr(state, instSource.nixExprPath);
+            Value vArg;
+            loadSourceExpr(state, instSource.nixExprPath, vArg);
 
             foreach (Strings::const_iterator, i, args) {
-                Expr * e2 = state.parseExprFromString(*i, absPath("."));
-                Expr * call = new ExprApp(e2, e1);
-                Value v; state.eval(call, v);
-                getDerivations(state, v, "", instSource.autoArgs, elems, true);
+                Expr * eFun = state.parseExprFromString(*i, absPath("."));
+                Value vFun, vTmp;
+                state.eval(eFun, vFun);
+                mkApp(vTmp, vFun, vArg);
+                getDerivations(state, vTmp, "", instSource.autoArgs, elems, true);
             }
 
             break;
@@ -413,10 +422,10 @@ static void queryInstSources(EvalState & state,
         }
 
         case srcAttrPath: {
+            Value vRoot;
+            loadSourceExpr(state, instSource.nixExprPath, vRoot);
             foreach (Strings::const_iterator, i, args) {
-                Value v;
-                findAlongAttrPath(state, *i, instSource.autoArgs,
-                    loadSourceExpr(state, instSource.nixExprPath), v);
+                Value & v(*findAlongAttrPath(state, *i, instSource.autoArgs, vRoot));
                 getDerivations(state, v, "", instSource.autoArgs, elems, true);
             }
             break;