From 444af855523b00b0959273bab9d7343312817cb3 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Wed, 5 Oct 2022 12:09:57 +0200 Subject: [PATCH 1/3] Temporarily disable the debugger during completion evaluation readline is not re-entrant, so entering the debugger from the completioncallback results in an eventual segfault. The workaround is to temporarily disable the debugger when searching for possible completions. --- src/libcmd/repl.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index df8932087..bb254ff8d 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -384,6 +384,10 @@ StringSet NixRepl::completePrefix(const std::string & prefix) i++; } } else { + /* Temporarily disable the debugger, to avoid re-entering readline. */ + auto debug_repl = state->debugRepl; + state->debugRepl = nullptr; + Finally restoreDebug([&]() { state->debugRepl = debug_repl; }); try { /* This is an expression that should evaluate to an attribute set. Evaluate it to get the names of the From 16f1720fd2ac1c74043492e71a3e0a3327db4919 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Thu, 3 Nov 2022 10:11:28 +0100 Subject: [PATCH 2/3] SourceExprCommand: swallow EvalError, add tests for this Completing things that would error would print an ugly error in the middle of your command line. Avoid printing this error. --- src/libcmd/installables.cc | 85 ++++++++++++++++++++------------------ tests/completions.sh | 6 +++ 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index e097f23b3..e8836c247 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -207,55 +207,60 @@ Strings SourceExprCommand::getDefaultFlakeAttrPathPrefixes() void SourceExprCommand::completeInstallable(std::string_view prefix) { - if (file) { - completionType = ctAttrs; + try { + if (file) { + completionType = ctAttrs; - evalSettings.pureEval = false; - auto state = getEvalState(); - Expr *e = state->parseExprFromFile( - resolveExprPath(state->checkSourcePath(lookupFileArg(*state, *file))) - ); + evalSettings.pureEval = false; + auto state = getEvalState(); + Expr *e = state->parseExprFromFile( + resolveExprPath(state->checkSourcePath(lookupFileArg(*state, *file))) + ); - Value root; - state->eval(e, root); + Value root; + state->eval(e, root); - auto autoArgs = getAutoArgs(*state); + auto autoArgs = getAutoArgs(*state); - std::string prefix_ = std::string(prefix); - auto sep = prefix_.rfind('.'); - std::string searchWord; - if (sep != std::string::npos) { - searchWord = prefix_.substr(sep + 1, std::string::npos); - prefix_ = prefix_.substr(0, sep); - } else { - searchWord = prefix_; - prefix_ = ""; - } + std::string prefix_ = std::string(prefix); + auto sep = prefix_.rfind('.'); + std::string searchWord; + if (sep != std::string::npos) { + searchWord = prefix_.substr(sep + 1, std::string::npos); + prefix_ = prefix_.substr(0, sep); + } else { + searchWord = prefix_; + prefix_ = ""; + } - auto [v, pos] = findAlongAttrPath(*state, prefix_, *autoArgs, root); - Value &v1(*v); - state->forceValue(v1, pos); - Value v2; - state->autoCallFunction(*autoArgs, v1, v2); + auto [v, pos] = findAlongAttrPath(*state, prefix_, *autoArgs, root); + Value &v1(*v); + state->forceValue(v1, pos); + Value v2; + state->autoCallFunction(*autoArgs, v1, v2); - if (v2.type() == nAttrs) { - for (auto & i : *v2.attrs) { - std::string name = state->symbols[i.name]; - if (name.find(searchWord) == 0) { - if (prefix_ == "") - completions->add(name); - else - completions->add(prefix_ + "." + name); + if (v2.type() == nAttrs) { + for (auto & i : *v2.attrs) { + std::string name = state->symbols[i.name]; + if (name.find(searchWord) == 0) { + if (prefix_ == "") + completions->add(name); + else + completions->add(prefix_ + "." + name); + } } } + } else { + completeFlakeRefWithFragment( + getEvalState(), + lockFlags, + getDefaultFlakeAttrPathPrefixes(), + getDefaultFlakeAttrPaths(), + prefix); } - } else { - completeFlakeRefWithFragment( - getEvalState(), - lockFlags, - getDefaultFlakeAttrPathPrefixes(), - getDefaultFlakeAttrPaths(), - prefix); + } catch (EvalError& e) { + // swallow eval error + (void)e; } } diff --git a/tests/completions.sh b/tests/completions.sh index 522aa1c86..19dc61098 100644 --- a/tests/completions.sh +++ b/tests/completions.sh @@ -28,6 +28,10 @@ cat < bar/flake.nix }; } EOF +mkdir -p err +cat < err/flake.nix +throw "error" +EOF # Test the completion of a subcommand [[ "$(NIX_GET_COMPLETIONS=1 nix buil)" == $'normal\nbuild\t' ]] @@ -60,3 +64,5 @@ NIX_GET_COMPLETIONS=3 nix build --option allow-import-from | grep -- "allow-impo # Attr path completions [[ "$(NIX_GET_COMPLETIONS=2 nix eval ./foo\#sam)" == $'attrs\n./foo#sampleOutput\t' ]] [[ "$(NIX_GET_COMPLETIONS=4 nix eval --file ./foo/flake.nix outp)" == $'attrs\noutputs\t' ]] +[[ "$(NIX_GET_COMPLETIONS=4 nix eval --file ./err/flake.nix outp 2>&1)" == $'attrs' ]] +[[ "$(NIX_GET_COMPLETIONS=2 nix eval ./err\# 2>&1)" == $'attrs' ]] From 60dea270d0bc430930f5560ebac71a2dc0ab2b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= <7226587+thufschmitt@users.noreply.github.com> Date: Wed, 16 Nov 2022 10:34:32 +0100 Subject: [PATCH 3/3] Swallow the error in a more idiomatic way --- src/libcmd/installables.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index e8836c247..f63b9eeae 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -258,9 +258,8 @@ void SourceExprCommand::completeInstallable(std::string_view prefix) getDefaultFlakeAttrPaths(), prefix); } - } catch (EvalError& e) { - // swallow eval error - (void)e; + } catch (EvalError&) { + // Don't want eval errors to mess-up with the completion engine, so let's just swallow them } }