From 17965bf11c55daee81729151f9876e55fdeaf9c1 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 27 Apr 2024 23:44:12 +0200 Subject: [PATCH] libstore: un-callback-ify Store::queryRealisationUncached Change-Id: I4a328f46eaac3bb8b19ddc091306de83348be9cf --- src/libstore/binary-cache-store.cc | 17 +++---- src/libstore/binary-cache-store.hh | 3 +- src/libstore/build/local-derivation-goal.cc | 7 ++- src/libstore/dummy-store.cc | 5 +- src/libstore/legacy-ssh-store.cc | 3 +- src/libstore/local-store.cc | 27 ++++------- src/libstore/local-store.hh | 3 +- src/libstore/remote-store.cc | 51 +++++++++------------ src/libstore/remote-store.hh | 3 +- src/libstore/store-api.cc | 32 +++++-------- src/libstore/store-api.hh | 3 +- 11 files changed, 60 insertions(+), 94 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 88b8db972..85ca36667 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -442,21 +442,16 @@ StorePath BinaryCacheStore::addTextToStore( })->path; } -void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept +std::shared_ptr BinaryCacheStore::queryRealisationUncached(const DrvOutput & id) { auto outputInfoFilePath = realisationsPrefix + "/" + id.to_string() + ".doi"; - try { - auto data = getFile(outputInfoFilePath); - if (!data) return callback({}); + auto data = getFile(outputInfoFilePath); + if (!data) return {}; - auto realisation = Realisation::fromJSON( - nlohmann::json::parse(*data), outputInfoFilePath); - return callback(std::make_shared(realisation)); - } catch (...) { - callback.rethrow(); - } + auto realisation = Realisation::fromJSON( + nlohmann::json::parse(*data), outputInfoFilePath); + return std::make_shared(realisation); } void BinaryCacheStore::registerDrvOutput(const Realisation& info) { diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index b6fbcfbc8..510965d12 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -134,8 +134,7 @@ public: void registerDrvOutput(const Realisation & info) override; - void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override; + std::shared_ptr queryRealisationUncached(const DrvOutput &) override; void narFromPath(const StorePath & path, Sink & sink) override; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 1b014605d..5b365accd 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1353,14 +1353,13 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In // corresponds to an allowed derivation { throw Error("registerDrvOutput"); } - void queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept override + std::shared_ptr queryRealisationUncached(const DrvOutput & id) override // XXX: This should probably be allowed if the realisation corresponds to // an allowed derivation { if (!goal.isAllowed(id)) - callback(nullptr); - next->queryRealisation(id, std::move(callback)); + return nullptr; + return next->queryRealisation(id); } void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index f1c2b43b1..ae2b91370 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -67,9 +67,8 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store void narFromPath(const StorePath & path, Sink & sink) override { unsupported("narFromPath"); } - void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override - { callback(nullptr); } + std::shared_ptr queryRealisationUncached(const DrvOutput &) override + { return nullptr; } virtual ref getFSAccessor() override { unsupported("getFSAccessor"); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 5044876cb..23ccfb178 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -407,8 +407,7 @@ public: return std::nullopt; } - void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override + std::shared_ptr queryRealisationUncached(const DrvOutput &) override // TODO: Implement { unsupported("queryRealisation"); } }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ee401d7fc..a27e43989 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1856,24 +1856,17 @@ std::optional LocalStore::queryRealisation_( return { res }; } -void LocalStore::queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept +std::shared_ptr LocalStore::queryRealisationUncached(const DrvOutput & id) { - try { - auto maybeRealisation - = retrySQLite>([&]() { - auto state(_state.lock()); - return queryRealisation_(*state, id); - }); - if (maybeRealisation) - callback( - std::make_shared(maybeRealisation.value())); - else - callback(nullptr); - - } catch (...) { - callback.rethrow(); - } + auto maybeRealisation + = retrySQLite>([&]() { + auto state(_state.lock()); + return queryRealisation_(*state, id); + }); + if (maybeRealisation) + return std::make_shared(maybeRealisation.value()); + else + return nullptr; } ContentAddress LocalStore::hashCAPath( diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index daf50989c..14f024ca9 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -294,8 +294,7 @@ public: std::optional queryRealisation_(State & state, const DrvOutput & id); std::optional> queryRealisationCore_(State & state, const DrvOutput & id); - void queryRealisationUncached(const DrvOutput&, - Callback> callback) noexcept override; + std::shared_ptr queryRealisationUncached(const DrvOutput&) override; std::optional getVersion() override; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 94c426c02..1d3fa93c1 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -612,39 +612,32 @@ void RemoteStore::registerDrvOutput(const Realisation & info) conn.processStderr(); } -void RemoteStore::queryRealisationUncached(const DrvOutput & id, - Callback> callback) noexcept +std::shared_ptr RemoteStore::queryRealisationUncached(const DrvOutput & id) { - try { - auto conn(getConnection()); + auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 27) { - warn("the daemon is too old to support content-addressed derivations, please upgrade it to 2.4"); - return callback(nullptr); - } + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 27) { + warn("the daemon is too old to support content-addressed derivations, please upgrade it to 2.4"); + return nullptr; + } - conn->to << WorkerProto::Op::QueryRealisation; - conn->to << id.to_string(); - conn.processStderr(); + conn->to << WorkerProto::Op::QueryRealisation; + conn->to << id.to_string(); + conn.processStderr(); - auto real = [&]() -> std::shared_ptr { - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 31) { - auto outPaths = WorkerProto::Serialise>::read( - *this, *conn); - if (outPaths.empty()) - return nullptr; - return std::make_shared(Realisation { .id = id, .outPath = *outPaths.begin() }); - } else { - auto realisations = WorkerProto::Serialise>::read( - *this, *conn); - if (realisations.empty()) - return nullptr; - return std::make_shared(*realisations.begin()); - } - }(); - - callback(std::shared_ptr(real)); - } catch (...) { return callback.rethrow(); } + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 31) { + auto outPaths = WorkerProto::Serialise>::read( + *this, *conn); + if (outPaths.empty()) + return nullptr; + return std::make_shared(Realisation { .id = id, .outPath = *outPaths.begin() }); + } else { + auto realisations = WorkerProto::Serialise>::read( + *this, *conn); + if (realisations.empty()) + return nullptr; + return std::make_shared(*realisations.begin()); + } } void RemoteStore::copyDrvsFromEvalStore( diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 7ce47bee2..b1b7f93e9 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -108,8 +108,7 @@ public: void registerDrvOutput(const Realisation & info) override; - void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept override; + std::shared_ptr queryRealisationUncached(const DrvOutput &) override; void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 553f9587a..7ebab3933 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -743,29 +743,21 @@ void Store::queryRealisation(const DrvOutput & id, return callback.rethrow(); } - auto callbackPtr - = std::make_shared(std::move(callback)); + try { + auto info = queryRealisationUncached(id); - queryRealisationUncached( - id, - { [this, id, callbackPtr]( - std::future> fut) { - try { - auto info = fut.get(); + if (diskCache) { + if (info) + diskCache->upsertRealisation(getUri(), *info); + else + diskCache->upsertAbsentRealisation(getUri(), id); + } - if (diskCache) { - if (info) - diskCache->upsertRealisation(getUri(), *info); - else - diskCache->upsertAbsentRealisation(getUri(), id); - } + callback(std::shared_ptr(info)); - (*callbackPtr)(std::shared_ptr(info)); - - } catch (...) { - callbackPtr->rethrow(); - } - } }); + } catch (...) { + callback.rethrow(); + } } std::shared_ptr Store::queryRealisation(const DrvOutput & id) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 8cdfff576..1aab3c8a3 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -402,8 +402,7 @@ public: protected: virtual std::shared_ptr queryPathInfoUncached(const StorePath & path) = 0; - virtual void queryRealisationUncached(const DrvOutput &, - Callback> callback) noexcept = 0; + virtual std::shared_ptr queryRealisationUncached(const DrvOutput &) = 0; public: