Give queryPartialDerivationOutputMap an evalStore parameter

This makes it more useful. In general, the derivation will be in one
store, and the realisation info is in another.

This also helps us avoid duplication. See how `resolveDerivedPath` is
now simpler because it uses `queryPartialDerivationOutputMap`. In NixOS#8369
we get more flavors of derived path, and need more code to resolve them
all, and this problem only gets worse.

The fact that we need a new method to deal with the multiple dispatch is
unfortunate, but this generally relates to the fact that `Store` is a
sub-par interface, too bulky/unwieldy and conflating separate concerns.
Solving that is out of scope of this PR.

This is part of the RFC 92 work. See tracking issue NixOS#6316

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

src/libstore/local-store.cc

@@ -1022,9 +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 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;
@@ -1036,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)

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;
 

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;
 }

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)
     {}
 };
 

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);
     }
 }
 

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;

src/libstore/store-api.cc

@@ -496,22 +496,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;

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.