From 85ce455b854761b1fd4985ed21ef5a8881eb3c11 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 20 May 2020 17:25:02 -0600 Subject: [PATCH 01/33] get code lines from the nix file --- src/libutil/error.cc | 69 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 59628d875..7aff9381b 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -53,6 +53,52 @@ string showErrPos(const ErrPos &errPos) } } +void getCodeLines(NixCode &nixCode) +{ + if (nixCode.errPos.line <= 0) + return; + + // check this magic value! + if (nixCode.errPos.file == "(string)") + return; + + try { + AutoCloseFD fd = open(nixCode.errPos.file.c_str(), O_RDONLY | O_CLOEXEC); + if (!fd) + throw SysError("opening file '%1%'", nixCode.errPos.file); + + // count the newlines. + + int count = 0; + string line; + int pl = nixCode.errPos.line - 1; + do + { + line = readLine(fd.get()); + ++count; + if (count < pl) + { + ; + } + else if (count == pl) { + nixCode.prevLineOfCode = line; + } else if (count == pl + 1) { + nixCode.errLineOfCode = line; + } else if (count == pl + 2) { + nixCode.nextLineOfCode = line; + break; + } + } while (true); + } + catch (EndOfFile &eof) { + ; + } + catch (std::exception &e) { + printError("error reading nix file: %s\n%s", nixCode.errPos.file, e.what()); + } +} + + void printCodeLines(std::ostream &out, const string &prefix, const NixCode &nixCode) { // previous line of code. @@ -197,16 +243,21 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) out << prefix << std::endl; } - // lines of code. - if (einfo.nixCode.has_value() && einfo.nixCode->errLineOfCode.has_value()) { - printCodeLines(out, prefix, *einfo.nixCode); - out << prefix << std::endl; - } - // hint - if (einfo.hint.has_value()) { - out << prefix << *einfo.hint << std::endl; - out << prefix << std::endl; + if (einfo.nixCode.has_value()) { + NixCode nixcode = *einfo.nixCode; + getCodeLines(nixcode); + + // lines of code. + if (nixcode.errLineOfCode.has_value()) { + printCodeLines(out, prefix, nixcode); + out << prefix << std::endl; + } + + // hint + out << prefix << *einfo.hint << std::endl; + out << prefix << std::endl; + } return out; From 6a420d672ca690ef4235ac7a5833c1789a7d8b10 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 20 May 2020 22:18:26 -0600 Subject: [PATCH 02/33] print LOC for stdin, string args --- src/libexpr/attr-path.cc | 2 +- src/libexpr/eval.hh | 2 +- src/libexpr/nixexpr.hh | 7 ++-- src/libexpr/parser.y | 27 +++++++++---- src/libutil/error.cc | 86 +++++++++++++++++++++++++++++----------- src/libutil/error.hh | 9 +++++ 6 files changed, 97 insertions(+), 36 deletions(-) diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index 9a9531a3f..b8eef9286 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -121,7 +121,7 @@ Pos findDerivationFilename(EvalState & state, Value & v, std::string what) Symbol file = state.symbols.create(filename); - return { file, lineno, 0 }; + return { foFile, file, lineno, 0 }; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 1485dc7fe..8471c2f8b 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -249,7 +249,7 @@ private: friend struct ExprAttrs; friend struct ExprLet; - Expr * parse(const char * text, const Path & path, + Expr * parse(const char * text, FileOrigin origin, const Path & path, const Path & basePath, StaticEnv & staticEnv); public: diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 47d0e85ec..5a98b9149 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -24,11 +24,12 @@ MakeError(RestrictedPathError, Error); struct Pos { + FileOrigin origin; Symbol file; unsigned int line, column; - Pos() : line(0), column(0) { }; - Pos(const Symbol & file, unsigned int line, unsigned int column) - : file(file), line(line), column(column) { }; + Pos() : origin(foString), line(0), column(0) { }; + Pos(FileOrigin origin, const Symbol & file, unsigned int line, unsigned int column) + : origin(origin), file(file), line(line), column(column) { }; operator bool() const { return line != 0; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 0417a3c21..1a4f3d56b 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -30,7 +30,8 @@ namespace nix { SymbolTable & symbols; Expr * result; Path basePath; - Symbol path; + Symbol file; + FileOrigin origin; ErrorInfo error; Symbol sLetBody; ParseData(EvalState & state) @@ -250,7 +251,7 @@ static Expr * stripIndentation(const Pos & pos, SymbolTable & symbols, vectorpath, loc.first_line, loc.first_column); + return Pos(data->origin, data->file, loc.first_line, loc.first_column); } #define CUR_POS makeCurPos(*yylocp, data) @@ -576,13 +577,24 @@ formal namespace nix { -Expr * EvalState::parse(const char * text, +Expr * EvalState::parse(const char * text, FileOrigin origin, const Path & path, const Path & basePath, StaticEnv & staticEnv) { yyscan_t scanner; ParseData data(*this); + data.origin = origin; + switch (origin) { + case foFile: + data.file = data.symbols.create(path); + break; + case foStdin: + case foString: + data.file = data.symbols.create(text); + break; + default: + throw Error("invalid FileOrigin in parse"); + } data.basePath = basePath; - data.path = data.symbols.create(path); yylex_init(&scanner); yy_scan_string(text, scanner); @@ -632,13 +644,13 @@ Expr * EvalState::parseExprFromFile(const Path & path) Expr * EvalState::parseExprFromFile(const Path & path, StaticEnv & staticEnv) { - return parse(readFile(path).c_str(), path, dirOf(path), staticEnv); + return parse(readFile(path).c_str(), foFile, path, dirOf(path), staticEnv); } Expr * EvalState::parseExprFromString(std::string_view s, const Path & basePath, StaticEnv & staticEnv) { - return parse(s.data(), "(string)", basePath, staticEnv); + return parse(s.data(), foString, "", basePath, staticEnv); } @@ -651,7 +663,8 @@ Expr * EvalState::parseExprFromString(std::string_view s, const Path & basePath) Expr * EvalState::parseStdin() { //Activity act(*logger, lvlTalkative, format("parsing standard input")); - return parseExprFromString(drainFD(0), absPath(".")); + // return parseExprFromString(foStdin, drainFD(0), absPath(".")); + return parse(drainFD(0).data(), foStdin, "", absPath("."), staticBaseEnv); } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 7aff9381b..ee415e18c 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -59,22 +59,51 @@ void getCodeLines(NixCode &nixCode) return; // check this magic value! - if (nixCode.errPos.file == "(string)") - return; - - try { - AutoCloseFD fd = open(nixCode.errPos.file.c_str(), O_RDONLY | O_CLOEXEC); - if (!fd) - throw SysError("opening file '%1%'", nixCode.errPos.file); + if (nixCode.errPos.origin == foFile) { + try { + AutoCloseFD fd = open(nixCode.errPos.file.c_str(), O_RDONLY | O_CLOEXEC); + if (!fd) + throw SysError("opening file '%1%'", nixCode.errPos.file); + // count the newlines. + int count = 0; + string line; + int pl = nixCode.errPos.line - 1; + do + { + line = readLine(fd.get()); + ++count; + if (count < pl) + { + ; + } + else if (count == pl) { + nixCode.prevLineOfCode = line; + } else if (count == pl + 1) { + nixCode.errLineOfCode = line; + } else if (count == pl + 2) { + nixCode.nextLineOfCode = line; + break; + } + } while (true); + } + catch (EndOfFile &eof) { + ; + } + catch (std::exception &e) { + printError("error reading nix file: %s\n%s", nixCode.errPos.file, e.what()); + } + } else { + std::istringstream iss(nixCode.errPos.file); // count the newlines. - int count = 0; string line; int pl = nixCode.errPos.line - 1; + do { - line = readLine(fd.get()); + std::getline(iss, line); + // std::cout << "getline result: " << std::getline(iss, line) << std::endl; ++count; if (count < pl) { @@ -88,14 +117,11 @@ void getCodeLines(NixCode &nixCode) nixCode.nextLineOfCode = line; break; } + + if (!iss.good()) + break; } while (true); } - catch (EndOfFile &eof) { - ; - } - catch (std::exception &e) { - printError("error reading nix file: %s\n%s", nixCode.errPos.file, e.what()); - } } @@ -225,15 +251,27 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) // filename, line, column. if (einfo.nixCode.has_value()) { - if (einfo.nixCode->errPos.file != "") { - out << fmt("%1%in file: " ANSI_BLUE "%2% %3%" ANSI_NORMAL, - prefix, - einfo.nixCode->errPos.file, - showErrPos(einfo.nixCode->errPos)) << std::endl; - out << prefix << std::endl; - } else { - out << fmt("%1%from command line argument", prefix) << std::endl; - out << prefix << std::endl; + switch (einfo.nixCode->errPos.origin) { + case foFile: { + out << fmt("%1%in file: " ANSI_BLUE "%2% %3%" ANSI_NORMAL, + prefix, + einfo.nixCode->errPos.file, + showErrPos(einfo.nixCode->errPos)) << std::endl; + out << prefix << std::endl; + break; + } + case foString: { + out << fmt("%1%from command line argument", prefix) << std::endl; + out << prefix << std::endl; + break; + } + case foStdin: { + out << fmt("%1%from stdin", prefix) << std::endl; + out << prefix << std::endl; + break; + } + default: + throw Error("invalid FileOrigin in errPos"); } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index b374c2780..d60c0fefe 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -32,10 +32,18 @@ typedef enum { lvlVomit } Verbosity; +typedef enum { + foFile, + foStdin, + foString +} FileOrigin; + + struct ErrPos { int line = 0; int column = 0; string file; + FileOrigin origin; operator bool() const { @@ -45,6 +53,7 @@ struct ErrPos { template ErrPos& operator=(const P &pos) { + origin = pos.origin; line = pos.line; column = pos.column; file = pos.file; From 0e49de6a2b1a751c744c9e608968e9e32a042db8 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 21 May 2020 14:28:45 -0600 Subject: [PATCH 03/33] position for stdin, string; (string) for trace; fix tests --- Makefile | 3 +- src/error-demo/error-demo.cc | 8 ++--- src/libexpr/nixexpr.cc | 17 ++++++++++- src/libutil/error.cc | 59 ++++++++++++++++++------------------ tests/misc.sh | 11 +++++-- 5 files changed, 60 insertions(+), 38 deletions(-) diff --git a/Makefile b/Makefile index 0ba011e2a..05c4bdc8a 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,8 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk + tests/plugins/local.mk \ + src/error-demo/local.mk -include Makefile.config diff --git a/src/error-demo/error-demo.cc b/src/error-demo/error-demo.cc index 0216092e3..8c73a8c83 100644 --- a/src/error-demo/error-demo.cc +++ b/src/error-demo/error-demo.cc @@ -118,7 +118,7 @@ int main() "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), + .errPos = Pos(foFile, problem_file, 40, 13), .prevLineOfCode = std::nullopt, .errLineOfCode = "this is the problem line of code", .nextLineOfCode = std::nullopt @@ -132,7 +132,7 @@ int main() "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), + .errPos = Pos(foFile, problem_file, 40, 13), .prevLineOfCode = "previous line of code", .errLineOfCode = "this is the problem line of code", .nextLineOfCode = "next line of code", @@ -147,7 +147,7 @@ int main() "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(foFile, problem_file, 40, 13) }}); // Error with only hint and name.. @@ -155,7 +155,7 @@ int main() ErrorInfo { .name = "error name", .hint = hintfmt("hint %1%", "only"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(foFile, problem_file, 40, 13) }}); return 0; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 91a508305..6ab36dd35 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -197,7 +197,22 @@ std::ostream & operator << (std::ostream & str, const Pos & pos) if (!pos) str << "undefined position"; else - str << (format(ANSI_BOLD "%1%" ANSI_NORMAL ":%2%:%3%") % (string) pos.file % pos.line % pos.column).str(); + { + auto f = format(ANSI_BOLD "%1%" ANSI_NORMAL ":%2%:%3%"); + switch (pos.origin) { + case foFile: + f % (string) pos.file; + break; + case foStdin: + case foString: + f % "(string)"; + break; + default: + throw Error("unhandled Pos origin!"); + } + str << (f % pos.line % pos.column).str(); + } + return str; } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index ee415e18c..585059f22 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -58,34 +58,35 @@ void getCodeLines(NixCode &nixCode) if (nixCode.errPos.line <= 0) return; - // check this magic value! if (nixCode.errPos.origin == foFile) { try { AutoCloseFD fd = open(nixCode.errPos.file.c_str(), O_RDONLY | O_CLOEXEC); if (!fd) - throw SysError("opening file '%1%'", nixCode.errPos.file); - - // count the newlines. - int count = 0; - string line; - int pl = nixCode.errPos.line - 1; - do + logError(SysError("opening file '%1%'", nixCode.errPos.file).info()); + else { - line = readLine(fd.get()); - ++count; - if (count < pl) + // count the newlines. + int count = 0; + string line; + int pl = nixCode.errPos.line - 1; + do { - ; - } - else if (count == pl) { - nixCode.prevLineOfCode = line; - } else if (count == pl + 1) { - nixCode.errLineOfCode = line; - } else if (count == pl + 2) { - nixCode.nextLineOfCode = line; - break; - } - } while (true); + line = readLine(fd.get()); + ++count; + if (count < pl) + { + ; + } + else if (count == pl) { + nixCode.prevLineOfCode = line; + } else if (count == pl + 1) { + nixCode.errLineOfCode = line; + } else if (count == pl + 2) { + nixCode.nextLineOfCode = line; + break; + } + } while (true); + } } catch (EndOfFile &eof) { ; @@ -103,7 +104,6 @@ void getCodeLines(NixCode &nixCode) do { std::getline(iss, line); - // std::cout << "getline result: " << std::getline(iss, line) << std::endl; ++count; if (count < pl) { @@ -261,12 +261,12 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) break; } case foString: { - out << fmt("%1%from command line argument", prefix) << std::endl; + out << fmt("%1%from command line argument %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; out << prefix << std::endl; break; } case foStdin: { - out << fmt("%1%from stdin", prefix) << std::endl; + out << fmt("%1%from stdin %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; out << prefix << std::endl; break; } @@ -291,11 +291,12 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) printCodeLines(out, prefix, nixcode); out << prefix << std::endl; } + } - // hint - out << prefix << *einfo.hint << std::endl; - out << prefix << std::endl; - + // hint + if (einfo.hint.has_value()) { + out << prefix << *einfo.hint << std::endl; + out << prefix << std::endl; } return out; diff --git a/tests/misc.sh b/tests/misc.sh index fd4908e25..0804622ea 100644 --- a/tests/misc.sh +++ b/tests/misc.sh @@ -16,6 +16,11 @@ nix-env --foo 2>&1 | grep "no operation" nix-env -q --foo 2>&1 | grep "unknown flag" # Eval Errors. -eval_res=$(nix-instantiate --eval -E 'let a = {} // a; in a.foo' 2>&1 || true) -echo $eval_res | grep "(string) (1:15)" -echo $eval_res | grep "infinite recursion encountered" +eval_arg_res=$(nix-instantiate --eval -E 'let a = {} // a; in a.foo' 2>&1 || true) +echo $eval_arg_res | grep "command line argument (1:15)" +echo $eval_arg_res | grep "infinite recursion encountered" + +eval_stdin_res=$(echo 'let a = {} // a; in a.foo' | nix-instantiate --eval -E - 2>&1 || true) +echo $eval_stdin_res | grep "stdin (1:15)" +echo $eval_stdin_res | grep "infinite recursion encountered" + From b7057fa6272ed1e2867ed5491463b6714aa34673 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 21 May 2020 16:04:18 -0600 Subject: [PATCH 04/33] remove error-demo from make; clean up comment --- Makefile | 3 +-- src/libexpr/parser.y | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 05c4bdc8a..f4ad2c468 100644 --- a/Makefile +++ b/Makefile @@ -17,8 +17,7 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk \ - src/error-demo/local.mk + tests/plugins/local.mk -include Makefile.config diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 1a4f3d56b..f8b5fb0d5 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -663,7 +663,6 @@ Expr * EvalState::parseExprFromString(std::string_view s, const Path & basePath) Expr * EvalState::parseStdin() { //Activity act(*logger, lvlTalkative, format("parsing standard input")); - // return parseExprFromString(foStdin, drainFD(0), absPath(".")); return parse(drainFD(0).data(), foStdin, "", absPath("."), staticBaseEnv); } From 2f19650768e2bbef0f7ad819c5aa3dce5084d56d Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 8 Jun 2020 11:21:17 -0600 Subject: [PATCH 05/33] add file origin to Pos in stests --- src/libutil/tests/logging.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index fbdc91253..fa5983f2e 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -163,7 +163,7 @@ namespace nix { "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), + .errPos = Pos(foFile, problem_file, 40, 13), .prevLineOfCode = "previous line of code", .errLineOfCode = "this is the problem line of code", .nextLineOfCode = "next line of code", @@ -186,7 +186,7 @@ namespace nix { "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(foFile, problem_file, 40, 13) }}); auto str = testing::internal::GetCapturedStderr(); @@ -202,7 +202,7 @@ namespace nix { .name = "error name", .hint = hintfmt("hint %1%", "only"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(foFile, problem_file, 40, 13) }}); auto str = testing::internal::GetCapturedStderr(); @@ -241,7 +241,7 @@ namespace nix { "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), + .errPos = Pos(foFile, problem_file, 40, 13), .prevLineOfCode = std::nullopt, .errLineOfCode = "this is the problem line of code", .nextLineOfCode = std::nullopt From 4d1a4f02178b1f77a4bcf2de0483500d89c1424c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 18 Jun 2020 15:25:26 -0600 Subject: [PATCH 06/33] addTrace --- src/libexpr/primops.cc | 4 ++-- src/libutil/error.cc | 52 ++++++++++++++++++++++++------------------ src/libutil/error.hh | 47 ++++++++++++++++++++++---------------- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 2f1a41a64..7b80d76b9 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -239,13 +239,13 @@ void prim_exec(EvalState & state, const Pos & pos, Value * * args, Value & v) try { parsed = state.parseExprFromString(output, pos.file); } catch (Error & e) { - e.addPrefix(fmt("While parsing the output from '%1%', at %2%\n", program, pos)); + e.addTrace(pos, hintfmt("While parsing the output from '%1%'", program)); throw; } try { state.eval(parsed, v); } catch (Error & e) { - e.addPrefix(fmt("While evaluating the output from '%1%', at %2%\n", program, pos)); + e.addTrace(pos, hintfmt("While evaluating the output from '%1%'", program)); throw; } } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 07d9791ad..5f5b54251 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -18,6 +18,14 @@ BaseError & BaseError::addPrefix(const FormatOrString & fs) return *this; } +// addPrefix is used for show-trace. Strings added with addPrefix +// will print ahead of the error itself. +BaseError & BaseError::addTrace(hintformat hint, ErrPos e) +{ + err.traces.push_front(Trace { .pos = e, .hint = hint}); + return *this; +} + // c++ std::exception descendants must have a 'const char* what()' function. // This stringifies the error and caches it for use by what(), or similarly by msg(). const string& BaseError::calcWhat() const @@ -58,7 +66,7 @@ string showErrPos(const ErrPos &errPos) void getCodeLines(NixCode &nixCode) { - if (nixCode.errPos.line <= 0) + if (nixCode.errPos.line <= 0) return; if (nixCode.errPos.origin == foFile) { @@ -72,13 +80,13 @@ void getCodeLines(NixCode &nixCode) int count = 0; string line; int pl = nixCode.errPos.line - 1; - do + do { line = readLine(fd.get()); ++count; - if (count < pl) + if (count < pl) { - ; + ; } else if (count == pl) { nixCode.prevLineOfCode = line; @@ -89,7 +97,7 @@ void getCodeLines(NixCode &nixCode) break; } } while (true); - } + } } catch (EndOfFile &eof) { ; @@ -97,20 +105,20 @@ void getCodeLines(NixCode &nixCode) catch (std::exception &e) { printError("error reading nix file: %s\n%s", nixCode.errPos.file, e.what()); } - } else { + } else { std::istringstream iss(nixCode.errPos.file); // count the newlines. int count = 0; string line; int pl = nixCode.errPos.line - 1; - do + do { std::getline(iss, line); ++count; - if (count < pl) + if (count < pl) { - ; + ; } else if (count == pl) { nixCode.prevLineOfCode = line; @@ -134,18 +142,18 @@ void printCodeLines(std::ostream &out, const string &prefix, const NixCode &nixC if (nixCode.prevLineOfCode.has_value()) { out << std::endl << fmt("%1% %|2$5d|| %3%", - prefix, - (nixCode.errPos.line - 1), - *nixCode.prevLineOfCode); + prefix, + (nixCode.errPos.line - 1), + *nixCode.prevLineOfCode); } if (nixCode.errLineOfCode.has_value()) { // line of code containing the error. out << std::endl << fmt("%1% %|2$5d|| %3%", - prefix, - (nixCode.errPos.line), - *nixCode.errLineOfCode); + prefix, + (nixCode.errPos.line), + *nixCode.errLineOfCode); // error arrows for the column range. if (nixCode.errPos.column > 0) { int start = nixCode.errPos.column; @@ -158,9 +166,9 @@ void printCodeLines(std::ostream &out, const string &prefix, const NixCode &nixC out << std::endl << fmt("%1% |%2%" ANSI_RED "%3%" ANSI_NORMAL, - prefix, - spaces, - arrows); + prefix, + spaces, + arrows); } } @@ -168,9 +176,9 @@ void printCodeLines(std::ostream &out, const string &prefix, const NixCode &nixC if (nixCode.nextLineOfCode.has_value()) { out << std::endl << fmt("%1% %|2$5d|| %3%", - prefix, - (nixCode.errPos.line + 1), - *nixCode.nextLineOfCode); + prefix, + (nixCode.errPos.line + 1), + *nixCode.nextLineOfCode); } } @@ -287,7 +295,7 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) if (einfo.nixCode.has_value()) { NixCode nixcode = *einfo.nixCode; getCodeLines(nixcode); - + // lines of code. if (nixcode.errLineOfCode.has_value()) { if (nl) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 8494b9412..10bdeea1d 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -25,20 +25,20 @@ namespace nix { /* -This file defines two main structs/classes used in nix error handling. + This file defines two main structs/classes used in nix error handling. -ErrorInfo provides a standard payload of error information, with conversion to string -happening in the logger rather than at the call site. + ErrorInfo provides a standard payload of error information, with conversion to string + happening in the logger rather than at the call site. -BaseError is the ancestor of nix specific exceptions (and Interrupted), and contains -an ErrorInfo. + BaseError is the ancestor of nix specific exceptions (and Interrupted), and contains + an ErrorInfo. -ErrorInfo structs are sent to the logger as part of an exception, or directly with the -logError or logWarning macros. + ErrorInfo structs are sent to the logger as part of an exception, or directly with the + logError or logWarning macros. -See the error-demo.cc program for usage examples. + See the error-demo.cc program for usage examples. -*/ + */ typedef enum { lvlError = 0, @@ -50,7 +50,7 @@ typedef enum { lvlVomit } Verbosity; -typedef enum { +typedef enum { foFile, foStdin, foString @@ -93,12 +93,18 @@ struct NixCode { std::optional nextLineOfCode; }; +struct Trace { + ErrPos pos; + hintformat hint; +}; + struct ErrorInfo { Verbosity level; string name; string description; std::optional hint; std::optional nixCode; + std::list traces; static std::optional programName; }; @@ -121,23 +127,23 @@ public: template BaseError(unsigned int status, const Args & ... args) - : err { .level = lvlError, - .hint = hintfmt(args...) - } + : err {.level = lvlError, + .hint = hintfmt(args...) + } , status(status) { } template BaseError(const std::string & fs, const Args & ... args) - : err { .level = lvlError, - .hint = hintfmt(fs, args...) - } + : err {.level = lvlError, + .hint = hintfmt(fs, args...) + } { } BaseError(hintformat hint) - : err { .level = lvlError, - .hint = hint - } + : err {.level = lvlError, + .hint = hint + } { } BaseError(ErrorInfo && e) @@ -160,6 +166,7 @@ public: const string & msg() const { return calcWhat(); } const string & prefix() const { return prefix_; } BaseError & addPrefix(const FormatOrString & fs); + BaseError & addTrace(ErrPos e, hintformat hint); const ErrorInfo & info() { calcWhat(); return err; } }; @@ -181,7 +188,7 @@ public: template SysError(const Args & ... args) - :Error("") + : Error("") { errNo = errno; auto hf = hintfmt(args...); From 54e8f550c9d5cc88c1161035d366871bb82d4a0c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 13:44:08 -0600 Subject: [PATCH 07/33] addErrorTrace --- src/libexpr/eval.cc | 24 +-- src/libexpr/primops.cc | 10 +- src/libmain/shared.cc | 10 +- src/libutil/error.cc | 51 +++++- src/libutil/error.hh | 13 +- src/nix-env/nix-env.cc | 4 +- src/nix/repl.cc | 376 +++++++++++++++++++++-------------------- src/nix/search.cc | 2 +- 8 files changed, 270 insertions(+), 220 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b90a64357..c249add86 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -592,19 +592,19 @@ LocalNoInlineNoReturn(void throwUndefinedVarError(const Pos & pos, const char * }); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2)) +LocalNoInline(void addErrorTrace(Error & e, const char * s, const string & s2)) { - e.addPrefix(format(s) % s2); + e.addTrace(std::nullopt, hintfmt(s) % s2); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const ExprLambda & fun, const Pos & pos)) +LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const ExprLambda & fun)) { - e.addPrefix(format(s) % fun.showNamePos() % pos); + e.addTrace(pos, hintfmt(s) % fun.showNamePos()); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2, const Pos & pos)) +LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const string & s2)) { - e.addPrefix(format(s) % s2 % pos); + e.addTrace(pos, hintfmt(s) % s2); } @@ -818,7 +818,7 @@ void EvalState::evalFile(const Path & path_, Value & v) try { eval(e, v); } catch (Error & e) { - addErrorPrefix(e, "while evaluating the file '%1%':\n", path2); + addErrorTrace(e, "while evaluating the file '%1%':", path2); throw; } @@ -1068,8 +1068,8 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) } catch (Error & e) { if (pos2 && pos2->file != state.sDerivationNix) - addErrorPrefix(e, "while evaluating the attribute '%1%' at %2%:\n", - showAttrPath(state, env, attrPath), *pos2); + addErrorTrace(e, *pos2, "while evaluating the attribute '%1%'", + showAttrPath(state, env, attrPath)); throw; } @@ -1241,7 +1241,9 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & po try { lambda.body->eval(*this, env2, v); } catch (Error & e) { - addErrorPrefix(e, "while evaluating %1%, called from %2%:\n", lambda, pos); + // TODO something different for 'called from' than usual addTrace? + // addErrorTrace(e, pos, "while evaluating %1%, called from %2%:", lambda); + addErrorTrace(e, pos, "while evaluating %1%:", lambda); throw; } else @@ -1516,7 +1518,7 @@ void EvalState::forceValueDeep(Value & v) try { recurse(*i.value); } catch (Error & e) { - addErrorPrefix(e, "while evaluating the attribute '%1%' at %2%:\n", i.name, *i.pos); + addErrorTrace(e, *i.pos, "while evaluating the attribute '%1%'", i.name); throw; } } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 7b80d76b9..791fef27d 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -471,7 +471,8 @@ static void prim_addErrorContext(EvalState & state, const Pos & pos, Value * * a v = *args[1]; } catch (Error & e) { PathSet context; - e.addPrefix(format("%1%\n") % state.coerceToString(pos, *args[0], context)); + // TODO: is this right, include this pos?? Test it. esp with LOC. + e.addTrace(pos, hintfmt("%1%") % state.coerceToString(pos, *args[0], context)); throw; } } @@ -563,7 +564,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * try { drvName = state.forceStringNoCtx(*attr->value, pos); } catch (Error & e) { - e.addPrefix(fmt("while evaluating the derivation attribute 'name' at %1%:\n", posDrvName)); + e.addTrace(posDrvName, hintfmt("while evaluating the derivation attribute 'name'")); throw; } @@ -696,8 +697,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } } catch (Error & e) { - e.addPrefix(format("while evaluating the attribute '%1%' of the derivation '%2%' at %3%:\n") - % key % drvName % posDrvName); + e.addTrace(posDrvName, + hintfmt("while evaluating the attribute '%1%' of the derivation '%2%'", + key, drvName)); throw; } } diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 1cb422967..27a4d5fc1 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -323,11 +323,13 @@ int handleExceptions(const string & programName, std::function fun) printError("Try '%1% --help' for more information.", programName); return 1; } catch (BaseError & e) { - if (settings.showTrace && e.prefix() != "") - printError(e.prefix()); + // TODO showTrace as argument, or have calcWhat check settings? + // if (settings.showTrace && e.prefix() != "") + // printError(e.prefix()); logError(e.info()); - if (e.prefix() != "" && !settings.showTrace) - printError("(use '--show-trace' to show detailed location information)"); + // TODO fix to detect non-empty trace here. + // if (e.prefix() != "" && !settings.showTrace) + // printError("(use '--show-trace' to show detailed location information)"); return e.status; } catch (std::bad_alloc & e) { printError(error + "out of memory"); diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 5f5b54251..222d9cbef 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -12,20 +12,43 @@ const std::string nativeSystem = SYSTEM; // addPrefix is used for show-trace. Strings added with addPrefix // will print ahead of the error itself. -BaseError & BaseError::addPrefix(const FormatOrString & fs) -{ - prefix_ = fs.s + prefix_; - return *this; -} +// BaseError & BaseError::addPrefix(const FormatOrString & fs) +// { +// prefix_ = fs.s + prefix_; +// return *this; +// } + +// const string & prefix() const +// { +// // build prefix string on demand?? + +// } +// ; // { return prefix_; } + // addPrefix is used for show-trace. Strings added with addPrefix // will print ahead of the error itself. -BaseError & BaseError::addTrace(hintformat hint, ErrPos e) +BaseError & BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint}); return *this; } +// const string& BaseError::calcTrace() const +// { +// if (trace_.has_value()) +// return *trace_; +// else { +// err.name = sname(); + +// std::ostringstream oss; +// oss << err; +// trace_ = oss.str(); + +// return *trace_; +// } +// } + // c++ std::exception descendants must have a 'const char* what()' function. // This stringifies the error and caches it for use by what(), or similarly by msg(). const string& BaseError::calcWhat() const @@ -284,6 +307,22 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) nl = true; } + // traces + for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) + { + + try { + auto pos = *iter->pos; + out << iter->hint.str() << showErrPos(pos) << std::endl; + NixCode nc { .errPos = pos }; + getCodeLines(nc); + printCodeLines(out, prefix, nc); + } catch(const std::bad_optional_access& e) { + out << iter->hint.str() << std::endl; + } + + } + // description if (einfo.description != "") { if (nl) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 10bdeea1d..75ba80365 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -94,7 +94,7 @@ struct NixCode { }; struct Trace { - ErrPos pos; + std::optional pos; hintformat hint; }; @@ -116,9 +116,12 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo); class BaseError : public std::exception { protected: - string prefix_; // used for location traces etc. + // string prefix_; // used for location traces etc. mutable ErrorInfo err; + // mutable std::optional trace_; + // const string& calcTrace() const; + mutable std::optional what_; const string& calcWhat() const; @@ -164,9 +167,9 @@ public: #endif const string & msg() const { return calcWhat(); } - const string & prefix() const { return prefix_; } - BaseError & addPrefix(const FormatOrString & fs); - BaseError & addTrace(ErrPos e, hintformat hint); + // const string & trace() const { return calcTrace(); } + // BaseError & addPrefix(const FormatOrString & fs); + BaseError & addTrace(std::optional e, hintformat hint); const ErrorInfo & info() { calcWhat(); return err; } }; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 8b0692035..f5dfbf9f6 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -593,7 +593,7 @@ static void upgradeDerivations(Globals & globals, } else newElems.push_back(i); } catch (Error & e) { - e.addPrefix(fmt("while trying to find an upgrade for '%s':\n", i.queryName())); + e.addTrace(std::nullopt, hintfmt("while trying to find an upgrade for '%s'", i.queryName())); throw; } } @@ -1185,7 +1185,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) } catch (AssertionError & e) { printMsg(lvlTalkative, "skipping derivation named '%1%' which gives an assertion failure", i.queryName()); } catch (Error & e) { - e.addPrefix(fmt("while querying the derivation named '%1%':\n", i.queryName())); + e.addTrace(std::nullopt, hintfmt("while querying the derivation named '%1%'", i.queryName())); throw; } } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 617d49614..a75157a82 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -101,74 +101,74 @@ NixRepl::~NixRepl() static NixRepl * curRepl; // ugly static char * completionCallback(char * s, int *match) { - auto possible = curRepl->completePrefix(s); - if (possible.size() == 1) { - *match = 1; - auto *res = strdup(possible.begin()->c_str() + strlen(s)); - if (!res) throw Error("allocation failure"); - return res; - } else if (possible.size() > 1) { - auto checkAllHaveSameAt = [&](size_t pos) { - auto &first = *possible.begin(); - for (auto &p : possible) { - if (p.size() <= pos || p[pos] != first[pos]) - return false; - } - return true; - }; - size_t start = strlen(s); - size_t len = 0; - while (checkAllHaveSameAt(start + len)) ++len; - if (len > 0) { - *match = 1; - auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); - if (!res) throw Error("allocation failure"); - return res; + auto possible = curRepl->completePrefix(s); + if (possible.size() == 1) { + *match = 1; + auto *res = strdup(possible.begin()->c_str() + strlen(s)); + if (!res) throw Error("allocation failure"); + return res; + } else if (possible.size() > 1) { + auto checkAllHaveSameAt = [&](size_t pos) { + auto &first = *possible.begin(); + for (auto &p : possible) { + if (p.size() <= pos || p[pos] != first[pos]) + return false; + } + return true; + }; + size_t start = strlen(s); + size_t len = 0; + while (checkAllHaveSameAt(start + len)) ++len; + if (len > 0) { + *match = 1; + auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); + if (!res) throw Error("allocation failure"); + return res; + } } - } - *match = 0; - return nullptr; + *match = 0; + return nullptr; } static int listPossibleCallback(char *s, char ***avp) { - auto possible = curRepl->completePrefix(s); + auto possible = curRepl->completePrefix(s); - if (possible.size() > (INT_MAX / sizeof(char*))) - throw Error("too many completions"); + if (possible.size() > (INT_MAX / sizeof(char*))) + throw Error("too many completions"); - int ac = 0; - char **vp = nullptr; + int ac = 0; + char **vp = nullptr; - auto check = [&](auto *p) { - if (!p) { - if (vp) { - while (--ac >= 0) - free(vp[ac]); - free(vp); - } - throw Error("allocation failure"); - } - return p; - }; + auto check = [&](auto *p) { + if (!p) { + if (vp) { + while (--ac >= 0) + free(vp[ac]); + free(vp); + } + throw Error("allocation failure"); + } + return p; + }; - vp = check((char **)malloc(possible.size() * sizeof(char*))); + vp = check((char **)malloc(possible.size() * sizeof(char*))); - for (auto & p : possible) - vp[ac++] = check(strdup(p.c_str())); + for (auto & p : possible) + vp[ac++] = check(strdup(p.c_str())); - *avp = vp; + *avp = vp; - return ac; + return ac; } namespace { - // Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. - volatile sig_atomic_t g_signal_received = 0; +// Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. +volatile sig_atomic_t g_signal_received = 0; - void sigintHandler(int signo) { - g_signal_received = signo; - } +void sigintHandler(int signo) { + g_signal_received = signo; +} } void NixRepl::mainLoop(const std::vector & files) @@ -211,12 +211,14 @@ void NixRepl::mainLoop(const std::vector & files) // input without clearing the input so far. continue; } else { - printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + printMsg(lvlError, e.msg()); } } catch (Error & e) { - printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + // printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + printMsg(lvlError, e.msg()); } catch (Interrupted & e) { - printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + // printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + printMsg(lvlError, e.msg()); } // We handled the current input fully, so we should clear it @@ -233,24 +235,24 @@ bool NixRepl::getLine(string & input, const std::string &prompt) sigset_t savedSignalMask, set; auto setupSignals = [&]() { - act.sa_handler = sigintHandler; - sigfillset(&act.sa_mask); - act.sa_flags = 0; - if (sigaction(SIGINT, &act, &old)) - throw SysError("installing handler for SIGINT"); + act.sa_handler = sigintHandler; + sigfillset(&act.sa_mask); + act.sa_flags = 0; + if (sigaction(SIGINT, &act, &old)) + throw SysError("installing handler for SIGINT"); - sigemptyset(&set); - sigaddset(&set, SIGINT); - if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) - throw SysError("unblocking SIGINT"); - }; + sigemptyset(&set); + sigaddset(&set, SIGINT); + if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) + throw SysError("unblocking SIGINT"); + }; auto restoreSignals = [&]() { - if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) - throw SysError("restoring signals"); + if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) + throw SysError("restoring signals"); - if (sigaction(SIGINT, &old, 0)) - throw SysError("restoring handler for SIGINT"); - }; + if (sigaction(SIGINT, &old, 0)) + throw SysError("restoring handler for SIGINT"); + }; setupSignals(); char * s = readline(prompt.c_str()); @@ -264,7 +266,7 @@ bool NixRepl::getLine(string & input, const std::string &prompt) } if (!s) - return false; + return false; input += s; input += '\n'; return true; @@ -397,21 +399,21 @@ bool NixRepl::processLine(string line) if (command == ":?" || command == ":help") { std::cout - << "The following commands are available:\n" - << "\n" - << " Evaluate and print expression\n" - << " = Bind expression to variable\n" - << " :a Add attributes from resulting set to scope\n" - << " :b Build derivation\n" - << " :e Open the derivation in $EDITOR\n" - << " :i Build derivation, then install result into current profile\n" - << " :l Load Nix expression and add it to scope\n" - << " :p Evaluate and print expression recursively\n" - << " :q Exit nix-repl\n" - << " :r Reload all files\n" - << " :s Build dependencies of derivation, then start nix-shell\n" - << " :t Describe result of evaluation\n" - << " :u Build derivation, then start nix-shell\n"; + << "The following commands are available:\n" + << "\n" + << " Evaluate and print expression\n" + << " = Bind expression to variable\n" + << " :a Add attributes from resulting set to scope\n" + << " :b Build derivation\n" + << " :e Open the derivation in $EDITOR\n" + << " :i Build derivation, then install result into current profile\n" + << " :l Load Nix expression and add it to scope\n" + << " :p Evaluate and print expression recursively\n" + << " :q Exit nix-repl\n" + << " :r Reload all files\n" + << " :s Build dependencies of derivation, then start nix-shell\n" + << " :t Describe result of evaluation\n" + << " :u Build derivation, then start nix-shell\n"; } else if (command == ":a" || command == ":add") { @@ -637,118 +639,118 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m switch (v.type) { - case tInt: - str << ANSI_CYAN << v.integer << ANSI_NORMAL; - break; + case tInt: + str << ANSI_CYAN << v.integer << ANSI_NORMAL; + break; - case tBool: - str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; - break; + case tBool: + str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; + break; - case tString: - str << ANSI_YELLOW; - printStringValue(str, v.string.s); - str << ANSI_NORMAL; - break; + case tString: + str << ANSI_YELLOW; + printStringValue(str, v.string.s); + str << ANSI_NORMAL; + break; - case tPath: - str << ANSI_GREEN << v.path << ANSI_NORMAL; // !!! escaping? - break; + case tPath: + str << ANSI_GREEN << v.path << ANSI_NORMAL; // !!! escaping? + break; - case tNull: - str << ANSI_CYAN "null" ANSI_NORMAL; - break; + case tNull: + str << ANSI_CYAN "null" ANSI_NORMAL; + break; - case tAttrs: { - seen.insert(&v); + case tAttrs: { + seen.insert(&v); - bool isDrv = state->isDerivation(v); + bool isDrv = state->isDerivation(v); - if (isDrv) { - str << "«derivation "; - Bindings::iterator i = v.attrs->find(state->sDrvPath); - PathSet context; - Path drvPath = i != v.attrs->end() ? state->coerceToPath(*i->pos, *i->value, context) : "???"; - str << drvPath << "»"; + if (isDrv) { + str << "«derivation "; + Bindings::iterator i = v.attrs->find(state->sDrvPath); + PathSet context; + Path drvPath = i != v.attrs->end() ? state->coerceToPath(*i->pos, *i->value, context) : "???"; + str << drvPath << "»"; + } + + else if (maxDepth > 0) { + str << "{ "; + + typedef std::map Sorted; + Sorted sorted; + for (auto & i : *v.attrs) + sorted[i.name] = i.value; + + for (auto & i : sorted) { + if (isVarName(i.first)) + str << i.first; + else + printStringValue(str, i.first.c_str()); + str << " = "; + if (seen.find(i.second) != seen.end()) + str << "«repeated»"; + else + try { + printValue(str, *i.second, maxDepth - 1, seen); + } catch (AssertionError & e) { + str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; + } + str << "; "; + } + + str << "}"; + } else + str << "{ ... }"; + + break; } - else if (maxDepth > 0) { - str << "{ "; + case tList1: + case tList2: + case tListN: + seen.insert(&v); - typedef std::map Sorted; - Sorted sorted; - for (auto & i : *v.attrs) - sorted[i.name] = i.value; + str << "[ "; + if (maxDepth > 0) + for (unsigned int n = 0; n < v.listSize(); ++n) { + if (seen.find(v.listElems()[n]) != seen.end()) + str << "«repeated»"; + else + try { + printValue(str, *v.listElems()[n], maxDepth - 1, seen); + } catch (AssertionError & e) { + str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; + } + str << " "; + } + else + str << "... "; + str << "]"; + break; - for (auto & i : sorted) { - if (isVarName(i.first)) - str << i.first; - else - printStringValue(str, i.first.c_str()); - str << " = "; - if (seen.find(i.second) != seen.end()) - str << "«repeated»"; - else - try { - printValue(str, *i.second, maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << "; "; - } + case tLambda: { + std::ostringstream s; + s << v.lambda.fun->pos; + str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; + break; + } - str << "}"; - } else - str << "{ ... }"; + case tPrimOp: + str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; + break; - break; - } + case tPrimOpApp: + str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; + break; - case tList1: - case tList2: - case tListN: - seen.insert(&v); + case tFloat: + str << v.fpoint; + break; - str << "[ "; - if (maxDepth > 0) - for (unsigned int n = 0; n < v.listSize(); ++n) { - if (seen.find(v.listElems()[n]) != seen.end()) - str << "«repeated»"; - else - try { - printValue(str, *v.listElems()[n], maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << " "; - } - else - str << "... "; - str << "]"; - break; - - case tLambda: { - std::ostringstream s; - s << v.lambda.fun->pos; - str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; - break; - } - - case tPrimOp: - str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; - break; - - case tPrimOpApp: - str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; - break; - - case tFloat: - str << v.fpoint; - break; - - default: - str << ANSI_RED "«unknown»" ANSI_NORMAL; - break; + default: + str << ANSI_RED "«unknown»" ANSI_NORMAL; + break; } return str; @@ -771,10 +773,10 @@ struct CmdRepl : StoreCommand, MixEvalArgs Examples examples() override { return { - Example{ - "Display all special commands within the REPL:", - "nix repl\n nix-repl> :?" - } + Example{ + "Display all special commands within the REPL:", + "nix repl\n nix-repl> :?" + } }; } diff --git a/src/nix/search.cc b/src/nix/search.cc index ba72c1e79..d9c730796 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -216,7 +216,7 @@ struct CmdSearch : SourceExprCommand, MixJSON } catch (AssertionError & e) { } catch (Error & e) { if (!toplevel) { - e.addPrefix(fmt("While evaluating the attribute '%s':\n", attrPath)); + e.addTrace(std::nullopt, hintfmt("While evaluating the attribute '%s'", attrPath)); throw; } } From 13e87535ffa195690213ed656e2b61218c6894a3 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 23 Jun 2020 09:36:58 -0600 Subject: [PATCH 08/33] traces to bottom --- src/libutil/error.cc | 47 +++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 222d9cbef..a0dfdbdbd 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -284,21 +284,20 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) if (einfo.nixCode.has_value()) { switch (einfo.nixCode->errPos.origin) { case foFile: { - out << fmt("%1%in file: " ANSI_BLUE "%2% %3%" ANSI_NORMAL, - prefix, - einfo.nixCode->errPos.file, - showErrPos(einfo.nixCode->errPos)) << std::endl; out << prefix << std::endl; + auto &pos = einfo.nixCode->errPos; + out << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file << std::endl; break; } case foString: { - out << fmt("%1%from command line argument %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; out << prefix << std::endl; + out << fmt("%1%from command line argument %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; break; } case foStdin: { - out << fmt("%1%from stdin %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; out << prefix << std::endl; + out << fmt("%1%from stdin %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; break; } default: @@ -307,22 +306,6 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) nl = true; } - // traces - for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) - { - - try { - auto pos = *iter->pos; - out << iter->hint.str() << showErrPos(pos) << std::endl; - NixCode nc { .errPos = pos }; - getCodeLines(nc); - printCodeLines(out, prefix, nc); - } catch(const std::bad_optional_access& e) { - out << iter->hint.str() << std::endl; - } - - } - // description if (einfo.description != "") { if (nl) @@ -352,6 +335,26 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) nl = true; } + // traces + for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) + { + try { + auto pos = *iter->pos; + if (nl) + out << std::endl << prefix; + out << std::endl << prefix; + out << iter->hint.str() << std::endl; + out << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file << std::endl; + nl = true; + NixCode nc { .errPos = pos }; + getCodeLines(nc); + printCodeLines(out, prefix, nc); + } catch(const std::bad_optional_access& e) { + out << iter->hint.str() << std::endl; + } + } + return out; } } From d0e78fbb03e89c8a070e0c50daeda06b055669fc Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 23 Jun 2020 10:51:58 -0600 Subject: [PATCH 09/33] re-add Pos origin in tests --- src/libutil/tests/logging.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 6a6fb4ac3..ac4a1b161 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -163,7 +163,7 @@ namespace nix { "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), + .errPos = Pos(foFile, problem_file, 40, 13), .prevLineOfCode = "previous line of code", .errLineOfCode = "this is the problem line of code", .nextLineOfCode = "next line of code", @@ -186,7 +186,7 @@ namespace nix { "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(foFile, problem_file, 40, 13) }}); auto str = testing::internal::GetCapturedStderr(); @@ -202,7 +202,7 @@ namespace nix { .name = "error name", .hint = hintfmt("hint %1%", "only"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(foFile, problem_file, 40, 13) }}); auto str = testing::internal::GetCapturedStderr(); @@ -241,7 +241,7 @@ namespace nix { "yellow", "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), + .errPos = Pos(foFile, problem_file, 40, 13), .prevLineOfCode = std::nullopt, .errLineOfCode = "this is the problem line of code", .nextLineOfCode = std::nullopt From 1d43a6e123e679112dfdfda393e3b6eef99eecf6 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 23 Jun 2020 15:30:13 -0600 Subject: [PATCH 10/33] use plain errPos instead of nixCode; fix tests --- src/libexpr/attr-set.hh | 2 +- src/libexpr/eval-inline.hh | 4 +- src/libexpr/eval.cc | 16 ++--- src/libexpr/nixexpr.cc | 2 +- src/libexpr/nixexpr.hh | 2 +- src/libexpr/parser.y | 19 +++--- src/libexpr/primops.cc | 90 +++++++++++++-------------- src/libexpr/primops/context.cc | 6 +- src/libexpr/primops/fetchGit.cc | 4 +- src/libexpr/primops/fetchMercurial.cc | 4 +- src/libexpr/primops/fetchTree.cc | 6 +- src/libexpr/primops/fromTOML.cc | 2 +- src/libutil/error.cc | 55 ++++------------ src/libutil/error.hh | 2 +- src/libutil/tests/logging.cc | 55 +++++++--------- 15 files changed, 116 insertions(+), 153 deletions(-) diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh index c601d09c2..7eaa16c59 100644 --- a/src/libexpr/attr-set.hh +++ b/src/libexpr/attr-set.hh @@ -78,7 +78,7 @@ public: if (!a) throw Error({ .hint = hintfmt("attribute '%s' missing", name), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); return *a; diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 3d544c903..30f6ec7db 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -11,7 +11,7 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s)) { throw EvalError({ .hint = hintfmt(s), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -25,7 +25,7 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const { throw TypeError({ .hint = hintfmt(s, showType(v)), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c249add86..3fd8aa285 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -529,7 +529,7 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const { throw EvalError({ .hint = hintfmt(s, s2), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -542,7 +542,7 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const { throw EvalError({ .hint = hintfmt(s, s2, s3), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -551,7 +551,7 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & p1, const char * s, const // p1 is where the error occurred; p2 is a position mentioned in the message. throw EvalError({ .hint = hintfmt(s, sym, p2), - .nixCode = NixCode { .errPos = p1 } + .errPos = p1 }); } @@ -559,7 +559,7 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s)) { throw TypeError({ .hint = hintfmt(s), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -572,7 +572,7 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const { throw TypeError({ .hint = hintfmt(s, fun.showNamePos(), s2), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -580,7 +580,7 @@ LocalNoInlineNoReturn(void throwAssertionError(const Pos & pos, const char * s, { throw AssertionError({ .hint = hintfmt(s, s1), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -588,7 +588,7 @@ LocalNoInlineNoReturn(void throwUndefinedVarError(const Pos & pos, const char * { throw UndefinedVarError({ .hint = hintfmt(s, s1), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -1938,7 +1938,7 @@ string ExternalValueBase::coerceToString(const Pos & pos, PathSet & context, boo { throw TypeError({ .hint = hintfmt("cannot coerce %1% to a string", showType()), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index d4e179008..2b22ef3b6 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -285,7 +285,7 @@ void ExprVar::bindVars(const StaticEnv & env) if (withLevel == -1) throw UndefinedVarError({ .hint = hintfmt("undefined variable '%1%'", name), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); fromWith = true; this->level = withLevel; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index b1bf9f67b..e4cbc660f 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -239,7 +239,7 @@ struct ExprLambda : Expr if (!arg.empty() && formals && formals->argNames.find(arg) != formals->argNames.end()) throw ParseError({ .hint = hintfmt("duplicate formal function argument '%1%'", arg), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); }; void setName(Symbol & name); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index b746e70af..472b33c2f 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -66,18 +66,17 @@ namespace nix { static void dupAttr(const AttrPath & attrPath, const Pos & pos, const Pos & prevPos) { throw ParseError({ - .hint = hintfmt("attribute '%1%' already defined at %2%", + .hint = hintfmt("attribute '%1%' already defined at %2%", showAttrPath(attrPath), prevPos), - .nixCode = NixCode { .errPos = pos }, + .errPos = pos }); } - static void dupAttr(Symbol attr, const Pos & pos, const Pos & prevPos) { throw ParseError({ .hint = hintfmt("attribute '%1%' already defined at %2%", attr, prevPos), - .nixCode = NixCode { .errPos = pos }, + .errPos = pos }); } @@ -149,7 +148,7 @@ static void addFormal(const Pos & pos, Formals * formals, const Formal & formal) throw ParseError({ .hint = hintfmt("duplicate formal function argument '%1%'", formal.name), - .nixCode = NixCode { .errPos = pos }, + .errPos = pos }); formals->formals.push_front(formal); } @@ -260,7 +259,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err { data->error = { .hint = hintfmt(error), - .nixCode = NixCode { .errPos = makeCurPos(*loc, data) } + .errPos = makeCurPos(*loc, data) }; } @@ -340,7 +339,7 @@ expr_function { if (!$2->dynamicAttrs.empty()) throw ParseError({ .hint = hintfmt("dynamic attributes not allowed in let"), - .nixCode = NixCode { .errPos = CUR_POS }, + .errPos = CUR_POS }); $$ = new ExprLet($2, $4); } @@ -420,7 +419,7 @@ expr_simple if (noURLLiterals) throw ParseError({ .hint = hintfmt("URL literals are disabled"), - .nixCode = NixCode { .errPos = CUR_POS } + .errPos = CUR_POS }); $$ = new ExprString(data->symbols.create($1)); } @@ -493,7 +492,7 @@ attrs } else throw ParseError({ .hint = hintfmt("dynamic attributes not allowed in inherit"), - .nixCode = NixCode { .errPos = makeCurPos(@2, data) }, + .errPos = makeCurPos(@2, data) }); } | { $$ = new AttrPath; } @@ -705,7 +704,7 @@ Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos ? "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), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3f6bd7969..6f7a691cd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -96,7 +96,7 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args } catch (InvalidPathError & e) { throw EvalError({ .hint = hintfmt("cannot import '%1%', since path '%2%' is not valid", path, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -177,7 +177,7 @@ void prim_importNative(EvalState & state, const Pos & pos, Value * * args, Value .hint = hintfmt( "cannot import '%1%', since path '%2%' is not valid", path, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -215,7 +215,7 @@ void prim_exec(EvalState & state, const Pos & pos, Value * * args, Value & v) if (count == 0) { throw EvalError({ .hint = hintfmt("at least one argument to 'exec' required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } PathSet context; @@ -230,7 +230,7 @@ void prim_exec(EvalState & state, const Pos & pos, Value * * args, Value & v) throw EvalError({ .hint = hintfmt("cannot execute '%1%', since path '%2%' is not valid", program, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -385,7 +385,7 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar if (startSet == args[0]->attrs->end()) throw EvalError({ .hint = hintfmt("attribute 'startSet' required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.forceList(*startSet->value, pos); @@ -399,7 +399,7 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar if (op == args[0]->attrs->end()) throw EvalError({ .hint = hintfmt("attribute 'operator' required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.forceValue(*op->value, pos); @@ -421,7 +421,7 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar if (key == e->attrs->end()) throw EvalError({ .hint = hintfmt("attribute 'key' required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.forceValue(*key->value, pos); @@ -557,7 +557,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (attr == args[0]->attrs->end()) throw EvalError({ .hint = hintfmt("required attribute 'name' missing"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); string drvName; Pos & posDrvName(*attr->pos); @@ -604,7 +604,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * else throw EvalError({ .hint = hintfmt("invalid value '%s' for 'outputHashMode' attribute", s), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); }; @@ -614,7 +614,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (outputs.find(j) != outputs.end()) throw EvalError({ .hint = hintfmt("duplicate derivation output '%1%'", j), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); /* !!! Check whether j is a valid attribute name. */ @@ -624,14 +624,14 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (j == "drv") throw EvalError({ .hint = hintfmt("invalid derivation output name 'drv'" ), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); outputs.insert(j); } if (outputs.empty()) throw EvalError({ .hint = hintfmt("derivation cannot have an empty set of outputs"), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); }; @@ -747,20 +747,20 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (drv.builder == "") throw EvalError({ .hint = hintfmt("required attribute 'builder' missing"), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); if (drv.platform == "") throw EvalError({ .hint = hintfmt("required attribute 'system' missing"), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); /* Check whether the derivation name is valid. */ if (isDerivation(drvName)) throw EvalError({ .hint = hintfmt("derivation names are not allowed to end in '%s'", drvExtension), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); if (outputHash) { @@ -768,7 +768,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (outputs.size() != 1 || *(outputs.begin()) != "out") throw Error({ .hint = hintfmt("multiple outputs are not supported in fixed-output derivations"), - .nixCode = NixCode { .errPos = posDrvName } + .errPos = posDrvName }); std::optional ht = parseHashTypeOpt(outputHashAlgo); @@ -882,7 +882,7 @@ static void prim_storePath(EvalState & state, const Pos & pos, Value * * args, V if (!state.store->isInStore(path)) throw EvalError({ .hint = hintfmt("path '%1%' is not in the Nix store", path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); Path path2 = state.store->toStorePath(path); if (!settings.readOnlyMode) @@ -903,7 +903,7 @@ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args, .hint = hintfmt( "cannot check the existence of '%1%', since path '%2%' is not valid", path, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -949,7 +949,7 @@ static void prim_readFile(EvalState & state, const Pos & pos, Value * * args, Va } catch (InvalidPathError & e) { throw EvalError({ .hint = hintfmt("cannot read '%1%', since path '%2%' is not valid", path, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } string s = readFile(state.checkSourcePath(state.toRealPath(path, context))); @@ -980,7 +980,7 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va if (i == v2.attrs->end()) throw EvalError({ .hint = hintfmt("attribute 'path' missing"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); PathSet context; @@ -991,7 +991,7 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va } catch (InvalidPathError & e) { throw EvalError({ .hint = hintfmt("cannot find '%1%', since path '%2%' is not valid", path, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -1011,7 +1011,7 @@ static void prim_hashFile(EvalState & state, const Pos & pos, Value * * args, Va if (!ht) throw Error({ .hint = hintfmt("unknown hash type '%1%'", type), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); PathSet context; // discarded @@ -1030,7 +1030,7 @@ static void prim_readDir(EvalState & state, const Pos & pos, Value * * args, Val } catch (InvalidPathError & e) { throw EvalError({ .hint = hintfmt("cannot read '%1%', since path '%2%' is not valid", path, e.path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } @@ -1106,7 +1106,7 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu "in 'toFile': the file named '%1%' must not contain a reference " "to a derivation but contains (%2%)", name, path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); refs.insert(state.store->parseStorePath(path)); } @@ -1177,7 +1177,7 @@ static void prim_filterSource(EvalState & state, const Pos & pos, Value * * args if (!context.empty()) throw EvalError({ .hint = hintfmt("string '%1%' cannot refer to other paths", path), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.forceValue(*args[0], pos); @@ -1186,7 +1186,7 @@ static void prim_filterSource(EvalState & state, const Pos & pos, Value * * args .hint = hintfmt( "first argument in call to 'filterSource' is not a function but %1%", showType(*args[0])), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); addPath(state, pos, std::string(baseNameOf(path)), path, args[0], FileIngestionMethod::Recursive, Hash(), v); @@ -1209,7 +1209,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value if (!context.empty()) throw EvalError({ .hint = hintfmt("string '%1%' cannot refer to other paths", path), - .nixCode = NixCode { .errPos = *attr.pos } + .errPos = *attr.pos }); } else if (attr.name == state.sName) name = state.forceStringNoCtx(*attr.value, *attr.pos); @@ -1223,13 +1223,13 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value else throw EvalError({ .hint = hintfmt("unsupported argument '%1%' to 'addPath'", attr.name), - .nixCode = NixCode { .errPos = *attr.pos } + .errPos = *attr.pos }); } if (path.empty()) throw EvalError({ .hint = hintfmt("'path' required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); if (name.empty()) name = baseNameOf(path); @@ -1290,7 +1290,7 @@ void prim_getAttr(EvalState & state, const Pos & pos, Value * * args, Value & v) if (i == args[1]->attrs->end()) throw EvalError({ .hint = hintfmt("attribute '%1%' missing", attr), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); // !!! add to stack trace? if (state.countCalls && i->pos) state.attrSelects[*i->pos]++; @@ -1373,7 +1373,7 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args, if (j == v2.attrs->end()) throw TypeError({ .hint = hintfmt("'name' attribute missing in a call to 'listToAttrs'"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); string name = state.forceStringNoCtx(*j->value, pos); @@ -1383,7 +1383,7 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args, if (j2 == v2.attrs->end()) throw TypeError({ .hint = hintfmt("'value' attribute missing in a call to 'listToAttrs'"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); v.attrs->push_back(Attr(sym, j2->value, j2->pos)); } @@ -1459,7 +1459,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args if (args[0]->type != tLambda) throw TypeError({ .hint = hintfmt("'functionArgs' requires a function"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); if (!args[0]->lambda.fun->matchAttrs) { @@ -1515,7 +1515,7 @@ static void elemAt(EvalState & state, const Pos & pos, Value & list, int n, Valu if (n < 0 || (unsigned int) n >= list.listSize()) throw Error({ .hint = hintfmt("list index %1% is out of bounds", n), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.forceValue(*list.listElems()[n], pos); v = *list.listElems()[n]; @@ -1545,7 +1545,7 @@ static void prim_tail(EvalState & state, const Pos & pos, Value * * args, Value if (args[0]->listSize() == 0) throw Error({ .hint = hintfmt("'tail' called on an empty list"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.mkList(v, args[0]->listSize() - 1); @@ -1690,7 +1690,7 @@ static void prim_genList(EvalState & state, const Pos & pos, Value * * args, Val if (len < 0) throw EvalError({ .hint = hintfmt("cannot create list of size %1%", len), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); state.mkList(v, len); @@ -1852,7 +1852,7 @@ static void prim_div(EvalState & state, const Pos & pos, Value * * args, Value & if (f2 == 0) throw EvalError({ .hint = hintfmt("division by zero"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); if (args[0]->type == tFloat || args[1]->type == tFloat) { @@ -1864,7 +1864,7 @@ static void prim_div(EvalState & state, const Pos & pos, Value * * args, Value & if (i1 == std::numeric_limits::min() && i2 == -1) throw EvalError({ .hint = hintfmt("overflow in integer division"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); mkInt(v, i1 / i2); @@ -1925,7 +1925,7 @@ static void prim_substring(EvalState & state, const Pos & pos, Value * * args, V if (start < 0) throw EvalError({ .hint = hintfmt("negative start position in 'substring'"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); mkString(v, (unsigned int) start >= s.size() ? "" : string(s, start, len), context); @@ -1948,7 +1948,7 @@ static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, if (!ht) throw Error({ .hint = hintfmt("unknown hash type '%1%'", type), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); PathSet context; // discarded @@ -1994,12 +1994,12 @@ void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) // limit is _GLIBCXX_REGEX_STATE_LIMIT for libstdc++ throw EvalError({ .hint = hintfmt("memory limit exceeded by regular expression '%s'", re), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } else { throw EvalError({ .hint = hintfmt("invalid regular expression '%s'", re), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } } @@ -2067,12 +2067,12 @@ static void prim_split(EvalState & state, const Pos & pos, Value * * args, Value // limit is _GLIBCXX_REGEX_STATE_LIMIT for libstdc++ throw EvalError({ .hint = hintfmt("memory limit exceeded by regular expression '%s'", re), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } else { throw EvalError({ .hint = hintfmt("invalid regular expression '%s'", re), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } } @@ -2106,7 +2106,7 @@ static void prim_replaceStrings(EvalState & state, const Pos & pos, Value * * ar if (args[0]->listSize() != args[1]->listSize()) throw EvalError({ .hint = hintfmt("'from' and 'to' arguments to 'replaceStrings' have different lengths"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); vector from; diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 301e8c5dd..dbb93bae6 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -148,7 +148,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg if (!state.store->isStorePath(i.name)) throw EvalError({ .hint = hintfmt("Context key '%s' is not a store path", i.name), - .nixCode = NixCode { .errPos = *i.pos } + .errPos = *i.pos }); if (!settings.readOnlyMode) state.store->ensurePath(state.store->parseStorePath(i.name)); @@ -165,7 +165,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg if (!isDerivation(i.name)) { throw EvalError({ .hint = hintfmt("Tried to add all-outputs context of %s, which is not a derivation, to a string", i.name), - .nixCode = NixCode { .errPos = *i.pos } + .errPos = *i.pos }); } context.insert("=" + string(i.name)); @@ -178,7 +178,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg if (iter->value->listSize() && !isDerivation(i.name)) { throw EvalError({ .hint = hintfmt("Tried to add derivation output context of %s, which is not a derivation, to a string", i.name), - .nixCode = NixCode { .errPos = *i.pos } + .errPos = *i.pos }); } for (unsigned int n = 0; n < iter->value->listSize(); ++n) { diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc index dd7229a3d..36b0db2bd 100644 --- a/src/libexpr/primops/fetchGit.cc +++ b/src/libexpr/primops/fetchGit.cc @@ -37,14 +37,14 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va else throw EvalError({ .hint = hintfmt("unsupported argument '%s' to 'fetchGit'", attr.name), - .nixCode = NixCode { .errPos = *attr.pos } + .errPos = *attr.pos }); } if (url.empty()) throw EvalError({ .hint = hintfmt("'url' argument required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } else diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 9bace8f89..59166b777 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -40,14 +40,14 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar else throw EvalError({ .hint = hintfmt("unsupported argument '%s' to 'fetchMercurial'", attr.name), - .nixCode = NixCode { .errPos = *attr.pos } + .errPos = *attr.pos }); } if (url.empty()) throw EvalError({ .hint = hintfmt("'url' argument required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } else diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 9be93710a..01d6ad8b0 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -68,7 +68,7 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V if (!attrs.count("type")) throw Error({ .hint = hintfmt("attribute 'type' is missing in call to 'fetchTree'"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); input = fetchers::inputFromAttrs(attrs); @@ -112,14 +112,14 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, else throw EvalError({ .hint = hintfmt("unsupported argument '%s' to '%s'", attr.name, who), - .nixCode = NixCode { .errPos = *attr.pos } + .errPos = *attr.pos }); } if (!url) throw EvalError({ .hint = hintfmt("'url' argument required"), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } else url = state.forceStringNoCtx(*args[0], pos); diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 7615d1379..b00827a4b 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -83,7 +83,7 @@ static void prim_fromTOML(EvalState & state, const Pos & pos, Value * * args, Va } catch (std::runtime_error & e) { throw EvalError({ .hint = hintfmt("while parsing a TOML string: %s", e.what()), - .nixCode = NixCode { .errPos = pos } + .errPos = pos }); } } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index a0dfdbdbd..322dfea3b 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -10,45 +10,14 @@ namespace nix { const std::string nativeSystem = SYSTEM; -// addPrefix is used for show-trace. Strings added with addPrefix -// will print ahead of the error itself. -// BaseError & BaseError::addPrefix(const FormatOrString & fs) -// { -// prefix_ = fs.s + prefix_; -// return *this; -// } - -// const string & prefix() const -// { -// // build prefix string on demand?? - -// } -// ; // { return prefix_; } - - -// addPrefix is used for show-trace. Strings added with addPrefix -// will print ahead of the error itself. +// Traces show the chain of calls in nix code. If an ErrPos is included the surrounding +// lines of code will print. BaseError & BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint}); return *this; } -// const string& BaseError::calcTrace() const -// { -// if (trace_.has_value()) -// return *trace_; -// else { -// err.name = sname(); - -// std::ostringstream oss; -// oss << err; -// trace_ = oss.str(); - -// return *trace_; -// } -// } - // c++ std::exception descendants must have a 'const char* what()' function. // This stringifies the error and caches it for use by what(), or similarly by msg(). const string& BaseError::calcWhat() const @@ -281,23 +250,25 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) einfo.programName.value_or("")); bool nl = false; // intersperse newline between sections. - if (einfo.nixCode.has_value()) { - switch (einfo.nixCode->errPos.origin) { + if (einfo.errPos.has_value()) { + switch (einfo.errPos->origin) { case foFile: { out << prefix << std::endl; - auto &pos = einfo.nixCode->errPos; - out << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << - ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file << std::endl; + auto &pos = *einfo.errPos; + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file; break; } case foString: { out << prefix << std::endl; - out << fmt("%1%from command line argument %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << + ANSI_BLUE << " from command line argument" << ANSI_NORMAL; break; } case foStdin: { out << prefix << std::endl; - out << fmt("%1%from stdin %2%", prefix, showErrPos(einfo.nixCode->errPos)) << std::endl; + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << + ANSI_BLUE << " from stdin" << ANSI_NORMAL; break; } default: @@ -314,8 +285,8 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) nl = true; } - if (einfo.nixCode.has_value()) { - NixCode nixcode = *einfo.nixCode; + if (einfo.errPos.has_value()) { + NixCode nixcode { .errPos = *einfo.errPos }; getCodeLines(nixcode); // lines of code. diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 75ba80365..d992c4ece 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -103,7 +103,7 @@ struct ErrorInfo { string name; string description; std::optional hint; - std::optional nixCode; + std::optional errPos; std::list traces; static std::optional programName; diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index ac4a1b161..0bbec482f 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -11,6 +11,13 @@ namespace nix { * logEI * --------------------------------------------------------------------------*/ + const char *test_file = + "previous line of code\n" + "this is the problem line of code\n" + "next line of code\n"; + const char *one_liner = + "this is the problem line of code"; + TEST(logEI, catpuresBasicProperties) { MakeError(TestError, Error); @@ -152,7 +159,7 @@ namespace nix { TEST(logError, logErrorWithPreviousAndNextLinesOfCode) { SymbolTable testTable; - auto problem_file = testTable.create("myfile.nix"); + auto problem_file = testTable.create(test_file); testing::internal::CaptureStderr(); @@ -162,21 +169,16 @@ namespace nix { .hint = hintfmt("this hint has %1% templated %2%!!", "yellow", "values"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13), - .prevLineOfCode = "previous line of code", - .errLineOfCode = "this is the problem line of code", - .nextLineOfCode = "next line of code", - }}); - + .errPos = Pos(foString, problem_file, 02, 13), + }); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\nin file: \x1B[34;1mmyfile.nix (40:13)\x1B[0m\n\nerror with code lines\n\n 39| previous line of code\n 40| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 41| next line of code\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\nerror with code lines\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); } - TEST(logError, logErrorWithoutLinesOfCode) { + TEST(logError, logErrorWithInvalidFile) { SymbolTable testTable; - auto problem_file = testTable.create("myfile.nix"); + auto problem_file = testTable.create("invalid filename"); testing::internal::CaptureStderr(); logError({ @@ -185,28 +187,23 @@ namespace nix { .hint = hintfmt("this hint has %1% templated %2%!!", "yellow", "values"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13) - }}); + .errPos = Pos(foFile, problem_file, 02, 13) + }); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\nin file: \x1B[34;1mmyfile.nix (40:13)\x1B[0m\n\nerror without any code lines.\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1minvalid filename\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m in file: \x1B[0minvalid filename\n\nerror without any code lines.\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); } TEST(logError, logErrorWithOnlyHintAndName) { - SymbolTable testTable; - auto problem_file = testTable.create("myfile.nix"); testing::internal::CaptureStderr(); logError({ .name = "error name", .hint = hintfmt("hint %1%", "only"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13) - }}); + }); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\nin file: \x1B[34;1mmyfile.nix (40:13)\x1B[0m\n\nhint \x1B[33;1monly\x1B[0m\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\nhint \x1B[33;1monly\x1B[0m\n"); } @@ -219,18 +216,18 @@ namespace nix { logWarning({ .name = "name", - .description = "error description", + .description = "warning description", .hint = hintfmt("there was a %1%", "warning"), }); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- name --- error-unit-test\x1B[0m\nerror description\n\nthere was a \x1B[33;1mwarning\x1B[0m\n"); + ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- name --- error-unit-test\x1B[0m\nwarning description\n\nthere was a \x1B[33;1mwarning\x1B[0m\n"); } TEST(logWarning, logWarningWithFileLineNumAndCode) { SymbolTable testTable; - auto problem_file = testTable.create("myfile.nix"); + auto problem_file = testTable.create(test_file); testing::internal::CaptureStderr(); @@ -240,16 +237,12 @@ namespace nix { .hint = hintfmt("this hint has %1% templated %2%!!", "yellow", "values"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13), - .prevLineOfCode = std::nullopt, - .errLineOfCode = "this is the problem line of code", - .nextLineOfCode = std::nullopt - }}); + .errPos = Pos(foStdin, problem_file, 2, 13), + }); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- warning name --- error-unit-test\x1B[0m\nin file: \x1B[34;1mmyfile.nix (40:13)\x1B[0m\n\nwarning description\n\n 40| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); + ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- warning name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from stdin\x1B[0m\n\nwarning description\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); } /* ---------------------------------------------------------------------------- From 00fe653ea5bcc9b130107919d3238f6f7da0808d Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 08:33:53 -0600 Subject: [PATCH 11/33] nixCode -> LinesOfCode --- src/libutil/error.cc | 104 ++++++++++++++++++++++++------------------- src/libutil/error.hh | 14 +++--- tests/misc.sh | 4 +- 3 files changed, 67 insertions(+), 55 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 322dfea3b..560a1b8f2 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -11,7 +11,7 @@ namespace nix { const std::string nativeSystem = SYSTEM; // Traces show the chain of calls in nix code. If an ErrPos is included the surrounding -// lines of code will print. +// lines of code will print. BaseError & BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint}); @@ -56,22 +56,25 @@ string showErrPos(const ErrPos &errPos) } } -void getCodeLines(NixCode &nixCode) +std::optional getCodeLines(const ErrPos &errPos) { - if (nixCode.errPos.line <= 0) - return; + if (errPos.line <= 0) + return std::nullopt; - if (nixCode.errPos.origin == foFile) { + if (errPos.origin == foFile) { + LinesOfCode loc; try { - AutoCloseFD fd = open(nixCode.errPos.file.c_str(), O_RDONLY | O_CLOEXEC); - if (!fd) - logError(SysError("opening file '%1%'", nixCode.errPos.file).info()); + AutoCloseFD fd = open(errPos.file.c_str(), O_RDONLY | O_CLOEXEC); + if (!fd) { + logError(SysError("opening file '%1%'", errPos.file).info()); + return std::nullopt; + } else { // count the newlines. int count = 0; string line; - int pl = nixCode.errPos.line - 1; + int pl = errPos.line - 1; do { line = readLine(fd.get()); @@ -81,28 +84,33 @@ void getCodeLines(NixCode &nixCode) ; } else if (count == pl) { - nixCode.prevLineOfCode = line; + loc.prevLineOfCode = line; } else if (count == pl + 1) { - nixCode.errLineOfCode = line; + loc.errLineOfCode = line; } else if (count == pl + 2) { - nixCode.nextLineOfCode = line; + loc.nextLineOfCode = line; break; } } while (true); + return loc; } } catch (EndOfFile &eof) { - ; + // TODO: return maybe partial loc? + return std::nullopt; } catch (std::exception &e) { - printError("error reading nix file: %s\n%s", nixCode.errPos.file, e.what()); + printError("error reading nix file: %s\n%s", errPos.file, e.what()); + return std::nullopt; } } else { - std::istringstream iss(nixCode.errPos.file); + std::istringstream iss(errPos.file); // count the newlines. int count = 0; string line; - int pl = nixCode.errPos.line - 1; + int pl = errPos.line - 1; + + LinesOfCode loc; do { @@ -113,42 +121,47 @@ void getCodeLines(NixCode &nixCode) ; } else if (count == pl) { - nixCode.prevLineOfCode = line; + loc.prevLineOfCode = line; } else if (count == pl + 1) { - nixCode.errLineOfCode = line; + loc.errLineOfCode = line; } else if (count == pl + 2) { - nixCode.nextLineOfCode = line; + loc.nextLineOfCode = line; break; } if (!iss.good()) break; } while (true); + + return loc; } } // if nixCode contains lines of code, print them to the ostream, indicating the error column. -void printCodeLines(std::ostream &out, const string &prefix, const NixCode &nixCode) +void printCodeLines(std::ostream &out, + const string &prefix, + const ErrPos &errPos, + const LinesOfCode &loc) { // previous line of code. - if (nixCode.prevLineOfCode.has_value()) { + if (loc.prevLineOfCode.has_value()) { out << std::endl << fmt("%1% %|2$5d|| %3%", prefix, - (nixCode.errPos.line - 1), - *nixCode.prevLineOfCode); + (errPos.line - 1), + *loc.prevLineOfCode); } - if (nixCode.errLineOfCode.has_value()) { + if (loc.errLineOfCode.has_value()) { // line of code containing the error. out << std::endl << fmt("%1% %|2$5d|| %3%", prefix, - (nixCode.errPos.line), - *nixCode.errLineOfCode); + (errPos.line), + *loc.errLineOfCode); // error arrows for the column range. - if (nixCode.errPos.column > 0) { - int start = nixCode.errPos.column; + if (errPos.column > 0) { + int start = errPos.column; std::string spaces; for (int i = 0; i < start; ++i) { spaces.append(" "); @@ -165,12 +178,12 @@ void printCodeLines(std::ostream &out, const string &prefix, const NixCode &nixC } // next line of code. - if (nixCode.nextLineOfCode.has_value()) { + if (loc.nextLineOfCode.has_value()) { out << std::endl << fmt("%1% %|2$5d|| %3%", prefix, - (nixCode.errPos.line + 1), - *nixCode.nextLineOfCode); + (errPos.line + 1), + *loc.nextLineOfCode); } } @@ -255,20 +268,20 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) case foFile: { out << prefix << std::endl; auto &pos = *einfo.errPos; - out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << - ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file; + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file; break; } case foString: { out << prefix << std::endl; - out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << - ANSI_BLUE << " from command line argument" << ANSI_NORMAL; + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << + ANSI_BLUE << " from command line argument" << ANSI_NORMAL; break; } case foStdin: { out << prefix << std::endl; - out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << - ANSI_BLUE << " from stdin" << ANSI_NORMAL; + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << + ANSI_BLUE << " from stdin" << ANSI_NORMAL; break; } default: @@ -286,14 +299,13 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) } if (einfo.errPos.has_value()) { - NixCode nixcode { .errPos = *einfo.errPos }; - getCodeLines(nixcode); + auto loc = getCodeLines(*einfo.errPos); // lines of code. - if (nixcode.errLineOfCode.has_value()) { + if (loc.has_value()) { if (nl) out << std::endl << prefix; - printCodeLines(out, prefix, nixcode); + printCodeLines(out, prefix, *einfo.errPos, *loc); nl = true; } } @@ -315,12 +327,12 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) out << std::endl << prefix; out << std::endl << prefix; out << iter->hint.str() << std::endl; - out << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << - ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file << std::endl; + out << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file << std::endl; nl = true; - NixCode nc { .errPos = pos }; - getCodeLines(nc); - printCodeLines(out, prefix, nc); + auto loc = getCodeLines(pos); + if (loc.has_value()) + printCodeLines(out, prefix, pos, *loc); } catch(const std::bad_optional_access& e) { out << iter->hint.str() << std::endl; } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index d992c4ece..7185033f5 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -56,6 +56,13 @@ typedef enum { foString } FileOrigin; +// the lines of code surrounding an error. +struct LinesOfCode { + std::optional prevLineOfCode; + std::optional errLineOfCode; + std::optional nextLineOfCode; +}; + // ErrPos indicates the location of an error in a nix file. struct ErrPos { int line = 0; @@ -86,13 +93,6 @@ struct ErrPos { } }; -struct NixCode { - ErrPos errPos; - std::optional prevLineOfCode; - std::optional errLineOfCode; - std::optional nextLineOfCode; -}; - struct Trace { std::optional pos; hintformat hint; diff --git a/tests/misc.sh b/tests/misc.sh index 0804622ea..77a52995a 100644 --- a/tests/misc.sh +++ b/tests/misc.sh @@ -17,10 +17,10 @@ nix-env -q --foo 2>&1 | grep "unknown flag" # Eval Errors. eval_arg_res=$(nix-instantiate --eval -E 'let a = {} // a; in a.foo' 2>&1 || true) -echo $eval_arg_res | grep "command line argument (1:15)" +echo $eval_arg_res | grep "at: (1:15) from command line argument" echo $eval_arg_res | grep "infinite recursion encountered" eval_stdin_res=$(echo 'let a = {} // a; in a.foo' | nix-instantiate --eval -E - 2>&1 || true) -echo $eval_stdin_res | grep "stdin (1:15)" +echo $eval_stdin_res | grep "at: (1:15) from stdin" echo $eval_stdin_res | grep "infinite recursion encountered" From 6fe660acf902fff137e52e8d8659a7b49897c74c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 12:33:05 -0600 Subject: [PATCH 12/33] re-remove --- src/error-demo/error-demo.cc | 162 ----------------------------------- 1 file changed, 162 deletions(-) delete mode 100644 src/error-demo/error-demo.cc diff --git a/src/error-demo/error-demo.cc b/src/error-demo/error-demo.cc deleted file mode 100644 index 8c73a8c83..000000000 --- a/src/error-demo/error-demo.cc +++ /dev/null @@ -1,162 +0,0 @@ -#include "logging.hh" -#include "nixexpr.hh" -#include "util.hh" - -#include -#include - -using namespace nix; - -MakeError(DemoError, Error); - -int main() -{ - verbosity = lvlVomit; - - // In each program where errors occur, this has to be set. - ErrorInfo::programName = std::optional("error-demo"); - - // 'DemoError' appears as error name. - try { - throw DemoError("demo error was thrown"); - } catch (Error &e) { - logger->logEI(e.info()); - } - - // appending to the hint from the previous error - try { - auto e = Error("initial error"); - throw DemoError(e.info()); - } catch (Error &e) { - ErrorInfo ei = e.info(); - // using normaltxt to avoid the default yellow highlighting. - ei.hint = hintfmt("%s; subsequent error message.", - normaltxt(e.info().hint ? e.info().hint->str() : "")); - logger->logEI(ei); - } - - // SysError; picks up errno - try { - auto x = readFile(-1); - } - catch (SysError &e) { - std::cout << "errno was: " << e.errNo << std::endl; - logError(e.info()); - } - - // current exception - try { - throw DemoError("DemoError handled as a %1%", "std::exception"); - } - catch (...) { - const std::exception_ptr &eptr = std::current_exception(); - try - { - std::rethrow_exception(eptr); - } - catch (std::exception& e) - { - std::cerr << e.what() << std::endl; - } - } - - // For completeness sake, show 'info' through 'vomit' levels. - // But this is maybe a heavy format for those. - logger->logEI( - ErrorInfo { .level = lvlInfo, - .name = "Info name", - .description = "Info description", - }); - - logger->logEI( - ErrorInfo { .level = lvlTalkative, - .name = "Talkative name", - .description = "Talkative description", - }); - - logger->logEI( - ErrorInfo { .level = lvlChatty, - .name = "Chatty name", - .description = "Chatty description", - }); - - logger->logEI( - ErrorInfo { .level = lvlDebug, - .name = "Debug name", - .description = "Debug description", - }); - - logger->logEI( - ErrorInfo { .level = lvlVomit, - .name = "Vomit name", - .description = "Vomit description", - }); - - // Error in a program; no hint and no nix code. - logError( - ErrorInfo { .name = "name", - .description = "error description", - }); - - // Warning with name, description, and hint. - // The hintfmt function makes all the substituted text yellow. - logWarning( - ErrorInfo { .name = "name", - .description = "error description", - .hint = hintfmt("there was a %1%", "warning"), - }); - - // Warning with nix file, line number, column, and the lines of - // code where a warning occurred. - SymbolTable testTable; - auto problem_file = testTable.create("myfile.nix"); - - logWarning( - ErrorInfo { .name = "warning name", - .description = "warning description", - .hint = hintfmt("this hint has %1% templated %2%!!", - "yellow", - "values"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13), - .prevLineOfCode = std::nullopt, - .errLineOfCode = "this is the problem line of code", - .nextLineOfCode = std::nullopt - }}); - - // Error with previous and next lines of code. - logError( - ErrorInfo { .name = "error name", - .description = "error with code lines", - .hint = hintfmt("this hint has %1% templated %2%!!", - "yellow", - "values"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13), - .prevLineOfCode = "previous line of code", - .errLineOfCode = "this is the problem line of code", - .nextLineOfCode = "next line of code", - }}); - - - // Error without lines of code. - logError( - ErrorInfo { .name = "error name", - .description = "error without any code lines.", - .hint = hintfmt("this hint has %1% templated %2%!!", - "yellow", - "values"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13) - }}); - - // Error with only hint and name.. - logError( - ErrorInfo { .name = "error name", - .hint = hintfmt("hint %1%", "only"), - .nixCode = NixCode { - .errPos = Pos(foFile, problem_file, 40, 13) - }}); - - return 0; -} From b18ed02b768560cf5551149a983afdb1e3c8053c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 13:10:41 -0600 Subject: [PATCH 13/33] repl indenting --- Makefile | 2 +- src/libutil/error.cc | 2 - src/libutil/error.hh | 9 +- src/nix/repl.cc | 360 +++++++++++++++++++++---------------------- 4 files changed, 183 insertions(+), 190 deletions(-) diff --git a/Makefile b/Makefile index 6f5a3ea90..332e6e971 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk + tests/plugins/local.mk -include Makefile.config diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 560a1b8f2..b5edfa710 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -10,8 +10,6 @@ namespace nix { const std::string nativeSystem = SYSTEM; -// Traces show the chain of calls in nix code. If an ErrPos is included the surrounding -// lines of code will print. BaseError & BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint}); diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 7185033f5..3879d9b5f 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -119,9 +119,6 @@ protected: // string prefix_; // used for location traces etc. mutable ErrorInfo err; - // mutable std::optional trace_; - // const string& calcTrace() const; - mutable std::optional what_; const string& calcWhat() const; @@ -167,11 +164,9 @@ public: #endif const string & msg() const { return calcWhat(); } - // const string & trace() const { return calcTrace(); } - // BaseError & addPrefix(const FormatOrString & fs); - BaseError & addTrace(std::optional e, hintformat hint); - const ErrorInfo & info() { calcWhat(); return err; } + + BaseError & addTrace(std::optional e, hintformat hint); }; #define MakeError(newClass, superClass) \ diff --git a/src/nix/repl.cc b/src/nix/repl.cc index a75157a82..26cee1e9f 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -101,74 +101,74 @@ NixRepl::~NixRepl() static NixRepl * curRepl; // ugly static char * completionCallback(char * s, int *match) { - auto possible = curRepl->completePrefix(s); - if (possible.size() == 1) { + auto possible = curRepl->completePrefix(s); + if (possible.size() == 1) { + *match = 1; + auto *res = strdup(possible.begin()->c_str() + strlen(s)); + if (!res) throw Error("allocation failure"); + return res; + } else if (possible.size() > 1) { + auto checkAllHaveSameAt = [&](size_t pos) { + auto &first = *possible.begin(); + for (auto &p : possible) { + if (p.size() <= pos || p[pos] != first[pos]) + return false; + } + return true; + }; + size_t start = strlen(s); + size_t len = 0; + while (checkAllHaveSameAt(start + len)) ++len; + if (len > 0) { *match = 1; - auto *res = strdup(possible.begin()->c_str() + strlen(s)); + auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); if (!res) throw Error("allocation failure"); return res; - } else if (possible.size() > 1) { - auto checkAllHaveSameAt = [&](size_t pos) { - auto &first = *possible.begin(); - for (auto &p : possible) { - if (p.size() <= pos || p[pos] != first[pos]) - return false; - } - return true; - }; - size_t start = strlen(s); - size_t len = 0; - while (checkAllHaveSameAt(start + len)) ++len; - if (len > 0) { - *match = 1; - auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); - if (!res) throw Error("allocation failure"); - return res; - } - } + } + } - *match = 0; - return nullptr; + *match = 0; + return nullptr; } static int listPossibleCallback(char *s, char ***avp) { - auto possible = curRepl->completePrefix(s); + auto possible = curRepl->completePrefix(s); - if (possible.size() > (INT_MAX / sizeof(char*))) - throw Error("too many completions"); + if (possible.size() > (INT_MAX / sizeof(char*))) + throw Error("too many completions"); - int ac = 0; - char **vp = nullptr; + int ac = 0; + char **vp = nullptr; - auto check = [&](auto *p) { - if (!p) { - if (vp) { - while (--ac >= 0) - free(vp[ac]); - free(vp); - } - throw Error("allocation failure"); - } - return p; - }; + auto check = [&](auto *p) { + if (!p) { + if (vp) { + while (--ac >= 0) + free(vp[ac]); + free(vp); + } + throw Error("allocation failure"); + } + return p; + }; - vp = check((char **)malloc(possible.size() * sizeof(char*))); + vp = check((char **)malloc(possible.size() * sizeof(char*))); - for (auto & p : possible) - vp[ac++] = check(strdup(p.c_str())); + for (auto & p : possible) + vp[ac++] = check(strdup(p.c_str())); - *avp = vp; + *avp = vp; - return ac; + return ac; } namespace { -// Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. -volatile sig_atomic_t g_signal_received = 0; + // Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. + volatile sig_atomic_t g_signal_received = 0; -void sigintHandler(int signo) { - g_signal_received = signo; -} + void sigintHandler(int signo) { + g_signal_received = signo; + } } void NixRepl::mainLoop(const std::vector & files) @@ -235,24 +235,24 @@ bool NixRepl::getLine(string & input, const std::string &prompt) sigset_t savedSignalMask, set; auto setupSignals = [&]() { - act.sa_handler = sigintHandler; - sigfillset(&act.sa_mask); - act.sa_flags = 0; - if (sigaction(SIGINT, &act, &old)) - throw SysError("installing handler for SIGINT"); + act.sa_handler = sigintHandler; + sigfillset(&act.sa_mask); + act.sa_flags = 0; + if (sigaction(SIGINT, &act, &old)) + throw SysError("installing handler for SIGINT"); - sigemptyset(&set); - sigaddset(&set, SIGINT); - if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) - throw SysError("unblocking SIGINT"); - }; + sigemptyset(&set); + sigaddset(&set, SIGINT); + if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) + throw SysError("unblocking SIGINT"); + }; auto restoreSignals = [&]() { - if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) - throw SysError("restoring signals"); + if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) + throw SysError("restoring signals"); - if (sigaction(SIGINT, &old, 0)) - throw SysError("restoring handler for SIGINT"); - }; + if (sigaction(SIGINT, &old, 0)) + throw SysError("restoring handler for SIGINT"); + }; setupSignals(); char * s = readline(prompt.c_str()); @@ -266,7 +266,7 @@ bool NixRepl::getLine(string & input, const std::string &prompt) } if (!s) - return false; + return false; input += s; input += '\n'; return true; @@ -399,21 +399,21 @@ bool NixRepl::processLine(string line) if (command == ":?" || command == ":help") { std::cout - << "The following commands are available:\n" - << "\n" - << " Evaluate and print expression\n" - << " = Bind expression to variable\n" - << " :a Add attributes from resulting set to scope\n" - << " :b Build derivation\n" - << " :e Open the derivation in $EDITOR\n" - << " :i Build derivation, then install result into current profile\n" - << " :l Load Nix expression and add it to scope\n" - << " :p Evaluate and print expression recursively\n" - << " :q Exit nix-repl\n" - << " :r Reload all files\n" - << " :s Build dependencies of derivation, then start nix-shell\n" - << " :t Describe result of evaluation\n" - << " :u Build derivation, then start nix-shell\n"; + << "The following commands are available:\n" + << "\n" + << " Evaluate and print expression\n" + << " = Bind expression to variable\n" + << " :a Add attributes from resulting set to scope\n" + << " :b Build derivation\n" + << " :e Open the derivation in $EDITOR\n" + << " :i Build derivation, then install result into current profile\n" + << " :l Load Nix expression and add it to scope\n" + << " :p Evaluate and print expression recursively\n" + << " :q Exit nix-repl\n" + << " :r Reload all files\n" + << " :s Build dependencies of derivation, then start nix-shell\n" + << " :t Describe result of evaluation\n" + << " :u Build derivation, then start nix-shell\n"; } else if (command == ":a" || command == ":add") { @@ -639,118 +639,118 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m switch (v.type) { - case tInt: - str << ANSI_CYAN << v.integer << ANSI_NORMAL; - break; + case tInt: + str << ANSI_CYAN << v.integer << ANSI_NORMAL; + break; - case tBool: - str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; - break; + case tBool: + str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; + break; - case tString: - str << ANSI_YELLOW; - printStringValue(str, v.string.s); - str << ANSI_NORMAL; - break; + case tString: + str << ANSI_YELLOW; + printStringValue(str, v.string.s); + str << ANSI_NORMAL; + break; - case tPath: - str << ANSI_GREEN << v.path << ANSI_NORMAL; // !!! escaping? - break; + case tPath: + str << ANSI_GREEN << v.path << ANSI_NORMAL; // !!! escaping? + break; - case tNull: - str << ANSI_CYAN "null" ANSI_NORMAL; - break; + case tNull: + str << ANSI_CYAN "null" ANSI_NORMAL; + break; - case tAttrs: { - seen.insert(&v); + case tAttrs: { + seen.insert(&v); - bool isDrv = state->isDerivation(v); + bool isDrv = state->isDerivation(v); - if (isDrv) { - str << "«derivation "; - Bindings::iterator i = v.attrs->find(state->sDrvPath); - PathSet context; - Path drvPath = i != v.attrs->end() ? state->coerceToPath(*i->pos, *i->value, context) : "???"; - str << drvPath << "»"; + if (isDrv) { + str << "«derivation "; + Bindings::iterator i = v.attrs->find(state->sDrvPath); + PathSet context; + Path drvPath = i != v.attrs->end() ? state->coerceToPath(*i->pos, *i->value, context) : "???"; + str << drvPath << "»"; + } + + else if (maxDepth > 0) { + str << "{ "; + + typedef std::map Sorted; + Sorted sorted; + for (auto & i : *v.attrs) + sorted[i.name] = i.value; + + for (auto & i : sorted) { + if (isVarName(i.first)) + str << i.first; + else + printStringValue(str, i.first.c_str()); + str << " = "; + if (seen.find(i.second) != seen.end()) + str << "«repeated»"; + else + try { + printValue(str, *i.second, maxDepth - 1, seen); + } catch (AssertionError & e) { + str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; + } + str << "; "; } - else if (maxDepth > 0) { - str << "{ "; + str << "}"; + } else + str << "{ ... }"; - typedef std::map Sorted; - Sorted sorted; - for (auto & i : *v.attrs) - sorted[i.name] = i.value; + break; + } - for (auto & i : sorted) { - if (isVarName(i.first)) - str << i.first; - else - printStringValue(str, i.first.c_str()); - str << " = "; - if (seen.find(i.second) != seen.end()) - str << "«repeated»"; - else - try { - printValue(str, *i.second, maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << "; "; - } + case tList1: + case tList2: + case tListN: + seen.insert(&v); - str << "}"; - } else - str << "{ ... }"; + str << "[ "; + if (maxDepth > 0) + for (unsigned int n = 0; n < v.listSize(); ++n) { + if (seen.find(v.listElems()[n]) != seen.end()) + str << "«repeated»"; + else + try { + printValue(str, *v.listElems()[n], maxDepth - 1, seen); + } catch (AssertionError & e) { + str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; + } + str << " "; + } + else + str << "... "; + str << "]"; + break; - break; - } + case tLambda: { + std::ostringstream s; + s << v.lambda.fun->pos; + str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; + break; + } - case tList1: - case tList2: - case tListN: - seen.insert(&v); + case tPrimOp: + str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; + break; - str << "[ "; - if (maxDepth > 0) - for (unsigned int n = 0; n < v.listSize(); ++n) { - if (seen.find(v.listElems()[n]) != seen.end()) - str << "«repeated»"; - else - try { - printValue(str, *v.listElems()[n], maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << " "; - } - else - str << "... "; - str << "]"; - break; + case tPrimOpApp: + str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; + break; - case tLambda: { - std::ostringstream s; - s << v.lambda.fun->pos; - str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; - break; - } + case tFloat: + str << v.fpoint; + break; - case tPrimOp: - str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; - break; - - case tPrimOpApp: - str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; - break; - - case tFloat: - str << v.fpoint; - break; - - default: - str << ANSI_RED "«unknown»" ANSI_NORMAL; - break; + default: + str << ANSI_RED "«unknown»" ANSI_NORMAL; + break; } return str; @@ -773,10 +773,10 @@ struct CmdRepl : StoreCommand, MixEvalArgs Examples examples() override { return { - Example{ - "Display all special commands within the REPL:", - "nix repl\n nix-repl> :?" - } + Example{ + "Display all special commands within the REPL:", + "nix repl\n nix-repl> :?" + } }; } From 93e9307329567c9f181318c4d4d6dc09de20ea48 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 13:14:49 -0600 Subject: [PATCH 14/33] repl indenting --- src/nix/repl.cc | 78 ++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 26cee1e9f..fdacf604b 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -103,28 +103,28 @@ static NixRepl * curRepl; // ugly static char * completionCallback(char * s, int *match) { auto possible = curRepl->completePrefix(s); if (possible.size() == 1) { + *match = 1; + auto *res = strdup(possible.begin()->c_str() + strlen(s)); + if (!res) throw Error("allocation failure"); + return res; + } else if (possible.size() > 1) { + auto checkAllHaveSameAt = [&](size_t pos) { + auto &first = *possible.begin(); + for (auto &p : possible) { + if (p.size() <= pos || p[pos] != first[pos]) + return false; + } + return true; + }; + size_t start = strlen(s); + size_t len = 0; + while (checkAllHaveSameAt(start + len)) ++len; + if (len > 0) { *match = 1; - auto *res = strdup(possible.begin()->c_str() + strlen(s)); + auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); if (!res) throw Error("allocation failure"); return res; - } else if (possible.size() > 1) { - auto checkAllHaveSameAt = [&](size_t pos) { - auto &first = *possible.begin(); - for (auto &p : possible) { - if (p.size() <= pos || p[pos] != first[pos]) - return false; - } - return true; - }; - size_t start = strlen(s); - size_t len = 0; - while (checkAllHaveSameAt(start + len)) ++len; - if (len > 0) { - *match = 1; - auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); - if (!res) throw Error("allocation failure"); - return res; - } + } } *match = 0; @@ -155,7 +155,7 @@ static int listPossibleCallback(char *s, char ***avp) { vp = check((char **)malloc(possible.size() * sizeof(char*))); for (auto & p : possible) - vp[ac++] = check(strdup(p.c_str())); + vp[ac++] = check(strdup(p.c_str())); *avp = vp; @@ -211,14 +211,12 @@ void NixRepl::mainLoop(const std::vector & files) // input without clearing the input so far. continue; } else { - printMsg(lvlError, e.msg()); + printMsg(lvlError, e.msg()); } } catch (Error & e) { - // printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); - printMsg(lvlError, e.msg()); + printMsg(lvlError, e.msg()); } catch (Interrupted & e) { - // printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); - printMsg(lvlError, e.msg()); + printMsg(lvlError, e.msg()); } // We handled the current input fully, so we should clear it @@ -399,21 +397,21 @@ bool NixRepl::processLine(string line) if (command == ":?" || command == ":help") { std::cout - << "The following commands are available:\n" - << "\n" - << " Evaluate and print expression\n" - << " = Bind expression to variable\n" - << " :a Add attributes from resulting set to scope\n" - << " :b Build derivation\n" - << " :e Open the derivation in $EDITOR\n" - << " :i Build derivation, then install result into current profile\n" - << " :l Load Nix expression and add it to scope\n" - << " :p Evaluate and print expression recursively\n" - << " :q Exit nix-repl\n" - << " :r Reload all files\n" - << " :s Build dependencies of derivation, then start nix-shell\n" - << " :t Describe result of evaluation\n" - << " :u Build derivation, then start nix-shell\n"; + << "The following commands are available:\n" + << "\n" + << " Evaluate and print expression\n" + << " = Bind expression to variable\n" + << " :a Add attributes from resulting set to scope\n" + << " :b Build derivation\n" + << " :e Open the derivation in $EDITOR\n" + << " :i Build derivation, then install result into current profile\n" + << " :l Load Nix expression and add it to scope\n" + << " :p Evaluate and print expression recursively\n" + << " :q Exit nix-repl\n" + << " :r Reload all files\n" + << " :s Build dependencies of derivation, then start nix-shell\n" + << " :t Describe result of evaluation\n" + << " :u Build derivation, then start nix-shell\n"; } else if (command == ":a" || command == ":add") { From 023912def37c8db64dec0339d39f2535e0d79e78 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 13:46:25 -0600 Subject: [PATCH 15/33] convenience form of addTrace --- src/libexpr/eval.cc | 6 +++--- src/libexpr/primops.cc | 12 ++++++------ src/libutil/error.hh | 6 ++++++ src/nix-env/nix-env.cc | 4 ++-- src/nix/search.cc | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3fd8aa285..1cbbb7ade 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -594,17 +594,17 @@ LocalNoInlineNoReturn(void throwUndefinedVarError(const Pos & pos, const char * LocalNoInline(void addErrorTrace(Error & e, const char * s, const string & s2)) { - e.addTrace(std::nullopt, hintfmt(s) % s2); + e.addTrace(std::nullopt, s, s2); } LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const ExprLambda & fun)) { - e.addTrace(pos, hintfmt(s) % fun.showNamePos()); + e.addTrace(pos, s, fun.showNamePos()); } LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const string & s2)) { - e.addTrace(pos, hintfmt(s) % s2); + e.addTrace(pos, s, s2); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 6f7a691cd..4cc3718e3 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -239,13 +239,13 @@ void prim_exec(EvalState & state, const Pos & pos, Value * * args, Value & v) try { parsed = state.parseExprFromString(output, pos.file); } catch (Error & e) { - e.addTrace(pos, hintfmt("While parsing the output from '%1%'", program)); + e.addTrace(pos, "While parsing the output from '%1%'", program); throw; } try { state.eval(parsed, v); } catch (Error & e) { - e.addTrace(pos, hintfmt("While evaluating the output from '%1%'", program)); + e.addTrace(pos, "While evaluating the output from '%1%'", program); throw; } } @@ -472,7 +472,7 @@ static void prim_addErrorContext(EvalState & state, const Pos & pos, Value * * a } catch (Error & e) { PathSet context; // TODO: is this right, include this pos?? Test it. esp with LOC. - e.addTrace(pos, hintfmt("%1%") % state.coerceToString(pos, *args[0], context)); + e.addTrace(pos, "%1%", state.coerceToString(pos, *args[0], context)); throw; } } @@ -564,7 +564,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * try { drvName = state.forceStringNoCtx(*attr->value, pos); } catch (Error & e) { - e.addTrace(posDrvName, hintfmt("while evaluating the derivation attribute 'name'")); + e.addTrace(posDrvName, "while evaluating the derivation attribute 'name'"); throw; } @@ -698,8 +698,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } catch (Error & e) { e.addTrace(posDrvName, - hintfmt("while evaluating the attribute '%1%' of the derivation '%2%'", - key, drvName)); + "while evaluating the attribute '%1%' of the derivation '%2%'", + key, drvName); throw; } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 3879d9b5f..09fc57fee 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -166,6 +166,12 @@ public: const string & msg() const { return calcWhat(); } const ErrorInfo & info() { calcWhat(); return err; } + template + BaseError & addTrace(std::optional e, const string &fs, const Args & ... args) + { + return addTrace(e, hintfmt(fs, args...)); + } + BaseError & addTrace(std::optional e, hintformat hint); }; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index f5dfbf9f6..c992b7d74 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -593,7 +593,7 @@ static void upgradeDerivations(Globals & globals, } else newElems.push_back(i); } catch (Error & e) { - e.addTrace(std::nullopt, hintfmt("while trying to find an upgrade for '%s'", i.queryName())); + e.addTrace(std::nullopt, "while trying to find an upgrade for '%s'", i.queryName()); throw; } } @@ -1185,7 +1185,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) } catch (AssertionError & e) { printMsg(lvlTalkative, "skipping derivation named '%1%' which gives an assertion failure", i.queryName()); } catch (Error & e) { - e.addTrace(std::nullopt, hintfmt("while querying the derivation named '%1%'", i.queryName())); + e.addTrace(std::nullopt, "while querying the derivation named '%1%'", i.queryName()); throw; } } diff --git a/src/nix/search.cc b/src/nix/search.cc index d9c730796..93c3f3f83 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -216,7 +216,7 @@ struct CmdSearch : SourceExprCommand, MixJSON } catch (AssertionError & e) { } catch (Error & e) { if (!toplevel) { - e.addTrace(std::nullopt, hintfmt("While evaluating the attribute '%s'", attrPath)); + e.addTrace(std::nullopt, "While evaluating the attribute '%s'", attrPath); throw; } } From 6359d71d6b6110ce3608cfd289cae5143e78f123 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 18:28:20 -0600 Subject: [PATCH 16/33] re-enable --show-trace check --- src/libmain/shared.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 27a4d5fc1..e1b1cd4fc 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -328,8 +328,8 @@ int handleExceptions(const string & programName, std::function fun) // printError(e.prefix()); logError(e.info()); // TODO fix to detect non-empty trace here. - // if (e.prefix() != "" && !settings.showTrace) - // printError("(use '--show-trace' to show detailed location information)"); + if (e.hasTrace() && !settings.showTrace) + printError("(use '--show-trace' to show detailed location information)"); return e.status; } catch (std::bad_alloc & e) { printError(error + "out of memory"); From 9c0e1fd4f1151f4780c1ff3db82322d8b28d2d8b Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 24 Jun 2020 18:31:28 -0600 Subject: [PATCH 17/33] add trace test; error formatting refinements --- src/libutil/error.cc | 86 ++++++++++++++++++++++++------------ src/libutil/error.hh | 2 + src/libutil/tests/logging.cc | 33 ++++++++++++-- 3 files changed, 89 insertions(+), 32 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index b5edfa710..1185a0204 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -185,6 +185,31 @@ void printCodeLines(std::ostream &out, } } +void printAtPos(const string &prefix, const ErrPos &pos, std::ostream &out) +{ + { + switch (pos.origin) { + case foFile: { + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file; + break; + } + case foString: { + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " from command line argument" << ANSI_NORMAL; + break; + } + case foStdin: { + out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << + ANSI_BLUE << " from stdin" << ANSI_NORMAL; + break; + } + default: + throw Error("invalid FileOrigin in errPos"); + } + } +} + std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) { auto errwidth = std::max(getWindowSize().second, 20); @@ -240,8 +265,12 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) } } - auto ndl = prefix.length() + levelString.length() + 3 + einfo.name.length() + einfo.programName.value_or("").length(); - auto dashwidth = ndl > (errwidth - 3) ? 3 : errwidth - ndl; + auto ndl = prefix.length() + + filterANSIEscapes(levelString, true).length() + + 7 + + einfo.name.length() + + einfo.programName.value_or("").length(); + auto dashwidth = std::max(errwidth - ndl, 3); std::string dashes(dashwidth, '-'); @@ -262,29 +291,8 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) bool nl = false; // intersperse newline between sections. if (einfo.errPos.has_value()) { - switch (einfo.errPos->origin) { - case foFile: { - out << prefix << std::endl; - auto &pos = *einfo.errPos; - out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << - ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file; - break; - } - case foString: { - out << prefix << std::endl; - out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << - ANSI_BLUE << " from command line argument" << ANSI_NORMAL; - break; - } - case foStdin: { - out << prefix << std::endl; - out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(*einfo.errPos) << - ANSI_BLUE << " from stdin" << ANSI_NORMAL; - break; - } - default: - throw Error("invalid FileOrigin in errPos"); - } + out << prefix << std::endl; + printAtPos(prefix, *einfo.errPos, out); nl = true; } @@ -320,13 +328,33 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) { try { - auto pos = *iter->pos; if (nl) out << std::endl << prefix; - out << std::endl << prefix; + + const string tracetitle(" show-trace output "); + + int fill = errwidth - tracetitle.length(); + int lw = 0; + int rw = 0; + const int min_dashes = 3; + if (fill > min_dashes * 2) { + if (fill % 2 != 0) { + lw = fill / 2; + rw = lw + 1; + } + else + { + lw = rw = fill / 2; + } + } + else + lw = rw = min_dashes; + + out << ANSI_BLUE << std::string(lw, '-') << tracetitle << std::string(rw, '-') << std::endl << prefix; out << iter->hint.str() << std::endl; - out << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << - ANSI_BLUE << " in file: " << ANSI_NORMAL << pos.file << std::endl; + + auto pos = *iter->pos; + printAtPos(prefix, pos, out); nl = true; auto loc = getCodeLines(pos); if (loc.has_value()) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 09fc57fee..d79ff8f8c 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -173,6 +173,8 @@ public: } BaseError & addTrace(std::optional e, hintformat hint); + + bool hasTrace() const { return !err.traces.empty(); } }; #define MakeError(newClass, superClass) \ diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 0bbec482f..6d0431133 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -11,12 +11,12 @@ namespace nix { * logEI * --------------------------------------------------------------------------*/ - const char *test_file = + const char *test_file = "previous line of code\n" "this is the problem line of code\n" "next line of code\n"; - const char *one_liner = - "this is the problem line of code"; + const char *one_liner = + "this is the other problem line of code"; TEST(logEI, catpuresBasicProperties) { @@ -245,6 +245,33 @@ namespace nix { ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- warning name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from stdin\x1B[0m\n\nwarning description\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); } + /* ---------------------------------------------------------------------------- + * traces + * --------------------------------------------------------------------------*/ + + TEST(addTrace, showTracesWithShowTrace) { + SymbolTable testTable; + auto problem_file = testTable.create(test_file); + + auto oneliner_file = testTable.create(one_liner); + + auto e = AssertionError(ErrorInfo { + .name = "wat", + .description = "a well-known problem occurred", + .hint = hintfmt("it has been %1% days since our last error", "zero"), + .errPos = Pos(foString, problem_file, 2, 13), + }); + + e.addTrace(Pos(foStdin, oneliner_file, 1, 19), "while trying to compute %1%", 42); + + testing::internal::CaptureStderr(); + + logError(e.info()); + + auto str = testing::internal::GetCapturedStderr(); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m--- show-trace output ---\nwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n"); + } + /* ---------------------------------------------------------------------------- * hintfmt * --------------------------------------------------------------------------*/ From 9ab808c92613b59a72a4d15cfda1adb6aa523e28 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 25 Jun 2020 09:23:12 -0600 Subject: [PATCH 18/33] showTrace flag for ErrorInfo; showTrace test. --- src/libmain/shared.cc | 4 +-- src/libutil/error.cc | 65 +++++++++++++++++++----------------- src/libutil/error.hh | 1 + src/libutil/tests/logging.cc | 25 ++++++++++++++ 4 files changed, 61 insertions(+), 34 deletions(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index e1b1cd4fc..7514b19d2 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -303,6 +303,7 @@ int handleExceptions(const string & programName, std::function fun) ReceiveInterrupts receiveInterrupts; // FIXME: need better place for this ErrorInfo::programName = baseNameOf(programName); + ErrorInfo::showTrace = settings.showTrace; string error = ANSI_RED "error:" ANSI_NORMAL " "; try { @@ -323,9 +324,6 @@ int handleExceptions(const string & programName, std::function fun) printError("Try '%1% --help' for more information.", programName); return 1; } catch (BaseError & e) { - // TODO showTrace as argument, or have calcWhat check settings? - // if (settings.showTrace && e.prefix() != "") - // printError(e.prefix()); logError(e.info()); // TODO fix to detect non-empty trace here. if (e.hasTrace() && !settings.showTrace) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 1185a0204..10a5300cb 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -34,6 +34,7 @@ const string& BaseError::calcWhat() const } std::optional ErrorInfo::programName = std::nullopt; +bool ErrorInfo::showTrace = false; std::ostream& operator<<(std::ostream &os, const hintformat &hf) { @@ -325,42 +326,44 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) } // traces - for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) - { - try { - if (nl) - out << std::endl << prefix; + if (ErrorInfo::showTrace) { + for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) + { + try { + if (nl) + out << std::endl << prefix; - const string tracetitle(" show-trace output "); + const string tracetitle(" show-trace output "); - int fill = errwidth - tracetitle.length(); - int lw = 0; - int rw = 0; - const int min_dashes = 3; - if (fill > min_dashes * 2) { - if (fill % 2 != 0) { - lw = fill / 2; - rw = lw + 1; + int fill = errwidth - tracetitle.length(); + int lw = 0; + int rw = 0; + const int min_dashes = 3; + if (fill > min_dashes * 2) { + if (fill % 2 != 0) { + lw = fill / 2; + rw = lw + 1; + } + else + { + lw = rw = fill / 2; + } } else - { - lw = rw = fill / 2; - } + lw = rw = min_dashes; + + out << ANSI_BLUE << std::string(lw, '-') << tracetitle << std::string(rw, '-') << std::endl << prefix; + out << iter->hint.str() << std::endl; + + auto pos = *iter->pos; + printAtPos(prefix, pos, out); + nl = true; + auto loc = getCodeLines(pos); + if (loc.has_value()) + printCodeLines(out, prefix, pos, *loc); + } catch(const std::bad_optional_access& e) { + out << iter->hint.str() << std::endl; } - else - lw = rw = min_dashes; - - out << ANSI_BLUE << std::string(lw, '-') << tracetitle << std::string(rw, '-') << std::endl << prefix; - out << iter->hint.str() << std::endl; - - auto pos = *iter->pos; - printAtPos(prefix, pos, out); - nl = true; - auto loc = getCodeLines(pos); - if (loc.has_value()) - printCodeLines(out, prefix, pos, *loc); - } catch(const std::bad_optional_access& e) { - out << iter->hint.str() << std::endl; } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index d79ff8f8c..dcde28453 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -107,6 +107,7 @@ struct ErrorInfo { std::list traces; static std::optional programName; + static bool showTrace; }; std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo); diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 6d0431133..7531d6c7a 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -251,6 +251,7 @@ namespace nix { TEST(addTrace, showTracesWithShowTrace) { SymbolTable testTable; + ErrorInfo::showTrace = true; auto problem_file = testTable.create(test_file); auto oneliner_file = testTable.create(one_liner); @@ -272,6 +273,30 @@ namespace nix { ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m--- show-trace output ---\nwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n"); } + TEST(addTrace, hideTracesWithoutShowTrace) { + SymbolTable testTable; + ErrorInfo::showTrace = false; + auto problem_file = testTable.create(test_file); + + auto oneliner_file = testTable.create(one_liner); + + auto e = AssertionError(ErrorInfo { + .name = "wat", + .description = "a well-known problem occurred", + .hint = hintfmt("it has been %1% days since our last error", "zero"), + .errPos = Pos(foString, problem_file, 2, 13), + }); + + e.addTrace(Pos(foStdin, oneliner_file, 1, 19), "while trying to compute %1%", 42); + + testing::internal::CaptureStderr(); + + logError(e.info()); + + auto str = testing::internal::GetCapturedStderr(); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n"); + } + /* ---------------------------------------------------------------------------- * hintfmt * --------------------------------------------------------------------------*/ From bc9e87412cb94f74d2515a00a591a7b5e33c90d7 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 25 Jun 2020 09:56:32 -0600 Subject: [PATCH 19/33] 'string' makes more sense in nix repl --- src/libutil/error.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 10a5300cb..49190eae1 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -197,7 +197,7 @@ void printAtPos(const string &prefix, const ErrPos &pos, std::ostream &out) } case foString: { out << prefix << ANSI_BLUE << "at: " << ANSI_YELLOW << showErrPos(pos) << - ANSI_BLUE << " from command line argument" << ANSI_NORMAL; + ANSI_BLUE << " from string" << ANSI_NORMAL; break; } case foStdin: { From ef24a0835d16baf1aa3f19ffa7ba7af54e6e63e6 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Sat, 27 Jun 2020 12:19:31 -0600 Subject: [PATCH 20/33] showtrace as function arg --- src/libutil/error.cc | 7 ++++--- src/libutil/error.hh | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 49190eae1..0210555c5 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -34,7 +34,7 @@ const string& BaseError::calcWhat() const } std::optional ErrorInfo::programName = std::nullopt; -bool ErrorInfo::showTrace = false; +// bool ErrorInfo::showTrace = false; std::ostream& operator<<(std::ostream &os, const hintformat &hf) { @@ -211,7 +211,8 @@ void printAtPos(const string &prefix, const ErrPos &pos, std::ostream &out) } } -std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) +// std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) +std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool showTrace) { auto errwidth = std::max(getWindowSize().second, 20); string prefix = ""; @@ -326,7 +327,7 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) } // traces - if (ErrorInfo::showTrace) { + if (showTrace) { for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) { try { diff --git a/src/libutil/error.hh b/src/libutil/error.hh index dcde28453..a9da72b54 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -107,10 +107,11 @@ struct ErrorInfo { std::list traces; static std::optional programName; - static bool showTrace; + // static bool showTrace; }; -std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo); +// std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo); +std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool showTrace); /* BaseError should generally not be caught, as it has Interrupted as a subclass. Catch Error instead. */ From 8f81fae116b449ef17561c3574d01574544c798c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 29 Jun 2020 10:20:51 -0600 Subject: [PATCH 21/33] showTrace flag in loggers --- src/libmain/loggers.cc | 6 +++--- src/libmain/progress-bar.cc | 11 ++++++++++- src/libmain/shared.cc | 2 +- src/libstore/daemon.cc | 13 +++++++++++-- src/libutil/error.cc | 3 ++- src/libutil/logging.cc | 31 ++++++++++++++++++++++++------- src/libutil/logging.hh | 6 +++++- src/libutil/tests/logging.cc | 4 ++-- src/libutil/util.cc | 2 +- src/nix/installables.cc | 2 +- 10 files changed, 60 insertions(+), 20 deletions(-) diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index c44bb6408..fd6ce912f 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -22,11 +22,11 @@ LogFormat parseLogFormat(const std::string & logFormatStr) { Logger * makeDefaultLogger() { switch (defaultLogFormat) { case LogFormat::raw: - return makeSimpleLogger(false); + return makeSimpleLogger(false, false); case LogFormat::rawWithLogs: - return makeSimpleLogger(true); + return makeSimpleLogger(true, false); case LogFormat::internalJson: - return makeJSONLogger(*makeSimpleLogger()); + return makeJSONLogger(*makeSimpleLogger(true, false)); case LogFormat::bar: return makeProgressBar(); case LogFormat::barWithLogs: diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 95a9187de..8da0f942e 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -81,12 +81,14 @@ private: bool printBuildLogs; bool isTTY; + bool showTrace; public: ProgressBar(bool printBuildLogs, bool isTTY) : printBuildLogs(printBuildLogs) , isTTY(isTTY) + , showTrace(false) { state_.lock()->active = isTTY; updateThread = std::thread([&]() { @@ -131,10 +133,17 @@ public: auto state(state_.lock()); std::stringstream oss; - oss << ei; + showErrorInfo(oss, ei, showTrace); + // oss << ei; log(*state, ei.level, oss.str()); } + bool getShowTrace() const override { + return showTrace; + } + void setShowTrace(bool showTrace) override { + this->showTrace = showTrace; + } void log(State & state, Verbosity lvl, const std::string & s) { diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 7514b19d2..722e384fc 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -303,7 +303,6 @@ int handleExceptions(const string & programName, std::function fun) ReceiveInterrupts receiveInterrupts; // FIXME: need better place for this ErrorInfo::programName = baseNameOf(programName); - ErrorInfo::showTrace = settings.showTrace; string error = ANSI_RED "error:" ANSI_NORMAL " "; try { @@ -324,6 +323,7 @@ int handleExceptions(const string & programName, std::function fun) printError("Try '%1% --help' for more information.", programName); return 1; } catch (BaseError & e) { + logger->setShowTrace(settings.showTrace); logError(e.info()); // TODO fix to detect non-empty trace here. if (e.hasTrace() && !settings.showTrace) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 842aef20c..6f42093d5 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -41,9 +41,10 @@ struct TunnelLogger : public Logger Sync state_; unsigned int clientVersion; + bool showTrace; TunnelLogger(FdSink & to, unsigned int clientVersion) - : to(to), clientVersion(clientVersion) { } + : to(to), clientVersion(clientVersion), showTrace(false) { } void enqueueMsg(const std::string & s) { @@ -78,13 +79,21 @@ struct TunnelLogger : public Logger if (ei.level > verbosity) return; std::stringstream oss; - oss << ei; + showErrorInfo(oss, ei, false); + // oss << ei; StringSink buf; buf << STDERR_NEXT << oss.str() << "\n"; // (fs.s + "\n"); enqueueMsg(*buf.s); } + bool getShowTrace() const override { + return showTrace; + } + void setShowTrace(bool showTrace) override { + this->showTrace = showTrace; + } + /* startWork() means that we're starting an operation for which we want to send out stderr to the client. */ void startWork() diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 0210555c5..0199ee5f8 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -26,7 +26,8 @@ const string& BaseError::calcWhat() const err.name = sname(); std::ostringstream oss; - oss << err; + showErrorInfo(oss, err, false); + // oss << err; what_ = oss.str(); return *what_; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 105fadb15..4de4c4012 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -18,7 +18,7 @@ void setCurActivity(const ActivityId activityId) curActivity = activityId; } -Logger * logger = makeSimpleLogger(true); +Logger * logger = makeSimpleLogger(true, false); void Logger::warn(const std::string & msg) { @@ -36,9 +36,10 @@ public: bool systemd, tty; bool printBuildLogs; + bool showTrace; - SimpleLogger(bool printBuildLogs) - : printBuildLogs(printBuildLogs) + SimpleLogger(bool printBuildLogs, bool showTrace) + : printBuildLogs(printBuildLogs), showTrace(showTrace) { systemd = getEnv("IN_SYSTEMD") == "1"; tty = isatty(STDERR_FILENO); @@ -48,6 +49,13 @@ public: return printBuildLogs; } + bool getShowTrace() const override { + return showTrace; + } + void setShowTrace(bool showTrace) override { + this->showTrace = showTrace; + } + void log(Verbosity lvl, const FormatOrString & fs) override { if (lvl > verbosity) return; @@ -72,7 +80,8 @@ public: void logEI(const ErrorInfo & ei) override { std::stringstream oss; - oss << ei; + showErrorInfo(oss, ei, showTrace); + // oss << ei; log(ei.level, oss.str()); } @@ -120,9 +129,9 @@ void writeToStderr(const string & s) } } -Logger * makeSimpleLogger(bool printBuildLogs) +Logger * makeSimpleLogger(bool printBuildLogs, bool showTrace) { - return new SimpleLogger(printBuildLogs); + return new SimpleLogger(printBuildLogs, showTrace); } std::atomic nextId{(uint64_t) getpid() << 32}; @@ -143,6 +152,13 @@ struct JSONLogger : Logger { return true; } + bool getShowTrace() const override { + return prevLogger.getShowTrace(); + } + void setShowTrace(bool showTrace) override { + prevLogger.setShowTrace(showTrace); + } + void addFields(nlohmann::json & json, const Fields & fields) { if (fields.empty()) return; @@ -173,7 +189,8 @@ struct JSONLogger : Logger { void logEI(const ErrorInfo & ei) override { std::ostringstream oss; - oss << ei; + showErrorInfo(oss, ei, getShowTrace()); + // oss << ei; nlohmann::json json; json["action"] = "msg"; diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index b1583eced..3c164c670 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -75,6 +75,9 @@ public: logEI(ei); } + virtual bool getShowTrace() const = 0; + virtual void setShowTrace(bool showTrace) = 0; + virtual void warn(const std::string & msg); virtual void startActivity(ActivityId act, Verbosity lvl, ActivityType type, @@ -146,7 +149,8 @@ struct PushActivity extern Logger * logger; -Logger * makeSimpleLogger(bool printBuildLogs = true); +Logger * makeSimpleLogger(bool printBuildLogs, bool showTrace); +// Logger * makeSimpleLogger(bool printBuildLogs = true, bool showTrace); Logger * makeJSONLogger(Logger & prevLogger); diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 7531d6c7a..1bd90f009 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -251,7 +251,7 @@ namespace nix { TEST(addTrace, showTracesWithShowTrace) { SymbolTable testTable; - ErrorInfo::showTrace = true; + // ErrorInfo::showTrace = true; auto problem_file = testTable.create(test_file); auto oneliner_file = testTable.create(one_liner); @@ -275,7 +275,7 @@ namespace nix { TEST(addTrace, hideTracesWithoutShowTrace) { SymbolTable testTable; - ErrorInfo::showTrace = false; + // ErrorInfo::showTrace = false; auto problem_file = testTable.create(test_file); auto oneliner_file = testTable.create(one_liner); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 1268b146a..92035c3ad 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -972,7 +972,7 @@ pid_t startProcess(std::function fun, const ProcessOptions & options) { auto wrapper = [&]() { if (!options.allowVfork) - logger = makeSimpleLogger(); + logger = makeSimpleLogger(true, false); // TODO remove args. try { #if __linux__ if (options.dieWithParent && prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 708a0dc88..166ba167f 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -212,7 +212,7 @@ std::string attrRegex = R"([A-Za-z_][A-Za-z0-9-_+]*)"; static std::regex attrPathRegex(fmt(R"(%1%(\.%1%)*)", attrRegex)); static std::vector> parseInstallables( - SourceExprCommand & cmd, ref store, std::vector ss, bool useDefaultInstallables) + SourceExprCommand & cmd, ref store, std::vetor ss, bool useDefaultInstallables) { std::vector> result; From c484a67914fe16c4f2f6e7779baf4bdfe6405a22 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 29 Jun 2020 15:46:21 -0600 Subject: [PATCH 22/33] trace formatting --- src/libutil/error.cc | 57 +++++++++++++++++++++++------------------ src/nix/installables.cc | 2 +- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 0199ee5f8..02eb3f7cb 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -328,41 +328,48 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show } // traces - if (showTrace) { + if (showTrace && !einfo.traces.empty()) + { + const string tracetitle(" show-trace output "); + + int fill = errwidth - tracetitle.length(); + int lw = 0; + int rw = 0; + const int min_dashes = 3; + if (fill > min_dashes * 2) { + if (fill % 2 != 0) { + lw = fill / 2; + rw = lw + 1; + } + else + { + lw = rw = fill / 2; + } + } + else + lw = rw = min_dashes; + + if (nl) + out << std::endl << prefix; + + out << ANSI_BLUE << std::string(lw, '-') << tracetitle << std::string(rw, '-') << ANSI_NORMAL; + for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) { try { - if (nl) - out << std::endl << prefix; - - const string tracetitle(" show-trace output "); - - int fill = errwidth - tracetitle.length(); - int lw = 0; - int rw = 0; - const int min_dashes = 3; - if (fill > min_dashes * 2) { - if (fill % 2 != 0) { - lw = fill / 2; - rw = lw + 1; - } - else - { - lw = rw = fill / 2; - } - } - else - lw = rw = min_dashes; - - out << ANSI_BLUE << std::string(lw, '-') << tracetitle << std::string(rw, '-') << std::endl << prefix; - out << iter->hint.str() << std::endl; + out << std::endl << prefix; + out << ANSI_BLUE << "trace: " << ANSI_NORMAL << iter->hint.str() << std::endl; auto pos = *iter->pos; printAtPos(prefix, pos, out); nl = true; auto loc = getCodeLines(pos); if (loc.has_value()) + { + out << std::endl; printCodeLines(out, prefix, pos, *loc); + out << std::endl; + } } catch(const std::bad_optional_access& e) { out << iter->hint.str() << std::endl; } diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 166ba167f..708a0dc88 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -212,7 +212,7 @@ std::string attrRegex = R"([A-Za-z_][A-Za-z0-9-_+]*)"; static std::regex attrPathRegex(fmt(R"(%1%(\.%1%)*)", attrRegex)); static std::vector> parseInstallables( - SourceExprCommand & cmd, ref store, std::vetor ss, bool useDefaultInstallables) + SourceExprCommand & cmd, ref store, std::vector ss, bool useDefaultInstallables) { std::vector> result; From 70bcb39d3fe272164c122979ee36cf2f2ea8f084 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 30 Jun 2020 15:44:19 -0600 Subject: [PATCH 23/33] double addtrace for 'called from' --- src/libexpr/eval.cc | 13 +++++-------- src/libutil/error.cc | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 1cbbb7ade..e89948b94 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -597,11 +597,6 @@ LocalNoInline(void addErrorTrace(Error & e, const char * s, const string & s2)) e.addTrace(std::nullopt, s, s2); } -LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const ExprLambda & fun)) -{ - e.addTrace(pos, s, fun.showNamePos()); -} - LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const string & s2)) { e.addTrace(pos, s, s2); @@ -1241,9 +1236,11 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & po try { lambda.body->eval(*this, env2, v); } catch (Error & e) { - // TODO something different for 'called from' than usual addTrace? - // addErrorTrace(e, pos, "while evaluating %1%, called from %2%:", lambda); - addErrorTrace(e, pos, "while evaluating %1%:", lambda); + addErrorTrace(e, lambda.pos, "while evaluating %s", + (lambda.name.set() + ? "'" + (string) lambda.name + "'" + : "anonymous lambdaction")); + addErrorTrace(e, pos, "from call site%s", ""); throw; } else diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 02eb3f7cb..229c1e6ed 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -330,7 +330,7 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show // traces if (showTrace && !einfo.traces.empty()) { - const string tracetitle(" show-trace output "); + const string tracetitle(" show-trace "); int fill = errwidth - tracetitle.length(); int lw = 0; From 9159dfe3d8053e0c77d32893e45f8c527ba83076 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 30 Jun 2020 16:31:55 -0600 Subject: [PATCH 24/33] comments and cleanup --- src/libexpr/nixexpr.cc | 6 +++--- src/libmain/progress-bar.cc | 1 - src/libmain/shared.cc | 1 - src/libstore/daemon.cc | 3 +-- src/libutil/error.cc | 3 --- src/libutil/error.hh | 3 --- src/libutil/logging.cc | 4 +--- src/libutil/logging.hh | 3 +-- src/libutil/util.cc | 2 +- 9 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 2b22ef3b6..d5698011f 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -197,11 +197,11 @@ std::ostream & operator << (std::ostream & str, const Pos & pos) if (!pos) str << "undefined position"; else - { + { auto f = format(ANSI_BOLD "%1%" ANSI_NORMAL ":%2%:%3%"); - switch (pos.origin) { + switch (pos.origin) { case foFile: - f % (string) pos.file; + f % (string) pos.file; break; case foStdin: case foString: diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 8da0f942e..265b16908 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -134,7 +134,6 @@ public: std::stringstream oss; showErrorInfo(oss, ei, showTrace); - // oss << ei; log(*state, ei.level, oss.str()); } diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 722e384fc..5cbdd19e5 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -325,7 +325,6 @@ int handleExceptions(const string & programName, std::function fun) } catch (BaseError & e) { logger->setShowTrace(settings.showTrace); logError(e.info()); - // TODO fix to detect non-empty trace here. if (e.hasTrace() && !settings.showTrace) printError("(use '--show-trace' to show detailed location information)"); return e.status; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 6f42093d5..186ccc5b4 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -80,10 +80,9 @@ struct TunnelLogger : public Logger std::stringstream oss; showErrorInfo(oss, ei, false); - // oss << ei; StringSink buf; - buf << STDERR_NEXT << oss.str() << "\n"; // (fs.s + "\n"); + buf << STDERR_NEXT << oss.str() << "\n"; enqueueMsg(*buf.s); } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 229c1e6ed..7d105d8b4 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -27,7 +27,6 @@ const string& BaseError::calcWhat() const std::ostringstream oss; showErrorInfo(oss, err, false); - // oss << err; what_ = oss.str(); return *what_; @@ -35,7 +34,6 @@ const string& BaseError::calcWhat() const } std::optional ErrorInfo::programName = std::nullopt; -// bool ErrorInfo::showTrace = false; std::ostream& operator<<(std::ostream &os, const hintformat &hf) { @@ -212,7 +210,6 @@ void printAtPos(const string &prefix, const ErrPos &pos, std::ostream &out) } } -// std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool showTrace) { auto errwidth = std::max(getWindowSize().second, 20); diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 4e86666fe..1b0fb43b8 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -111,10 +111,8 @@ struct ErrorInfo { std::list traces; static std::optional programName; - // static bool showTrace; }; -// std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo); std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool showTrace); /* BaseError should generally not be caught, as it has Interrupted as @@ -122,7 +120,6 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show class BaseError : public std::exception { protected: - // string prefix_; // used for location traces etc. mutable ErrorInfo err; mutable std::optional what_; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 4de4c4012..504f5fb52 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -81,7 +81,6 @@ public: { std::stringstream oss; showErrorInfo(oss, ei, showTrace); - // oss << ei; log(ei.level, oss.str()); } @@ -158,7 +157,7 @@ struct JSONLogger : Logger { void setShowTrace(bool showTrace) override { prevLogger.setShowTrace(showTrace); } - + void addFields(nlohmann::json & json, const Fields & fields) { if (fields.empty()) return; @@ -190,7 +189,6 @@ struct JSONLogger : Logger { { std::ostringstream oss; showErrorInfo(oss, ei, getShowTrace()); - // oss << ei; nlohmann::json json; json["action"] = "msg"; diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 3c164c670..273300b50 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -149,8 +149,7 @@ struct PushActivity extern Logger * logger; -Logger * makeSimpleLogger(bool printBuildLogs, bool showTrace); -// Logger * makeSimpleLogger(bool printBuildLogs = true, bool showTrace); +Logger * makeSimpleLogger(bool printBuildLogs = true, bool showTrace = false); Logger * makeJSONLogger(Logger & prevLogger); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 92035c3ad..1268b146a 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -972,7 +972,7 @@ pid_t startProcess(std::function fun, const ProcessOptions & options) { auto wrapper = [&]() { if (!options.allowVfork) - logger = makeSimpleLogger(true, false); // TODO remove args. + logger = makeSimpleLogger(); try { #if __linux__ if (options.dieWithParent && prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) From dabbb4538f8b021fbaaacf1e5c2b79fec4228421 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 30 Jun 2020 16:43:01 -0600 Subject: [PATCH 25/33] 'from string' --- tests/misc.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/misc.sh b/tests/misc.sh index 77a52995a..a81c9dbb1 100644 --- a/tests/misc.sh +++ b/tests/misc.sh @@ -17,7 +17,7 @@ nix-env -q --foo 2>&1 | grep "unknown flag" # Eval Errors. eval_arg_res=$(nix-instantiate --eval -E 'let a = {} // a; in a.foo' 2>&1 || true) -echo $eval_arg_res | grep "at: (1:15) from command line argument" +echo $eval_arg_res | grep "at: (1:15) from string" echo $eval_arg_res | grep "infinite recursion encountered" eval_stdin_res=$(echo 'let a = {} // a; in a.foo' | nix-instantiate --eval -E - 2>&1 || true) From a7d5d26443622496b21e083bddb0d900b647f87d Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 30 Jun 2020 22:05:21 -0600 Subject: [PATCH 26/33] fix tests with the 'from string' change --- src/libutil/error.cc | 2 +- src/libutil/tests/logging.cc | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 7d105d8b4..6b6b268a2 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -135,7 +135,7 @@ std::optional getCodeLines(const ErrPos &errPos) } } -// if nixCode contains lines of code, print them to the ostream, indicating the error column. +// print lines of code to the ostream, indicating the error column. void printCodeLines(std::ostream &out, const string &prefix, const ErrPos &errPos, diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 221b1be1b..a73c1f8d9 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -173,7 +173,7 @@ namespace nix { }); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\nerror with code lines\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\nerror with code lines\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); } TEST(logError, logErrorWithInvalidFile) { @@ -251,7 +251,6 @@ namespace nix { TEST(addTrace, showTracesWithShowTrace) { SymbolTable testTable; - // ErrorInfo::showTrace = true; auto problem_file = testTable.create(test_file); auto oneliner_file = testTable.create(one_liner); @@ -267,15 +266,16 @@ namespace nix { testing::internal::CaptureStderr(); + logger->setShowTrace(true); + logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m--- show-trace output ---\nwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n\n"); } TEST(addTrace, hideTracesWithoutShowTrace) { SymbolTable testTable; - // ErrorInfo::showTrace = false; auto problem_file = testTable.create(test_file); auto oneliner_file = testTable.create(one_liner); @@ -291,10 +291,12 @@ namespace nix { testing::internal::CaptureStderr(); + logger->setShowTrace(false); + logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from command line argument\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n"); } /* ---------------------------------------------------------------------------- From 2a39c083dca8f672c695898d56cf62081ac79eca Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 1 Jul 2020 10:37:31 -0600 Subject: [PATCH 27/33] non-pos trace test --- src/libutil/error.cc | 24 ++++++++++++++---------- src/libutil/logging.cc | 1 + src/libutil/tests/logging.cc | 4 +++- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 6b6b268a2..78e2a5ea2 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -187,6 +187,7 @@ void printCodeLines(std::ostream &out, void printAtPos(const string &prefix, const ErrPos &pos, std::ostream &out) { + if (pos) { switch (pos.origin) { case foFile: { @@ -290,7 +291,7 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show einfo.programName.value_or("")); bool nl = false; // intersperse newline between sections. - if (einfo.errPos.has_value()) { + if (einfo.errPos.has_value() && (*einfo.errPos)) { out << prefix << std::endl; printAtPos(prefix, *einfo.errPos, out); nl = true; @@ -355,17 +356,20 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show { try { out << std::endl << prefix; - out << ANSI_BLUE << "trace: " << ANSI_NORMAL << iter->hint.str() << std::endl; + out << ANSI_BLUE << "trace: " << ANSI_NORMAL << iter->hint.str(); - auto pos = *iter->pos; - printAtPos(prefix, pos, out); nl = true; - auto loc = getCodeLines(pos); - if (loc.has_value()) - { - out << std::endl; - printCodeLines(out, prefix, pos, *loc); - out << std::endl; + auto pos = *iter->pos; + if (pos) { + out << std::endl << prefix; + printAtPos(prefix, pos, out); + auto loc = getCodeLines(pos); + if (loc.has_value()) + { + out << std::endl << prefix; + printCodeLines(out, prefix, pos, *loc); + out << std::endl << prefix; + } } } catch(const std::bad_optional_access& e) { out << iter->hint.str() << std::endl; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 504f5fb52..7a58527b8 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -84,6 +84,7 @@ public: log(ei.level, oss.str()); } + void startActivity(ActivityId act, Verbosity lvl, ActivityType type, const std::string & s, const Fields & fields, ActivityId parent) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index a73c1f8d9..4ebc3a3bf 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -263,6 +263,7 @@ namespace nix { }); e.addTrace(Pos(foStdin, oneliner_file, 1, 19), "while trying to compute %1%", 42); + e.addTrace(std::nullopt, "while doing something without a %1%", "pos"); testing::internal::CaptureStderr(); @@ -271,7 +272,7 @@ namespace nix { logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n\n\x1B[34;1mtrace: \x1B[0mwhile doing something without a \x1B[33;1mpos\x1B[0m\n"); } TEST(addTrace, hideTracesWithoutShowTrace) { @@ -288,6 +289,7 @@ namespace nix { }); e.addTrace(Pos(foStdin, oneliner_file, 1, 19), "while trying to compute %1%", 42); + e.addTrace(std::nullopt, "while doing something without a %1%", "pos"); testing::internal::CaptureStderr(); From 3629b0585aaf13f7d25510c55ca06611cb2135af Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 1 Jul 2020 11:49:01 -0600 Subject: [PATCH 28/33] don't include errpos for addErrorContext --- src/libexpr/primops.cc | 3 +-- src/libutil/error.cc | 7 ++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4cc3718e3..dec917b38 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -471,8 +471,7 @@ static void prim_addErrorContext(EvalState & state, const Pos & pos, Value * * a v = *args[1]; } catch (Error & e) { PathSet context; - // TODO: is this right, include this pos?? Test it. esp with LOC. - e.addTrace(pos, "%1%", state.coerceToString(pos, *args[0], context)); + e.addTrace(std::nullopt, state.coerceToString(pos, *args[0], context)); throw; } } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 78e2a5ea2..3a9b924f2 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -305,7 +305,7 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show nl = true; } - if (einfo.errPos.has_value()) { + if (einfo.errPos.has_value() && (*einfo.errPos)) { auto loc = getCodeLines(*einfo.errPos); // lines of code. @@ -359,9 +359,10 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show out << ANSI_BLUE << "trace: " << ANSI_NORMAL << iter->hint.str(); nl = true; - auto pos = *iter->pos; - if (pos) { + if (*iter->pos) { + auto pos = iter->pos.value(); out << std::endl << prefix; + printAtPos(prefix, pos, out); auto loc = getCodeLines(pos); if (loc.has_value()) From a295b2ea96c1cee263147a7d1b1b937d63e954af Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 1 Jul 2020 12:02:02 -0600 Subject: [PATCH 29/33] if no errLoc, no Loc. --- src/libutil/error.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 3a9b924f2..65c5813a4 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -94,8 +94,10 @@ std::optional getCodeLines(const ErrPos &errPos) } } catch (EndOfFile &eof) { - // TODO: return maybe partial loc? - return std::nullopt; + if (loc.errLineOfCode.has_value()) + return loc; + else + return std::nullopt; } catch (std::exception &e) { printError("error reading nix file: %s\n%s", errPos.file, e.what()); From 8497891b99b37c590c82cfe787d9541a0fda31de Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 1 Jul 2020 13:50:18 -0600 Subject: [PATCH 30/33] spacing --- src/libutil/tests/logging.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 4ebc3a3bf..f51e40fa8 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -144,7 +144,6 @@ namespace nix { * logError * --------------------------------------------------------------------------*/ - TEST(logError, logErrorWithoutHintOrCode) { testing::internal::CaptureStderr(); From 5ae498872a832b0df93b551210f0a3a8b6ffaa35 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 2 Jul 2020 07:14:40 -0600 Subject: [PATCH 31/33] assert for invalid fileorigin --- src/libexpr/parser.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 472b33c2f..878f06c96 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -584,7 +584,7 @@ Expr * EvalState::parse(const char * text, FileOrigin origin, data.file = data.symbols.create(text); break; default: - throw Error("invalid FileOrigin in parse"); + assert(false); } data.basePath = basePath; From bf2788e4c1d92a8d625f016b2a3b4d34990f33e3 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 2 Jul 2020 09:04:31 -0600 Subject: [PATCH 32/33] move showTrace to new loggerSettings --- src/libexpr/eval.cc | 2 +- src/libmain/loggers.cc | 6 +++--- src/libmain/progress-bar.cc | 10 +--------- src/libmain/shared.cc | 3 +-- src/libstore/daemon.cc | 10 +--------- src/libstore/globals.hh | 4 ---- src/libstore/remote-store.cc | 3 ++- src/libutil/logging.cc | 34 ++++++++++++---------------------- src/libutil/logging.hh | 16 ++++++++++++---- src/libutil/tests/logging.cc | 4 ++-- 10 files changed, 35 insertions(+), 57 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e89948b94..c1a9af9b2 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1232,7 +1232,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & po /* Evaluate the body. This is conditional on showTrace, because catching exceptions makes this function not tail-recursive. */ - if (settings.showTrace) + if (loggerSettings.showTrace.get()) try { lambda.body->eval(*this, env2, v); } catch (Error & e) { diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index fd6ce912f..fa18f991d 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -22,11 +22,11 @@ LogFormat parseLogFormat(const std::string & logFormatStr) { Logger * makeDefaultLogger() { switch (defaultLogFormat) { case LogFormat::raw: - return makeSimpleLogger(false, false); + return makeSimpleLogger(false); case LogFormat::rawWithLogs: - return makeSimpleLogger(true, false); + return makeSimpleLogger(true); case LogFormat::internalJson: - return makeJSONLogger(*makeSimpleLogger(true, false)); + return makeJSONLogger(*makeSimpleLogger(true)); case LogFormat::bar: return makeProgressBar(); case LogFormat::barWithLogs: diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 265b16908..3f7d99a1d 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -81,14 +81,12 @@ private: bool printBuildLogs; bool isTTY; - bool showTrace; public: ProgressBar(bool printBuildLogs, bool isTTY) : printBuildLogs(printBuildLogs) , isTTY(isTTY) - , showTrace(false) { state_.lock()->active = isTTY; updateThread = std::thread([&]() { @@ -133,16 +131,10 @@ public: auto state(state_.lock()); std::stringstream oss; - showErrorInfo(oss, ei, showTrace); + showErrorInfo(oss, ei, loggerSettings.showTrace.get()); log(*state, ei.level, oss.str()); } - bool getShowTrace() const override { - return showTrace; - } - void setShowTrace(bool showTrace) override { - this->showTrace = showTrace; - } void log(State & state, Verbosity lvl, const std::string & s) { diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 5cbdd19e5..52718c231 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -323,9 +323,8 @@ int handleExceptions(const string & programName, std::function fun) printError("Try '%1% --help' for more information.", programName); return 1; } catch (BaseError & e) { - logger->setShowTrace(settings.showTrace); logError(e.info()); - if (e.hasTrace() && !settings.showTrace) + if (e.hasTrace() && !loggerSettings.showTrace.get()) printError("(use '--show-trace' to show detailed location information)"); return e.status; } catch (std::bad_alloc & e) { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 186ccc5b4..40d126192 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -41,10 +41,9 @@ struct TunnelLogger : public Logger Sync state_; unsigned int clientVersion; - bool showTrace; TunnelLogger(FdSink & to, unsigned int clientVersion) - : to(to), clientVersion(clientVersion), showTrace(false) { } + : to(to), clientVersion(clientVersion) { } void enqueueMsg(const std::string & s) { @@ -86,13 +85,6 @@ struct TunnelLogger : public Logger enqueueMsg(*buf.s); } - bool getShowTrace() const override { - return showTrace; - } - void setShowTrace(bool showTrace) override { - this->showTrace = showTrace; - } - /* startWork() means that we're starting an operation for which we want to send out stderr to the client. */ void startWork() diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 2fbcafff8..58cf08763 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -196,10 +196,6 @@ public: /* Whether to lock the Nix client and worker to the same CPU. */ bool lockCPU; - /* Whether to show a stack trace if Nix evaluation fails. */ - Setting showTrace{this, false, "show-trace", - "Whether to show a stack trace on evaluation errors."}; - Setting sandboxMode{this, #if __linux__ smEnabled diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index b7cc7a5fc..670a6d873 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -8,6 +8,7 @@ #include "derivations.hh" #include "pool.hh" #include "finally.hh" +#include "logging.hh" #include #include @@ -197,7 +198,7 @@ void RemoteStore::setOptions(Connection & conn) overrides.erase(settings.maxSilentTime.name); overrides.erase(settings.buildCores.name); overrides.erase(settings.useSubstitutes.name); - overrides.erase(settings.showTrace.name); + overrides.erase(loggerSettings.showTrace.name); conn.to << overrides.size(); for (auto & i : overrides) conn.to << i.first << i.second.value; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 7a58527b8..90c6afe81 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -1,5 +1,6 @@ #include "logging.hh" #include "util.hh" +#include "config.hh" #include #include @@ -7,6 +8,10 @@ namespace nix { +LoggerSettings loggerSettings; + +static GlobalConfig::Register r1(&loggerSettings); + static thread_local ActivityId curActivity = 0; ActivityId getCurActivity() @@ -18,7 +23,7 @@ void setCurActivity(const ActivityId activityId) curActivity = activityId; } -Logger * logger = makeSimpleLogger(true, false); +Logger * logger = makeSimpleLogger(true); void Logger::warn(const std::string & msg) { @@ -36,10 +41,9 @@ public: bool systemd, tty; bool printBuildLogs; - bool showTrace; - SimpleLogger(bool printBuildLogs, bool showTrace) - : printBuildLogs(printBuildLogs), showTrace(showTrace) + SimpleLogger(bool printBuildLogs) + : printBuildLogs(printBuildLogs) { systemd = getEnv("IN_SYSTEMD") == "1"; tty = isatty(STDERR_FILENO); @@ -49,13 +53,6 @@ public: return printBuildLogs; } - bool getShowTrace() const override { - return showTrace; - } - void setShowTrace(bool showTrace) override { - this->showTrace = showTrace; - } - void log(Verbosity lvl, const FormatOrString & fs) override { if (lvl > verbosity) return; @@ -80,7 +77,7 @@ public: void logEI(const ErrorInfo & ei) override { std::stringstream oss; - showErrorInfo(oss, ei, showTrace); + showErrorInfo(oss, ei, loggerSettings.showTrace.get()); log(ei.level, oss.str()); } @@ -129,9 +126,9 @@ void writeToStderr(const string & s) } } -Logger * makeSimpleLogger(bool printBuildLogs, bool showTrace) +Logger * makeSimpleLogger(bool printBuildLogs) { - return new SimpleLogger(printBuildLogs, showTrace); + return new SimpleLogger(printBuildLogs); } std::atomic nextId{(uint64_t) getpid() << 32}; @@ -152,13 +149,6 @@ struct JSONLogger : Logger { return true; } - bool getShowTrace() const override { - return prevLogger.getShowTrace(); - } - void setShowTrace(bool showTrace) override { - prevLogger.setShowTrace(showTrace); - } - void addFields(nlohmann::json & json, const Fields & fields) { if (fields.empty()) return; @@ -189,7 +179,7 @@ struct JSONLogger : Logger { void logEI(const ErrorInfo & ei) override { std::ostringstream oss; - showErrorInfo(oss, ei, getShowTrace()); + showErrorInfo(oss, ei, loggerSettings.showTrace.get()); nlohmann::json json; json["action"] = "msg"; diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 273300b50..09619aac6 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -2,6 +2,7 @@ #include "types.hh" #include "error.hh" +#include "config.hh" namespace nix { @@ -34,6 +35,16 @@ typedef enum { typedef uint64_t ActivityId; +struct LoggerSettings : Config +{ + Setting showTrace{this, + false, + "show-trace", + "Whether to show a stack trace on evaluation errors."}; +}; + +extern LoggerSettings loggerSettings; + class Logger { friend struct Activity; @@ -75,9 +86,6 @@ public: logEI(ei); } - virtual bool getShowTrace() const = 0; - virtual void setShowTrace(bool showTrace) = 0; - virtual void warn(const std::string & msg); virtual void startActivity(ActivityId act, Verbosity lvl, ActivityType type, @@ -149,7 +157,7 @@ struct PushActivity extern Logger * logger; -Logger * makeSimpleLogger(bool printBuildLogs = true, bool showTrace = false); +Logger * makeSimpleLogger(bool printBuildLogs = true); Logger * makeJSONLogger(Logger & prevLogger); diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index f51e40fa8..ef22e9966 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -266,7 +266,7 @@ namespace nix { testing::internal::CaptureStderr(); - logger->setShowTrace(true); + loggerSettings.showTrace.assign(true); logError(e.info()); @@ -292,7 +292,7 @@ namespace nix { testing::internal::CaptureStderr(); - logger->setShowTrace(false); + loggerSettings.showTrace.assign(false); logError(e.info()); From 5818271c6e617f5c956ba74d19611d4c6e76bfe8 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Thu, 2 Jul 2020 09:41:54 -0600 Subject: [PATCH 33/33] spacing --- src/libutil/error.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 65c5813a4..a4ee7afc2 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -7,7 +7,6 @@ namespace nix { - const std::string nativeSystem = SYSTEM; BaseError & BaseError::addTrace(std::optional e, hintformat hint)