From cbbd21ec073781f68029daff153dac2516dafc23 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 21 Dec 2021 08:42:19 +0100 Subject: [PATCH 1/2] Factor out the path realisation bit of IFD --- src/libexpr/primops.cc | 75 +++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 66af373d7..5f00fad1c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -76,6 +76,19 @@ void EvalState::realiseContext(const PathSet & context) } } +static Path realisePath(EvalState & state, const Pos & pos, Value & v, bool requireAbsolutePath = true) +{ + PathSet context; + + Path path = requireAbsolutePath + ? state.coerceToPath(pos, v, context) + : state.coerceToString(pos, v, context, false, false); + + state.realiseContext(context); + + return state.checkSourcePath(state.toRealPath(path, context)); +} + /* Add and attribute to the given attribute map from the output name to the output path, or a placeholder. @@ -109,11 +122,9 @@ static void mkOutputString(EvalState & state, Value & v, argument. */ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vScope, Value & v) { - PathSet context; - Path path = state.coerceToPath(pos, vPath, context); - + Path path; try { - state.realiseContext(context); + path = realisePath(state, pos, vPath); } catch (InvalidPathError & e) { throw EvalError({ .msg = hintfmt("cannot import '%1%', since path '%2%' is not valid", path, e.path), @@ -124,8 +135,6 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS throw; } - Path realPath = state.checkSourcePath(state.toRealPath(path, context)); - // FIXME auto isValidDerivationInStore = [&]() -> std::optional { if (!state.store->isStorePath(path)) @@ -177,7 +186,7 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS else { if (!vScope) - state.evalFile(realPath, v); + state.evalFile(path, v); else { state.forceAttrs(*vScope); @@ -195,8 +204,8 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS // No need to call staticEnv.sort(), because // args[0]->attrs is already sorted. - printTalkative("evaluating file '%1%'", realPath); - Expr * e = state.parseExprFromFile(resolveExprPath(realPath), staticEnv); + printTalkative("evaluating file '%1%'", path); + Expr * e = state.parseExprFromFile(resolveExprPath(path), staticEnv); e->eval(state, *env, v); } @@ -281,22 +290,19 @@ 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 Pos & pos, Value * * args, Value & v) { - PathSet context; - Path path = state.coerceToPath(pos, *args[0], context); - + Path path; try { - state.realiseContext(context); + path = realisePath(state, pos, *args[0]); } catch (InvalidPathError & e) { throw EvalError({ - .msg = hintfmt( - "cannot import '%1%', since path '%2%' is not valid", - path, e.path), + .msg = hintfmt("cannot import '%1%', since path '%2%' is not valid", path, e.path), .errPos = pos }); + } catch (Error & e) { + e.addTrace(pos, "while importing '%s'", path); + throw; } - path = state.checkSourcePath(path); - string sym = state.forceStringNoCtx(*args[1], pos); void *handle = dlopen(path.c_str(), RTLD_LAZY | RTLD_LOCAL); @@ -1349,10 +1355,9 @@ static RegisterPrimOp primop_storePath({ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args, Value & v) { - PathSet context; - Path path = state.coerceToPath(pos, *args[0], context); + Path path; try { - state.realiseContext(context); + path = realisePath(state, pos, *args[0]); } catch (InvalidPathError & e) { throw EvalError({ .msg = hintfmt( @@ -1363,7 +1368,7 @@ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args, } try { - mkBool(v, pathExists(state.checkSourcePath(path))); + mkBool(v, pathExists(path)); } catch (SysError & e) { /* Don't give away info from errors while canonicalising ‘path’ in restricted mode. */ @@ -1426,17 +1431,16 @@ static RegisterPrimOp primop_dirOf({ /* Return the contents of a file as a string. */ static void prim_readFile(EvalState & state, const Pos & pos, Value * * args, Value & v) { - PathSet context; - Path path = state.coerceToPath(pos, *args[0], context); + Path path; try { - state.realiseContext(context); + path = realisePath(state, pos, *args[0]); } catch (InvalidPathError & e) { throw EvalError({ .msg = hintfmt("cannot read '%1%', since path '%2%' is not valid", path, e.path), .errPos = pos }); } - string s = readFile(state.checkSourcePath(state.toRealPath(path, context))); + string s = readFile(path); if (s.find((char) 0) != string::npos) throw Error("the contents of the file '%1%' cannot be represented as a Nix string", path); mkString(v, s.c_str()); @@ -1475,11 +1479,10 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va pos ); - PathSet context; - string path = state.coerceToString(pos, *i->value, context, false, false); + Path path; try { - state.realiseContext(context); + path = realisePath(state, pos, *i->value, false); } catch (InvalidPathError & e) { throw EvalError({ .msg = hintfmt("cannot find '%1%', since path '%2%' is not valid", path, e.path), @@ -1512,15 +1515,14 @@ static void prim_hashFile(EvalState & state, const Pos & pos, Value * * args, Va .errPos = pos }); - PathSet context; - Path path = state.coerceToPath(pos, *args[1], context); + Path path; try { - state.realiseContext(context); + path = realisePath(state, pos, *args[1]); } catch (InvalidPathError & e) { throw EvalError("cannot read '%s' since path '%s' is not valid, at %s", path, e.path, pos); } - mkString(v, hashFile(*ht, state.checkSourcePath(state.toRealPath(path, context))).to_string(Base16, false)); + mkString(v, hashFile(*ht, path).to_string(Base16, false)); } static RegisterPrimOp primop_hashFile({ @@ -1537,10 +1539,9 @@ static RegisterPrimOp primop_hashFile({ /* Read a directory (without . or ..) */ static void prim_readDir(EvalState & state, const Pos & pos, Value * * args, Value & v) { - PathSet ctx; - Path path = state.coerceToPath(pos, *args[0], ctx); + Path path; try { - state.realiseContext(ctx); + path = realisePath(state, pos, *args[0]); } catch (InvalidPathError & e) { throw EvalError({ .msg = hintfmt("cannot read '%1%', since path '%2%' is not valid", path, e.path), @@ -1548,7 +1549,7 @@ static void prim_readDir(EvalState & state, const Pos & pos, Value * * args, Val }); } - DirEntries entries = readDirectory(state.checkSourcePath(path)); + DirEntries entries = readDirectory(path); state.mkAttrs(v, entries.size()); for (auto & ent : entries) { From d90f9d4b9994dc1f15b9d664ae313f06261d6058 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 20 Dec 2021 19:46:55 +0100 Subject: [PATCH 2/2] Fix IFD with CA derivations Rewrite the string taken by the IFD-like primops to contain the actual output paths of the derivations rather than the placeholders Fix #5805 --- src/libexpr/eval.hh | 5 +++- src/libexpr/primops.cc | 43 +++++++++++++++++++++++------------ tests/ca/import-derivation.sh | 6 +++++ tests/local.mk | 2 +- 4 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 tests/ca/import-derivation.sh diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 1aab8e166..0ba570434 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -350,7 +350,10 @@ public: /* Print statistics. */ void printStats(); - void realiseContext(const PathSet & context); + /* 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 PathSet & context); private: diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5f00fad1c..aff8f951e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -35,9 +35,10 @@ namespace nix { InvalidPathError::InvalidPathError(const Path & path) : EvalError("path '%s' is not valid", path), path(path) {} -void EvalState::realiseContext(const PathSet & context) +StringMap EvalState::realiseContext(const PathSet & context) { std::vector drvs; + StringMap res; for (auto & i : context) { auto [ctxS, outputName] = decodeContext(i); @@ -46,10 +47,12 @@ void EvalState::realiseContext(const PathSet & context) throw InvalidPathError(store->printStorePath(ctx)); if (!outputName.empty() && ctx.isDerivation()) { drvs.push_back({ctx, {outputName}}); + } else { + res.insert_or_assign(ctxS, ctxS); } } - if (drvs.empty()) return; + if (drvs.empty()) return {}; if (!evalSettings.enableImportFromDerivation) throw Error( @@ -61,19 +64,29 @@ void EvalState::realiseContext(const PathSet & context) for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d }); store->buildPaths(buildReqs); + /* Get all the output paths corresponding to the placeholders we had */ + for (auto & [drvPath, outputs] : drvs) { + auto outputPaths = store->queryDerivationOutputMap(drvPath); + for (auto & outputName : outputs) { + if (outputPaths.count(outputName) == 0) + throw Error("derivation '%s' does not have an output named '%s'", + store->printStorePath(drvPath), outputName); + res.insert_or_assign( + downstreamPlaceholder(*store, drvPath, outputName), + store->printStorePath(outputPaths.at(outputName)) + ); + } + } + /* Add the output of this derivations to the allowed paths. */ if (allowedPaths) { - for (auto & [drvPath, outputs] : drvs) { - auto outputPaths = store->queryDerivationOutputMap(drvPath); - for (auto & outputName : outputs) { - if (outputPaths.count(outputName) == 0) - throw Error("derivation '%s' does not have an output named '%s'", - store->printStorePath(drvPath), outputName); - allowPath(outputPaths.at(outputName)); - } + for (auto & [_placeholder, outputPath] : res) { + allowPath(outputPath); } } + + return res; } static Path realisePath(EvalState & state, const Pos & pos, Value & v, bool requireAbsolutePath = true) @@ -84,9 +97,10 @@ static Path realisePath(EvalState & state, const Pos & pos, Value & v, bool requ ? state.coerceToPath(pos, v, context) : state.coerceToString(pos, v, context, false, false); - state.realiseContext(context); + StringMap rewrites = state.realiseContext(context); - return state.checkSourcePath(state.toRealPath(path, context)); + return state.checkSourcePath( + state.toRealPath(rewriteStrings(path, rewrites), context)); } /* Add and attribute to the given attribute map from the output name to @@ -344,7 +358,7 @@ void prim_exec(EvalState & state, const Pos & pos, Value * * args, Value & v) for (unsigned int i = 1; i < args[0]->listSize(); ++i) commandArgs.emplace_back(state.coerceToString(pos, *elems[i], context, false, false)); try { - state.realiseContext(context); + auto _ = state.realiseContext(context); // FIXME: Handle CA derivations } catch (InvalidPathError & e) { throw EvalError({ .msg = hintfmt("cannot execute '%1%', since path '%2%' is not valid", @@ -1876,7 +1890,8 @@ static void addPath( try { // FIXME: handle CA derivation outputs (where path needs to // be rewritten to the actual output). - state.realiseContext(context); + auto rewrites = state.realiseContext(context); + path = state.toRealPath(rewriteStrings(path, rewrites), context); StorePathSet refs; diff --git a/tests/ca/import-derivation.sh b/tests/ca/import-derivation.sh new file mode 100644 index 000000000..e98e0fbd0 --- /dev/null +++ b/tests/ca/import-derivation.sh @@ -0,0 +1,6 @@ +source common.sh + +export NIX_TESTS_CA_BY_DEFAULT=1 + +cd .. && source import-derivation.sh + diff --git a/tests/local.mk b/tests/local.mk index 936b72c2a..c22e779a6 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -11,7 +11,7 @@ nix_tests = \ local-store.sh remote-store.sh export.sh export-graph.sh \ db-migration.sh \ timeout.sh secure-drv-outputs.sh nix-channel.sh \ - multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \ + multiple-outputs.sh import-derivation.sh ca/import-derivation.sh fetchurl.sh optimise-store.sh \ binary-cache.sh \ substitute-with-invalid-ca.sh \ binary-cache-build-remote.sh \