From d90f9d4b9994dc1f15b9d664ae313f06261d6058 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 20 Dec 2021 19:46:55 +0100 Subject: [PATCH] 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 \