From 2b4e3f04a433d96e779fb91fcd9fa01329413d02 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 23 Jul 2024 16:57:45 +0200 Subject: [PATCH 01/11] markdown.cc: Format Slightly custom because the automated formatting messes up the braced initializer with named fields. --- src/libcmd/markdown.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libcmd/markdown.cc b/src/libcmd/markdown.cc index 88c3f640bad..882f95faa11 100644 --- a/src/libcmd/markdown.cc +++ b/src/libcmd/markdown.cc @@ -4,8 +4,8 @@ #include "terminal.hh" #if HAVE_LOWDOWN -# include -# include +# include +# include #endif namespace nix { @@ -15,7 +15,8 @@ std::string renderMarkdownToTerminal(std::string_view markdown) #if HAVE_LOWDOWN int windowWidth = getWindowSize().second; - struct lowdown_opts opts { + struct lowdown_opts opts + { .type = LOWDOWN_TERM, .maxdepth = 20, .cols = (size_t) std::max(windowWidth - 5, 60), From 97b0114ab84144e3574c74c7fb0e0e38681f3b6a Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 23 Jul 2024 16:54:04 +0200 Subject: [PATCH 02/11] renderMarkdownToTerminal: Add _NIX_TEST_RAW_MARKDOWN env var For testing only. --- src/libcmd/markdown.cc | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libcmd/markdown.cc b/src/libcmd/markdown.cc index 882f95faa11..6a0d05d9fba 100644 --- a/src/libcmd/markdown.cc +++ b/src/libcmd/markdown.cc @@ -1,5 +1,6 @@ #include "markdown.hh" -#include "util.hh" +#include "environment-variables.hh" +#include "error.hh" #include "finally.hh" #include "terminal.hh" @@ -10,9 +11,9 @@ namespace nix { -std::string renderMarkdownToTerminal(std::string_view markdown) -{ #if HAVE_LOWDOWN +static std::string doRenderMarkdownToTerminal(std::string_view markdown) +{ int windowWidth = getWindowSize().second; struct lowdown_opts opts @@ -52,9 +53,21 @@ std::string renderMarkdownToTerminal(std::string_view markdown) throw Error("allocation error while rendering Markdown"); return filterANSIEscapes(std::string(buf->data, buf->size), !isTTY()); +} + +std::string renderMarkdownToTerminal(std::string_view markdown) +{ + if (auto e = getEnv("_NIX_TEST_RAW_MARKDOWN"); e && *e == "1") + return std::string(markdown); + else + return doRenderMarkdownToTerminal(markdown); +} + #else +std::string renderMarkdownToTerminal(std::string_view markdown) +{ return std::string(markdown); -#endif } +#endif -} +} // namespace nix From 712ce2feacdc0cde7249ae6da5f8063858e80ae2 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 23 Jul 2024 17:50:51 +0200 Subject: [PATCH 03/11] ReadlineLikeInteracter::getLine: Add _NIX_TEST_REPL_ECHO env var ... for testing --- src/libcmd/repl-interacter.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc index b285c8a9ae5..a12e7d8c799 100644 --- a/src/libcmd/repl-interacter.cc +++ b/src/libcmd/repl-interacter.cc @@ -19,6 +19,7 @@ extern "C" { #include "repl-interacter.hh" #include "file-system.hh" #include "repl.hh" +#include "environment-variables.hh" namespace nix { @@ -179,6 +180,19 @@ bool ReadlineLikeInteracter::getLine(std::string & input, ReplPromptType promptT return false; input += s; input += '\n'; + +#ifndef USE_READLINE + // editline doesn't echo the input to the output when non-interactive, unlike readline + // this results in a different behavior when running tests. The echoing is + // quite useful for reading the test output, so we add it here. + if (auto e = getEnv("_NIX_TEST_REPL_ECHO"); s && e && *e == "1") + { + // This is probably not right for multi-line input, but we don't use that + // in the characterisation tests, so it's fine. + std::cout << "nix-repl> " << s << std::endl; + } +#endif + return true; } From ca2cc26e12d7264fb141206e0bc35ade2378b8f8 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 23 Jul 2024 17:52:28 +0200 Subject: [PATCH 04/11] tests/functional/repl: Improve precision and readability ... as well as match buildReadlineNoMarkdown. Unfortunately it doesn't support long inputs or multiline inputs for now. This needs to make better use of the interacter interface. --- src/libcmd/repl-interacter.cc | 16 ++--- tests/functional/repl.sh | 11 ++++ .../repl/doc-comment-curried-args.expected | 26 ++++---- .../repl/doc-comment-curried-args.in | 2 +- .../repl/doc-comment-formals.expected | 13 ++-- .../repl/doc-comment-function.expected | 7 ++- tests/functional/repl/doc-compact.expected | 12 ++-- tests/functional/repl/doc-constant.expected | 60 +++++++++++-------- tests/functional/repl/doc-floatedIn.expected | 12 ++-- .../repl/doc-lambda-flavors.expected | 36 +++++------ .../functional/repl/doc-measurement.expected | 12 ++-- tests/functional/repl/doc-multiply.expected | 18 +++--- .../functional/repl/doc-unambiguous.expected | 12 ++-- .../repl/pretty-print-idempotent.expected | 12 +++- 14 files changed, 152 insertions(+), 97 deletions(-) diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc index a12e7d8c799..76fe38780b6 100644 --- a/src/libcmd/repl-interacter.cc +++ b/src/libcmd/repl-interacter.cc @@ -176,22 +176,22 @@ bool ReadlineLikeInteracter::getLine(std::string & input, ReplPromptType promptT return true; } - if (!s) - return false; - input += s; - input += '\n'; - -#ifndef USE_READLINE // editline doesn't echo the input to the output when non-interactive, unlike readline // this results in a different behavior when running tests. The echoing is // quite useful for reading the test output, so we add it here. if (auto e = getEnv("_NIX_TEST_REPL_ECHO"); s && e && *e == "1") { +#ifndef USE_READLINE // This is probably not right for multi-line input, but we don't use that // in the characterisation tests, so it's fine. - std::cout << "nix-repl> " << s << std::endl; - } + std::cout << promptForType(promptType) << s << std::endl; #endif + } + + if (!s) + return false; + input += s; + input += '\n'; return true; } diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index 4f5bb36aae9..a6c075abd84 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -262,6 +262,14 @@ badExitCode=0 nixVersion="$(nix eval --impure --raw --expr 'builtins.nixVersion' --extra-experimental-features nix-command)" +# I couldn't get readline and editline to agree on the newline before the prompt, +# so let's just force it to be one empty line. Ideally we get the two to agree +# or use a simpler interacter for testing. +stripEmptyLinesBeforePrompt() { + # --null-data: treat input as NUL-terminated instead of newline-terminated + sed --null-data 's/\n\n*nix-repl>/\n\nnix-repl>/g' +} + runRepl () { # That is right, we are also filtering out the testdir _without underscores_. @@ -273,8 +281,11 @@ runRepl () { testDirNoUnderscores="${testDir//_/}" # TODO: pass arguments to nix repl; see lang.sh + _NIX_TEST_RAW_MARKDOWN=1 \ + _NIX_TEST_REPL_ECHO=1 \ nix repl 2>&1 \ | stripColors \ + | stripEmptyLinesBeforePrompt \ | sed \ -e "s@$testDir@/path/to/tests/functional@g" \ -e "s@$testDirNoUnderscores@/path/to/tests/functional@g" \ diff --git a/tests/functional/repl/doc-comment-curried-args.expected b/tests/functional/repl/doc-comment-curried-args.expected index c10c171e1e8..f97f593c667 100644 --- a/tests/functional/repl/doc-comment-curried-args.expected +++ b/tests/functional/repl/doc-comment-curried-args.expected @@ -1,24 +1,30 @@ Nix Type :? for help. + +nix-repl> :l doc-comments.nix Added variables. -Function curriedArgs - … defined at - /path/to/tests/functional/repl/doc-comments.nix:48:5 +nix-repl> :doc curriedArgs +Function `curriedArgs`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:48:5 - A documented function. +A documented function. +nix-repl> x = curriedArgs 1 +nix-repl> "Note that users may not expect this to behave as it currently does" "Note that users may not expect this to behave as it currently does" -Function curriedArgs - … defined at - /path/to/tests/functional/repl/doc-comments.nix:50:5 +nix-repl> :doc x +Function `curriedArgs`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:50:5 - The function returned by applying once +The function returned by applying once -"This won't produce documentation, because we can't actually document arbitrary values" +nix-repl> "This won't produce docs; no support for arbitrary values" +"This won't produce docs; no support for arbitrary values" +nix-repl> :doc x 2 error: value does not have documentation - +nix-repl> diff --git a/tests/functional/repl/doc-comment-curried-args.in b/tests/functional/repl/doc-comment-curried-args.in index 8dbbfc37052..06ba21dcca9 100644 --- a/tests/functional/repl/doc-comment-curried-args.in +++ b/tests/functional/repl/doc-comment-curried-args.in @@ -3,5 +3,5 @@ x = curriedArgs 1 "Note that users may not expect this to behave as it currently does" :doc x -"This won't produce documentation, because we can't actually document arbitrary values" +"This won't produce docs; no support for arbitrary values" :doc x 2 diff --git a/tests/functional/repl/doc-comment-formals.expected b/tests/functional/repl/doc-comment-formals.expected index 704c0050bd8..4c9757f89ef 100644 --- a/tests/functional/repl/doc-comment-formals.expected +++ b/tests/functional/repl/doc-comment-formals.expected @@ -1,13 +1,16 @@ Nix Type :? for help. + +nix-repl> :l doc-comments.nix Added variables. +nix-repl> "Note that this is not yet complete" "Note that this is not yet complete" -Function documentedFormals - … defined at - /path/to/tests/functional/repl/doc-comments.nix:57:5 - - Finds x +nix-repl> :doc documentedFormals +Function `documentedFormals`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:57:5 +Finds x +nix-repl> diff --git a/tests/functional/repl/doc-comment-function.expected b/tests/functional/repl/doc-comment-function.expected index 5ec465a96c9..9718ccc66a8 100644 --- a/tests/functional/repl/doc-comment-function.expected +++ b/tests/functional/repl/doc-comment-function.expected @@ -1,8 +1,9 @@ Nix Type :? for help. -Function defined at - /path/to/tests/functional/repl/doc-comment-function.nix:2:1 - A doc comment for a file that only contains a function +nix-repl> :doc import ./doc-comment-function.nix +Function defined at /path/to/tests/functional/repl/doc-comment-function.nix:2:1 +A doc comment for a file that only contains a function +nix-repl> diff --git a/tests/functional/repl/doc-compact.expected b/tests/functional/repl/doc-compact.expected index 4b05b653c45..17603d81369 100644 --- a/tests/functional/repl/doc-compact.expected +++ b/tests/functional/repl/doc-compact.expected @@ -1,11 +1,13 @@ Nix Type :? for help. -Added variables. -Function compact - … defined at - /path/to/tests/functional/repl/doc-comments.nix:18:20 +nix-repl> :l doc-comments.nix +Added variables. - boom +nix-repl> :doc compact +Function `compact`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:18:20 +boom +nix-repl> diff --git a/tests/functional/repl/doc-constant.expected b/tests/functional/repl/doc-constant.expected index c6655833376..e0092453e41 100644 --- a/tests/functional/repl/doc-constant.expected +++ b/tests/functional/repl/doc-constant.expected @@ -1,23 +1,27 @@ Nix Type :? for help. + +nix-repl> :l doc-comments.nix Added variables. +nix-repl> :doc constant error: value does not have documentation -Attribute version +nix-repl> :doc lib.version +Attribute `version` - … defined at - /path/to/tests/functional/repl/doc-comments.nix:30:3 + … defined at /path/to/tests/functional/repl/doc-comments.nix:30:3 - Immovably fixed. +Immovably fixed. -Attribute empty +nix-repl> :doc lib.attr.empty +Attribute `empty` - … defined at - /path/to/tests/functional/repl/doc-comments.nix:33:3 + … defined at /path/to/tests/functional/repl/doc-comments.nix:33:3 - Unchangeably constant. +Unchangeably constant. +nix-repl> :doc lib.attr.undocument error: … while evaluating the attribute 'attr.undocument' at /path/to/tests/functional/repl/doc-comments.nix:33:3: @@ -32,59 +36,65 @@ error: | ^ Did you mean undocumented? -Attribute constant +nix-repl> :doc (import ./doc-comments.nix).constant +Attribute `constant` - … defined at - /path/to/tests/functional/repl/doc-comments.nix:27:3 + … defined at /path/to/tests/functional/repl/doc-comments.nix:27:3 - Firmly rigid. +Firmly rigid. -Attribute version +nix-repl> :doc (import ./doc-comments.nix).lib.version +Attribute `version` - … defined at - /path/to/tests/functional/repl/doc-comments.nix:30:3 + … defined at /path/to/tests/functional/repl/doc-comments.nix:30:3 - Immovably fixed. +Immovably fixed. -Attribute empty +nix-repl> :doc (import ./doc-comments.nix).lib.attr.empty +Attribute `empty` - … defined at - /path/to/tests/functional/repl/doc-comments.nix:33:3 + … defined at /path/to/tests/functional/repl/doc-comments.nix:33:3 - Unchangeably constant. +Unchangeably constant. -Attribute undocumented +nix-repl> :doc (import ./doc-comments.nix).lib.attr.undocumented +Attribute `undocumented` - … defined at - /path/to/tests/functional/repl/doc-comments.nix:35:3 + … defined at /path/to/tests/functional/repl/doc-comments.nix:35:3 - No documentation found. +No documentation found. +nix-repl> :doc missing error: undefined variable 'missing' at «string»:1:1: 1| missing | ^ +nix-repl> :doc constanz error: undefined variable 'constanz' at «string»:1:1: 1| constanz | ^ +nix-repl> :doc missing.attr error: undefined variable 'missing' at «string»:1:1: 1| missing.attr | ^ +nix-repl> :doc lib.missing error: attribute 'missing' missing at «string»:1:1: 1| lib.missing | ^ +nix-repl> :doc lib.missing.attr error: attribute 'missing' missing at «string»:1:1: 1| lib.missing.attr | ^ +nix-repl> :doc lib.attr.undocumental error: … while evaluating the attribute 'attr.undocumental' at /path/to/tests/functional/repl/doc-comments.nix:33:3: @@ -99,4 +109,4 @@ error: | ^ Did you mean undocumented? - +nix-repl> diff --git a/tests/functional/repl/doc-floatedIn.expected b/tests/functional/repl/doc-floatedIn.expected index 30f13572586..d3f1c3f65a5 100644 --- a/tests/functional/repl/doc-floatedIn.expected +++ b/tests/functional/repl/doc-floatedIn.expected @@ -1,11 +1,13 @@ Nix Type :? for help. -Added variables. -Function floatedIn - … defined at - /path/to/tests/functional/repl/doc-comments.nix:16:5 +nix-repl> :l doc-comments.nix +Added variables. - This also works. +nix-repl> :doc floatedIn +Function `floatedIn`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:16:5 +This also works. +nix-repl> diff --git a/tests/functional/repl/doc-lambda-flavors.expected b/tests/functional/repl/doc-lambda-flavors.expected index 43f483ce943..268cac05e9e 100644 --- a/tests/functional/repl/doc-lambda-flavors.expected +++ b/tests/functional/repl/doc-lambda-flavors.expected @@ -1,29 +1,31 @@ Nix Type :? for help. -Added variables. -Function nonStrict - … defined at - /path/to/tests/functional/repl/doc-comments.nix:37:70 +nix-repl> :l doc-comments.nix +Added variables. - My syntax is not strict, but I'm strict anyway. +nix-repl> :doc nonStrict +Function `nonStrict`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:37:70 -Function strict - … defined at - /path/to/tests/functional/repl/doc-comments.nix:38:63 +My syntax is not strict, but I'm strict anyway. - I don't have to be strict, but I am anyway. +nix-repl> :doc strict +Function `strict`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:38:63 -Function strictPre - … defined at - /path/to/tests/functional/repl/doc-comments.nix:40:48 +I don't have to be strict, but I am anyway. - Here's one way to do this +nix-repl> :doc strictPre +Function `strictPre`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:40:48 -Function strictPost - … defined at - /path/to/tests/functional/repl/doc-comments.nix:41:53 +Here's one way to do this - Here's another way to do this +nix-repl> :doc strictPost +Function `strictPost`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:41:53 +Here's another way to do this +nix-repl> diff --git a/tests/functional/repl/doc-measurement.expected b/tests/functional/repl/doc-measurement.expected index 8598aaedbc7..14ae11e800f 100644 --- a/tests/functional/repl/doc-measurement.expected +++ b/tests/functional/repl/doc-measurement.expected @@ -1,11 +1,13 @@ Nix Type :? for help. -Added variables. -Function measurement - … defined at - /path/to/tests/functional/repl/doc-comments.nix:13:17 +nix-repl> :l doc-comments.nix +Added variables. - 👈 precisely this wide 👉 +nix-repl> :doc measurement +Function `measurement`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:13:17 +👈 precisely this wide 👉 +nix-repl> diff --git a/tests/functional/repl/doc-multiply.expected b/tests/functional/repl/doc-multiply.expected index db82af91fc6..5cefda51650 100644 --- a/tests/functional/repl/doc-multiply.expected +++ b/tests/functional/repl/doc-multiply.expected @@ -1,15 +1,19 @@ Nix Type :? for help. + +nix-repl> :l doc-comments.nix Added variables. -Function multiply - … defined at - /path/to/tests/functional/repl/doc-comments.nix:10:14 +nix-repl> :doc multiply +Function `multiply`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:10:14 - Perform arithmetic multiplication. It's kind of like - repeated addition, very neat. - | multiply 2 3 - | => 6 +Perform *arithmetic* multiplication. It's kind of like repeated **addition**, very neat. +```nix +multiply 2 3 +=> 6 +``` +nix-repl> diff --git a/tests/functional/repl/doc-unambiguous.expected b/tests/functional/repl/doc-unambiguous.expected index 825aa1ee170..0872750a16a 100644 --- a/tests/functional/repl/doc-unambiguous.expected +++ b/tests/functional/repl/doc-unambiguous.expected @@ -1,11 +1,13 @@ Nix Type :? for help. -Added variables. -Function unambiguous - … defined at - /path/to/tests/functional/repl/doc-comments.nix:24:5 +nix-repl> :l doc-comments.nix +Added variables. - Very close +nix-repl> :doc unambiguous +Function `unambiguous`\ + … defined at /path/to/tests/functional/repl/doc-comments.nix:24:5 +Very close +nix-repl> diff --git a/tests/functional/repl/pretty-print-idempotent.expected b/tests/functional/repl/pretty-print-idempotent.expected index f38b9b56969..f09aec88d53 100644 --- a/tests/functional/repl/pretty-print-idempotent.expected +++ b/tests/functional/repl/pretty-print-idempotent.expected @@ -1,29 +1,39 @@ Nix Type :? for help. + +nix-repl> :l pretty-print-idempotent.nix Added variables. +nix-repl> oneDeep { homepage = "https://example.com"; } +nix-repl> oneDeep { homepage = "https://example.com"; } +nix-repl> twoDeep { layerOne = { ... }; } +nix-repl> twoDeep { layerOne = { ... }; } +nix-repl> oneDeepList [ "https://example.com" ] +nix-repl> oneDeepList [ "https://example.com" ] +nix-repl> twoDeepList [ [ ... ] ] +nix-repl> twoDeepList [ [ ... ] ] - +nix-repl> From c4ae9bb45b72c852ecff8bf5d5153951116e2039 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 24 Jul 2024 01:01:00 +0200 Subject: [PATCH 05/11] tests/functional/repl: Normalize final prompt --- tests/functional/repl.sh | 15 +++++++++++++-- .../repl/doc-comment-curried-args.expected | 2 -- .../functional/repl/doc-comment-formals.expected | 2 -- .../functional/repl/doc-comment-function.expected | 2 -- tests/functional/repl/doc-compact.expected | 2 -- tests/functional/repl/doc-constant.expected | 2 -- tests/functional/repl/doc-floatedIn.expected | 2 -- tests/functional/repl/doc-lambda-flavors.expected | 2 -- tests/functional/repl/doc-measurement.expected | 2 -- tests/functional/repl/doc-multiply.expected | 2 -- tests/functional/repl/doc-unambiguous.expected | 2 -- .../repl/pretty-print-idempotent.expected | 2 -- 12 files changed, 13 insertions(+), 24 deletions(-) diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index a6c075abd84..a149916ac9c 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -262,14 +262,23 @@ badExitCode=0 nixVersion="$(nix eval --impure --raw --expr 'builtins.nixVersion' --extra-experimental-features nix-command)" +# TODO: write a repl interacter for testing. Papering over the differences between readline / editline and between platforms is a pain. + # I couldn't get readline and editline to agree on the newline before the prompt, -# so let's just force it to be one empty line. Ideally we get the two to agree -# or use a simpler interacter for testing. +# so let's just force it to be one empty line. stripEmptyLinesBeforePrompt() { # --null-data: treat input as NUL-terminated instead of newline-terminated sed --null-data 's/\n\n*nix-repl>/\n\nnix-repl>/g' } +# We don't get a final prompt on darwin, so we strip this as well. +stripFinalPrompt() { + # Strip the final prompt and/or any trailing spaces + sed --null-data \ + -e 's/\(.*[^\n]\)\n\n*nix-repl>[ \n]*$/\1/' \ + -e 's/[ \n]*$/\n/' +} + runRepl () { # That is right, we are also filtering out the testdir _without underscores_. @@ -285,7 +294,9 @@ runRepl () { _NIX_TEST_REPL_ECHO=1 \ nix repl 2>&1 \ | stripColors \ + | tr -d '\0' \ | stripEmptyLinesBeforePrompt \ + | stripFinalPrompt \ | sed \ -e "s@$testDir@/path/to/tests/functional@g" \ -e "s@$testDirNoUnderscores@/path/to/tests/functional@g" \ diff --git a/tests/functional/repl/doc-comment-curried-args.expected b/tests/functional/repl/doc-comment-curried-args.expected index f97f593c667..56607e911e8 100644 --- a/tests/functional/repl/doc-comment-curried-args.expected +++ b/tests/functional/repl/doc-comment-curried-args.expected @@ -26,5 +26,3 @@ nix-repl> "This won't produce docs; no support for arbitrary values" nix-repl> :doc x 2 error: value does not have documentation - -nix-repl> diff --git a/tests/functional/repl/doc-comment-formals.expected b/tests/functional/repl/doc-comment-formals.expected index 4c9757f89ef..1024919f4b9 100644 --- a/tests/functional/repl/doc-comment-formals.expected +++ b/tests/functional/repl/doc-comment-formals.expected @@ -12,5 +12,3 @@ Function `documentedFormals`\ … defined at /path/to/tests/functional/repl/doc-comments.nix:57:5 Finds x - -nix-repl> diff --git a/tests/functional/repl/doc-comment-function.expected b/tests/functional/repl/doc-comment-function.expected index 9718ccc66a8..3889c4f7860 100644 --- a/tests/functional/repl/doc-comment-function.expected +++ b/tests/functional/repl/doc-comment-function.expected @@ -5,5 +5,3 @@ nix-repl> :doc import ./doc-comment-function.nix Function defined at /path/to/tests/functional/repl/doc-comment-function.nix:2:1 A doc comment for a file that only contains a function - -nix-repl> diff --git a/tests/functional/repl/doc-compact.expected b/tests/functional/repl/doc-compact.expected index 17603d81369..79f1fd44f59 100644 --- a/tests/functional/repl/doc-compact.expected +++ b/tests/functional/repl/doc-compact.expected @@ -9,5 +9,3 @@ Function `compact`\ … defined at /path/to/tests/functional/repl/doc-comments.nix:18:20 boom - -nix-repl> diff --git a/tests/functional/repl/doc-constant.expected b/tests/functional/repl/doc-constant.expected index e0092453e41..5787e04dc19 100644 --- a/tests/functional/repl/doc-constant.expected +++ b/tests/functional/repl/doc-constant.expected @@ -108,5 +108,3 @@ error: 1| lib.attr.undocumental | ^ Did you mean undocumented? - -nix-repl> diff --git a/tests/functional/repl/doc-floatedIn.expected b/tests/functional/repl/doc-floatedIn.expected index d3f1c3f65a5..82bb80b9501 100644 --- a/tests/functional/repl/doc-floatedIn.expected +++ b/tests/functional/repl/doc-floatedIn.expected @@ -9,5 +9,3 @@ Function `floatedIn`\ … defined at /path/to/tests/functional/repl/doc-comments.nix:16:5 This also works. - -nix-repl> diff --git a/tests/functional/repl/doc-lambda-flavors.expected b/tests/functional/repl/doc-lambda-flavors.expected index 268cac05e9e..ab5c956390f 100644 --- a/tests/functional/repl/doc-lambda-flavors.expected +++ b/tests/functional/repl/doc-lambda-flavors.expected @@ -27,5 +27,3 @@ Function `strictPost`\ … defined at /path/to/tests/functional/repl/doc-comments.nix:41:53 Here's another way to do this - -nix-repl> diff --git a/tests/functional/repl/doc-measurement.expected b/tests/functional/repl/doc-measurement.expected index 14ae11e800f..555cac9a2a0 100644 --- a/tests/functional/repl/doc-measurement.expected +++ b/tests/functional/repl/doc-measurement.expected @@ -9,5 +9,3 @@ Function `measurement`\ … defined at /path/to/tests/functional/repl/doc-comments.nix:13:17 👈 precisely this wide 👉 - -nix-repl> diff --git a/tests/functional/repl/doc-multiply.expected b/tests/functional/repl/doc-multiply.expected index 5cefda51650..21523e24c81 100644 --- a/tests/functional/repl/doc-multiply.expected +++ b/tests/functional/repl/doc-multiply.expected @@ -15,5 +15,3 @@ Perform *arithmetic* multiplication. It's kind of like repeated **addition**, ve multiply 2 3 => 6 ``` - -nix-repl> diff --git a/tests/functional/repl/doc-unambiguous.expected b/tests/functional/repl/doc-unambiguous.expected index 0872750a16a..0db5505d781 100644 --- a/tests/functional/repl/doc-unambiguous.expected +++ b/tests/functional/repl/doc-unambiguous.expected @@ -9,5 +9,3 @@ Function `unambiguous`\ … defined at /path/to/tests/functional/repl/doc-comments.nix:24:5 Very close - -nix-repl> diff --git a/tests/functional/repl/pretty-print-idempotent.expected b/tests/functional/repl/pretty-print-idempotent.expected index f09aec88d53..311855dae36 100644 --- a/tests/functional/repl/pretty-print-idempotent.expected +++ b/tests/functional/repl/pretty-print-idempotent.expected @@ -35,5 +35,3 @@ nix-repl> twoDeepList [ [ ... ] ] - -nix-repl> From 6e680a664421927cd7d99f956a1e8c4b5bdc7db6 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 24 Jul 2024 01:01:14 +0200 Subject: [PATCH 06/11] tests/functional/repl: Improve failure reporting --- tests/functional/repl.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index a149916ac9c..706e0f5dba9 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -311,7 +311,10 @@ for test in $(cd "$testDir/repl"; echo *.in); do in="$testDir/repl/$test.in" actual="$testDir/repl/$test.actual" expected="$testDir/repl/$test.expected" - (cd "$testDir/repl"; set +x; runRepl 2>&1) < "$in" > "$actual" + (cd "$testDir/repl"; set +x; runRepl 2>&1) < "$in" > "$actual" || { + echo "FAIL: $test (exit code $?)" >&2 + badExitCode=1 + } diffAndAcceptInner "$test" "$actual" "$expected" done From 7d4d34a27d8d95e8a63d3b3940bd0ee7b623ff82 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 24 Jul 2024 01:02:21 +0200 Subject: [PATCH 07/11] eval-gc.cc: Fix warning --- src/libexpr/eval-gc.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libexpr/eval-gc.cc b/src/libexpr/eval-gc.cc index 73ab809e5bd..1bc8cd28f50 100644 --- a/src/libexpr/eval-gc.cc +++ b/src/libexpr/eval-gc.cc @@ -84,7 +84,9 @@ void fixupBoehmStackPointer(void ** sp_ptr, void * _pthread_id) { void *& sp = *sp_ptr; auto pthread_id = reinterpret_cast(_pthread_id); +# ifndef __APPLE__ pthread_attr_t pattr; +# endif size_t osStackSize; void * osStackLow; void * osStackBase; From 0bd2d363750db35926460cbc4c3e62dcb1d09bf5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 24 Jul 2024 12:53:37 +0200 Subject: [PATCH 08/11] Document renderMarkdownToTerminal --- src/libcmd/markdown.hh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libcmd/markdown.hh b/src/libcmd/markdown.hh index a04d32a4fe4..164f99b9881 100644 --- a/src/libcmd/markdown.hh +++ b/src/libcmd/markdown.hh @@ -5,6 +5,13 @@ namespace nix { +/** + * Render the given Markdown text to the terminal. + * + * If Nix is compiled without Markdown support, this function will return the input text as-is. + * + * The renderer takes into account the terminal width, and wraps text accordingly. + */ std::string renderMarkdownToTerminal(std::string_view markdown); } From e48e0cbab0eed9b9174c27d3f3ddfa1afff5186b Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 24 Jul 2024 12:54:40 +0200 Subject: [PATCH 09/11] markdown.hh: Improve includes --- src/libcmd/markdown.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcmd/markdown.hh b/src/libcmd/markdown.hh index 164f99b9881..66db1736c65 100644 --- a/src/libcmd/markdown.hh +++ b/src/libcmd/markdown.hh @@ -1,7 +1,7 @@ #pragma once ///@file -#include "types.hh" +#include namespace nix { From 3172e88af544ca53a54930ccf3f580b15141f01c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 20 Jul 2024 22:46:09 +0200 Subject: [PATCH 10/11] Make abort() call sites log first --- src/libcmd/repl.cc | 2 +- src/libexpr/attr-path.cc | 2 +- src/libexpr/eval.cc | 8 +++---- src/libexpr/nixexpr.cc | 4 ++-- src/libexpr/primops.cc | 2 +- src/libexpr/print-ambiguous.cc | 2 +- src/libexpr/print.cc | 4 ++-- src/libexpr/symbol-table.hh | 3 ++- src/libexpr/value.hh | 2 +- src/libfetchers/attrs.cc | 4 ++-- src/libmain/loggers.cc | 2 +- .../build/drv-output-substitution-goal.hh | 2 +- src/libstore/build/goal.hh | 4 ++-- src/libstore/build/substitution-goal.hh | 2 +- src/libstore/build/worker.cc | 4 ++-- src/libstore/daemon.cc | 2 +- src/libstore/gc.cc | 2 +- src/libstore/globals.cc | 2 +- src/libstore/nar-info-disk-cache.cc | 4 ++-- .../unix/build/local-derivation-goal.cc | 2 +- src/libstore/unix/pathlocks.cc | 2 +- src/libutil/chunked-vector.hh | 4 +++- src/libutil/error.cc | 12 ++++++++++ src/libutil/error.hh | 24 +++++++++++++++++++ src/libutil/file-content-address.cc | 2 +- src/libutil/fs-sink.cc | 2 +- src/libutil/git.cc | 2 +- src/libutil/hash.cc | 2 +- src/libutil/logging.cc | 2 +- src/libutil/serialise.cc | 8 +++---- src/libutil/sync.hh | 4 +++- src/libutil/unix/monitor-fd.hh | 4 +++- src/libutil/unix/processes.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix-store/nix-store.cc | 2 +- 35 files changed, 88 insertions(+), 45 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index b5d0816dd2c..bf0d820c270 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -217,7 +217,7 @@ ReplExitStatus NixRepl::mainLoop() case ProcessLineResult::PromptAgain: break; default: - abort(); + unreachable(); } } catch (ParseError & e) { if (e.msg().find("unexpected end of file") != std::string::npos) { diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index d61d9363070..2f67260c532 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -134,7 +134,7 @@ std::pair findPackageFilename(EvalState & state, Value & v return {SourcePath{path.accessor, CanonPath(fn.substr(0, colon))}, lineno}; } catch (std::invalid_argument & e) { fail(); - abort(); + unreachable(); } } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b192f9b4b18..32bc68e6ded 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -149,7 +149,7 @@ std::string_view showType(ValueType type, bool withArticle) case nFloat: return WA("a", "float"); case nThunk: return WA("a", "thunk"); } - abort(); + unreachable(); } @@ -771,7 +771,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & case ReplExitStatus::Continue: break; default: - abort(); + unreachable(); } } } @@ -1140,7 +1140,7 @@ inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const PosIdx po void Expr::eval(EvalState & state, Env & env, Value & v) { - abort(); + unreachable(); } @@ -1573,7 +1573,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & .withFrame(*fun.payload.lambda.env, lambda) .debugThrow(); } - abort(); // can't happen + unreachable(); } } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 6c6769cfd44..dbc74faf9a7 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -25,7 +25,7 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) void Expr::show(const SymbolTable & symbols, std::ostream & str) const { - abort(); + unreachable(); } void ExprInt::show(const SymbolTable & symbols, std::ostream & str) const @@ -271,7 +271,7 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath) void Expr::bindVars(EvalState & es, const std::shared_ptr & env) { - abort(); + unreachable(); } void ExprInt::bindVars(EvalState & es, const std::shared_ptr & env) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5a373a43bda..0b3b19b5764 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -426,7 +426,7 @@ static void prim_typeOf(EvalState & state, const PosIdx pos, Value * * args, Val t = args[0]->external()->typeOf(); break; case nFloat: t = "float"; break; - case nThunk: abort(); + case nThunk: unreachable(); } v.mkString(t); } diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc index 5d55b45da3b..a40c98643e3 100644 --- a/src/libexpr/print-ambiguous.cc +++ b/src/libexpr/print-ambiguous.cc @@ -94,7 +94,7 @@ void printAmbiguous( break; default: printError("Nix evaluator internal error: printAmbiguous: invalid value type"); - abort(); + unreachable(); } } diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index bc17d6bfe26..4d1a6868c67 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -475,7 +475,7 @@ class Printer else output << "primop"; } else { - abort(); + unreachable(); } output << "»"; @@ -504,7 +504,7 @@ class Printer if (options.ansiColors) output << ANSI_NORMAL; } else { - abort(); + unreachable(); } } diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh index c7a3563b040..dee7369e80b 100644 --- a/src/libexpr/symbol-table.hh +++ b/src/libexpr/symbol-table.hh @@ -7,6 +7,7 @@ #include "types.hh" #include "chunked-vector.hh" +#include "error.hh" namespace nix { @@ -113,7 +114,7 @@ public: SymbolStr operator[](Symbol s) const { if (s.id == 0 || s.id > store.size()) - abort(); + unreachable(); return SymbolStr(store[s.id - 1]); } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 1f4d72d3926..fdc6c84c4d3 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -285,7 +285,7 @@ public: if (invalidIsThunk) return nThunk; else - abort(); + unreachable(); } inline void finishValue(InternalType newType, Payload newPayload) diff --git a/src/libfetchers/attrs.cc b/src/libfetchers/attrs.cc index b788c5948e0..25d04cdc950 100644 --- a/src/libfetchers/attrs.cc +++ b/src/libfetchers/attrs.cc @@ -33,7 +33,7 @@ nlohmann::json attrsToJSON(const Attrs & attrs) json[attr.first] = *v; } else if (auto v = std::get_if>(&attr.second)) { json[attr.first] = v->t; - } else abort(); + } else unreachable(); } return json; } @@ -99,7 +99,7 @@ std::map attrsToQuery(const Attrs & attrs) query.insert_or_assign(attr.first, *v); } else if (auto v = std::get_if>(&attr.second)) { query.insert_or_assign(attr.first, v->t ? "1" : "0"); - } else abort(); + } else unreachable(); } return query; } diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index 9829859de32..a4e0530c8f9 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -36,7 +36,7 @@ Logger * makeDefaultLogger() { return logger; } default: - abort(); + unreachable(); } } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 80705492662..8c60d01987a 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -36,7 +36,7 @@ public: Co init() override; Co realisationFetched(std::shared_ptr outputInfo, nix::ref sub); - void timedOut(Error && ex) override { abort(); }; + void timedOut(Error && ex) override { unreachable(); }; std::string key() override; diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 162c392d066..9c6a40c845c 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -400,12 +400,12 @@ public: virtual void handleChildOutput(Descriptor fd, std::string_view data) { - abort(); + unreachable(); } virtual void handleEOF(Descriptor fd) { - abort(); + unreachable(); } void trace(std::string_view s); diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 86e4f542382..c1de45379f1 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -50,7 +50,7 @@ public: PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~PathSubstitutionGoal(); - void timedOut(Error && ex) override { abort(); }; + void timedOut(Error && ex) override { unreachable(); }; /** * We prepend "a$" to the key name to ensure substitution goals diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 7fc41b1214a..ab0ba67b521 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -216,7 +216,7 @@ void Worker::childStarted(GoalPtr goal, const std::set std::string BaseSetting::to_string() const if (value == smEnabled) return "true"; else if (value == smRelaxed) return "relaxed"; else if (value == smDisabled) return "false"; - else abort(); + else unreachable(); } template<> void BaseSetting::convertToArg(Args & args, const std::string & category) diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 288f618d5d7..83e63794e0d 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -164,7 +164,7 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache Cache & getCache(State & state, const std::string & uri) { auto i = state.caches.find(uri); - if (i == state.caches.end()) abort(); + if (i == state.caches.end()) unreachable(); return i->second; } @@ -211,7 +211,7 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache { auto r(state->insertCache.use()(uri)(time(0))(storeDir)(wantMassQuery)(priority)); - if (!r.next()) { abort(); } + if (!r.next()) { unreachable(); } ret.id = (int) r.getInt(0); } diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 0dd102200a5..ac5b4dd0b7e 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -165,7 +165,7 @@ void LocalDerivationGoal::killSandbox(bool getStats) buildResult.cpuSystem = stats.cpuSystem; } #else - abort(); + unreachable(); #endif } diff --git a/src/libstore/unix/pathlocks.cc b/src/libstore/unix/pathlocks.cc index af21319a758..1ec4579ec96 100644 --- a/src/libstore/unix/pathlocks.cc +++ b/src/libstore/unix/pathlocks.cc @@ -45,7 +45,7 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait) if (lockType == ltRead) type = LOCK_SH; else if (lockType == ltWrite) type = LOCK_EX; else if (lockType == ltNone) type = LOCK_UN; - else abort(); + else unreachable(); if (wait) { while (flock(desc, type) != 0) { diff --git a/src/libutil/chunked-vector.hh b/src/libutil/chunked-vector.hh index d914e2542f0..4709679a62a 100644 --- a/src/libutil/chunked-vector.hh +++ b/src/libutil/chunked-vector.hh @@ -6,6 +6,8 @@ #include #include +#include "error.hh" + namespace nix { /** @@ -30,7 +32,7 @@ private: auto & addChunk() { if (size_ >= std::numeric_limits::max() - ChunkSize) - abort(); + unreachable(); chunks.emplace_back(); chunks.back().reserve(ChunkSize); return chunks.back(); diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 33c391963f3..b1858911aa5 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -430,4 +430,16 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s return out; } +void panic(std::string_view msg) +{ + printError(msg); + printError("This was a fatal error, aborting."); + abort(); +} + +void panic(const char * file, int line, const char * func) +{ + panic(std::string("Unexpected condition in ") + func + " at " + file + ":" + std::to_string(line)); +} + } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index d7fe902d6c0..572a1baf729 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -273,4 +273,28 @@ using NativeSysError = */ void throwExceptionSelfCheck(); +/** + * Print a message and abort(). + * + * @note: This assumes that the logger is operational + */ +[[noreturn]] +void panic(std::string_view msg); + +/** + * Print a basic error message with source position and abort(). + * Use the unreachable macro to call this. + * + * @note: This assumes that the logger is operational + */ +[[noreturn]] +void panic(const char * file, int line, const char * func); + +/** + * Print a basic error message with source position and abort(). + * + * @note: This assumes that the logger is operational + */ +#define unreachable() (panic(__FILE__, __LINE__, __func__)) + } diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc index 438dac7daa6..86378dd673d 100644 --- a/src/libutil/file-content-address.cc +++ b/src/libutil/file-content-address.cc @@ -63,7 +63,7 @@ std::string_view renderFileIngestionMethod(FileIngestionMethod method) case FileIngestionMethod::Git: return "git"; default: - abort(); + unreachable(); } } diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index 3246e0902fb..f15324d0a9f 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -53,7 +53,7 @@ void copyRecursive( throw Error("file '%1%' has an unsupported type", from); default: - abort(); + unreachable(); } } diff --git a/src/libutil/git.cc b/src/libutil/git.cc index a6968a43eed..af91fa64399 100644 --- a/src/libutil/git.cc +++ b/src/libutil/git.cc @@ -201,7 +201,7 @@ std::optional convertMode(SourceAccessor::Type type) case SourceAccessor::tRegular: return Mode::Regular; case SourceAccessor::tDirectory: return Mode::Directory; case SourceAccessor::tMisc: return std::nullopt; - default: abort(); + default: unreachable(); } } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 35b913e4249..ab2a8695dd4 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -25,7 +25,7 @@ static size_t regularHashSize(HashAlgorithm type) { case HashAlgorithm::SHA256: return sha256HashSize; case HashAlgorithm::SHA512: return sha512HashSize; } - abort(); + unreachable(); } diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 55751b4cf0d..29427f2f636 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -189,7 +189,7 @@ struct JSONLogger : Logger { else if (f.type == Logger::Field::tString) arr.push_back(f.s); else - abort(); + unreachable(); } void write(const nlohmann::json & json) diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 36b99905aab..ee435154573 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -260,7 +260,7 @@ std::unique_ptr sourceToSink(std::function fun) }); } - if (!*coro) { abort(); } + if (!*coro) { unreachable(); } if (!cur.empty()) { CoroutineContext ctx; @@ -271,12 +271,12 @@ std::unique_ptr sourceToSink(std::function fun) void finish() override { if (!coro) return; - if (!*coro) abort(); + if (!*coro) unreachable(); { CoroutineContext ctx; (*coro)(true); } - if (*coro) abort(); + if (*coro) unreachable(); } }; @@ -316,7 +316,7 @@ std::unique_ptr sinkToSource( }); } - if (!*coro) { eof(); abort(); } + if (!*coro) { eof(); unreachable(); } if (pos == cur.size()) { if (!cur.empty()) { diff --git a/src/libutil/sync.hh b/src/libutil/sync.hh index 20dd6ee52bc..c1b699ffc30 100644 --- a/src/libutil/sync.hh +++ b/src/libutil/sync.hh @@ -7,6 +7,8 @@ #include #include +#include "error.hh" + namespace nix { /** @@ -47,7 +49,7 @@ public: friend SyncBase; Lock(SyncBase * s) : s(s), lk(s->mutex) { } public: - Lock(Lock && l) : s(l.s) { abort(); } + Lock(Lock && l) : s(l.s) { unreachable(); } Lock(const Lock & l) = delete; ~Lock() { } diff --git a/src/libutil/unix/monitor-fd.hh b/src/libutil/unix/monitor-fd.hh index 103894de968..b6610feff98 100644 --- a/src/libutil/unix/monitor-fd.hh +++ b/src/libutil/unix/monitor-fd.hh @@ -40,7 +40,9 @@ public: #endif ; auto count = poll(fds, 1, -1); - if (count == -1) abort(); // can't happen + if (count == -1) + unreachable(); + /* This shouldn't happen, but can on macOS due to a bug. See rdar://37550628. diff --git a/src/libutil/unix/processes.cc b/src/libutil/unix/processes.cc index 1af559a21b9..c5ce74acc33 100644 --- a/src/libutil/unix/processes.cc +++ b/src/libutil/unix/processes.cc @@ -182,7 +182,7 @@ static pid_t doFork(bool allowVfork, ChildWrapperFunction & fun) #endif if (pid != 0) return pid; fun(); - abort(); + unreachable(); } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 5e170c99d37..40c20054268 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1159,7 +1159,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) case cvEqual: ch = '='; break; case cvGreater: ch = '<'; break; case cvUnavail: ch = '-'; break; - default: abort(); + default: unreachable(); } if (xmlOutput) { diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index f073074e851..b4de42ba17b 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -480,7 +480,7 @@ static void opQuery(Strings opFlags, Strings opArgs) } default: - abort(); + unreachable(); } } From 55a654abfd0f9a0d8e70b9d2dec410888a3e76db Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 24 Jul 2024 17:53:17 +0200 Subject: [PATCH 11/11] Make panic() and unreachable() robust Plus one or two tweaks. --- src/libutil/error.cc | 28 +++++++++++++++++++++++++--- src/libutil/error.hh | 8 ++------ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index b1858911aa5..ccd008c7c33 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -1,3 +1,5 @@ +#include + #include "error.hh" #include "environment-variables.hh" #include "signals.hh" @@ -430,16 +432,36 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s return out; } +/** Write to stderr in a robust and minimal way, considering that the process + * may be in a bad state. + */ +static void writeErr(std::string_view buf) +{ + while (!buf.empty()) { + auto n = write(STDERR_FILENO, buf.data(), buf.size()); + if (n < 0) { + if (errno == EINTR) continue; + abort(); + } + buf = buf.substr(n); + } +} + void panic(std::string_view msg) { - printError(msg); - printError("This was a fatal error, aborting."); + writeErr("\n\n" ANSI_RED "terminating due to unexpected unrecoverable internal error: " ANSI_NORMAL ); + writeErr(msg); + writeErr("\n"); abort(); } void panic(const char * file, int line, const char * func) { - panic(std::string("Unexpected condition in ") + func + " at " + file + ":" + std::to_string(line)); + char buf[512]; + int n = snprintf(buf, sizeof(buf), "Unexpected condition in %s at %s:%d", func, file, line); + if (n < 0) + panic("Unexpected condition and could not format error message"); + panic(std::string_view(buf, std::min(static_cast(sizeof(buf)), n))); } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 572a1baf729..58d9026222f 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -275,17 +275,13 @@ void throwExceptionSelfCheck(); /** * Print a message and abort(). - * - * @note: This assumes that the logger is operational */ [[noreturn]] void panic(std::string_view msg); /** * Print a basic error message with source position and abort(). - * Use the unreachable macro to call this. - * - * @note: This assumes that the logger is operational + * Use the unreachable() macro to call this. */ [[noreturn]] void panic(const char * file, int line, const char * func); @@ -295,6 +291,6 @@ void panic(const char * file, int line, const char * func); * * @note: This assumes that the logger is operational */ -#define unreachable() (panic(__FILE__, __LINE__, __func__)) +#define unreachable() (::nix::panic(__FILE__, __LINE__, __func__)) }