From 363f37d0843d87e03bd4dcb600ce04e7be60d7e1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 14 Apr 2016 15:32:24 +0200 Subject: [PATCH] Make the search path lazier with non-fatal errors Thus, -I / $NIX_PATH entries are now downloaded only when they are needed for evaluation. An error to download an entry is a non-fatal warning (just like non-existant paths). This does change the semantics of builtins.nixPath, which now returns the original, rather than resulting path. E.g., before we had [ { path = "/nix/store/hgm3yxf1lrrwa3z14zpqaj5p9vs0qklk-nixexprs.tar.xz"; prefix = "nixpkgs"; } ... ] but now [ { path = "https://nixos.org/channels/nixos-16.03/nixexprs.tar.xz"; prefix = "nixpkgs"; } ... ] Fixes #792. --- src/libexpr/eval.cc | 10 ++++--- src/libexpr/eval.hh | 10 +++++-- src/libexpr/parser.y | 60 +++++++++++++++++++++++++++++------------- src/libexpr/primops.cc | 23 ++++++++-------- tests/tarball.sh | 6 +++++ 5 files changed, 75 insertions(+), 34 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 8ce2f3dfa..03d359725 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -278,7 +278,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) /* Initialise the Nix expression search path. */ Strings paths = parseNixPath(getEnv("NIX_PATH", "")); - for (auto & i : _searchPath) addToSearchPath(i, true); + for (auto & i : _searchPath) addToSearchPath(i); for (auto & i : paths) addToSearchPath(i); addToSearchPath("nix=" + settings.nixDataDir + "/nix/corepkgs"); @@ -301,11 +301,15 @@ Path EvalState::checkSourcePath(const Path & path_) if (!restricted) return path_; /* Resolve symlinks. */ + debug(format("checking access to ‘%s’") % path_); Path path = canonPath(path_, true); - for (auto & i : searchPath) - if (path == i.second || isInDir(path, i.second)) + for (auto & i : searchPath) { + auto r = resolveSearchPathElem(i); + if (!r.first) continue; + if (path == r.second || isInDir(path, r.second)) return path; + } /* To support import-from-derivation, allow access to anything in the store. FIXME: only allow access to paths that have been diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 50093d3fc..80e369f2d 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -56,7 +56,8 @@ typedef std::map SrcToStore; std::ostream & operator << (std::ostream & str, const Value & v); -typedef list > SearchPath; +typedef std::pair SearchPathElem; +typedef std::list SearchPath; /* Initialise the Boehm GC, if applicable. */ @@ -98,12 +99,14 @@ private: SearchPath searchPath; + std::map> searchPathResolved; + public: EvalState(const Strings & _searchPath, ref store); ~EvalState(); - void addToSearchPath(const string & s, bool warn = false); + void addToSearchPath(const string & s); Path checkSourcePath(const Path & path); @@ -125,6 +128,9 @@ public: Path findFile(const string & path); Path findFile(SearchPath & searchPath, const string & path, const Pos & pos = noPos); + /* If the specified search path element is a URI, download it. */ + std::pair resolveSearchPathElem(const SearchPathElem & elem); + /* Evaluate an expression to normal form, storing the result in value `v'. */ void eval(Expr * e, Value & v); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 11dc7bb5c..20ae1a696 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -600,7 +600,7 @@ Expr * EvalState::parseExprFromString(const string & s, const Path & basePath) } -void EvalState::addToSearchPath(const string & s, bool warn) +void EvalState::addToSearchPath(const string & s) { size_t pos = s.find('='); string prefix; @@ -612,16 +612,7 @@ void EvalState::addToSearchPath(const string & s, bool warn) path = string(s, pos + 1); } - if (isUri(path)) - path = makeDownloader()->downloadCached(store, path, true); - - path = absPath(path); - if (pathExists(path)) { - debug(format("adding path ‘%1%’ to the search path") % path); - /* Resolve symlinks in the path to support restricted mode. */ - searchPath.push_back(std::pair(prefix, canonPath(path, true))); - } else if (warn) - printMsg(lvlError, format("warning: Nix search path entry ‘%1%’ does not exist, ignoring") % path); + searchPath.emplace_back(prefix, path); } @@ -634,17 +625,19 @@ Path EvalState::findFile(const string & path) Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos & pos) { for (auto & i : searchPath) { - assert(!isUri(i.second)); - Path res; + std::string suffix; if (i.first.empty()) - res = i.second + "/" + path; + suffix = "/" + path; else { - if (path.compare(0, i.first.size(), i.first) != 0 || - (path.size() > i.first.size() && path[i.first.size()] != '/')) + auto s = i.first.size(); + if (path.compare(0, s, i.first) != 0 || + (path.size() > s && path[s] != '/')) continue; - res = i.second + - (path.size() == i.first.size() ? "" : "/" + string(path, i.first.size())); + suffix = path.size() == s ? "" : "/" + string(path, s); } + auto r = resolveSearchPathElem(i); + if (!r.first) continue; + Path res = r.second + suffix; if (pathExists(res)) return canonPath(res); } format f = format( @@ -655,4 +648,35 @@ Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos } +std::pair EvalState::resolveSearchPathElem(const SearchPathElem & elem) +{ + auto i = searchPathResolved.find(elem.second); + if (i != searchPathResolved.end()) return i->second; + + std::pair res; + + if (isUri(elem.second)) { + try { + res = { true, makeDownloader()->downloadCached(store, elem.second, true) }; + } catch (DownloadError & e) { + printMsg(lvlError, format("warning: Nix search path entry ‘%1%’ cannot be downloaded, ignoring") % elem.second); + res = { false, "" }; + } + } else { + auto path = absPath(elem.second); + if (pathExists(path)) + res = { true, path }; + else { + printMsg(lvlError, format("warning: Nix search path entry ‘%1%’ does not exist, ignoring") % elem.second); + res = { false, "" }; + } + } + + debug(format("resolved search path element ‘%s’ to ‘%s’") % elem.second % res.second); + + searchPathResolved[elem.second] = res; + return res; +} + + } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 816827d6a..51680ad62 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -778,7 +778,6 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va SearchPath searchPath; - PathSet context; for (unsigned int n = 0; n < args[0]->listSize(); ++n) { Value & v2(*args[0]->listElems()[n]); state.forceAttrs(v2, pos); @@ -791,21 +790,23 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va i = v2.attrs->find(state.symbols.create("path")); if (i == v2.attrs->end()) throw EvalError(format("attribute ‘path’ missing, at %1%") % pos); - string path = state.coerceToPath(pos, *i->value, context); - searchPath.push_back(std::pair(prefix, state.checkSourcePath(path))); + PathSet context; + string path = state.coerceToString(pos, *i->value, context, false, false); + + try { + state.realiseContext(context); + } catch (InvalidPathError & e) { + throw EvalError(format("cannot find ‘%1%’, since path ‘%2%’ is not valid, at %3%") + % path % e.path % pos); + } + + searchPath.emplace_back(prefix, path); } string path = state.forceStringNoCtx(*args[1], pos); - try { - state.realiseContext(context); - } catch (InvalidPathError & e) { - throw EvalError(format("cannot find ‘%1%’, since path ‘%2%’ is not valid, at %3%") - % path % e.path % pos); - } - - mkPath(v, state.findFile(searchPath, path, pos).c_str()); + mkPath(v, state.checkSourcePath(state.findFile(searchPath, path, pos)).c_str()); } /* Read a directory (without . or ..) */ diff --git a/tests/tarball.sh b/tests/tarball.sh index ae18490c2..254c4b626 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -21,3 +21,9 @@ nix-build -o $TMPDIR/result file://$tarball nix-build -o $TMPDIR/result '' -I foo=file://$tarball nix-build -o $TMPDIR/result -E "import (fetchTarball file://$tarball)" + +nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar.xz +nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball.tar.xz +(! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball.tar.xz) + +nix-instantiate --eval -E '' -I fnord=file://no-such-tarball.tar.xz -I fnord=.