Skip to content

New CLI flag --replace-eval-errors #13095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

New CLI flag --replace-eval-errors #13095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions doc/manual/source/command-ref/nix-instantiate.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ standard input.
When used with `--eval`, print the resulting value as an JSON
representation of the abstract syntax tree rather than as a Nix expression.

- `--replace-eval-errors`

When used with `--eval` and `--json`, replace any evaluation errors with the string
`"«evaluation error»"`.
Comment on lines +117 to +118
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really what it does. It should say something like: any attribute that throws an evaluation error is represented by a JSON property with the string value «evaluation error».


- `--xml`

When used with `--eval`, print the resulting value as an XML
Expand Down Expand Up @@ -205,3 +210,10 @@ $ nix-instantiate --eval --xml --strict --expr '{ x = {}; }'
</attrs>
</expr>
```

Replacing evaluation errors:

```console
$ nix-instantiate --eval --json --replace-eval-errors --expr '{ a = throw "fail"; }'
{"a":"«evaluation error»"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this representation of eval errors is ambiguous, since

nix-instantiate --eval --json --expr '{ a = "«evaluation error»"; }'
{"a":"«evaluation error»"}

produces the same output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is true, we already have such ambiguity, for instance:

nix-instantiate --eval --json --expr --strict '{ foo.outPath = "bar"; }'
{"foo":"bar"}

That said, something like {"__nix_evaluation_error": null} would be subjectively less ambiguous, and allows for a message or other data to be put where it says null.

Considering that this originated from a tryEval improvement, we don't have to be so afraid of the "chaos" in the message anymore!

```
2 changes: 1 addition & 1 deletion src/libexpr-tests/json.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace nix {
std::string getJSONValue(Value& value) {
std::stringstream ss;
NixStringContext ps;
printValueAsJSON(state, true, value, noPos, ss, ps);
printValueAsJSON(state, true, false, value, noPos, ss, ps);
return ss.str();
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/include/nix/expr/value-to-json.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@

namespace nix {

nlohmann::json printValueAsJSON(EvalState & state, bool strict,
nlohmann::json printValueAsJSON(EvalState & state, bool strict, bool replaceEvalErrors,
Value & v, const PosIdx pos, NixStringContext & context, bool copyToStore = true);

void printValueAsJSON(EvalState & state, bool strict,
void printValueAsJSON(EvalState & state, bool strict, bool replaceEvalErrors,
Value & v, const PosIdx pos, std::ostream & str, NixStringContext & context, bool copyToStore = true);


Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/primops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1388,7 +1388,7 @@ static void derivationStrictInternal(

if (i->name == state.sStructuredAttrs) continue;

jsonObject->emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
jsonObject->emplace(key, printValueAsJSON(state, true, false, *i->value, pos, context));

if (i->name == state.sBuilder)
drv.builder = state.forceString(*i->value, context, pos, context_below);
Expand Down Expand Up @@ -2328,7 +2328,7 @@ static void prim_toJSON(EvalState & state, const PosIdx pos, Value * * args, Val
{
std::ostringstream out;
NixStringContext context;
printValueAsJSON(state, true, *args[0], pos, out, context);
printValueAsJSON(state, true, false, *args[0], pos, out, context);
v.mkString(toView(out), context);
}

Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/primops/fetchTree.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ static void fetchTree(
attrs.emplace(state.symbols[attr.name], uint64_t(intValue));
} else if (state.symbols[attr.name] == "publicKeys") {
experimentalFeatureSettings.require(Xp::VerifiedFetches);
attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, pos, context).dump());
attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, false, *attr.value, pos, context).dump());
}
else
state.error<TypeError>("argument '%s' to '%s' is %s while a string, Boolean or integer is expected",
Expand Down
28 changes: 22 additions & 6 deletions src/libexpr/value-to-json.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
#include <cstdlib>
#include <iomanip>
#include <nlohmann/json.hpp>
#include <typeinfo>


namespace nix {
using json = nlohmann::json;
// TODO: rename. It doesn't print.
json printValueAsJSON(EvalState & state, bool strict,
json printValueAsJSON(EvalState & state, bool strict, bool replaceEvalErrors,
Value & v, const PosIdx pos, NixStringContext & context, bool copyToStore)
{
checkInterrupt();
Expand Down Expand Up @@ -54,13 +55,27 @@ json printValueAsJSON(EvalState & state, bool strict,
break;
}
if (auto i = v.attrs()->get(state.sOutPath))
return printValueAsJSON(state, strict, *i->value, i->pos, context, copyToStore);
return printValueAsJSON(state, strict, replaceEvalErrors, *i->value, i->pos, context, copyToStore);
else {
out = json::object();
for (auto & a : v.attrs()->lexicographicOrder(state.symbols)) {
try {
out.emplace(state.symbols[a->name], printValueAsJSON(state, strict, *a->value, a->pos, context, copyToStore));
out.emplace(state.symbols[a->name], printValueAsJSON(state, strict, replaceEvalErrors, *a->value, a->pos, context, copyToStore));
} catch (Error & e) {
std::cerr << "Caught an Error of type: " << typeid(e).name() << std::endl;
// std::cerr << "Caught an Error of type: " << e.message() << std::endl;
// std::cerr << "Caught an Error of type: " << e.what() << std::endl;

// TODO: Figure out what Error is here?
// We seem to be not catching FileNotFoundError.
bool isEvalError = dynamic_cast<EvalError *>(&e);
bool isFileNotFoundError = dynamic_cast<FileNotFound *>(&e);
// Restrict replaceEvalErrors only only evaluation errors
if (replaceEvalErrors && (isEvalError || isFileNotFoundError)) {
out.emplace(state.symbols[a->name], "«evaluation error»");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the semantics of builtins.toJSON and derivation with structured attrs (since it calls printValueAsJSON()), making them ignore exceptions which seems like a bad thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ties in with the comment about making it a flag instead of a setting.
In line with that, this could then refer to a new C++ parameter on printValueAsJSON, instead of state.settings.

continue;
}

e.addTrace(state.positions[a->pos],
HintFmt("while evaluating attribute '%1%'", state.symbols[a->name]));
throw;
Expand All @@ -75,8 +90,9 @@ json printValueAsJSON(EvalState & state, bool strict,
int i = 0;
for (auto elem : v.listItems()) {
try {
out.push_back(printValueAsJSON(state, strict, *elem, pos, context, copyToStore));
out.push_back(printValueAsJSON(state, strict, replaceEvalErrors, *elem, pos, context, copyToStore));
} catch (Error & e) {
// TODO: Missing catch
e.addTrace(state.positions[pos],
HintFmt("while evaluating list element at index %1%", i));
throw;
Expand Down Expand Up @@ -106,11 +122,11 @@ json printValueAsJSON(EvalState & state, bool strict,
return out;
}

void printValueAsJSON(EvalState & state, bool strict,
void printValueAsJSON(EvalState & state, bool strict, bool replaceEvalErrors,
Value & v, const PosIdx pos, std::ostream & str, NixStringContext & context, bool copyToStore)
{
try {
str << printValueAsJSON(state, strict, v, pos, context, copyToStore);
str << printValueAsJSON(state, strict, replaceEvalErrors, v, pos, context, copyToStore);
} catch (nlohmann::json::exception & e) {
throw JSONSerializationError("JSON serialization error: %s", e.what());
}
Expand Down
2 changes: 1 addition & 1 deletion src/libflake/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ static void parseFlakeInputAttr(
if (attr.name == state.symbols.create("publicKeys")) {
experimentalFeatureSettings.require(Xp::VerifiedFetches);
NixStringContext emptyContext = {};
attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, attr.pos, emptyContext).dump());
attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, false, *attr.value, attr.pos, emptyContext).dump());
} else
state.error<TypeError>("flake input attribute '%s' is %s while a string, Boolean, or integer is expected",
state.symbols[attr.name], showType(*attr.value)).debugThrow();
Expand Down
2 changes: 1 addition & 1 deletion src/libutil/posix-source-accessor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void PosixSourceAccessor::readFile(
#endif
));
if (!fd)
throw SysError("opening file '%1%'", ap.string());
throw SysError("opening file9 '%1%'", ap.string());

struct stat st;
if (fstat(fromDescriptorReadOnly(fd.get()), &st) == -1)
Expand Down
2 changes: 1 addition & 1 deletion src/nix-env/nix-env.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ static void queryJSON(Globals & globals, std::vector<PackageInfo> & elems, bool
metaObj[j] = nullptr;
} else {
NixStringContext context;
metaObj[j] = printValueAsJSON(*globals.state, true, *v, noPos, context);
metaObj[j] = printValueAsJSON(*globals.state, true, false, *v, noPos, context);
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/nix-instantiate/nix-instantiate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static int rootNr = 0;
enum OutputKind { okPlain, okRaw, okXML, okJSON };

void processExpr(EvalState & state, const Strings & attrPaths,
bool parseOnly, bool strict, Bindings & autoArgs,
bool parseOnly, bool strict, bool replaceEvalErrors, Bindings & autoArgs,
bool evalOnly, OutputKind output, bool location, Expr * e)
{
if (parseOnly) {
Expand Down Expand Up @@ -58,7 +58,7 @@ void processExpr(EvalState & state, const Strings & attrPaths,
else if (output == okXML)
printValueAsXML(state, strict, location, vRes, std::cout, context, noPos);
else if (output == okJSON) {
printValueAsJSON(state, strict, vRes, v.determinePos(noPos), std::cout, context);
printValueAsJSON(state, strict, replaceEvalErrors, vRes, v.determinePos(noPos), std::cout, context);
std::cout << std::endl;
} else {
if (strict) state.forceValueDeep(vRes);
Expand Down Expand Up @@ -106,6 +106,7 @@ static int main_nix_instantiate(int argc, char * * argv)
OutputKind outputKind = okPlain;
bool xmlOutputSourceLocation = true;
bool strict = false;
bool replaceEvalErrors = false;
Strings attrPaths;
bool wantsReadWrite = false;

Expand Down Expand Up @@ -147,6 +148,8 @@ static int main_nix_instantiate(int argc, char * * argv)
xmlOutputSourceLocation = false;
else if (*arg == "--strict")
strict = true;
else if (*arg == "--replace-eval-errors")
replaceEvalErrors = true;
else if (*arg == "--dry-run")
settings.readOnlyMode = true;
else if (*arg != "" && arg->at(0) == '-')
Expand Down Expand Up @@ -184,7 +187,7 @@ static int main_nix_instantiate(int argc, char * * argv)

if (readStdin) {
Expr * e = state->parseStdin();
processExpr(*state, attrPaths, parseOnly, strict, autoArgs,
processExpr(*state, attrPaths, parseOnly, strict, replaceEvalErrors, autoArgs,
evalOnly, outputKind, xmlOutputSourceLocation, e);
} else if (files.empty() && !fromArgs)
files.push_back("./default.nix");
Expand All @@ -193,7 +196,7 @@ static int main_nix_instantiate(int argc, char * * argv)
Expr * e = fromArgs
? state->parseExprFromString(i, state->rootPath("."))
: state->parseExprFromFile(resolveExprPath(lookupFileArg(*state, i)));
processExpr(*state, attrPaths, parseOnly, strict, autoArgs,
processExpr(*state, attrPaths, parseOnly, strict, replaceEvalErrors, autoArgs,
evalOnly, outputKind, xmlOutputSourceLocation, e);
}

Expand Down
9 changes: 8 additions & 1 deletion src/nix/eval.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace nix::fs { using namespace std::filesystem; }
struct CmdEval : MixJSON, InstallableValueCommand, MixReadOnlyOption
{
bool raw = false;
bool replaceEvalErrors = false;
std::optional<std::string> apply;
std::optional<std::filesystem::path> writeTo;

Expand All @@ -39,6 +40,12 @@ struct CmdEval : MixJSON, InstallableValueCommand, MixReadOnlyOption
.labels = {"path"},
.handler = {&writeTo},
});

addFlag({
.longName = "replace-eval-errors",
.description = "When used with `--json` the Nix evaluator will replace evaluation errors with a fixed value.",
.handler = {&replaceEvalErrors, true},
});
}

std::string description() override
Expand Down Expand Up @@ -118,7 +125,7 @@ struct CmdEval : MixJSON, InstallableValueCommand, MixReadOnlyOption
}

else if (json) {
printJSON(printValueAsJSON(*state, true, *v, pos, context, false));
printJSON(printValueAsJSON(*state, true, replaceEvalErrors, *v, pos, context, false));
}

else {
Expand Down
7 changes: 7 additions & 0 deletions src/nix/eval.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ R""(
# cat ./out/subdir/bla
123

* Replace evaluation errors:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Replace evaluation errors" is very vague (though the example does make it clear).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is clear when you have in mind the model of evaluation as term rewriting, although that's not how we introduce evaluation at all. In fact, we don't have a description of evaluation in the manual.

Reminded me of

although that doesn't go all that much into term rewriting either.


```console
$ nix eval --json --replace-eval-errors --expr '{ a = throw "fail"; }'
{"a":"«evaluation error»"}
```

# Description

This command evaluates the given Nix expression, and prints the result on standard output.
Expand Down
17 changes: 17 additions & 0 deletions tests/functional/eval.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,23 @@ nix-instantiate --eval -E 'assert 1 + 2 == 3; true'
[[ $(nix-instantiate -A int --eval - < "./eval.nix") == 123 ]]
[[ "$(nix-instantiate --eval -E '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]]

expected="$(echo '{
"missingAttr":"«evaluation error»",
"insideAList":["«evaluation error»"],
"deeper":{v:"«evaluation error»"},
"failedAssertion":"«evaluation error»",
"missingFile":"«evaluation error»",
"missingImport":"«evaluation error»",
"outOfBounds":"«evaluation error»",
"failedCoersion":"«evaluation error»",
"failedAddition":"«evaluation error»"
}' | tr -d '\n')"
actual="$(nix-instantiate --eval --json --strict --replace-eval-errors "./replace-eval-errors.nix")"
[[ $actual == "$expected" ]] || diff --unified <(echo "$actual") <(echo "$expected") >&2

actual="$(nix eval --json --replace-eval-errors -f "./replace-eval-errors.nix")"
[[ $actual == "$expected" ]] || diff --unified <(echo "$actual") <(echo "$expected") >&2

# Check that symlink cycles don't cause a hang.
ln -sfn cycle.nix "$TEST_ROOT/cycle.nix"
(! nix eval --file "$TEST_ROOT/cycle.nix")
Expand Down
11 changes: 11 additions & 0 deletions tests/functional/replace-eval-errors.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
missingAttr = let bar = { }; in bar.notExist;
insideAList = [ (throw "a throw") ];
deeper = { v = throw "v"; };
failedAssertion = assert true; assert false; null;
missingFile = builtins.readFile ./missing-file.txt;
missingImport = import ./missing-import.nix;
outOfBounds = builtins.elemAt [ 1 2 3 ] 100;
failedCoersion = "${1}";
failedAddition = 1.0 + "a string";
}
Loading