From b4c6adfd35b4e6096268954590804bb26a0a12dc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 10 May 2022 16:12:48 +0200 Subject: [PATCH] Fix findFile(), coerceToPath() --- src/libcmd/common-eval-args.cc | 11 +++-- src/libcmd/common-eval-args.hh | 3 +- src/libcmd/installables.cc | 5 +- src/libexpr/eval.cc | 66 ++++++++++++++++---------- src/libexpr/eval.hh | 20 ++++---- src/libexpr/flake/flake.cc | 2 +- src/libexpr/parser.y | 38 +++++++-------- src/libexpr/primops.cc | 7 +-- src/nix-build/nix-build.cc | 5 +- src/nix-env/nix-env.cc | 4 +- src/nix-instantiate/nix-instantiate.cc | 7 ++- src/nix/prefetch.cc | 2 +- src/nix/repl.cc | 2 +- 13 files changed, 92 insertions(+), 80 deletions(-) diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 5b6e82388..40435f710 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -89,17 +89,18 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) return res.finish(); } -Path lookupFileArg(EvalState & state, std::string_view s) +SourcePath lookupFileArg(EvalState & state, std::string_view s) { if (isUri(s)) { - return state.store->toRealPath( - fetchers::downloadTarball( - state.store, resolveUri(s), "source", false).first.storePath); + auto storePath = fetchers::downloadTarball( + state.store, resolveUri(s), "source", false).first.storePath; + auto & accessor = state.registerAccessor(makeFSInputAccessor(state.store->toRealPath(storePath))); + return {accessor, "/"}; } else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') { Path p(s.substr(1, s.size() - 2)); return state.findFile(p); } else - return absPath(std::string(s)); + return state.rootPath(absPath(std::string(s))); } } diff --git a/src/libcmd/common-eval-args.hh b/src/libcmd/common-eval-args.hh index 03fa226aa..5af1e7f44 100644 --- a/src/libcmd/common-eval-args.hh +++ b/src/libcmd/common-eval-args.hh @@ -7,6 +7,7 @@ namespace nix { class Store; class EvalState; class Bindings; +struct SourcePath; struct MixEvalArgs : virtual Args { @@ -22,6 +23,6 @@ private: std::map autoArgs; }; -Path lookupFileArg(EvalState & state, std::string_view s); +SourcePath lookupFileArg(EvalState & state, std::string_view s); } diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index fc71ed156..e8a223add 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -210,8 +210,7 @@ void SourceExprCommand::completeInstallable(std::string_view prefix) Expr *e = state->parseExprFromFile( resolveExprPath( - state->rootPath( - lookupFileArg(*state, *file)))); + lookupFileArg(*state, *file))); Value root; state->eval(e, root); @@ -762,7 +761,7 @@ std::vector> SourceExprCommand::parseInstallables( state->eval(e, *vFile); } else if (file) - state->evalFile(state->rootPath(lookupFileArg(*state, *file)), *vFile); + state->evalFile(lookupFileArg(*state, *file), *vFile); else { auto e = state->parseExprFromString(*expr, absPath(".")); state->eval(e, *vFile); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 33ae0aa60..99f006900 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -485,22 +485,22 @@ EvalState::EvalState( if (rootFS->hasAccessControl()) { for (auto & i : searchPath) { - auto r = resolveSearchPathElem(i); - if (!r.first) continue; - - auto path = r.second; - - if (store->isInStore(r.second)) { - try { - StorePathSet closure; - store->computeFSClosure(store->toStorePath(r.second).first, closure); - for (auto & path : closure) - allowPath(path); - } catch (InvalidPath &) { - allowPath(r.second); - } - } else - allowPath(r.second); + if (auto path = resolveSearchPathElem(i)) { + // FIXME + #if 0 + if (store->isInStore(*path)) { + try { + StorePathSet closure; + store->computeFSClosure(store->toStorePath(*path).first, closure); + for (auto & p : closure) + allowPath(p); + } catch (InvalidPath &) { + allowPath(*r); + } + } else + allowPath(*r); + #endif + } } } @@ -1812,10 +1812,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) state.throwEvalError(i_pos, "cannot add %1% to a float", showType(vTmp)); } else { if (s.empty()) s.reserve(es->size()); - /* Skip canonization of first path, which would only be - non-canonical in the first place if it's coming from a - ./${foo} type path. */ - auto part = state.coerceToString(i_pos, vTmp, context, false, firstType == nString, !first); + auto part = state.coerceToString(i_pos, vTmp, context, false, firstType == nString); sSize += part->size(); s.emplace_back(std::move(part)); } @@ -2017,7 +2014,7 @@ std::optional EvalState::tryAttrsToString(const PosIdx pos, Value & } BackedStringView EvalState::coerceToString(const PosIdx pos, Value & v, PathSet & context, - bool coerceMore, bool copyToStore, bool canonicalizePath) + bool coerceMore, bool copyToStore) { forceValue(v, pos); @@ -2105,11 +2102,28 @@ StorePath EvalState::copyPathToStore(PathSet & context, const SourcePath & path) SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, PathSet & context) { - auto path = coerceToString(pos, v, context, false, false).toOwned(); - if (path == "" || path[0] != '/') - throwEvalError(pos, "string '%1%' doesn't represent an absolute path", path); - // FIXME - return rootPath(path); + forceValue(v, pos); + + if (v.type() == nString) { + copyContext(v, context); + return {*rootFS, v.string.s}; + } + + if (v.type() == nPath) + return v.path(); + + #if 0 + if (v.type() == nAttrs) { + auto maybeString = tryAttrsToString(pos, v, context, coerceMore, copyToStore); + if (maybeString) + return std::move(*maybeString); + auto i = v.attrs->find(sOutPath); + if (i == v.attrs->end()) throwTypeError(pos, "cannot coerce a set to a string"); + return coerceToString(pos, *i->value, context, coerceMore, copyToStore); + } + #endif + + throwTypeError(pos, "cannot coerce %1% to a path", v); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index c007a235b..7e312ba31 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -50,14 +50,15 @@ struct Env void copyContext(const Value & v, PathSet & context); -std::ostream & printValue(const EvalState & state, std::ostream & str, const Value & v); -std::string printValue(const EvalState & state, const Value & v); - - +// FIXME: maybe change this to an std::variant. typedef std::pair SearchPathElem; typedef std::list SearchPath; +std::ostream & printValue(const EvalState & state, std::ostream & str, const Value & v); +std::string printValue(const EvalState & state, const Value & v); + + /* Initialise the Boehm GC, if applicable. */ void initGC(); @@ -130,7 +131,7 @@ private: SearchPath searchPath; - std::map> searchPathResolved; + std::map> searchPathResolved; /* Cache used by checkSourcePath(). */ std::unordered_map resolvedPaths; @@ -209,11 +210,11 @@ public: void resetFileCache(); /* Look up a file in the search path. */ - Path findFile(const std::string_view path); - Path findFile(SearchPath & searchPath, const std::string_view path, const PosIdx pos = noPos); + SourcePath findFile(const std::string_view path); + SourcePath findFile(SearchPath & searchPath, const std::string_view path, const PosIdx pos = noPos); /* If the specified search path element is a URI, download it. */ - std::pair resolveSearchPathElem(const SearchPathElem & elem); + std::optional resolveSearchPathElem(const SearchPathElem & elem); /* Evaluate an expression to normal form, storing the result in value `v'. */ @@ -303,8 +304,7 @@ public: booleans and lists to a string. If `copyToStore' is set, referenced paths are copied to the Nix store as a side effect. */ BackedStringView coerceToString(const PosIdx pos, Value & v, PathSet & context, - bool coerceMore = false, bool copyToStore = true, - bool canonicalizePath = true); + bool coerceMore = false, bool copyToStore = true); StorePath copyPathToStore(PathSet & context, const SourcePath & path); diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index a55f5280a..d1a4c0402 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -268,7 +268,7 @@ static Flake readFlake( PathSet emptyContext = {}; flake.config.settings.emplace( state.symbols[setting.name], - state.coerceToString(setting.pos, *setting.value, emptyContext, false, true, true) .toOwned()); + state.coerceToString(setting.pos, *setting.value, emptyContext, false, true).toOwned()); } else if (setting.value->type() == nInt) flake.config.settings.emplace( diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 2bdb3bd96..6ede0d27a 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -759,13 +759,13 @@ void EvalState::addToSearchPath(const std::string & s) } -Path EvalState::findFile(const std::string_view path) +SourcePath EvalState::findFile(const std::string_view path) { return findFile(searchPath, path); } -Path EvalState::findFile(SearchPath & searchPath, const std::string_view path, const PosIdx pos) +SourcePath EvalState::findFile(SearchPath & searchPath, const std::string_view path, const PosIdx pos) { for (auto & i : searchPath) { std::string suffix; @@ -778,15 +778,14 @@ Path EvalState::findFile(SearchPath & searchPath, const std::string_view path, c continue; suffix = path.size() == s ? "" : concatStrings("/", path.substr(s)); } - auto r = resolveSearchPathElem(i); - if (!r.first) continue; - Path res = r.second + suffix; - if (pathExists(res)) return canonPath(res); + if (auto path = resolveSearchPathElem(i)) { + auto res = path->append("/" + suffix); + if (res.pathExists()) return res; + } } if (hasPrefix(path, "nix/")) - abort(); - //return packPath(SourcePath {corepkgsFS, (std::string) path.substr(3)}); + return {*corepkgsFS, (std::string) path.substr(3)}; throw ThrownError({ .msg = hintfmt(evalSettings.pureEval @@ -798,38 +797,39 @@ Path EvalState::findFile(SearchPath & searchPath, const std::string_view path, c } -std::pair EvalState::resolveSearchPathElem(const SearchPathElem & elem) +std::optional EvalState::resolveSearchPathElem(const SearchPathElem & elem) { auto i = searchPathResolved.find(elem.second); if (i != searchPathResolved.end()) return i->second; - std::pair res; + std::optional res; if (isUri(elem.second)) { try { - res = { true, store->toRealPath(fetchers::downloadTarball( - store, resolveUri(elem.second), "source", false).first.storePath) }; + auto storePath = fetchers::downloadTarball( + store, resolveUri(elem.second), "source", false).first.storePath; + auto & accessor = registerAccessor(makeFSInputAccessor(store->toRealPath(storePath))); + res.emplace(SourcePath {accessor, "/"}); } catch (FileTransferError & e) { logWarning({ .msg = hintfmt("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 }; + auto path = rootPath(absPath(elem.second)); + if (path.pathExists()) + res.emplace(path); else { logWarning({ .msg = hintfmt("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); + if (res) + debug("resolved search path element '%s' to '%s'", elem.second, *res); - searchPathResolved[elem.second] = res; + searchPathResolved.emplace(elem.second, res); return res; } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 2f47c54d3..b170b2763 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -113,6 +113,7 @@ static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, co }(); try { + #if 0 if (!context.empty()) { auto rewrites = state.realiseContext(context); // FIXME: check that path.accessor == rootFS? @@ -120,6 +121,7 @@ static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, co // FIXME: return store accessor return state.rootPath(realPath); } else + #endif return path; } catch (Error & e) { e.addTrace(state.positions[pos], "while realising the context of path '%s'", path); @@ -1545,12 +1547,7 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V auto path = state.forceStringNoCtx(*args[1], pos); - #if 0 - // FIXME: checkSourcePath? v.mkPath(state.findFile(searchPath, path, pos)); - #endif - - throw ThrownError("findFile('%s'): not implemented", path); } static RegisterPrimOp primop_findFile(RegisterPrimOp::Info { diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 4c6d696fb..bcd87adad 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -307,9 +307,8 @@ static void main_nix_build(int argc, char * * argv) exprs.push_back( state->parseExprFromFile( resolveExprPath( - state->rootPath( - lookupFileArg(*state, - inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i))))); + lookupFileArg(*state, + inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i)))); } } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 9d56c3339..3a8558c64 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1475,7 +1475,9 @@ static int main_nix_env(int argc, char * * argv) globals.state->repair = repair; if (file != "") - globals.instSource.nixExprPath = lookupFileArg(*globals.state, file); + // FIXME: check that the accessor returned by + // lookupFileArg() is the root FS. + globals.instSource.nixExprPath = lookupFileArg(*globals.state, file).path; globals.instSource.autoArgs = myArgs.getAutoArgs(*globals.state); diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index ce933cf30..614201bbb 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -167,9 +167,8 @@ static int main_nix_instantiate(int argc, char * * argv) if (findFile) { for (auto & i : files) { - Path p = state->findFile(i); - if (p == "") throw Error("unable to find '%1%'", i); - std::cout << p << std::endl; + auto p = state->findFile(i); + std::cout << p.readFile() << std::endl; } return 0; } @@ -184,7 +183,7 @@ static int main_nix_instantiate(int argc, char * * argv) for (auto & i : files) { Expr * e = fromArgs ? state->parseExprFromString(i, absPath(".")) - : state->parseExprFromFile(resolveExprPath(state->rootPath(lookupFileArg(*state, i)))); + : state->parseExprFromFile(resolveExprPath(lookupFileArg(*state, i))); processExpr(*state, attrPaths, parseOnly, strict, autoArgs, evalOnly, outputKind, xmlOutputSourceLocation, e); } diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index 3a1c142c1..73d0cc23e 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -195,7 +195,7 @@ static int main_nix_prefetch_url(int argc, char * * argv) Value vRoot; state->evalFile( resolveExprPath( - state->rootPath(lookupFileArg(*state, args.empty() ? "." : args[0]))), + lookupFileArg(*state, args.empty() ? "." : args[0])), vRoot); Value & v(*findAlongAttrPath(*state, attrPath, autoArgs, vRoot).first); state->forceAttrs(v, noPos); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 4a02865ac..9dcc9b251 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -636,7 +636,7 @@ void NixRepl::loadFile(const Path & path) loadedFiles.remove(path); loadedFiles.push_back(path); Value v, v2; - state->evalFile(state->rootPath(lookupFileArg(*state, path)), v); + state->evalFile(lookupFileArg(*state, path), v); state->autoCallFunction(*autoArgs, v, v2); addAttrsToScope(v2); }