From 2b2a1ff51bc936e9eda0e216ceb02703ebb1278a Mon Sep 17 00:00:00 2001 From: Michael Bishop Date: Wed, 4 Mar 2020 22:22:20 -0400 Subject: [PATCH] fix: log all ifd Co-authored-by: Dylan Green --- src/libexpr/eval.cc | 2 +- src/libexpr/eval.hh | 5 ++- src/libexpr/primops.cc | 71 ++++++++++++++++++++------------ src/libstore/build-result.hh | 9 +++- src/libutil/logging.hh | 1 + tests/import-from-derivation.nix | 10 +++++ tests/remote-store.sh | 13 +----- 7 files changed, 69 insertions(+), 42 deletions(-) create mode 100644 tests/import-from-derivation.nix diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 0b424367031..d3162a8c69f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1655,7 +1655,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & // 1. Unify this and above code. Heavily redundant. // 2. Create a fake env (arg1, arg2, etc.) and a fake expr (arg1: arg2: etc: builtins.name arg1 arg2 etc) // so the debugger allows to inspect the wrong parameters passed to the builtin. - primOp->primOp->fun(*this, noPos, vArgs, vCur); + primOp->primOp->fun(*this, pos, vArgs, vCur); } catch (Error & e) { addErrorTrace(e, pos, "while calling the '%1%' builtin", name); throw; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index bb3ac2b227e..20af9930cca 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -593,7 +593,7 @@ public: * Realise the given context, and return a mapping from the placeholders * used to construct the associated value to their final store path */ - [[nodiscard]] StringMap realiseContext(const NixStringContext & context); + [[nodiscard]] StringMap realiseContext(const NixStringContext & context, const PosIdx pos, const std::string_view reason); private: @@ -746,6 +746,9 @@ struct EvalSettings : Config Setting traceVerbose{this, false, "trace-verbose", "Whether `builtins.traceVerbose` should trace its first argument when evaluated."}; + + Setting logImportFromDerivation{this, false, "log-import-from-derivation", + "Emit log messages for all imports from derivations at the 'info' log level"}; }; extern EvalSettings evalSettings; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 0be39fa7d7c..ea3b4f35660 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -11,6 +11,7 @@ #include "value-to-json.hh" #include "value-to-xml.hh" #include "primops.hh" +#include "build-result.hh" #include #include @@ -38,9 +39,9 @@ namespace nix { InvalidPathError::InvalidPathError(const Path & path) : EvalError("path '%s' is not valid", path), path(path) {} -StringMap EvalState::realiseContext(const NixStringContext & context) +StringMap EvalState::realiseContext(const NixStringContext & context, const PosIdx pos, const std::string_view reason) { - std::vector drvs; + std::vector drvs; StringMap res; for (auto & c : context) { @@ -50,10 +51,8 @@ StringMap EvalState::realiseContext(const NixStringContext & context) }; std::visit(overloaded { [&](const NixStringContextElem::Built & b) { - drvs.push_back(DerivedPath::Built { - .drvPath = b.drvPath, - .outputs = OutputsSpec::Names { b.output }, - }); + auto ctxS = store->printStorePath(b.drvPath); + drvs.push_back(b); ensureValid(b.drvPath); }, [&](const NixStringContextElem::Opaque & o) { @@ -77,18 +76,38 @@ StringMap EvalState::realiseContext(const NixStringContext & context) "cannot build '%1%' during evaluation because the option 'allow-import-from-derivation' is disabled", store->printStorePath(drvs.begin()->drvPath))); + if (evalSettings.logImportFromDerivation) { + for (auto & drv : drvs) { + Activity act{ + *logger, lvlInfo, actFromDerivation, + fmt("Derivation %s Output %s: importing from derivation %s via %s", + store->printStorePath(drv.drvPath), drv.output, positions[pos], reason), + }; + } + } + /* Build/substitute the context. */ std::vector buildReqs; - for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d }); - store->buildPaths(buildReqs); + for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { + DerivedPath::Built { + .drvPath = d.drvPath, + .outputs = OutputsSpec::Names { d.output }, + } + }); + auto results = store->buildPathsWithResults(buildReqs); /* Get all the output paths corresponding to the placeholders we had */ - for (auto & drv : drvs) { - auto outputs = resolveDerivedPath(*store, drv); - for (auto & [outputName, outputPath] : outputs) { + for (auto & result : results) { + auto built = std::get(result.path); + if (!result.success()) { + auto e = result.error(); + e.addTrace(positions[pos], fmt("Derivation %s Output %s via %s", store->printStorePath(built.drvPath), built.outputs.to_string(), reason)); + throw e; + } + for (auto & [outputName, realisation] : result.builtOutputs) { res.insert_or_assign( - downstreamPlaceholder(*store, drv.drvPath, outputName), - store->printStorePath(outputPath) + downstreamPlaceholder(*store, built.drvPath, outputName), + store->printStorePath(realisation.outPath) ); } } @@ -109,14 +128,14 @@ struct RealisePathFlags { bool checkForPureEval = true; }; -static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, const RealisePathFlags flags = {}) +static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, const std::string_view reason, const RealisePathFlags flags = {}) { NixStringContext context; - auto path = state.coerceToPath(noPos, v, context, "while realising the context of a path"); + auto path = state.coerceToPath(pos, v, context, "while realising the context of a path"); try { - StringMap rewrites = state.realiseContext(context); + StringMap rewrites = state.realiseContext(context, pos, reason); auto realPath = state.rootPath(CanonPath(state.toRealPath(rewriteStrings(path.path.abs(), rewrites), context))); @@ -169,7 +188,7 @@ static void mkOutputString( argument. */ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * vScope, Value & v) { - auto path = realisePath(state, pos, vPath); + auto path = realisePath(state, pos, vPath, "scopedImport"); auto path2 = path.path.abs(); // FIXME @@ -332,7 +351,7 @@ extern "C" typedef void (*ValueInitializer)(EvalState & state, Value & v); /* Load a ValueInitializer from a DSO and return whatever it initializes */ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - auto path = realisePath(state, pos, *args[0]); + auto path = realisePath(state, pos, *args[0], "importNative"); std::string sym(state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument passed to builtins.importNative")); @@ -376,7 +395,7 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v) false, false).toOwned()); } try { - auto _ = state.realiseContext(context); // FIXME: Handle CA derivations + auto _ = state.realiseContext(context, pos, "exec"); // FIXME: Handle CA derivations } catch (InvalidPathError & e) { state.error("cannot execute '%1%', since path '%2%' is not valid", program, e.path).atPos(pos).debugThrow(); } @@ -1523,7 +1542,7 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, can’t just catch the exception here because we still want to throw if something in the evaluation of `*args[0]` tries to access an unauthorized path). */ - auto path = realisePath(state, pos, *args[0], { .checkForPureEval = false }); + auto path = realisePath(state, pos, *args[0], "pathExists", { .checkForPureEval = false }); try { v.mkBool(state.checkSourcePath(path).pathExists()); @@ -1600,7 +1619,7 @@ static RegisterPrimOp primop_dirOf({ /* Return the contents of a file as a string. */ static void prim_readFile(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - auto path = realisePath(state, pos, *args[0]); + auto path = realisePath(state, pos, *args[0], "readFile"); auto s = path.readFile(); if (s.find((char) 0) != std::string::npos) state.debugThrowLastTrace(Error("the contents of the file '%1%' cannot be represented as a Nix string", path)); @@ -1657,7 +1676,7 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V false, false).toOwned(); try { - auto rewrites = state.realiseContext(context); + auto rewrites = state.realiseContext(context, pos, "findFile"); path = rewriteStrings(path, rewrites); } catch (InvalidPathError & e) { state.debugThrowLastTrace(EvalError({ @@ -1691,7 +1710,7 @@ static void prim_hashFile(EvalState & state, const PosIdx pos, Value * * args, V .errPos = state.positions[pos] })); - auto path = realisePath(state, pos, *args[1]); + auto path = realisePath(state, pos, *args[1], "hashFile"); v.mkString(hashString(*ht, path.readFile()).to_string(Base16, false)); } @@ -1718,7 +1737,7 @@ static std::string_view fileTypeToString(InputAccessor::Type type) static void prim_readFileType(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - auto path = realisePath(state, pos, *args[0]); + auto path = realisePath(state, pos, *args[0], "readFileType"); /* Retrieve the directory entry type and stringize it. */ v.mkString(fileTypeToString(path.lstat().type)); } @@ -1736,7 +1755,7 @@ static RegisterPrimOp primop_readFileType({ /* Read a directory (without . or ..) */ static void prim_readDir(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - auto path = realisePath(state, pos, *args[0]); + auto path = realisePath(state, pos, *args[0], "readDir"); // Retrieve directory entries for all nodes in a directory. // This is similar to `getFileType` but is optimized to reduce system calls @@ -2082,7 +2101,7 @@ static void addPath( try { // FIXME: handle CA derivation outputs (where path needs to // be rewritten to the actual output). - auto rewrites = state.realiseContext(context); + auto rewrites = state.realiseContext(context, noPos, "addPath"); path = state.toRealPath(rewriteStrings(path, rewrites), context); StorePathSet refs; diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index b7a56e7917d..8ce10199955 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -105,9 +105,14 @@ struct BuildResult return status == Built || status == Substituted || status == AlreadyValid || status == ResolvesToAlreadyValid; } - void rethrow() + [[noreturn]] void rethrow() { - throw Error("%s", errorMsg); + throw error(); + } + + Error error() + { + return Error("%s", errorMsg); } }; diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 5aa6bee956a..477fc0a2c2e 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -23,6 +23,7 @@ typedef enum { actQueryPathInfo = 109, actPostBuildHook = 110, actBuildWaiting = 111, + actFromDerivation = 112, } ActivityType; typedef enum { diff --git a/tests/import-from-derivation.nix b/tests/import-from-derivation.nix new file mode 100644 index 00000000000..61b84be6ef4 --- /dev/null +++ b/tests/import-from-derivation.nix @@ -0,0 +1,10 @@ +with import ./config.nix; +import ( + mkDerivation { + name = "foo"; + bla = import ./dependencies.nix; + buildCommand = " + echo \\\"hi\\\" > $out + "; + } +) diff --git a/tests/remote-store.sh b/tests/remote-store.sh index ea32a20d30e..2b411901a50 100644 --- a/tests/remote-store.sh +++ b/tests/remote-store.sh @@ -19,18 +19,7 @@ else fi # Test import-from-derivation through the daemon. -[[ $(nix eval --impure --raw --expr ' - with import ./config.nix; - import ( - mkDerivation { - name = "foo"; - bla = import ./dependencies.nix; - buildCommand = " - echo \\\"hi\\\" > $out - "; - } - ) -') = hi ]] +[[ $(nix eval --option "log-import-from-derivation" true --impure --raw ./import-from-derivation.nix) = hi ]] storeCleared=1 NIX_REMOTE_=$NIX_REMOTE $SHELL ./user-envs.sh