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).
This commit is contained in:
		
							parent
							
								
									06bb2d95b4
								
							
						
					
					
						commit
						ef4f5ba85e
					
				
					 4 changed files with 62 additions and 50 deletions
				
			
		|  | @ -6,9 +6,8 @@ | |||
| namespace nix { | ||||
| 
 | ||||
| 
 | ||||
| // !!! Shouldn't we return a pointer to a Value?
 | ||||
| void findAlongAttrPath(EvalState & state, const string & attrPath, | ||||
|     Bindings & autoArgs, Expr * e, Value & v) | ||||
| Value * findAlongAttrPath(EvalState & state, const string & attrPath, | ||||
|     Bindings & autoArgs, Value & vIn) | ||||
| { | ||||
|     Strings tokens = tokenizeString<Strings>(attrPath, "."); | ||||
| 
 | ||||
|  | @ -17,7 +16,7 @@ void findAlongAttrPath(EvalState & state, const string & attrPath, | |||
| 
 | ||||
|     string curPath; | ||||
| 
 | ||||
|     state.mkThunk_(v, e); | ||||
|     Value * v = &vIn; | ||||
| 
 | ||||
|     foreach (Strings::iterator, i, tokens) { | ||||
| 
 | ||||
|  | @ -31,10 +30,10 @@ void findAlongAttrPath(EvalState & state, const string & attrPath, | |||
|         if (string2Int(attr, attrIndex)) apType = apIndex; | ||||
| 
 | ||||
|         /* Evaluate the expression. */ | ||||
|         Value vTmp; | ||||
|         state.autoCallFunction(autoArgs, v, vTmp); | ||||
|         v = vTmp; | ||||
|         state.forceValue(v); | ||||
|         Value * vNew = state.allocValue(); | ||||
|         state.autoCallFunction(autoArgs, *v, *vNew); | ||||
|         v = vNew; | ||||
|         state.forceValue(*v); | ||||
| 
 | ||||
|         /* It should evaluate to either an attribute set or an
 | ||||
|            expression, according to what is specified in the | ||||
|  | @ -42,31 +41,33 @@ void findAlongAttrPath(EvalState & state, const string & attrPath, | |||
| 
 | ||||
|         if (apType == apAttr) { | ||||
| 
 | ||||
|             if (v.type != tAttrs) | ||||
|             if (v->type != tAttrs) | ||||
|                 throw TypeError( | ||||
|                     format("the expression selected by the selection path `%1%' should be an attribute set but is %2%") | ||||
|                     % curPath % showType(v)); | ||||
|                     % curPath % showType(*v)); | ||||
| 
 | ||||
|             Bindings::iterator a = v.attrs->find(state.symbols.create(attr)); | ||||
|             if (a == v.attrs->end()) | ||||
|             Bindings::iterator a = v->attrs->find(state.symbols.create(attr)); | ||||
|             if (a == v->attrs->end()) | ||||
|                 throw Error(format("attribute `%1%' in selection path `%2%' not found") % attr % curPath); | ||||
|             v = *a->value; | ||||
|             v = &*a->value; | ||||
|         } | ||||
| 
 | ||||
|         else if (apType == apIndex) { | ||||
| 
 | ||||
|             if (v.type != tList) | ||||
|             if (v->type != tList) | ||||
|                 throw TypeError( | ||||
|                     format("the expression selected by the selection path `%1%' should be a list but is %2%") | ||||
|                     % curPath % showType(v)); | ||||
|                     % curPath % showType(*v)); | ||||
| 
 | ||||
|             if (attrIndex >= v.list.length) | ||||
|             if (attrIndex >= v->list.length) | ||||
|                 throw Error(format("list index %1% in selection path `%2%' is out of range") % attrIndex % curPath); | ||||
| 
 | ||||
|             v = *v.list.elems[attrIndex]; | ||||
|             v = v->list.elems[attrIndex]; | ||||
|         } | ||||
| 
 | ||||
|     } | ||||
| 
 | ||||
|     return v; | ||||
| } | ||||
| 
 | ||||
| 
 | ||||
|  |  | |||
|  | @ -7,7 +7,7 @@ | |||
| 
 | ||||
| namespace nix { | ||||
| 
 | ||||
| void findAlongAttrPath(EvalState & state, const string & attrPath, | ||||
|     Bindings & autoArgs, Expr * e, Value & v); | ||||
| Value * findAlongAttrPath(EvalState & state, const string & attrPath, | ||||
|     Bindings & autoArgs, Value & vIn); | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -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; | ||||
|  |  | |||
|  | @ -44,9 +44,11 @@ void processExpr(EvalState & state, const Strings & attrPaths, | |||
|         return; | ||||
|     } | ||||
| 
 | ||||
|     Value vRoot; | ||||
|     state.eval(e, vRoot); | ||||
| 
 | ||||
|     foreach (Strings::const_iterator, i, attrPaths) { | ||||
|         Value v; | ||||
|         findAlongAttrPath(state, *i, autoArgs, e, v); | ||||
|         Value & v(*findAlongAttrPath(state, *i, autoArgs, vRoot)); | ||||
|         state.forceValue(v); | ||||
| 
 | ||||
|         PathSet context; | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue