From 6b5f82e78beede7a0eb58e736411273c493bf94f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 16 May 2025 13:16:18 +0200 Subject: [PATCH] libstore: deoptionalize queryPartialDerivationOutputMap derivation outpaths are now statically known at all times. the one snag here is that the wires encode even statically known paths as optionals, forcing us to check for this any time we receive an output map. remotes answering with nullopt paths for derivations we still support now would be a protocol error on its own though, so we do not diagnose it deeply. Change-Id: Ib7080b2a0c45c3506233e87c8ef6842576f61050 --- lix/legacy/nix-build.cc | 8 ++--- lix/libstore/build/derivation-goal.cc | 3 +- lix/libstore/derived-path.cc | 5 +-- lix/libstore/gc.cc | 9 +++-- lix/libstore/local-store.cc | 6 ++-- lix/libstore/local-store.hh | 2 +- lix/libstore/misc.cc | 52 +++++++++------------------ lix/libstore/remote-store.cc | 19 ++++++---- lix/libstore/remote-store.hh | 2 +- lix/libstore/store-api.cc | 15 +++----- lix/libstore/store-api.hh | 7 ++-- lix/nix/develop.cc | 4 +-- 12 files changed, 51 insertions(+), 81 deletions(-) diff --git a/lix/legacy/nix-build.cc b/lix/legacy/nix-build.cc index dcf4e811f..2fd7b58f8 100644 --- a/lix/legacy/nix-build.cc +++ b/lix/legacy/nix-build.cc @@ -402,7 +402,7 @@ static void main_nix_build(AsyncIoRoot & aio, std::string programName, Strings a if (shellDrv) { auto shellDrvOutputs = aio.blockOn(store->queryPartialDerivationOutputMap(shellDrv.value(), &*evalStore)); - shell = store->printStorePath(shellDrvOutputs.at("out").value()) + "/bin/bash"; + shell = store->printStorePath(shellDrvOutputs.at("out")) + "/bin/bash"; } // Set the environment. @@ -448,7 +448,7 @@ static void main_nix_build(AsyncIoRoot & aio, std::string programName, Strings a aio.blockOn(store->queryPartialDerivationOutputMap(inputDrv, &*evalStore)); for (auto & i : inputNode) { auto o = outputs.at(i); - aio.blockOn(store->computeFSClosure(*o, inputs)); + aio.blockOn(store->computeFSClosure(o, inputs)); } }; @@ -593,9 +593,7 @@ static void main_nix_build(AsyncIoRoot & aio, std::string programName, Strings a auto builtOutputs = aio.blockOn(store->queryPartialDerivationOutputMap(drvPath, &*evalStore)); - auto maybeOutputPath = builtOutputs.at(outputName); - assert(maybeOutputPath); - auto outputPath = *maybeOutputPath; + auto outputPath = builtOutputs.at(outputName); if (auto store2 = store.try_cast_shared()) { std::string symlink = drvPrefix; diff --git a/lix/libstore/build/derivation-goal.cc b/lix/libstore/build/derivation-goal.cc index 3787fe057..0db5b1762 100644 --- a/lix/libstore/build/derivation-goal.cc +++ b/lix/libstore/build/derivation-goal.cc @@ -440,8 +440,7 @@ try { auto depOutputs = TRY_AWAIT(worker.store.queryPartialDerivationOutputMap(i, &worker.evalStore)); for (auto & j : depOutputs) - if (j.second) - outputsToDrv.insert_or_assign(*j.second, i); + outputsToDrv.insert_or_assign(j.second, i); } /* Check each path (slow!). */ diff --git a/lix/libstore/derived-path.cc b/lix/libstore/derived-path.cc index 5867a0524..bd55a9daa 100644 --- a/lix/libstore/derived-path.cc +++ b/lix/libstore/derived-path.cc @@ -43,10 +43,7 @@ try { const auto outputMap = TRY_AWAIT(store.queryPartialDerivationOutputMap(drvPath.path)); for (const auto & [output, outputPathOpt] : outputMap) { if (!outputs.contains(output)) continue; - if (outputPathOpt) - res["outputs"][output] = store.printStorePath(*outputPathOpt); - else - res["outputs"][output] = nullptr; + res["outputs"][output] = store.printStorePath(outputPathOpt); } co_return res; } catch (...) { diff --git a/lix/libstore/gc.cc b/lix/libstore/gc.cc index 309c30fc6..bcc046cdc 100644 --- a/lix/libstore/gc.cc +++ b/lix/libstore/gc.cc @@ -774,13 +774,12 @@ try { /* If keep-derivations is set and this is a derivation, then visit the derivation outputs. */ if (gcKeepDerivations && path->isDerivation()) { - for (auto & [name, maybeOutPath] : + for (auto & [name, outPath] : TRY_AWAIT(queryPartialDerivationOutputMap(*path))) { - if (maybeOutPath && - TRY_AWAIT(isValidPath(*maybeOutPath)) && - TRY_AWAIT(queryPathInfo(*maybeOutPath))->deriver == *path) - enqueue(*maybeOutPath); + if (TRY_AWAIT(isValidPath(outPath)) && + TRY_AWAIT(queryPathInfo(outPath))->deriver == *path) + enqueue(outPath); } } diff --git a/lix/libstore/local-store.cc b/lix/libstore/local-store.cc index 287f9a932..7fbc9d33f 100644 --- a/lix/libstore/local-store.cc +++ b/lix/libstore/local-store.cc @@ -1026,15 +1026,15 @@ try { } -kj::Promise>>> +kj::Promise>> LocalStore::queryStaticPartialDerivationOutputMap(const StorePath & path) try { co_return TRY_AWAIT( // NOLINTNEXTLINE(cppcoreguidelines-avoid-capturing-lambda-coroutines) - retrySQLite([&]() -> kj::Promise>>> { + retrySQLite([&]() -> kj::Promise>> { try { auto state = co_await _dbState.lock(); - std::map> outputs; + std::map outputs; uint64_t drvId; drvId = queryValidPathId(*state, path); auto use(state->stmts->QueryDerivationOutputs.use()(drvId)); diff --git a/lix/libstore/local-store.hh b/lix/libstore/local-store.hh index c76d49c4e..f5758157b 100644 --- a/lix/libstore/local-store.hh +++ b/lix/libstore/local-store.hh @@ -201,7 +201,7 @@ public: kj::Promise> queryValidDerivers(const StorePath & path) override; - kj::Promise>>> + kj::Promise>> queryStaticPartialDerivationOutputMap(const StorePath & path) override; kj::Promise>> diff --git a/lix/libstore/misc.cc b/lix/libstore/misc.cc index 39a9066ad..6185b4129 100644 --- a/lix/libstore/misc.cc +++ b/lix/libstore/misc.cc @@ -37,9 +37,9 @@ try { res.insert(i); if (includeDerivers && path.isDerivation()) - for (auto& [_, maybeOutPath] : TRY_AWAIT(queryPartialDerivationOutputMap(path))) - if (maybeOutPath && TRY_AWAIT(isValidPath(*maybeOutPath))) - res.insert(*maybeOutPath); + for (auto& [_, outPath] : TRY_AWAIT(queryPartialDerivationOutputMap(path))) + if (TRY_AWAIT(isValidPath(outPath))) + res.insert(outPath); co_return res; } catch (...) { co_return result::current_exception(); @@ -56,9 +56,9 @@ try { res.insert(ref); if (includeOutputs && path.isDerivation()) - for (auto& [_, maybeOutPath] : TRY_AWAIT(queryPartialDerivationOutputMap(path))) - if (maybeOutPath && TRY_AWAIT(isValidPath(*maybeOutPath))) - res.insert(*maybeOutPath); + for (auto& [_, outPath] : TRY_AWAIT(queryPartialDerivationOutputMap(path))) + if (TRY_AWAIT(isValidPath(outPath))) + res.insert(outPath); if (includeDerivers && info->deriver && TRY_AWAIT(isValidPath(*info->deriver))) res.insert(*info->deriver); @@ -240,29 +240,18 @@ struct QueryMissingContext } StorePathSet invalid; - /* true for regular derivations, and CA derivations for which we - have a trust mapping for all wanted outputs. */ - auto knownOutputPaths = true; - for (auto & [outputName, pathOpt] : + for (auto & [outputName, path] : aio.blockOn(store.queryPartialDerivationOutputMap(drvPath))) { - if (!pathOpt) { - knownOutputPaths = false; - break; - } - if (bfd.outputs.contains(outputName) && !aio.blockOn(store.isValidPath(*pathOpt))) - invalid.insert(*pathOpt); + if (bfd.outputs.contains(outputName) && !aio.blockOn(store.isValidPath(path))) + invalid.insert(path); } - if (knownOutputPaths && invalid.empty()) return; + if (invalid.empty()) return; auto drv = make_ref(aio.blockOn(store.derivationFromPath(drvPath))); ParsedDerivation parsedDrv(StorePath(drvPath), *drv); - if (!knownOutputPaths && settings.useSubstitutes && parsedDrv.substitutesAllowed()) { - throw UnimplementedError("ca derivations are not supported"); - } - - if (knownOutputPaths && settings.useSubstitutes && parsedDrv.substitutesAllowed()) { + if (settings.useSubstitutes && parsedDrv.substitutesAllowed()) { auto drvState = make_ref>(DrvState(invalid.size())); for (auto & output : invalid) { pool.enqueueWithAio([=, this](AsyncIoRoot & aio) { @@ -365,18 +354,18 @@ resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, Store * evalSt try { auto drvPath = bfd.drvPath.path; - auto outputsOpt_ = TRY_AWAIT(store.queryPartialDerivationOutputMap(drvPath, evalStore_)); + auto outputs_ = TRY_AWAIT(store.queryPartialDerivationOutputMap(drvPath, evalStore_)); - auto outputsOpt = std::visit(overloaded { + co_return std::visit(overloaded { [&](const OutputsSpec::All &) { // Keep all outputs - return std::move(outputsOpt_); + return std::move(outputs_); }, [&](const OutputsSpec::Names & names) { // Get just those mentioned by name - std::map> outputsOpt; + std::map outputsOpt; for (auto & output : names) { - auto * pOutputPathOpt = get(outputsOpt_, output); + auto * pOutputPathOpt = get(outputs_, output); if (!pOutputPathOpt) throw Error( "the derivation '%s' doesn't have an output named '%s'", @@ -386,15 +375,6 @@ try { return outputsOpt; }, }, bfd.outputs.raw); - - OutputPathMap outputs; - for (auto & [outputName, outputPathOpt] : outputsOpt) { - if (!outputPathOpt) - throw MissingRealisation(bfd.drvPath.to_string(store), outputName); - auto & outputPath = *outputPathOpt; - outputs.insert_or_assign(outputName, outputPath); - } - co_return outputs; } catch (...) { co_return result::current_exception(); } diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index 9090b8626..46f47eb16 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -338,7 +338,7 @@ try { } -kj ::Promise>>> +kj ::Promise>> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_) try { if (GET_PROTOCOL_MINOR(TRY_AWAIT(getProtocol())) >= 22) { @@ -346,9 +346,19 @@ try { auto conn(TRY_AWAIT(getConnection())); conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path); conn.processStderr(); - co_return WorkerProto::Serialise>>::read( + auto tmp = WorkerProto::Serialise>>::read( *this, *conn ); + std::map result; + for (auto & [name, outPath] : tmp) { + if (!outPath) { + throw Error( + "remote responded with unknown outpath for %s^%s", path.to_string(), name + ); + } + result.emplace(std::move(name), std::move(*outPath)); + } + co_return result; } else { auto & evalStore = *evalStore_; auto outputs = TRY_AWAIT(evalStore.queryStaticPartialDerivationOutputMap(path)); @@ -357,10 +367,7 @@ try { for (auto && [outputName, optPath] : TRY_AWAIT(queryPartialDerivationOutputMap(path, nullptr))) { - if (optPath) - outputs.insert_or_assign(std::move(outputName), std::move(optPath)); - else - outputs.insert({std::move(outputName), std::nullopt}); + outputs.insert_or_assign(std::move(outputName), std::move(optPath)); } co_return outputs; } diff --git a/lix/libstore/remote-store.hh b/lix/libstore/remote-store.hh index 99eb96f02..6232cd191 100644 --- a/lix/libstore/remote-store.hh +++ b/lix/libstore/remote-store.hh @@ -69,7 +69,7 @@ public: kj::Promise> queryDerivationOutputs(const StorePath & path) override; - kj::Promise>>> + kj::Promise>> queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore = nullptr) override; kj::Promise>> queryPathFromHashPart(const std::string & hashPart) override; diff --git a/lix/libstore/store-api.cc b/lix/libstore/store-api.cc index 354124974..38100db55 100644 --- a/lix/libstore/store-api.cc +++ b/lix/libstore/store-api.cc @@ -508,10 +508,10 @@ bool Store::PathInfoCacheValue::isKnownNow() return std::chrono::steady_clock::now() < time_point + ttl; } -kj::Promise>>> +kj::Promise>> Store::queryStaticPartialDerivationOutputMap(const StorePath & path) try { - std::map> outputs; + std::map outputs; auto drv = TRY_AWAIT(readInvalidDerivation(path)); for (auto & [outputName, output] : drv.outputsAndPaths(*this)) { outputs.emplace(outputName, output.second); @@ -521,7 +521,7 @@ try { co_return result::current_exception(); } -kj::Promise>>> +kj::Promise>> Store::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_) try { auto & evalStore = evalStore_ ? *evalStore_ : *this; @@ -534,14 +534,7 @@ try { kj::Promise> Store::queryDerivationOutputMap(const StorePath & path, Store * evalStore) try { - auto resp = TRY_AWAIT(queryPartialDerivationOutputMap(path, evalStore)); - OutputPathMap result; - for (auto & [outName, optOutPath] : resp) { - if (!optOutPath) - throw MissingRealisation(printStorePath(path), outName); - result.insert_or_assign(outName, *optOutPath); - } - co_return result; + co_return TRY_AWAIT(queryPartialDerivationOutputMap(path, evalStore)); } catch (...) { co_return result::current_exception(); } diff --git a/lix/libstore/store-api.hh b/lix/libstore/store-api.hh index 07cdf68f8..eea29f348 100644 --- a/lix/libstore/store-api.hh +++ b/lix/libstore/store-api.hh @@ -448,10 +448,9 @@ public: /** * Query the mapping outputName => outputPath for the given - * derivation. All outputs are mentioned so ones mising the mapping - * are mapped to `std::nullopt`. + * derivation. Naming is a historical accident. */ - virtual kj::Promise>>> + virtual kj::Promise>> queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore = nullptr); /** @@ -462,7 +461,7 @@ public: * Just a helper function for implementing * `queryPartialDerivationOutputMap`. */ - virtual kj::Promise>>> + virtual kj::Promise>> queryStaticPartialDerivationOutputMap(const StorePath & path); /** diff --git a/lix/nix/develop.cc b/lix/nix/develop.cc index 59acd316f..07c3506e7 100644 --- a/lix/nix/develop.cc +++ b/lix/nix/develop.cc @@ -260,10 +260,8 @@ try { }}, bmNormal, evalStore)); - for (auto & [_0, optPath] : TRY_AWAIT(evalStore->queryPartialDerivationOutputMap(shellDrvPath))) + for (auto & [_0, outPath] : TRY_AWAIT(evalStore->queryPartialDerivationOutputMap(shellDrvPath))) { - assert(optPath); - auto & outPath = *optPath; assert(TRY_AWAIT(store->isValidPath(outPath))); auto outPathS = store->toRealPath(outPath); if (lstat(outPathS).st_size)