From 8a90195cdbf545442afa4b548574f139aa742310 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 25 Mar 2025 20:14:45 +0100 Subject: [PATCH] libexpr: don't debugThrow from async code running a debugger is never-async since it holds an EvalState reference. fixes #761 Change-Id: Ie1b4df6f81bb5614f11ea183eb9d1058fde73121 --- lix/legacy/nix-build.cc | 2 +- lix/legacy/nix-env.cc | 2 +- lix/legacy/nix-instantiate.cc | 11 ++++----- lix/libcmd/common-eval-args.cc | 12 ++++++---- lix/libcmd/common-eval-args.hh | 4 +++- lix/libcmd/installables.cc | 8 +++---- lix/libcmd/repl.cc | 3 ++- lix/libexpr/eval.cc | 20 +++++++++------- lix/libexpr/eval.hh | 32 ++++++++++++++++++++++--- lix/libexpr/primops.cc | 2 +- lix/libexpr/value-to-json.cc | 2 +- lix/nix/prefetch.cc | 2 +- subprojects/nix-eval-jobs/src/worker.cc | 5 +++- tests/unit/libcmd/args.cc | 11 +++++---- 14 files changed, 77 insertions(+), 39 deletions(-) diff --git a/lix/legacy/nix-build.cc b/lix/legacy/nix-build.cc index 2e8a0767e..86c4795dc 100644 --- a/lix/legacy/nix-build.cc +++ b/lix/legacy/nix-build.cc @@ -255,7 +255,7 @@ static void main_nix_build(AsyncIoRoot & aio, std::string programName, Strings a evaluator->paths.resolveExprPath(aio.blockOn(lookupFileArg( *evaluator, inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i - ))) + )).unwrap()) )); } } diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index 1a9f72877..03776a2f1 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -1558,7 +1558,7 @@ static int main_nix_env(AsyncIoRoot & aio, std::string programName, Strings argv globals.instSource.nixExprPath = std::make_shared( file != "" - ? aio.blockOn(lookupFileArg(*globals.state, file)) + ? aio.blockOn(lookupFileArg(*globals.state, file)).unwrap() : CanonPath(nixExprPath)); globals.instSource.autoArgs = myArgs.getAutoArgs(*globals.state); diff --git a/lix/legacy/nix-instantiate.cc b/lix/legacy/nix-instantiate.cc index 90c485bdf..4983e499a 100644 --- a/lix/legacy/nix-instantiate.cc +++ b/lix/legacy/nix-instantiate.cc @@ -167,7 +167,7 @@ static int main_nix_instantiate(AsyncIoRoot & aio, std::string programName, Stri if (findFile) { for (auto & i : files) { - auto p = aio.blockOn(evaluator->paths.findFile(i)); + auto p = aio.blockOn(evaluator->paths.findFile(i)).unwrap(); std::cout << p.canonical().abs() << std::endl; } return 0; @@ -181,11 +181,10 @@ static int main_nix_instantiate(AsyncIoRoot & aio, std::string programName, Stri files.push_back("./default.nix"); for (auto & i : files) { - Expr & e = fromArgs - ? evaluator->parseExprFromString(i, CanonPath::fromCwd()) - : evaluator->parseExprFromFile( - evaluator->paths.resolveExprPath(aio.blockOn(lookupFileArg(*evaluator, i))) - ); + Expr & e = fromArgs ? evaluator->parseExprFromString(i, CanonPath::fromCwd()) + : evaluator->parseExprFromFile(evaluator->paths.resolveExprPath( + aio.blockOn(lookupFileArg(*evaluator, i)).unwrap() + )); processExpr(*state, attrPaths, parseOnly, strict, autoArgs, evalOnly, outputKind, xmlOutputSourceLocation, e); } diff --git a/lix/libcmd/common-eval-args.cc b/lix/libcmd/common-eval-args.cc index 2204e104a..39c3a1423 100644 --- a/lix/libcmd/common-eval-args.cc +++ b/lix/libcmd/common-eval-args.cc @@ -188,7 +188,8 @@ Bindings * MixEvalArgs::getAutoArgs(Evaluator & state) return res.finish(); } -kj::Promise> lookupFileArg(Evaluator & state, std::string_view fileArg) +kj::Promise>> +lookupFileArg(Evaluator & state, std::string_view fileArg) try { if (EvalSettings::isPseudoUrl(fileArg)) { auto const url = EvalSettings::resolvePseudoUrl(fileArg); @@ -199,19 +200,20 @@ try { /* locked */ false )); StorePath const storePath = downloaded.tree.storePath; - co_return CanonPath(state.store->toRealPath(storePath)); + co_return SourcePath(CanonPath(state.store->toRealPath(storePath))); } else if (fileArg.starts_with("flake:")) { experimentalFeatureSettings.require(Xp::Flakes); static constexpr size_t FLAKE_LEN = std::string_view("flake:").size(); auto flakeRef = parseFlakeRef(std::string(fileArg.substr(FLAKE_LEN)), {}, true, false); auto storePath = TRY_AWAIT(TRY_AWAIT(flakeRef.resolve(state.store)).fetchTree(state.store)) .first.storePath; - co_return CanonPath(state.store->toRealPath(storePath)); - } else if (fileArg.size() > 2 && fileArg.at(0) == '<' && fileArg.at(fileArg.size() - 1) == '>') { + co_return SourcePath(CanonPath(state.store->toRealPath(storePath))); + } else if (fileArg.size() > 2 && fileArg.at(0) == '<' && fileArg.at(fileArg.size() - 1) == '>') + { Path p(fileArg.substr(1, fileArg.size() - 2)); co_return TRY_AWAIT(state.paths.findFile(p)); } else { - co_return CanonPath::fromCwd(fileArg); + co_return SourcePath(CanonPath::fromCwd(fileArg)); } } catch (...) { co_return result::current_exception(); diff --git a/lix/libcmd/common-eval-args.hh b/lix/libcmd/common-eval-args.hh index cb7781769..5403dafc1 100644 --- a/lix/libcmd/common-eval-args.hh +++ b/lix/libcmd/common-eval-args.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "lix/libexpr/eval-error.hh" #include "lix/libexpr/eval.hh" #include "lix/libutil/args.hh" #include "lix/libmain/common-args.hh" @@ -49,6 +50,7 @@ private: * * @exception nix::ThrownError for failed search path lookup. Probably others. */ -kj::Promise> lookupFileArg(Evaluator & state, std::string_view fileArg); +kj::Promise>> +lookupFileArg(Evaluator & state, std::string_view fileArg); } diff --git a/lix/libcmd/installables.cc b/lix/libcmd/installables.cc index 5111a48ba..e2fb2ea57 100644 --- a/lix/libcmd/installables.cc +++ b/lix/libcmd/installables.cc @@ -215,9 +215,9 @@ void SourceExprCommand::completeInstallable(EvalState & state, AddCompletions & auto evaluator = getEvaluator(); - Expr & e = evaluator->parseExprFromFile( - state.ctx.paths.resolveExprPath(state.aio.blockOn(lookupFileArg(*evaluator, *file))) - ); + Expr & e = evaluator->parseExprFromFile(state.ctx.paths.resolveExprPath( + state.aio.blockOn(lookupFileArg(*evaluator, *file)).unwrap() + )); Value root; state.eval(e, root); @@ -457,7 +457,7 @@ Installables SourceExprCommand::parseInstallables( state.eval(e, *vFile); } else if (file) - state.evalFile(state.aio.blockOn(lookupFileArg(*evaluator, *file)), *vFile); + state.evalFile(state.aio.blockOn(lookupFileArg(*evaluator, *file)).unwrap(), *vFile); else { auto & e = evaluator->parseExprFromString(*expr, CanonPath::fromCwd()); state.eval(e, *vFile); diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 899fdc4a8..d79f6b46b 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -34,6 +34,7 @@ #include "lix/libutil/signals.hh" #include "lix/libexpr/print.hh" #include "lix/libexpr/gc-small-vector.hh" +#include "lix/libutil/types.hh" #include "lix/libutil/users.hh" #if HAVE_BOEHMGC @@ -899,7 +900,7 @@ void NixRepl::loadFile(const Path & path) loadedFiles.remove(path); loadedFiles.push_back(path); Value v, v2; - state.evalFile(state.aio.blockOn(lookupFileArg(evaluator, path)), v); + state.evalFile(state.aio.blockOn(lookupFileArg(evaluator, path)).unwrap(always_progresses), v); state.autoCallFunction(*autoArgs, v, v2); addAttrsToScope(v2); } diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 311f31a78..95ba6023f 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -2322,7 +2322,7 @@ BackedStringView EvalState::coerceToString( v._path : copyToStore ? ctx.store->printStorePath( - aio.blockOn(ctx.paths.copyPathToStore(context, v.path(), ctx.repair))) + aio.blockOn(ctx.paths.copyPathToStore(context, v.path(), ctx.repair)).unwrap()) : v.path().to_string(); } @@ -2391,10 +2391,11 @@ BackedStringView EvalState::coerceToString( } -kj::Promise> EvalPaths::copyPathToStore(NixStringContext & context, const SourcePath & path, RepairFlag repair) +kj::Promise>> +EvalPaths::copyPathToStore(NixStringContext & context, const SourcePath & path, RepairFlag repair) try { if (nix::isDerivation(path.canonical().abs())) - errors.make("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); + co_return errors.make("file names are not allowed to end in '%1%'", drvExtension); auto i = srcToStore.find(path); @@ -2792,13 +2793,14 @@ Expr & Evaluator::parseStdin() } -kj::Promise> EvalPaths::findFile(const std::string_view path) +kj::Promise>> +EvalPaths::findFile(const std::string_view path) { return findFile(searchPath_, path); } -kj::Promise> +kj::Promise>> EvalPaths::findFile(const SearchPath & searchPath, const std::string_view path, const PosIdx pos) try { for (auto & i : searchPath.elements) { @@ -2812,18 +2814,18 @@ try { auto r = *rOpt; Path res = suffix == "" ? r : concatStrings(r, "/", suffix); - if (pathExists(res)) co_return CanonPath(canonPath(res)); + if (pathExists(res)) co_return SourcePath(CanonPath(canonPath(res))); } if (path.starts_with("nix/")) - co_return CanonPath(concatStrings(corepkgsPrefix, path.substr(4))); + co_return SourcePath(CanonPath(concatStrings(corepkgsPrefix, path.substr(4)))); - errors.make( + co_return errors.make( evalSettings.pureEval ? "cannot look up '<%s>' in pure evaluation mode (use '--impure' to override)" : "file '%s' was not found in the Nix search path (add it using $NIX_PATH or -I)", path - ).atPos(pos).debugThrow(); + ).atPos(pos); } catch (...) { co_return result::current_exception(); } diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index a464ac05a..de8d715dd 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -451,11 +451,37 @@ public: */ Path toRealPath(const Path & path, const NixStringContext & context); + /** + * findFile wants to throw a debuggable error when the requested file + * is not found, but it can't invoke the debugger itself because it's + * async code. This wraps the result-or-error to allow it regardless. + * This happens for copyPathToStore as well, with another error type. + */ + template + struct PathResult : private std::variant> + { + PathResult(T p) : std::variant>(std::move(p)) {} + PathResult(EvalErrorBuilder e) : std::variant>(std::move(e)) {} + + T unwrap(NeverAsync = {}) && + { + return std::visit( + overloaded{ + [](T & p) -> T { return std::move(p); }, + [](EvalErrorBuilder & e) -> T { + std::move(e).debugThrow(); + } + }, + static_cast> &>(*this) + ); + } + }; + /** * Look up a file in the search path. */ - kj::Promise> findFile(const std::string_view path); - kj::Promise> + kj::Promise>> findFile(const std::string_view path); + kj::Promise>> findFile(const SearchPath & searchPath, const std::string_view path, const PosIdx pos = noPos); /** @@ -468,7 +494,7 @@ public: kj::Promise>> resolveSearchPathPath(const SearchPath::Path & path); - kj::Promise> copyPathToStore( + kj::Promise>> copyPathToStore( NixStringContext & context, const SourcePath & path, RepairFlag repair = NoRepair ); diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 98f3e70ef..be6b7baf1 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -1339,7 +1339,7 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V auto path = state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument passed to builtins.findFile"); v.mkPath(state.ctx.paths.checkSourcePath( - state.aio.blockOn(state.ctx.paths.findFile(searchPath, path, pos)) + state.aio.blockOn(state.ctx.paths.findFile(searchPath, path, pos)).unwrap() )); } diff --git a/lix/libexpr/value-to-json.cc b/lix/libexpr/value-to-json.cc index e996032a0..85187641a 100644 --- a/lix/libexpr/value-to-json.cc +++ b/lix/libexpr/value-to-json.cc @@ -37,7 +37,7 @@ JSON printValueAsJSON(EvalState & state, bool strict, if (copyToStore) out = state.ctx.store->printStorePath(state.aio.blockOn( state.ctx.paths.copyPathToStore(context, v.path(), state.ctx.repair) - )); + ).unwrap()); else { out = v.path().to_string(); } diff --git a/lix/nix/prefetch.cc b/lix/nix/prefetch.cc index 9a429122c..6d0dbdb79 100644 --- a/lix/nix/prefetch.cc +++ b/lix/nix/prefetch.cc @@ -204,7 +204,7 @@ static int main_nix_prefetch_url(AsyncIoRoot & aio, std::string programName, Str Value vRoot; state->evalFile( evaluator->paths.resolveExprPath( - aio.blockOn(lookupFileArg(*evaluator, args.empty() ? "." : args[0]))), + aio.blockOn(lookupFileArg(*evaluator, args.empty() ? "." : args[0])).unwrap()), vRoot); Value & v(*findAlongAttrPath(*state, attrPath, autoArgs, vRoot).first); state->forceAttrs(v, noPos, "while evaluating the source attribute to prefetch"); diff --git a/subprojects/nix-eval-jobs/src/worker.cc b/subprojects/nix-eval-jobs/src/worker.cc index 3ca2fb841..feaef7955 100644 --- a/subprojects/nix-eval-jobs/src/worker.cc +++ b/subprojects/nix-eval-jobs/src/worker.cc @@ -51,7 +51,10 @@ static nix::Value *releaseExprTopLevelValue(nix::EvalState &state, nix::CanonPath::fromCwd()); state.eval(e, vTop); } else { - state.evalFile(state.aio.blockOn(nix::lookupFileArg(state.ctx, args.releaseExpr)), vTop); + state.evalFile( + state.aio.blockOn(nix::lookupFileArg(state.ctx, args.releaseExpr)) + .unwrap(), + vTop); } auto vRoot = state.ctx.mem.allocValue(); diff --git a/tests/unit/libcmd/args.cc b/tests/unit/libcmd/args.cc index 5d8dc2ce7..3fd98cdc4 100644 --- a/tests/unit/libcmd/args.cc +++ b/tests/unit/libcmd/args.cc @@ -35,14 +35,16 @@ TEST(Arguments, lookupFileArg) { auto store = aio.blockOn(openStore("dummy://")); auto state = std::make_shared(aio, searchPath, store, store); - SourcePath const foundUnitData = aio.blockOn(lookupFileArg(*state, "")); + SourcePath const foundUnitData = + aio.blockOn(lookupFileArg(*state, "")).unwrap(always_progresses); EXPECT_EQ(foundUnitData.canonical(), canonDataPath); // lookupFileArg should not resolve if anything else is before or after it. - SourcePath const yepEvenSpaces = aio.blockOn(lookupFileArg(*state, " ")); + SourcePath const yepEvenSpaces = + aio.blockOn(lookupFileArg(*state, " ")).unwrap(always_progresses); EXPECT_EQ(yepEvenSpaces.canonical(), CanonPath::fromCwd(" ")); EXPECT_EQ( - aio.blockOn(lookupFileArg(*state, "/nixos")).canonical(), + aio.blockOn(lookupFileArg(*state, "/nixos")).unwrap(always_progresses).canonical(), CanonPath::fromCwd("/nixos") ); @@ -53,7 +55,8 @@ TEST(Arguments, lookupFileArg) { EXPECT_NE(msg.find(CHANNEL_URL), msg.npos); } - SourcePath const normalFile = aio.blockOn(lookupFileArg(*state, unitDataPath)); + SourcePath const normalFile = + aio.blockOn(lookupFileArg(*state, unitDataPath)).unwrap(always_progresses); EXPECT_EQ(normalFile.canonical(), canonDataPath); }