FIXME: MVP, will overwrite

src/libstore/build/substituter-set.cc

@@ -0,0 +1,135 @@
+#include "substituter-set.hh"
+#include "store-api.hh"
+
+namespace nix {
+
+    SubstituterSet::SubstituterSet(std::list<ref<Store>> substituters, std::optional<ContentAddress> contentAddress,
+                                   std::optional<StorePath> storePath, Store *baseStore) : contentAddress(
+            contentAddress), storePath(storePath),
+            baseStore(baseStore) {
+        for (const auto &x: substituters) {
+            subResults.push_back({.store = x});
+        }
+    }
+
+    size_t SubstituterSet::remaining() const {
+        return subResults.size();
+    }
+
+    // FIXME: Does this behave correctly with a missing result?
+    // I think we'll need these things to return nothing on an empty list
+    ref<Store> SubstituterSet::getSubstituter() {
+        return subResults[currentSubstituter].store;
+    }
+
+    std::shared_ptr<const ValidPathInfo>
+    SubstituterSet::getInfo() const {
+        return subResults[currentSubstituter].result->pathInfo;
+    }
+
+    std::optional<StorePath>
+    SubstituterSet::getSubPath() const {
+        return subResults[currentSubstituter].result->subPath;
+    }
+
+    // FIXME: maybe return Ok() or Errors here? Then we use getSubstituter to access, and can log the errors better
+    std::optional<ref<Store>> SubstituterSet::nextSubstituter() {
+        if (subResults.empty()) {
+            return {};
+        }
+        // What if results.size == 0?
+        if (currentSubstituter >= subResults.size()) {
+            // What if we continue calling this?
+            currentSubstituter = 0;
+        } else {
+            currentSubstituter++;
+            if (currentSubstituter == subResults.size()) {
+                // none left
+                return {};
+            }
+        }
+        if (!subResults[currentSubstituter].result) {
+            // Wait for the future if necessary
+            // FIXME: choose first available substituter with matching priority
+            if (subResults[currentSubstituter].future.valid()) {
+                subResults[currentSubstituter].result = subResults[currentSubstituter].future.get();
+            } else {
+                // Future was never initialized
+                // FIXME: maybe call init()? risk is some bug causing infinite looping
+                assert(false);
+            }
+        }
+        if (!subResults[currentSubstituter].result->valid) {
+            return nextSubstituter();
+        }
+        return getSubstituter();
+    }
+
+    // Checks whether a substituter may to have the store path.
+    // This function queries a single substituter and returns the result
+    // FIXME: intended to be thread-safe, need to test
+    // This is a non-member function because it's called async
+    SubstituterSet::SubstituterResult substituterValidate(ref<Store> substituter, std::optional<ContentAddress> contentAddress, Store* baseStore, std::optional<StorePath> storePath) {
+        // Explicitly mark the result as false until we finish all the checks
+        auto result = SubstituterSet::SubstituterResult{.valid = false};
+
+        if (contentAddress) {
+            result.subPath = substituter->makeFixedOutputPathFromCA(storePath->name(), *contentAddress);
+            if (substituter->storeDir == baseStore->storeDir)
+                assert(result.subPath == storePath);
+        } else if (substituter->storeDir != baseStore->storeDir) {
+            result.valid = false;
+            return result;
+        }
+
+        try {
+            result.pathInfo = substituter->queryPathInfo(result.subPath ? *result.subPath : *storePath);
+        } catch (const InvalidPath &e) {
+            result.error = e;
+            return result;
+        } catch (const SubstituterDisabled &e) {
+            result.error = e;
+            return result;
+        } catch (Error &e) {
+            // FIXME: Handle this error message better?
+            // Or combine with the others?
+            logError(e.info());
+            result.error = e;
+            return result;
+        }
+
+        if (result.pathInfo->path != storePath) {
+            if (result.pathInfo->isContentAddressed(*substituter) && result.pathInfo->references.empty()) {
+                auto info2 = std::make_shared<ValidPathInfo>(*result.pathInfo);
+                info2->path = *storePath;
+                result.pathInfo = info2;
+            } else {
+                printError("asked '%s' for '%s' but got '%s'",
+                           substituter->getUri(), baseStore->printStorePath(*storePath),
+                           substituter->printStorePath(result.pathInfo->path));
+                return result;
+            }
+        }
+
+        /* Bail out early if this substituter lacks a valid
+           signature. LocalStore::addToStore() also checks for this, but
+           only after we've downloaded the path. */
+        if (!substituter->isTrusted && baseStore->pathInfoIsUntrusted(*result.pathInfo)) {
+            warn("ignoring substitute for '%s' from '%s', as it's not signed by any of the keys in 'trusted-public-keys'",
+                 baseStore->printStorePath(*storePath), substituter->getUri());
+            return result;
+        }
+
+        result.valid = true;
+        return result;
+    }
+
+    void SubstituterSet::init() {
+        for (auto &sub: subResults) {
+            if (!sub.result && !sub.future.valid()) {
+                sub.future = std::async(std::launch::async, substituterValidate, sub.store, contentAddress, baseStore, storePath);
+            }
+        }
+    }
+
+}

src/libstore/build/substituter-set.hh

@@ -0,0 +1,64 @@
+#pragma once
+
+#include "store-api.hh"
+#include <future>
+#include <limits>
+
+namespace nix {
+
+    struct SubstituterSet {
+        struct SubstituterResult {
+            // FIXME: there's probably a better way to set this and the error messages
+            bool valid = false;
+            std::optional<Error> error;
+            // FIXME: is this the correct name?
+            std::shared_ptr<const ValidPathInfo> pathInfo;
+            std::optional<StorePath> subPath;
+        };
+
+        struct SubstituterInfo {
+            ref<Store> store;
+            std::optional<SubstituterResult> result;
+            std::future<SubstituterResult> future;
+        };
+
+        /* Content address for recomputing store path */
+        std::optional<ContentAddress> contentAddress;
+
+        /* The store path that should be realised through a substitute. */
+        std::optional<StorePath> storePath;
+
+        // The store associated with the current job?
+        // FIXME: no bare pointers
+        Store* baseStore = nullptr;
+
+        // Saved list of substituters and the query results
+        std::vector<SubstituterInfo> subResults;
+
+        // Reference to the substituter in use.
+        // FIXME: this really shouldn't be an index
+        size_t currentSubstituter = std::numeric_limits<size_t>::max();
+
+    public:
+        SubstituterSet() = default;
+
+        SubstituterSet(std::list<ref<Store>> substituters, std::optional<ContentAddress> ca,
+                       std::optional<StorePath> storePath, Store* baseStore);
+
+        // Perform initialization steps to query the substituters.
+        void init();
+
+        size_t remaining() const;
+
+        // Returns the current substituter.
+        // FIXME: safety
+        ref<Store> getSubstituter();
+
+        std::shared_ptr<const ValidPathInfo> getInfo() const;
+        std::optional<StorePath> getSubPath() const;
+
+        // Invalidates the current substituter and returns the next valid one.
+        // To be used when the current substituter fails.
+        std::optional<ref<Store>> nextSubstituter();
+    };
+}

src/libstore/build/substitution-goal.cc

@@ -59,7 +59,10 @@ void PathSubstitutionGoal::init()
     if (settings.readOnlyMode)
         throw Error("cannot substitute path '%s' - no write access to the Nix store", worker.store.printStorePath(storePath));
 
-    subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>();
+    substituters = SubstituterSet(settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>()
+            , ca, storePath, &worker.store);
+
+    substituters.init();
 
     tryNext();
 }
@@ -71,7 +74,9 @@ void PathSubstitutionGoal::tryNext()
 
     cleanup();
 
-    if (subs.size() == 0) {
+    if (auto next = substituters.nextSubstituter(); next.has_value()) {
+        sub = next.value();
+    } else {
         /* None left.  Terminate this goal and let someone else deal
            with it. */
 
@@ -91,56 +96,10 @@ void PathSubstitutionGoal::tryNext()
         return;
     }
 
-    sub = subs.front();
-    subs.pop_front();
-
-    if (ca) {
-        subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca);
-        if (sub->storeDir == worker.store.storeDir)
-            assert(subPath == storePath);
-    } else if (sub->storeDir != worker.store.storeDir) {
-        tryNext();
-        return;
-    }
-
-    try {
-        // FIXME: make async
-        info = sub->queryPathInfo(subPath ? *subPath : storePath);
-    } catch (InvalidPath &) {
-        tryNext();
-        return;
-    } catch (SubstituterDisabled &) {
-        if (settings.tryFallback) {
-            tryNext();
-            return;
-        }
-        throw;
-    } catch (Error & e) {
-        if (settings.tryFallback) {
-            logError(e.info());
-            tryNext();
-            return;
-        }
-        throw;
-    }
-
-    if (info->path != storePath) {
-        if (info->isContentAddressed(*sub) && info->references.empty()) {
-            auto info2 = std::make_shared<ValidPathInfo>(*info);
-            info2->path = storePath;
-            info = info2;
-        } else {
-            printError("asked '%s' for '%s' but got '%s'",
-                sub->getUri(), worker.store.printStorePath(storePath), sub->printStorePath(info->path));
-            tryNext();
-            return;
-        }
-    }
-
     /* Update the total expected download size. */
-    auto narInfo = std::dynamic_pointer_cast<const NarInfo>(info);
+    auto narInfo = std::dynamic_pointer_cast<const NarInfo>(substituters.getInfo());
 
-    maintainExpectedNar = std::make_unique<MaintainCount<uint64_t>>(worker.expectedNarSize, info->narSize);
+    maintainExpectedNar = std::make_unique<MaintainCount<uint64_t>>(worker.expectedNarSize, substituters.getInfo()->narSize);
 
     maintainExpectedDownload =
         narInfo && narInfo->fileSize
@@ -149,20 +108,9 @@ void PathSubstitutionGoal::tryNext()
 
     worker.updateProgress();
 
-    /* Bail out early if this substituter lacks a valid
-       signature. LocalStore::addToStore() also checks for this, but
-       only after we've downloaded the path. */
-    if (!sub->isTrusted && worker.store.pathInfoIsUntrusted(*info))
-    {
-        warn("ignoring substitute for '%s' from '%s', as it's not signed by any of the keys in 'trusted-public-keys'",
-            worker.store.printStorePath(storePath), sub->getUri());
-        tryNext();
-        return;
-    }
-
     /* To maintain the closure invariant, we first have to realise the
        paths referenced by this one. */
-    for (auto & i : info->references)
+    for (auto & i : substituters.getInfo()->references)
         if (i != storePath) /* ignore self-references */
             addWaitee(worker.makePathSubstitutionGoal(i));
 
@@ -185,7 +133,7 @@ void PathSubstitutionGoal::referencesValid()
         return;
     }
 
-    for (auto & i : info->references)
+    for (auto & i : substituters.getInfo()->references)
         if (i != storePath) /* ignore self-references */
             assert(worker.store.isValidPath(i));
 
@@ -223,7 +171,7 @@ void PathSubstitutionGoal::tryToRun()
             PushActivity pact(act.id);
 
             copyStorePath(*sub, worker.store,
-                subPath ? *subPath : storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs);
+                substituters.getSubPath() ? *substituters.getSubPath(): storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs);
 
             promise.set_value();
         } catch (...) {

src/libstore/build/substitution-goal.hh

@@ -2,6 +2,7 @@
 
 #include "lock.hh"
 #include "store-api.hh"
+#include "substituter-set.hh"
 #include "goal.hh"
 
 namespace nix {
@@ -13,12 +14,8 @@ struct PathSubstitutionGoal : public Goal
     /* The store path that should be realised through a substitute. */
     StorePath storePath;
 
-    /* The path the substituter refers to the path as. This will be
-       different when the stores have different names. */
-    std::optional<StorePath> subPath;
-
-    /* The remaining substituters. */
-    std::list<ref<Store>> subs;
+    /* The set of substituters in use. */
+    SubstituterSet substituters;
 
     /* The current substituter. */
     std::shared_ptr<Store> sub;
@@ -26,9 +23,6 @@ struct PathSubstitutionGoal : public Goal
     /* Whether a substituter failed. */
     bool substituterFailed = false;
 
-    /* Path info returned by the substituter's query info operation. */
-    std::shared_ptr<const ValidPathInfo> info;
-
     /* Pipe for the substituter's standard output. */
     Pipe outPipe;
 

src/libstore/local-store.cc

@@ -1121,6 +1121,7 @@ StorePathSet LocalStore::querySubstitutablePaths(const StorePathSet & paths)
 
 
 // FIXME: move this, it's not specific to LocalStore.
+// FIXME: use SubstituterSet
 void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos)
 {
     if (!settings.useSubstitutes) return;