Merge pull request #8724 from obsidiansystems/queryPartialDerivationOutputMap-evalStore

Give `queryPartialDerivationOutputMap` an `evalStore` parameter

11 files changed, 104 insertions, 72 deletions

diff --git a/src/libcmd/built-path.hh b/src/libcmd/built-path.hh
index c563a46e9..744e8090b 100644
--- a/src/libcmd/built-path.hh
+++ b/src/libcmd/built-path.hh
@@ -1,4 +1,5 @@
 #include "derived-path.hh"
+#include "realisation.hh"
 
 namespace nix {
 
diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index 7e9f6c100..214f3d49e 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -1251,11 +1251,13 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
     void queryReferrers(const StorePath & path, StorePathSet & referrers) override
     { }
 
-    std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path) override
+    std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(
+        const StorePath & path,
+        Store * evalStore = nullptr) override
     {
         if (!goal.isAllowed(path))
             throw InvalidPath("cannot query output map for unknown path '%s' in recursive Nix", printStorePath(path));
-        return next->queryPartialDerivationOutputMap(path);
+        return next->queryPartialDerivationOutputMap(path, evalStore);
     }
 
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override
diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh
index 6ea80c92e..7a4261ce0 100644
--- a/src/libstore/derived-path.hh
+++ b/src/libstore/derived-path.hh
@@ -3,7 +3,6 @@
 
 #include "util.hh"
 #include "path.hh"
-#include "realisation.hh"
 #include "outputs-spec.hh"
 #include "comparator.hh"
 
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index c468cb51e..f8e0a0ca2 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1022,10 +1022,9 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path)
 
 
 std::map<std::string, std::optional<StorePath>>
-LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)
+LocalStore::queryStaticPartialDerivationOutputMap(const StorePath & path)
 {
-    auto path = path_;
-    auto outputs = retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() {
+    return retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() {
         auto state(_state.lock());
         std::map<std::string, std::optional<StorePath>> outputs;
         uint64_t drvId;
@@ -1037,21 +1036,6 @@ LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)
 
         return outputs;
     });
-
-    if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
-        return outputs;
-
-    auto drv = readInvalidDerivation(path);
-    auto drvHashes = staticOutputHashes(*this, drv);
-    for (auto& [outputName, hash] : drvHashes) {
-        auto realisation = queryRealisation(DrvOutput{hash, outputName});
-        if (realisation)
-            outputs.insert_or_assign(outputName, realisation->outPath);
-        else
-            outputs.insert({outputName, std::nullopt});
-    }
-
-    return outputs;
 }
 
 std::optional<StorePath> LocalStore::queryPathFromHashPart(const std::string & hashPart)
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index d76662f33..2bdd46fa2 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -165,7 +165,7 @@ public:
 
     StorePathSet queryValidDerivers(const StorePath & path) override;
 
-    std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path) override;
+    std::map<std::string, std::optional<StorePath>> queryStaticPartialDerivationOutputMap(const StorePath & path) override;
 
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override;
 
diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc
index 50336c779..14160dc8b 100644
--- a/src/libstore/misc.cc
+++ b/src/libstore/misc.cc
@@ -310,43 +310,34 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
 
 OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, Store * evalStore_)
 {
-    auto & evalStore = evalStore_ ? *evalStore_ : store;
+    auto outputsOpt_ = store.queryPartialDerivationOutputMap(bfd.drvPath, evalStore_);
 
-    OutputPathMap outputs;
-    auto drv = evalStore.readDerivation(bfd.drvPath);
-    auto outputHashes = staticOutputHashes(store, drv);
-    auto drvOutputs = drv.outputsAndOptPaths(store);
-    auto outputNames = std::visit(overloaded {
+    auto outputsOpt = std::visit(overloaded {
         [&](const OutputsSpec::All &) {
-            StringSet names;
-            for (auto & [outputName, _] : drv.outputs)
-                names.insert(outputName);
-            return names;
+            // Keep all outputs
+            return std::move(outputsOpt_);
         },
         [&](const OutputsSpec::Names & names) {
-            return static_cast<std::set<std::string>>(names);
+            // Get just those mentioned by name
+            std::map<std::string, std::optional<StorePath>> outputsOpt;
+            for (auto & output : names) {
+                auto * pOutputPathOpt = get(outputsOpt_, output);
+                if (!pOutputPathOpt)
+                    throw Error(
+                        "the derivation '%s' doesn't have an output named '%s'",
+                        store.printStorePath(bfd.drvPath), output);
+                outputsOpt.insert_or_assign(output, std::move(*pOutputPathOpt));
+            }
+            return outputsOpt;
         },
     }, bfd.outputs.raw());
-    for (auto & output : outputNames) {
-        auto outputHash = get(outputHashes, output);
-        if (!outputHash)
-            throw Error(
-                "the derivation '%s' doesn't have an output named '%s'",
-                store.printStorePath(bfd.drvPath), output);
-        if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
-            DrvOutput outputId { *outputHash, output };
-            auto realisation = store.queryRealisation(outputId);
-            if (!realisation)
-                throw MissingRealisation(outputId);
-            outputs.insert_or_assign(output, realisation->outPath);
-        } else {
-            // If ca-derivations isn't enabled, assume that
-            // the output path is statically known.
-            auto drvOutput = get(drvOutputs, output);
-            assert(drvOutput);
-            assert(drvOutput->second);
-            outputs.insert_or_assign(output, *drvOutput->second);
-        }
+
+    OutputPathMap outputs;
+    for (auto & [outputName, outputPathOpt] : outputsOpt) {
+        if (!outputPathOpt)
+            throw MissingRealisation(store.printStorePath(bfd.drvPath), outputName);
+        auto & outputPath = *outputPathOpt;
+        outputs.insert_or_assign(outputName, outputPath);
     }
     return outputs;
 }
diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh
index 2a093c128..0548b30c1 100644
--- a/src/libstore/realisation.hh
+++ b/src/libstore/realisation.hh
@@ -5,6 +5,7 @@
 
 #include "hash.hh"
 #include "path.hh"
+#include "derived-path.hh"
 #include <nlohmann/json_fwd.hpp>
 #include "comparator.hh"
 #include "crypto.hh"
@@ -143,9 +144,13 @@ class MissingRealisation : public Error
 {
 public:
     MissingRealisation(DrvOutput & outputId)
-        : Error( "cannot operate on an output of the "
+        : MissingRealisation(outputId.outputName, outputId.strHash())
+    {}
+    MissingRealisation(std::string_view drv, std::string outputName)
+        : Error( "cannot operate on output '%s' of the "
                 "unbuilt derivation '%s'",
-                outputId.to_string())
+                outputName,
+                drv)
     {}
 };
 
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 1e2104e1f..bfe2258a4 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -378,27 +378,36 @@ StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path)
 }
 
 
-std::map<std::string, std::optional<StorePath>> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path)
+std::map<std::string, std::optional<StorePath>> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_)
 {
     if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) {
-        auto conn(getConnection());
-        conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path);
-        conn.processStderr();
-        return WorkerProto::Serialise<std::map<std::string, std::optional<StorePath>>>::read(*this, *conn);
+        if (!evalStore_) {
+            auto conn(getConnection());
+            conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path);
+            conn.processStderr();
+            return WorkerProto::Serialise<std::map<std::string, std::optional<StorePath>>>::read(*this, *conn);
+        } else {
+            auto & evalStore = *evalStore_;
+            auto outputs = evalStore.queryStaticPartialDerivationOutputMap(path);
+            // union with the first branch overriding the statically-known ones
+            // when non-`std::nullopt`.
+            for (auto && [outputName, optPath] : queryPartialDerivationOutputMap(path, nullptr)) {
+                if (optPath)
+                    outputs.insert_or_assign(std::move(outputName), std::move(optPath));
+                else
+                    outputs.insert({std::move(outputName), std::nullopt});
+            }
+            return outputs;
+        }
     } else {
+        auto & evalStore = evalStore_ ? *evalStore_ : *this;
         // Fallback for old daemon versions.
         // For floating-CA derivations (and their co-dependencies) this is an
         // under-approximation as it only returns the paths that can be inferred
         // from the derivation itself (and not the ones that are known because
         // the have been built), but as old stores don't handle floating-CA
         // derivations this shouldn't matter
-        auto derivation = readDerivation(path);
-        auto outputsWithOptPaths = derivation.outputsAndOptPaths(*this);
-        std::map<std::string, std::optional<StorePath>> ret;
-        for (auto & [outputName, outputAndPath] : outputsWithOptPaths) {
-            ret.emplace(outputName, outputAndPath.second);
-        }
-        return ret;
+        return evalStore.queryStaticPartialDerivationOutputMap(path);
     }
 }
 
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index cb7a71acf..b12f5437f 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -63,7 +63,7 @@ public:
 
     StorePathSet queryDerivationOutputs(const StorePath & path) override;
 
-    std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path) override;
+    std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore = nullptr) override;
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override;
 
     StorePathSet querySubstitutablePaths(const StorePathSet & paths) override;
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index b16f5a6b7..4ea16a1c0 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -492,22 +492,50 @@ bool Store::PathInfoCacheValue::isKnownNow()
     return std::chrono::steady_clock::now() < time_point + ttl;
 }
 
-std::map<std::string, std::optional<StorePath>> Store::queryPartialDerivationOutputMap(const StorePath & path)
+std::map<std::string, std::optional<StorePath>> Store::queryStaticPartialDerivationOutputMap(const StorePath & path)
 {
     std::map<std::string, std::optional<StorePath>> outputs;
     auto drv = readInvalidDerivation(path);
-    for (auto& [outputName, output] : drv.outputsAndOptPaths(*this)) {
+    for (auto & [outputName, output] : drv.outputsAndOptPaths(*this)) {
         outputs.emplace(outputName, output.second);
     }
     return outputs;
 }
 
+std::map<std::string, std::optional<StorePath>> Store::queryPartialDerivationOutputMap(
+    const StorePath & path,
+    Store * evalStore_)
+{
+    auto & evalStore = evalStore_ ? *evalStore_ : *this;
+
+    auto outputs = evalStore.queryStaticPartialDerivationOutputMap(path);
+
+    if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
+        return outputs;
+
+    auto drv = evalStore.readInvalidDerivation(path);
+    auto drvHashes = staticOutputHashes(*this, drv);
+    for (auto & [outputName, hash] : drvHashes) {
+        auto realisation = queryRealisation(DrvOutput{hash, outputName});
+        if (realisation) {
+            outputs.insert_or_assign(outputName, realisation->outPath);
+        } else {
+            // queryStaticPartialDerivationOutputMap is not guaranteed
+            // to return std::nullopt for outputs which are not
+            // statically known.
+            outputs.insert({outputName, std::nullopt});
+        }
+    }
+
+    return outputs;
+}
+
 OutputPathMap Store::queryDerivationOutputMap(const StorePath & path) {
     auto resp = queryPartialDerivationOutputMap(path);
     OutputPathMap result;
     for (auto & [outName, optOutPath] : resp) {
         if (!optOutPath)
-            throw Error("output '%s' of derivation '%s' has no store path mapped to it", outName, printStorePath(path));
+            throw MissingRealisation(printStorePath(path), outName);
         result.insert_or_assign(outName, *optOutPath);
     }
     return result;
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 14a862eef..7b412d2dd 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -425,7 +425,20 @@ public:
      * derivation. All outputs are mentioned so ones mising the mapping
      * are mapped to `std::nullopt`.
      */
-    virtual std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path);
+    virtual std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(
+        const StorePath & path,
+        Store * evalStore = nullptr);
+
+    /**
+     * Like `queryPartialDerivationOutputMap` but only considers
+     * statically known output paths (i.e. those that can be gotten from
+     * the derivation itself.
+     *
+     * Just a helper function for implementing
+     * `queryPartialDerivationOutputMap`.
+     */
+    virtual std::map<std::string, std::optional<StorePath>> queryStaticPartialDerivationOutputMap(
+        const StorePath & path);
 
     /**
      * Query the mapping outputName=>outputPath for the given derivation.