Merge pull request #4387 from obsidiansystems/non-local-store-build

Make `nix-build --store whatever` work

src/build-remote/build-remote.cc

@@ -71,11 +71,15 @@ static int main_build_remote(int argc, char * * argv)
 
         initPlugins();
 
-        auto store = openStore().cast<LocalStore>();
+        auto store = openStore();
 
         /* It would be more appropriate to use $XDG_RUNTIME_DIR, since
            that gets cleared on reboot, but it wouldn't work on macOS. */
-        currentLoad = store->stateDir + "/current-load";
+        auto currentLoadName = "/current-load";
+        if (auto localStore = store.dynamic_pointer_cast<LocalFSStore>())
+            currentLoad = std::string { localStore->stateDir } + currentLoadName;
+        else
+            currentLoad = settings.nixStateDir + currentLoadName;
 
         std::shared_ptr<Store> sshStore;
         AutoCloseFD bestSlotLock;
@@ -288,8 +292,9 @@ connected:
 
         if (!missing.empty()) {
             Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
+            if (auto localStore = store.dynamic_pointer_cast<LocalStore>())
                 for (auto & i : missing)
-                store->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */
+                    localStore->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */
             copyPaths(ref<Store>(sshStore), store, missing, NoRepair, NoCheckSigs, NoSubstitute);
         }
 

src/libstore/binary-cache-store.hh

@@ -108,13 +108,6 @@ public:
 
     void narFromPath(const StorePath & path, Sink & sink) override;
 
-    BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
-        BuildMode buildMode) override
-    { unsupported("buildDerivation"); }
-
-    void ensurePath(const StorePath & path) override
-    { unsupported("ensurePath"); }
-
     ref<FSAccessor> getFSAccessor() override;
 
     void addSignatures(const StorePath & storePath, const StringSet & sigs) override;

src/libstore/build/derivation-goal.cc

@@ -597,6 +597,14 @@ void DerivationGoal::tryToBuild()
     PathSet lockFiles;
     /* FIXME: Should lock something like the drv itself so we don't build same
        CA drv concurrently */
+    if (dynamic_cast<LocalStore *>(&worker.store))
+        /* If we aren't a local store, we might need to use the local store as
+           a build remote, but that would cause a deadlock. */
+        /* FIXME: Make it so we can use ourselves as a build remote even if we
+           are the local store (separate locking for building vs scheduling? */
+        /* FIXME: find some way to lock for scheduling for the other stores so
+           a forking daemon with --store still won't farm out redundant builds.
+           */
         for (auto & i : drv->outputsAndOptPaths(worker.store))
             if (i.second.second)
                 lockFiles.insert(worker.store.Store::toRealPath(*i.second.second));
@@ -681,6 +689,12 @@ void DerivationGoal::tryToBuild()
 
 void DerivationGoal::tryLocalBuild() {
     /* Make sure that we are allowed to start a build. */
+    if (!dynamic_cast<LocalStore *>(&worker.store)) {
+        throw Error(
+            "unable to build with a primary store that isn't a local store; "
+            "either pass a different '--store' or enable remote builds."
+            "\nhttps://nixos.org/nix/manual/#chap-distributed-builds");
+    }
     unsigned int curBuilds = worker.getNrLocalBuilds();
     if (curBuilds >= settings.maxBuildJobs) {
         worker.waitForBuildSlot(shared_from_this());
@@ -849,14 +863,16 @@ void DerivationGoal::buildDone()
                So instead, check if the disk is (nearly) full now.  If
                so, we don't mark this build as a permanent failure. */
 #if HAVE_STATVFS
+            if (auto localStore = dynamic_cast<LocalStore *>(&worker.store)) {
                 uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable
                 struct statvfs st;
-            if (statvfs(worker.store.realStoreDir.c_str(), &st) == 0 &&
+                if (statvfs(localStore->realStoreDir.c_str(), &st) == 0 &&
                     (uint64_t) st.f_bavail * st.f_bsize < required)
                     diskFull = true;
                 if (statvfs(tmpDir.c_str(), &st) == 0 &&
                     (uint64_t) st.f_bavail * st.f_bsize < required)
                     diskFull = true;
+            }
 #endif
 
             deleteTmpDir(false);
@@ -1216,13 +1232,16 @@ void DerivationGoal::startBuilder()
             useChroot = !(derivationIsImpure(derivationType)) && !noChroot;
     }
 
-    if (worker.store.storeDir != worker.store.realStoreDir) {
+    if (auto localStoreP = dynamic_cast<LocalStore *>(&worker.store)) {
+        auto & localStore = *localStoreP;
+        if (localStore.storeDir != localStore.realStoreDir) {
             #if __linux__
                 useChroot = true;
             #else
                 throw Error("building using a diverted store is not supported on this platform");
             #endif
         }
+    }
 
     /* Create a temporary directory where the build will take
        place. */
@@ -2181,7 +2200,8 @@ void DerivationGoal::startDaemon()
     Store::Params params;
     params["path-info-cache-size"] = "0";
     params["store"] = worker.store.storeDir;
-    params["root"] = worker.store.rootDir;
+    if (auto localStore = dynamic_cast<LocalStore *>(&worker.store))
+        params["root"] = localStore->rootDir;
     params["state"] = "/no-such-path";
     params["log"] = "/no-such-path";
     auto store = make_ref<RestrictedStore>(params,
@@ -3269,7 +3289,13 @@ void DerivationGoal::registerOutputs()
             }
         }
 
+        auto localStoreP = dynamic_cast<LocalStore *>(&worker.store);
+        if (!localStoreP)
+            throw Unsupported("can only register outputs with local store, but this is %s", worker.store.getUri());
+        auto & localStore = *localStoreP;
+
         if (buildMode == bmCheck) {
+
             if (!worker.store.isValidPath(newInfo.path)) continue;
             ValidPathInfo oldInfo(*worker.store.queryPathInfo(newInfo.path));
             if (newInfo.narHash != oldInfo.narHash) {
@@ -3294,8 +3320,8 @@ void DerivationGoal::registerOutputs()
             /* Since we verified the build, it's now ultimately trusted. */
             if (!oldInfo.ultimate) {
                 oldInfo.ultimate = true;
-                worker.store.signPathInfo(oldInfo);
+                localStore.signPathInfo(oldInfo);
-                worker.store.registerValidPaths({{oldInfo.path, oldInfo}});
+                localStore.registerValidPaths({{oldInfo.path, oldInfo}});
             }
 
             continue;
@@ -3311,13 +3337,13 @@ void DerivationGoal::registerOutputs()
         }
 
         if (curRound == nrRounds) {
-            worker.store.optimisePath(actualPath); // FIXME: combine with scanForReferences()
+            localStore.optimisePath(actualPath); // FIXME: combine with scanForReferences()
             worker.markContentsGood(newInfo.path);
         }
 
         newInfo.deriver = drvPath;
         newInfo.ultimate = true;
-        worker.store.signPathInfo(newInfo);
+        localStore.signPathInfo(newInfo);
 
         finish(newInfo.path);
 
@@ -3325,7 +3351,7 @@ void DerivationGoal::registerOutputs()
            isn't statically known so that we can safely unlock the path before
            the next iteration */
         if (newInfo.ca)
-            worker.store.registerValidPaths({{newInfo.path, newInfo}});
+            localStore.registerValidPaths({{newInfo.path, newInfo}});
 
         infos.emplace(outputName, std::move(newInfo));
     }
@@ -3398,11 +3424,16 @@ void DerivationGoal::registerOutputs()
        paths referenced by each of them.  If there are cycles in the
        outputs, this will fail. */
     {
+        auto localStoreP = dynamic_cast<LocalStore *>(&worker.store);
+        if (!localStoreP)
+            throw Unsupported("can only register outputs with local store, but this is %s", worker.store.getUri());
+        auto & localStore = *localStoreP;
+
         ValidPathInfos infos2;
         for (auto & [outputName, newInfo] : infos) {
             infos2.insert_or_assign(newInfo.path, newInfo);
         }
-        worker.store.registerValidPaths(infos2);
+        localStore.registerValidPaths(infos2);
     }
 
     /* In case of a fixed-output derivation hash mismatch, throw an
@@ -3600,7 +3631,12 @@ Path DerivationGoal::openLogFile()
     auto baseName = std::string(baseNameOf(worker.store.printStorePath(drvPath)));
 
     /* Create a log file. */
-    Path dir = fmt("%s/%s/%s/", worker.store.logDir, worker.store.drvsLogDir, string(baseName, 0, 2));
+    Path logDir;
+    if (auto localStore = dynamic_cast<LocalStore *>(&worker.store))
+        logDir = localStore->logDir;
+    else
+        logDir = settings.nixLogDir;
+    Path dir = fmt("%s/%s/%s/", logDir, LocalFSStore::drvsLogDir, string(baseName, 0, 2));
     createDirs(dir);
 
     Path logFileName = fmt("%s/%s%s", dir, string(baseName, 2),

src/libstore/build/entry-points.cc

@@ -5,7 +5,7 @@
 
 namespace nix {
 
-void LocalStore::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode)
+void Store::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode)
 {
     Worker worker(*this);
 
@@ -43,7 +43,7 @@ void LocalStore::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths,
     }
 }
 
-BuildResult LocalStore::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
+BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
     BuildMode buildMode)
 {
     Worker worker(*this);
@@ -63,7 +63,7 @@ BuildResult LocalStore::buildDerivation(const StorePath & drvPath, const BasicDe
 }
 
 
-void LocalStore::ensurePath(const StorePath & path)
+void Store::ensurePath(const StorePath & path)
 {
     /* If the path is already valid, we're done. */
     if (isValidPath(path)) return;

src/libstore/build/substitution-goal.cc

@@ -142,9 +142,7 @@ void SubstitutionGoal::tryNext()
     /* 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 (worker.store.requireSigs
+    if (!sub->isTrusted && worker.store.pathInfoIsTrusted(*info))
-        && !sub->isTrusted
-        && !info->checkSignatures(worker.store, worker.store.getPublicKeys()))
     {
         logWarning({
             .name = "Invalid path signature",

src/libstore/build/worker.cc

@@ -8,7 +8,7 @@
 
 namespace nix {
 
-Worker::Worker(LocalStore & store)
+Worker::Worker(Store & store)
     : act(*logger, actRealise)
     , actDerivations(*logger, actBuilds)
     , actSubstitutions(*logger, actCopyPaths)
@@ -229,7 +229,9 @@ void Worker::run(const Goals & _topGoals)
 
         checkInterrupt();
 
-        store.autoGC(false);
+        // TODO GC interface?
+        if (auto localStore = dynamic_cast<LocalStore *>(&store))
+            localStore->autoGC(false);
 
         /* Call every wake goal (in the ordering established by
            CompareGoalPtrs). */

src/libstore/build/worker.hh

@@ -2,9 +2,12 @@
 
 #include "types.hh"
 #include "lock.hh"
-#include "local-store.hh"
+#include "store-api.hh"
 #include "goal.hh"
 
+#include <future>
+#include <thread>
+
 namespace nix {
 
 /* Forward definition. */
@@ -102,7 +105,7 @@ public:
     /* Set if at least one derivation is not deterministic in check mode. */
     bool checkMismatch;
 
-    LocalStore & store;
+    Store & store;
 
     std::unique_ptr<HookInstance> hook;
 
@@ -124,7 +127,7 @@ public:
        it answers with "decline-permanently", we don't try again. */
     bool tryBuildHook = true;
 
-    Worker(LocalStore & store);
+    Worker(Store & store);
     ~Worker();
 
     /* Make a goal (with caching). */

src/libstore/dummy-store.cc

@@ -55,13 +55,6 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store
     void narFromPath(const StorePath & path, Sink & sink) override
     { unsupported("narFromPath"); }
 
-    void ensurePath(const StorePath & path) override
-    { unsupported("ensurePath"); }
-
-    BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
-        BuildMode buildMode) override
-    { unsupported("buildDerivation"); }
-
     std::optional<const Realisation> queryRealisation(const DrvOutput&) override
     { unsupported("queryRealisation"); }
 };

src/libstore/local-store.cc

@@ -1098,7 +1098,6 @@ void LocalStore::invalidatePath(State & state, const StorePath & path)
     }
 }
 
-
 const PublicKeys & LocalStore::getPublicKeys()
 {
     auto state(_state.lock());
@@ -1107,11 +1106,15 @@ const PublicKeys & LocalStore::getPublicKeys()
     return *state->publicKeys;
 }
 
+bool LocalStore::pathInfoIsTrusted(const ValidPathInfo & info)
+{
+    return requireSigs && !info.checkSignatures(*this, getPublicKeys());
+}
 
 void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
     RepairFlag repair, CheckSigsFlag checkSigs)
 {
-    if (requireSigs && checkSigs && !info.checkSignatures(*this, getPublicKeys()))
+    if (checkSigs && pathInfoIsTrusted(info))
         throw Error("cannot add path '%s' because it lacks a valid signature", printStorePath(info.path));
 
     addTempRoot(info.path);

src/libstore/local-store.hh

@@ -136,6 +136,8 @@ public:
     void querySubstitutablePathInfos(const StorePathCAMap & paths,
         SubstitutablePathInfos & infos) override;
 
+    bool pathInfoIsTrusted(const ValidPathInfo &) override;
+
     void addToStore(const ValidPathInfo & info, Source & source,
         RepairFlag repair, CheckSigsFlag checkSigs) override;
 
@@ -145,15 +147,6 @@ public:
     StorePath addTextToStore(const string & name, const string & s,
         const StorePathSet & references, RepairFlag repair) override;
 
-    void buildPaths(
-        const std::vector<StorePathWithOutputs> & paths,
-        BuildMode buildMode) override;
-
-    BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
-        BuildMode buildMode) override;
-
-    void ensurePath(const StorePath & path) override;
-
     void addTempRoot(const StorePath & path) override;
 
     void addIndirectRoot(const Path & path) override;

src/libstore/store-api.cc

@@ -747,29 +747,6 @@ const Store::Stats & Store::getStats()
 }
 
 
-void Store::buildPaths(const std::vector<StorePathWithOutputs> & paths, BuildMode buildMode)
-{
-    StorePathSet paths2;
-
-    for (auto & path : paths) {
-        if (path.path.isDerivation()) {
-            auto outPaths = queryPartialDerivationOutputMap(path.path);
-            for (auto & outputName : path.outputs) {
-                auto currentOutputPathIter = outPaths.find(outputName);
-                if (currentOutputPathIter == outPaths.end() ||
-                    !currentOutputPathIter->second ||
-                    !isValidPath(*currentOutputPathIter->second))
-                    unsupported("buildPaths");
-            }
-        } else
-            paths2.insert(path.path);
-    }
-
-    if (queryValidPaths(paths2).size() != paths2.size())
-        unsupported("buildPaths");
-}
-
-
 void copyStorePath(ref<Store> srcStore, ref<Store> dstStore,
     const StorePath & storePath, RepairFlag repair, CheckSigsFlag checkSigs)
 {

src/libstore/store-api.hh

@@ -372,6 +372,21 @@ public:
     void queryPathInfo(const StorePath & path,
         Callback<ref<const ValidPathInfo>> callback) noexcept;
 
+    /* Check whether the given valid path info is sufficiently attested, by
+       either being signed by a trusted public key or content-addressed, in
+       order to be included in the given store.
+
+       These same checks would be performed in addToStore, but this allows an
+       earlier failure in the case where dependencies need to be added too, but
+       the addToStore wouldn't fail until those dependencies are added. Also,
+       we don't really want to add the dependencies listed in a nar info we
+       don't trust anyyways.
+       */
+    virtual bool pathInfoIsTrusted(const ValidPathInfo &)
+    {
+        return true;
+    }
+
 protected:
 
     virtual void queryPathInfoUncached(const StorePath & path,
@@ -519,17 +534,17 @@ public:
            explicitly choosing to allow it).
     */
     virtual BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
-        BuildMode buildMode = bmNormal) = 0;
+        BuildMode buildMode = bmNormal);
 
     /* Ensure that a path is valid.  If it is not currently valid, it
        may be made valid by running a substitute (if defined for the
        path). */
-    virtual void ensurePath(const StorePath & path) = 0;
+    virtual void ensurePath(const StorePath & path);
 
     /* Add a store path as a temporary root of the garbage collector.
        The root disappears as soon as we exit. */
     virtual void addTempRoot(const StorePath & path)
-    { unsupported("addTempRoot"); }
+    { warn("not creating temp root, store doesn't support GC"); }
 
     /* Add an indirect root, which is merely a symlink to `path' from
        /nix/var/nix/gcroots/auto/<hash of `path'>.  `path' is supposed

tests/binary-cache-build-remote.sh

@@ -0,0 +1,16 @@
+source common.sh
+
+clearStore
+clearCacheCache
+
+# Fails without remote builders
+(! nix-build --store "file://$cacheDir" dependencies.nix)
+
+# Succeeds with default store as build remote.
+outPath=$(nix-build --store "file://$cacheDir" --builders 'auto - - 1 1' -j0 dependencies.nix)
+
+# Test that the path exactly exists in the destination store.
+nix path-info --store "file://$cacheDir" $outPath
+
+# Succeeds without any build capability because no-op
+nix-build --store "file://$cacheDir" -j0 dependencies.nix

tests/binary-cache.sh

@@ -1,15 +1,20 @@
 source common.sh
 
+# We can produce drvs directly into the binary cache
 clearStore
-clearCache
+clearCacheCache
+nix-instantiate --store "file://$cacheDir" dependencies.nix
 
 # Create the binary cache.
+clearStore
+clearCache
 outPath=$(nix-build dependencies.nix --no-out-link)
 
 nix copy --to file://$cacheDir $outPath
 
 
-basicTests() {
+basicDownloadTests() {
+    # No uploading tests bcause upload with force HTTP doesn't work.
 
     # By default, a binary cache doesn't support "nix-env -qas", but does
     # support installation.
@@ -44,12 +49,12 @@ basicTests() {
 
 
 # Test LocalBinaryCacheStore.
-basicTests
+basicDownloadTests
 
 
 # Test HttpBinaryCacheStore.
 export _NIX_FORCE_HTTP=1
-basicTests
+basicDownloadTests
 
 
 # Test whether Nix notices if the NAR doesn't match the hash in the NAR info.

tests/local.mk

@@ -9,7 +9,9 @@ nix_tests = \
   local-store.sh remote-store.sh export.sh export-graph.sh \
   timeout.sh secure-drv-outputs.sh nix-channel.sh \
   multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \
-  binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \
+  binary-cache.sh \
+  binary-cache-build-remote.sh \
+  nix-profile.sh repair.sh dump-db.sh case-hack.sh \
   check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \
   placeholders.sh nix-shell.sh \
   linux-sandbox.sh \