Merge pull request #13420 from NixOS/query-missing

Introduce MissingPaths struct for queryMissing() / printMissing()

Author: roberth

src/libmain/include/nix/main/shared.hh

@@ -35,15 +35,17 @@ void printVersion(const std::string & programName);
 void printGCWarning();
 
 class Store;
+struct MissingPaths;
 
 void printMissing(
     ref<Store> store,
     const std::vector<DerivedPath> & paths,
     Verbosity lvl = lvlInfo);
 
-void printMissing(ref<Store> store, const StorePathSet & willBuild,
-    const StorePathSet & willSubstitute, const StorePathSet & unknown,
-    uint64_t downloadSize, uint64_t narSize, Verbosity lvl = lvlInfo);
+void printMissing(
+    ref<Store> store,
+    const MissingPaths & missing,
+    Verbosity lvl = lvlInfo);
 
 std::string getArg(const std::string & opt,
     Strings::iterator & i, const Strings::iterator & end);

src/libmain/shared.cc

@@ -46,43 +46,41 @@ void printGCWarning()
 
 void printMissing(ref<Store> store, const std::vector<DerivedPath> & paths, Verbosity lvl)
 {
-    uint64_t downloadSize, narSize;
-    StorePathSet willBuild, willSubstitute, unknown;
-    store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize);
-    printMissing(store, willBuild, willSubstitute, unknown, downloadSize, narSize, lvl);
+    printMissing(store, store->queryMissing(paths), lvl);
 }
 
 
-void printMissing(ref<Store> store, const StorePathSet & willBuild,
-    const StorePathSet & willSubstitute, const StorePathSet & unknown,
-    uint64_t downloadSize, uint64_t narSize, Verbosity lvl)
+void printMissing(
+    ref<Store> store,
+    const MissingPaths & missing,
+    Verbosity lvl)
 {
-    if (!willBuild.empty()) {
-        if (willBuild.size() == 1)
+    if (!missing.willBuild.empty()) {
+        if (missing.willBuild.size() == 1)
             printMsg(lvl, "this derivation will be built:");
         else
-            printMsg(lvl, "these %d derivations will be built:", willBuild.size());
-        auto sorted = store->topoSortPaths(willBuild);
+            printMsg(lvl, "these %d derivations will be built:", missing.willBuild.size());
+        auto sorted = store->topoSortPaths(missing.willBuild);
         reverse(sorted.begin(), sorted.end());
         for (auto & i : sorted)
             printMsg(lvl, "  %s", store->printStorePath(i));
     }
 
-    if (!willSubstitute.empty()) {
-        const float downloadSizeMiB = downloadSize / (1024.f * 1024.f);
-        const float narSizeMiB = narSize / (1024.f * 1024.f);
-        if (willSubstitute.size() == 1) {
+    if (!missing.willSubstitute.empty()) {
+        const float downloadSizeMiB = missing.downloadSize / (1024.f * 1024.f);
+        const float narSizeMiB = missing.narSize / (1024.f * 1024.f);
+        if (missing.willSubstitute.size() == 1) {
             printMsg(lvl, "this path will be fetched (%.2f MiB download, %.2f MiB unpacked):",
                 downloadSizeMiB,
                 narSizeMiB);
         } else {
             printMsg(lvl, "these %d paths will be fetched (%.2f MiB download, %.2f MiB unpacked):",
-                willSubstitute.size(),
+                missing.willSubstitute.size(),
                 downloadSizeMiB,
                 narSizeMiB);
         }
         std::vector<const StorePath *> willSubstituteSorted = {};
-        std::for_each(willSubstitute.begin(), willSubstitute.end(),
+        std::for_each(missing.willSubstitute.begin(), missing.willSubstitute.end(),
                    [&](const StorePath &p) { willSubstituteSorted.push_back(&p); });
         std::sort(willSubstituteSorted.begin(), willSubstituteSorted.end(),
                   [](const StorePath *lhs, const StorePath *rhs) {
@@ -95,10 +93,10 @@ void printMissing(ref<Store> store, const StorePathSet & willBuild,
             printMsg(lvl, "  %s", store->printStorePath(*p));
     }
 
-    if (!unknown.empty()) {
+    if (!missing.unknown.empty()) {
         printMsg(lvl, "don't know how to build these paths%s:",
                 (settings.readOnlyMode ? " (may be caused by read-only store access)" : ""));
-        for (auto & i : unknown)
+        for (auto & i : missing.unknown)
             printMsg(lvl, "  %s", store->printStorePath(i));
     }
 }

src/libstore/build/worker.cc

@@ -324,9 +324,7 @@ void Worker::run(const Goals & _topGoals)
     }
 
     /* Call queryMissing() to efficiently query substitutes. */
-    StorePathSet willBuild, willSubstitute, unknown;
-    uint64_t downloadSize, narSize;
-    store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize);
+    store.queryMissing(topPaths);
 
     debug("entered goal loop");
 

src/libstore/daemon.cc

@@ -948,14 +948,12 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     case WorkerProto::Op::QueryMissing: {
         auto targets = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
         logger->startWork();
-        StorePathSet willBuild, willSubstitute, unknown;
-        uint64_t downloadSize, narSize;
-        store->queryMissing(targets, willBuild, willSubstitute, unknown, downloadSize, narSize);
+        auto missing = store->queryMissing(targets);
         logger->stopWork();
-        WorkerProto::write(*store, wconn, willBuild);
-        WorkerProto::write(*store, wconn, willSubstitute);
-        WorkerProto::write(*store, wconn, unknown);
-        conn.to << downloadSize << narSize;
+        WorkerProto::write(*store, wconn, missing.willBuild);
+        WorkerProto::write(*store, wconn, missing.willSubstitute);
+        WorkerProto::write(*store, wconn, missing.unknown);
+        conn.to << missing.downloadSize << missing.narSize;
         break;
     }
 

src/libstore/include/nix/store/remote-store.hh

@@ -149,9 +149,7 @@ struct RemoteStore :
 
     void addSignatures(const StorePath & storePath, const StringSet & sigs) override;
 
-    void queryMissing(const std::vector<DerivedPath> & targets,
-        StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
-        uint64_t & downloadSize, uint64_t & narSize) override;
+    MissingPaths queryMissing(const std::vector<DerivedPath> & targets) override;
 
     void addBuildLog(const StorePath & drvPath, std::string_view log) override;
 

src/libstore/include/nix/store/store-api.hh

@@ -71,6 +71,18 @@ struct KeyedBuildResult;
 
 typedef std::map<StorePath, std::optional<ContentAddress>> StorePathCAMap;
 
+/**
+ * Information about what paths will be built or substituted, returned
+ * by Store::queryMissing().
+ */
+struct MissingPaths
+{
+    StorePathSet willBuild;
+    StorePathSet willSubstitute;
+    StorePathSet unknown;
+    uint64_t downloadSize{0};
+    uint64_t narSize{0};
+};
 
 /**
  * About the class hierarchy of the store types:
@@ -694,9 +706,7 @@ public:
      * derivations that will be built, and the set of output paths that
      * will be substituted.
      */
-    virtual void queryMissing(const std::vector<DerivedPath> & targets,
-        StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
-        uint64_t & downloadSize, uint64_t & narSize);
+    virtual MissingPaths queryMissing(const std::vector<DerivedPath> & targets);
 
     /**
      * Sort a set of paths topologically under the references

src/libstore/misc.cc

@@ -98,23 +98,17 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv)
     return nullptr;
 }
 
-void Store::queryMissing(const std::vector<DerivedPath> & targets,
-    StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_,
-    uint64_t & downloadSize_, uint64_t & narSize_)
+MissingPaths Store::queryMissing(const std::vector<DerivedPath> & targets)
 {
     Activity act(*logger, lvlDebug, actUnknown, "querying info about missing paths");
 
-    downloadSize_ = narSize_ = 0;
-
     // FIXME: make async.
     ThreadPool pool(fileTransferSettings.httpConnections);
 
     struct State
     {
         std::unordered_set<std::string> done;
-        StorePathSet & unknown, & willSubstitute, & willBuild;
-        uint64_t & downloadSize;
-        uint64_t & narSize;
+        MissingPaths res;
     };
 
     struct DrvState
@@ -125,7 +119,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
         DrvState(size_t left) : left(left) { }
     };
 
-    Sync<State> state_(State{{}, unknown_, willSubstitute_, willBuild_, downloadSize_, narSize_});
+    Sync<State> state_;
 
     std::function<void(DerivedPath)> doPath;
 
@@ -143,7 +137,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
     auto mustBuildDrv = [&](const StorePath & drvPath, const Derivation & drv) {
         {
             auto state(state_.lock());
-            state->willBuild.insert(drvPath);
+            state->res.willBuild.insert(drvPath);
         }
 
         for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) {
@@ -203,7 +197,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
             if (!isValidPath(drvPath)) {
                 // FIXME: we could try to substitute the derivation.
                 auto state(state_.lock());
-                state->unknown.insert(drvPath);
+                state->res.unknown.insert(drvPath);
                 return;
             }
 
@@ -282,7 +276,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
 
             if (infos.empty()) {
                 auto state(state_.lock());
-                state->unknown.insert(bo.path);
+                state->res.unknown.insert(bo.path);
                 return;
             }
 
@@ -291,9 +285,9 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
 
             {
                 auto state(state_.lock());
-                state->willSubstitute.insert(bo.path);
-                state->downloadSize += info->second.downloadSize;
-                state->narSize += info->second.narSize;
+                state->res.willSubstitute.insert(bo.path);
+                state->res.downloadSize += info->second.downloadSize;
+                state->res.narSize += info->second.narSize;
             }
 
             for (auto & ref : info->second.references)
@@ -306,6 +300,8 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
         pool.enqueue(std::bind(doPath, path));
 
     pool.process();
+
+    return std::move(state_.lock()->res);
 }
 
 

src/libstore/remote-store.cc

@@ -855,9 +855,7 @@ void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & s
 }
 
 
-void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
-    StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
-    uint64_t & downloadSize, uint64_t & narSize)
+MissingPaths RemoteStore::queryMissing(const std::vector<DerivedPath> & targets)
 {
     {
         auto conn(getConnection());
@@ -868,16 +866,16 @@ void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
         conn->to << WorkerProto::Op::QueryMissing;
         WorkerProto::write(*this, *conn, targets);
         conn.processStderr();
-        willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        unknown = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
-        conn->from >> downloadSize >> narSize;
-        return;
+        MissingPaths res;
+        res.willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
+        res.willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
+        res.unknown = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
+        conn->from >> res.downloadSize >> res.narSize;
+        return res;
     }
 
  fallback:
-    return Store::queryMissing(targets, willBuild, willSubstitute,
-        unknown, downloadSize, narSize);
+    return Store::queryMissing(targets);
 }
 
 

src/libstore/restricted-store.cc

@@ -143,13 +143,7 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor
         unsupported("addSignatures");
     }
 
-    void queryMissing(
-        const std::vector<DerivedPath> & targets,
-        StorePathSet & willBuild,
-        StorePathSet & willSubstitute,
-        StorePathSet & unknown,
-        uint64_t & downloadSize,
-        uint64_t & narSize) override;
+    MissingPaths queryMissing(const std::vector<DerivedPath> & targets) override;
 
     virtual std::optional<std::string> getBuildLogExact(const StorePath & path) override
     {
@@ -306,27 +300,27 @@ std::vector<KeyedBuildResult> RestrictedStore::buildPathsWithResults(
     return results;
 }
 
-void RestrictedStore::queryMissing(
-    const std::vector<DerivedPath> & targets,
-    StorePathSet & willBuild,
-    StorePathSet & willSubstitute,
-    StorePathSet & unknown,
-    uint64_t & downloadSize,
-    uint64_t & narSize)
+MissingPaths RestrictedStore::queryMissing(const std::vector<DerivedPath> & targets)
 {
     /* This is slightly impure since it leaks information to the
        client about what paths will be built/substituted or are
        already present. Probably not a big deal. */
 
     std::vector<DerivedPath> allowed;
+    StorePathSet unknown;
     for (auto & req : targets) {
         if (goal.isAllowed(req))
             allowed.emplace_back(req);
         else
             unknown.insert(pathPartOfReq(req));
     }
 
-    next->queryMissing(allowed, willBuild, willSubstitute, unknown, downloadSize, narSize);
+    auto res = next->queryMissing(allowed);
+
+    for (auto & p : unknown)
+        res.unknown.insert(p);
+
+    return res;
 }
 
 }

src/libstore/store-api.cc

@@ -790,15 +790,12 @@ void Store::substitutePaths(const StorePathSet & paths)
     for (auto & path : paths)
         if (!path.isDerivation())
             paths2.emplace_back(DerivedPath::Opaque{path});
-    uint64_t downloadSize, narSize;
-    StorePathSet willBuild, willSubstitute, unknown;
-    queryMissing(paths2,
-        willBuild, willSubstitute, unknown, downloadSize, narSize);
+    auto missing = queryMissing(paths2);
 
-    if (!willSubstitute.empty())
+    if (!missing.willSubstitute.empty())
         try {
             std::vector<DerivedPath> subs;
-            for (auto & p : willSubstitute) subs.emplace_back(DerivedPath::Opaque{p});
+            for (auto & p : missing.willSubstitute) subs.emplace_back(DerivedPath::Opaque{p});
             buildPaths(subs);
         } catch (Error & e) {
             logWarning(e.info());