Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: NixOS/nix
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: c5b42c5a4213
Choose a base ref
...
head repository: NixOS/nix
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 488a82684229
Choose a head ref
  • 5 commits
  • 35 files changed
  • 1 contributor

Commits on Jan 21, 2021

  1. Improve error formatting

    Changes:
    
    * The divider lines are gone. These were in practice a bit confusing,
      in particular with --show-trace or --keep-going, since then there
      were multiple lines, suggesting a start/end which wasn't the case.
    
    * Instead, multi-line error messages are now indented to align with
      the prefix (e.g. "error: ").
    
    * The 'description' field is gone since we weren't really using it.
    
    * 'hint' is renamed to 'msg' since it really wasn't a hint.
    
    * The error is now printed *before* the location info.
    
    * The 'name' field is no longer printed since most of the time it
      wasn't very useful since it was just the name of the exception (like
      EvalError). Ideally in the future this would be a unique, easily
      googleable error ID (like rustc).
    
    * "trace:" is now just "…". This assumes error contexts start with
      something like "while doing X".
    
    Example before:
    
      error: --- AssertionError ---------------------------------------------------------------------------------------- nix
      at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix
    
           6|
           7|   x = assert false; 1;
            |       ^
           8|
    
      assertion 'false' failed
      ----------------------------------------------------- show-trace -----------------------------------------------------
      trace: while evaluating the attribute 'x' of the derivation 'hello-2.10'
      at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix
    
         191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
         192|           name = "${attrs.pname}-${attrs.version}";
            |           ^
         193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {
    
    Example after:
    
      error: assertion 'false' failed
    
             at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix
    
                  6|
                  7|   x = assert false; 1;
                   |       ^
                  8|
    
             … while evaluating the attribute 'x' of the derivation 'hello-2.10'
    
             at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix
    
                191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
                192|           name = "${attrs.pname}-${attrs.version}";
                   |           ^
                193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {
    edolstra committed Jan 21, 2021

    Unverified

    The committer email address is not verified.
    Copy the full SHA
    8d4268d View commit details
  2. Remove trailing whitespace

    edolstra committed Jan 21, 2021
    Copy the full SHA
    4060834 View commit details
  3. Change error position formatting

    It's now
    
      at /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix:7:7:
    
    instead of
    
      at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix
    
    The new format is more standard and clickable.
    edolstra committed Jan 21, 2021
    Copy the full SHA
    55849e1 View commit details
  4. Fix macOS build

    edolstra committed Jan 21, 2021
    Copy the full SHA
    0eb22db View commit details

Commits on Jan 25, 2021

  1. Merge pull request #4467 from edolstra/error-formatting

    Improve error formatting
    edolstra authored Jan 25, 2021
    Copy the full SHA
    488a826 View commit details
54 changes: 24 additions & 30 deletions src/build-remote/build-remote.cc
Original file line number Diff line number Diff line change
@@ -176,13 +176,14 @@ static int main_build_remote(int argc, char * * argv)
else
{
// build the hint template.
string hintstring = "derivation: %s\nrequired (system, features): (%s, %s)";
hintstring += "\n%s available machines:";
hintstring += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)";
string errorText =
"Failed to find a machine for remote build!\n"
"derivation: %s\nrequired (system, features): (%s, %s)";
errorText += "\n%s available machines:";
errorText += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)";

for (unsigned int i = 0; i < machines.size(); ++i) {
hintstring += "\n(%s, %s, %s, %s)";
}
for (unsigned int i = 0; i < machines.size(); ++i)
errorText += "\n(%s, %s, %s, %s)";

// add the template values.
string drvstr;
@@ -191,25 +192,21 @@ static int main_build_remote(int argc, char * * argv)
else
drvstr = "<unknown>";

auto hint = hintformat(hintstring);
hint
% drvstr
% neededSystem
% concatStringsSep<StringSet>(", ", requiredFeatures)
% machines.size();

for (auto & m : machines) {
hint % concatStringsSep<vector<string>>(", ", m.systemTypes)
% m.maxJobs
% concatStringsSep<StringSet>(", ", m.supportedFeatures)
% concatStringsSep<StringSet>(", ", m.mandatoryFeatures);
}
auto error = hintformat(errorText);
error
% drvstr
% neededSystem
% concatStringsSep<StringSet>(", ", requiredFeatures)
% machines.size();

for (auto & m : machines)
error
% concatStringsSep<vector<string>>(", ", m.systemTypes)
% m.maxJobs
% concatStringsSep<StringSet>(", ", m.supportedFeatures)
% concatStringsSep<StringSet>(", ", m.mandatoryFeatures);

logErrorInfo(canBuildLocally ? lvlChatty : lvlWarn, {
.name = "Remote build",
.description = "Failed to find a machine for remote build!",
.hint = hint
});
printMsg(canBuildLocally ? lvlChatty : lvlWarn, error);

std::cerr << "# decline\n";
}
@@ -234,12 +231,9 @@ static int main_build_remote(int argc, char * * argv)

} catch (std::exception & e) {
auto msg = chomp(drainFD(5, false));
logError({
.name = "Remote build",
.hint = hintfmt("cannot build on '%s': %s%s",
bestMachine->storeUri, e.what(),
(msg.empty() ? "" : ": " + msg))
});
printError("cannot build on '%s': %s%s",
bestMachine->storeUri, e.what(),
msg.empty() ? "" : ": " + msg);
bestMachine->enabled = false;
continue;
}
2 changes: 1 addition & 1 deletion src/libexpr/attr-set.hh
Original file line number Diff line number Diff line change
@@ -77,7 +77,7 @@ public:
auto a = get(name);
if (!a)
throw Error({
.hint = hintfmt("attribute '%s' missing", name),
.msg = hintfmt("attribute '%s' missing", name),
.errPos = pos
});

4 changes: 2 additions & 2 deletions src/libexpr/eval-inline.hh
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ namespace nix {
LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s))
{
throw EvalError({
.hint = hintfmt(s),
.msg = hintfmt(s),
.errPos = pos
});
}
@@ -24,7 +24,7 @@ LocalNoInlineNoReturn(void throwTypeError(const char * s, const Value & v))
LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const Value & v))
{
throw TypeError({
.hint = hintfmt(s, showType(v)),
.msg = hintfmt(s, showType(v)),
.errPos = pos
});
}
18 changes: 9 additions & 9 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
@@ -622,7 +622,7 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2))
LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const string & s2))
{
throw EvalError({
.hint = hintfmt(s, s2),
.msg = hintfmt(s, s2),
.errPos = pos
});
}
@@ -635,7 +635,7 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2, con
LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const string & s2, const string & s3))
{
throw EvalError({
.hint = hintfmt(s, s2, s3),
.msg = hintfmt(s, s2, s3),
.errPos = pos
});
}
@@ -644,47 +644,47 @@ 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),
.msg = hintfmt(s, sym, p2),
.errPos = p1
});
}

LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s))
{
throw TypeError({
.hint = hintfmt(s),
.msg = hintfmt(s),
.errPos = pos
});
}

LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const ExprLambda & fun, const Symbol & s2))
{
throw TypeError({
.hint = hintfmt(s, fun.showNamePos(), s2),
.msg = hintfmt(s, fun.showNamePos(), s2),
.errPos = pos
});
}

LocalNoInlineNoReturn(void throwAssertionError(const Pos & pos, const char * s, const string & s1))
{
throw AssertionError({
.hint = hintfmt(s, s1),
.msg = hintfmt(s, s1),
.errPos = pos
});
}

LocalNoInlineNoReturn(void throwUndefinedVarError(const Pos & pos, const char * s, const string & s1))
{
throw UndefinedVarError({
.hint = hintfmt(s, s1),
.msg = hintfmt(s, s1),
.errPos = pos
});
}

LocalNoInlineNoReturn(void throwMissingArgumentError(const Pos & pos, const char * s, const string & s1))
{
throw MissingArgumentError({
.hint = hintfmt(s, s1),
.msg = hintfmt(s, s1),
.errPos = pos
});
}
@@ -2057,7 +2057,7 @@ void EvalState::printStats()
string ExternalValueBase::coerceToString(const Pos & pos, PathSet & context, bool copyMore, bool copyToStore) const
{
throw TypeError({
.hint = hintfmt("cannot coerce %1% to a string", showType()),
.msg = hintfmt("cannot coerce %1% to a string", showType()),
.errPos = pos
});
}
2 changes: 1 addition & 1 deletion src/libexpr/nixexpr.cc
Original file line number Diff line number Diff line change
@@ -284,7 +284,7 @@ void ExprVar::bindVars(const StaticEnv & env)
"undefined variable" error now. */
if (withLevel == -1)
throw UndefinedVarError({
.hint = hintfmt("undefined variable '%1%'", name),
.msg = hintfmt("undefined variable '%1%'", name),
.errPos = pos
});
fromWith = true;
2 changes: 1 addition & 1 deletion src/libexpr/nixexpr.hh
Original file line number Diff line number Diff line change
@@ -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),
.msg = hintfmt("duplicate formal function argument '%1%'", arg),
.errPos = pos
});
};
30 changes: 14 additions & 16 deletions src/libexpr/parser.y
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@ namespace nix {
Path basePath;
Symbol file;
FileOrigin origin;
ErrorInfo error;
std::optional<ErrorInfo> error;
Symbol sLetBody;
ParseData(EvalState & state)
: state(state)
@@ -66,16 +66,16 @@ namespace nix {
static void dupAttr(const AttrPath & attrPath, const Pos & pos, const Pos & prevPos)
{
throw ParseError({
.hint = hintfmt("attribute '%1%' already defined at %2%",
showAttrPath(attrPath), prevPos),
.msg = hintfmt("attribute '%1%' already defined at %2%",
showAttrPath(attrPath), prevPos),
.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),
.msg = hintfmt("attribute '%1%' already defined at %2%", attr, prevPos),
.errPos = pos
});
}
@@ -146,7 +146,7 @@ static void addFormal(const Pos & pos, Formals * formals, const Formal & formal)
{
if (!formals->argNames.insert(formal.name).second)
throw ParseError({
.hint = hintfmt("duplicate formal function argument '%1%'",
.msg = hintfmt("duplicate formal function argument '%1%'",
formal.name),
.errPos = pos
});
@@ -258,7 +258,7 @@ static inline Pos makeCurPos(const YYLTYPE & loc, ParseData * data)
void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * error)
{
data->error = {
.hint = hintfmt(error),
.msg = hintfmt(error),
.errPos = makeCurPos(*loc, data)
};
}
@@ -338,7 +338,7 @@ expr_function
| LET binds IN expr_function
{ if (!$2->dynamicAttrs.empty())
throw ParseError({
.hint = hintfmt("dynamic attributes not allowed in let"),
.msg = hintfmt("dynamic attributes not allowed in let"),
.errPos = CUR_POS
});
$$ = new ExprLet($2, $4);
@@ -418,7 +418,7 @@ expr_simple
static bool noURLLiterals = settings.isExperimentalFeatureEnabled("no-url-literals");
if (noURLLiterals)
throw ParseError({
.hint = hintfmt("URL literals are disabled"),
.msg = hintfmt("URL literals are disabled"),
.errPos = CUR_POS
});
$$ = new ExprString(data->symbols.create($1));
@@ -491,7 +491,7 @@ attrs
delete str;
} else
throw ParseError({
.hint = hintfmt("dynamic attributes not allowed in inherit"),
.msg = hintfmt("dynamic attributes not allowed in inherit"),
.errPos = makeCurPos(@2, data)
});
}
@@ -576,7 +576,7 @@ Expr * EvalState::parse(const char * text, FileOrigin origin,
ParseData data(*this);
data.origin = origin;
switch (origin) {
case foFile:
case foFile:
data.file = data.symbols.create(path);
break;
case foStdin:
@@ -593,7 +593,7 @@ Expr * EvalState::parse(const char * text, FileOrigin origin,
int res = yyparse(scanner, &data);
yylex_destroy(scanner);

if (res) throw ParseError(data.error);
if (res) throw ParseError(data.error.value());

data.result->bindVars(staticEnv);

@@ -703,7 +703,7 @@ Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos
return corepkgsPrefix + path.substr(4);

throw ThrownError({
.hint = hintfmt(evalSettings.pureEval
.msg = hintfmt(evalSettings.pureEval
? "cannot look up '<%s>' in pure evaluation mode (use '--impure' to override)"
: "file '%s' was not found in the Nix search path (add it using $NIX_PATH or -I)",
path),
@@ -725,8 +725,7 @@ std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathEl
store, resolveUri(elem.second), "source", false).first.storePath) };
} catch (FileTransferError & e) {
logWarning({
.name = "Entry download",
.hint = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second)
.msg = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second)
});
res = { false, "" };
}
@@ -736,8 +735,7 @@ std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathEl
res = { true, path };
else {
logWarning({
.name = "Entry not found",
.hint = hintfmt("warning: Nix search path entry '%1%' does not exist, ignoring", elem.second)
.msg = hintfmt("warning: Nix search path entry '%1%' does not exist, ignoring", elem.second)
});
res = { false, "" };
}
Loading