Merge pull request #3958 from obsidiansystems/ca-floating-upstream

CA derivations that depend on other CA derivations

src/libstore/build.cc

@@ -1008,6 +1008,8 @@ private:
     void tryLocalBuild();
     void buildDone();
 
+    void resolvedFinished();
+
     /* Is the build hook willing to perform the build? */
     HookReply tryBuildHook();
 
@@ -1483,8 +1485,40 @@ void DerivationGoal::inputsRealised()
     /* Determine the full set of input paths. */
 
     /* First, the input derivations. */
-    if (useDerivation)
-        for (auto & [depDrvPath, wantedDepOutputs] : dynamic_cast<Derivation *>(drv.get())->inputDrvs) {
+    if (useDerivation) {
+        auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
+
+        if (!fullDrv.inputDrvs.empty() && fullDrv.type() == DerivationType::CAFloating) {
+            /* We are be able to resolve this derivation based on the
+               now-known results of dependencies. If so, we become a stub goal
+               aliasing that resolved derivation goal */
+            std::optional attempt = fullDrv.tryResolve(worker.store);
+            assert(attempt);
+            Derivation drvResolved { *std::move(attempt) };
+
+            auto pathResolved = writeDerivation(worker.store, drvResolved);
+            /* Add to memotable to speed up downstream goal's queries with the
+               original derivation. */
+            drvPathResolutions.lock()->insert_or_assign(drvPath, pathResolved);
+
+            auto msg = fmt("Resolved derivation: '%s' -> '%s'",
+                worker.store.printStorePath(drvPath),
+                worker.store.printStorePath(pathResolved));
+            act = std::make_unique<Activity>(*logger, lvlInfo, actBuildWaiting, msg,
+                Logger::Fields {
+                       worker.store.printStorePath(drvPath),
+                       worker.store.printStorePath(pathResolved),
+                   });
+
+            auto resolvedGoal = worker.makeDerivationGoal(
+                pathResolved, wantedOutputs, buildMode);
+            addWaitee(resolvedGoal);
+
+            state = &DerivationGoal::resolvedFinished;
+            return;
+        }
+
+        for (auto & [depDrvPath, wantedDepOutputs] : fullDrv.inputDrvs) {
             /* Add the relevant output closures of the input derivation
                `i' as input paths.  Only add the closures of output paths
                that are specified as inputs. */
@@ -1504,6 +1538,7 @@ void DerivationGoal::inputsRealised()
                         worker.store.printStorePath(drvPath), j, worker.store.printStorePath(drvPath));
             }
         }
+    }
 
     /* Second, the input sources. */
     worker.store.computeFSClosure(drv->inputSrcs, inputPaths);
@@ -1962,6 +1997,9 @@ void DerivationGoal::buildDone()
     done(BuildResult::Built);
 }
 
+void DerivationGoal::resolvedFinished() {
+    done(BuildResult::Built);
+}
 
 HookReply DerivationGoal::tryBuildHook()
 {
@@ -4257,11 +4295,13 @@ void DerivationGoal::registerOutputs()
     /* Register each output path as valid, and register the sets of
        paths referenced by each of them.  If there are cycles in the
        outputs, this will fail. */
-    ValidPathInfos infos2;
-    for (auto & [outputName, newInfo] : infos) {
-        infos2.push_back(newInfo);
+    {
+        ValidPathInfos infos2;
+        for (auto & [outputName, newInfo] : infos) {
+            infos2.push_back(newInfo);
+        }
+        worker.store.registerValidPaths(infos2);
     }
-    worker.store.registerValidPaths(infos2);
 
     /* In case of a fixed-output derivation hash mismatch, throw an
        exception now that we have registered the output as valid. */
@@ -4273,12 +4313,21 @@ void DerivationGoal::registerOutputs()
        means it's safe to link the derivation to the output hash. We must do
        that for floating CA derivations, which otherwise couldn't be cached,
        but it's fine to do in all cases. */
-    for (auto & [outputName, newInfo] : infos) {
-        /* FIXME: we will want to track this mapping in the DB whether or
-           not we have a drv file. */
-        if (useDerivation)
-            worker.store.linkDeriverToPath(drvPath, outputName, newInfo.path);
+    bool isCaFloating = drv->type() == DerivationType::CAFloating;
+
+    auto drvPathResolved = drvPath;
+    if (!useDerivation && isCaFloating) {
+        /* Once a floating CA derivations reaches this point, it
+           must already be resolved, so we don't bother trying to
+           downcast drv to get would would just be an empty
+           inputDrvs field. */
+        Derivation drv2 { *drv };
+        drvPathResolved = writeDerivation(worker.store, drv2);
     }
+
+    if (useDerivation || isCaFloating)
+        for (auto & [outputName, newInfo] : infos)
+            worker.store.linkDeriverToPath(drvPathResolved, outputName, newInfo.path);
 }
 
 
@@ -4568,7 +4617,7 @@ void DerivationGoal::flushLine()
 
 std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDerivationOutputMap()
 {
-    if (drv->type() != DerivationType::CAFloating) {
+    if (!useDerivation || drv->type() != DerivationType::CAFloating) {
         std::map<std::string, std::optional<StorePath>> res;
         for (auto & [name, output] : drv->outputs)
             res.insert_or_assign(name, output.path(worker.store, drv->name, name));
@@ -4580,7 +4629,7 @@ std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDeri
 
 OutputPathMap DerivationGoal::queryDerivationOutputMap()
 {
-    if (drv->type() != DerivationType::CAFloating) {
+    if (!useDerivation || drv->type() != DerivationType::CAFloating) {
         OutputPathMap res;
         for (auto & [name, output] : drv->outputsAndOptPaths(worker.store))
             res.insert_or_assign(name, *output.second);

src/libstore/derivations.cc

@@ -69,7 +69,7 @@ bool BasicDerivation::isBuiltin() const
 
 
 StorePath writeDerivation(Store & store,
-    const Derivation & drv, RepairFlag repair)
+    const Derivation & drv, RepairFlag repair, bool readOnly)
 {
     auto references = drv.inputSrcs;
     for (auto & i : drv.inputDrvs)
@@ -79,7 +79,7 @@ StorePath writeDerivation(Store & store,
        held during a garbage collection). */
     auto suffix = std::string(drv.name) + drvExtension;
     auto contents = drv.unparse(store, false);
-    return settings.readOnlyMode
+    return readOnly || settings.readOnlyMode
         ? store.computeStorePathForText(suffix, contents, references)
         : store.addTextToStore(suffix, contents, references, repair);
 }
@@ -644,4 +644,57 @@ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath
     return "/" + hashString(htSHA256, clearText).to_string(Base32, false);
 }
 
+
+// N.B. Outputs are left unchanged
+static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) {
+
+    debug("Rewriting the derivation");
+
+    for (auto &rewrite: rewrites) {
+        debug("rewriting %s as %s", rewrite.first, rewrite.second);
+    }
+
+    drv.builder = rewriteStrings(drv.builder, rewrites);
+    for (auto & arg: drv.args) {
+        arg = rewriteStrings(arg, rewrites);
+    }
+
+    StringPairs newEnv;
+    for (auto & envVar: drv.env) {
+        auto envName = rewriteStrings(envVar.first, rewrites);
+        auto envValue = rewriteStrings(envVar.second, rewrites);
+        newEnv.emplace(envName, envValue);
+    }
+    drv.env = newEnv;
+}
+
+
+Sync<DrvPathResolutions> drvPathResolutions;
+
+std::optional<BasicDerivation> Derivation::tryResolve(Store & store) {
+    BasicDerivation resolved { *this };
+
+    // Input paths that we'll want to rewrite in the derivation
+    StringMap inputRewrites;
+
+    for (auto & input : inputDrvs) {
+        auto inputDrvOutputs = store.queryPartialDerivationOutputMap(input.first);
+        StringSet newOutputNames;
+        for (auto & outputName : input.second) {
+            auto actualPathOpt = inputDrvOutputs.at(outputName);
+            if (!actualPathOpt)
+                return std::nullopt;
+            auto actualPath = *actualPathOpt;
+            inputRewrites.emplace(
+                downstreamPlaceholder(store, input.first, outputName),
+                store.printStorePath(actualPath));
+            resolved.inputSrcs.insert(std::move(actualPath));
+        }
+    }
+
+    rewriteDerivation(store, resolved, inputRewrites);
+
+    return resolved;
+}
+
 }

src/libstore/derivations.hh

@@ -4,6 +4,7 @@
 #include "types.hh"
 #include "hash.hh"
 #include "content-address.hh"
+#include "sync.hh"
 
 #include <map>
 #include <variant>
@@ -100,7 +101,7 @@ struct BasicDerivation
     StringPairs env;
     std::string name;
 
-    BasicDerivation() { }
+    BasicDerivation() = default;
     virtual ~BasicDerivation() { };
 
     bool isBuiltin() const;
@@ -127,7 +128,17 @@ struct Derivation : BasicDerivation
     std::string unparse(const Store & store, bool maskOutputs,
         std::map<std::string, StringSet> * actualInputs = nullptr) const;
 
-    Derivation() { }
+    /* Return the underlying basic derivation but with these changes:
+
+	   1. Input drvs are emptied, but the outputs of them that were used are
+	      added directly to input sources.
+
+       2. Input placeholders are replaced with realized input store paths. */
+    std::optional<BasicDerivation> tryResolve(Store & store);
+
+    Derivation() = default;
+    Derivation(const BasicDerivation & bd) : BasicDerivation(bd) { }
+    Derivation(BasicDerivation && bd) : BasicDerivation(std::move(bd)) { }
 };
 
 
@@ -137,7 +148,9 @@ enum RepairFlag : bool { NoRepair = false, Repair = true };
 
 /* Write a derivation to the Nix store, and return its path. */
 StorePath writeDerivation(Store & store,
-    const Derivation & drv, RepairFlag repair = NoRepair);
+    const Derivation & drv,
+    RepairFlag repair = NoRepair,
+    bool readOnly = false);
 
 /* Read a derivation from a file. */
 Derivation parseDerivation(const Store & store, std::string && s, std::string_view name);
@@ -191,6 +204,16 @@ typedef std::map<StorePath, DrvHashModulo> DrvHashes;
 
 extern DrvHashes drvHashes; // FIXME: global, not thread-safe
 
+/* Memoisation of `readDerivation(..).resove()`. */
+typedef std::map<
+    StorePath,
+    std::optional<StorePath>
+> DrvPathResolutions;
+
+// FIXME: global, though at least thread-safe.
+// FIXME: arguably overlaps with hashDerivationModulo memo table.
+extern Sync<DrvPathResolutions> drvPathResolutions;
+
 bool wantOutput(const string & output, const std::set<string> & wanted);
 
 struct Source;

src/libstore/local-store.cc

@@ -721,7 +721,7 @@ uint64_t LocalStore::queryValidPathId(State & state, const StorePath & path)
 {
     auto use(state.stmtQueryPathInfo.use()(printStorePath(path)));
     if (!use.next())
-        throw Error("path '%s' is not valid", printStorePath(path));
+        throw InvalidPath("path '%s' is not valid", printStorePath(path));
     return use.getInt(0);
 }
 
@@ -796,18 +796,58 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path)
 }
 
 
-std::map<std::string, std::optional<StorePath>> LocalStore::queryPartialDerivationOutputMap(const StorePath & path)
+std::map<std::string, std::optional<StorePath>> LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)
 {
+    auto path = path_;
     std::map<std::string, std::optional<StorePath>> outputs;
-    BasicDerivation drv = readDerivation(path);
+    Derivation drv = readDerivation(path);
     for (auto & [outName, _] : drv.outputs) {
         outputs.insert_or_assign(outName, std::nullopt);
     }
+    bool haveCached = false;
+    {
+        auto resolutions = drvPathResolutions.lock();
+        auto resolvedPathOptIter = resolutions->find(path);
+        if (resolvedPathOptIter != resolutions->end()) {
+            auto & [_, resolvedPathOpt] = *resolvedPathOptIter;
+            if (resolvedPathOpt)
+                path = *resolvedPathOpt;
+            haveCached = true;
+        }
+    }
+    /* can't just use else-if instead of `!haveCached` because we need to unlock
+       `drvPathResolutions` before it is locked in `Derivation::resolve`. */
+    if (!haveCached && drv.type() == DerivationType::CAFloating) {
+        /* Try resolve drv and use that path instead. */
+        auto attempt = drv.tryResolve(*this);
+        if (!attempt)
+            /* If we cannot resolve the derivation, we cannot have any path
+               assigned so we return the map of all std::nullopts. */
+            return outputs;
+        /* Just compute store path */
+        auto pathResolved = writeDerivation(*this, *std::move(attempt), NoRepair, true);
+        /* Store in memo table. */
+        /* FIXME: memo logic should not be local-store specific, should have
+           wrapper-method instead. */
+        drvPathResolutions.lock()->insert_or_assign(path, pathResolved);
+        path = std::move(pathResolved);
+    }
     return retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() {
         auto state(_state.lock());
 
-        auto useQueryDerivationOutputs(state->stmtQueryDerivationOutputs.use()
-            (queryValidPathId(*state, path)));
+        uint64_t drvId;
+        try {
+            drvId = queryValidPathId(*state, path);
+        } catch (InvalidPath &) {
+            /* FIXME? if the derivation doesn't exist, we cannot have a mapping
+               for it. */
+            return outputs;
+        }
+
+        auto useQueryDerivationOutputs {
+            state->stmtQueryDerivationOutputs.use()
+            (drvId)
+        };
 
         while (useQueryDerivationOutputs.next())
             outputs.insert_or_assign(

tests/content-addressed.nix

@@ -29,4 +29,26 @@ rec {
     outputHashMode = "recursive";
     outputHashAlgo = "sha256";
   };
+  dependentCA = mkDerivation {
+    name = "dependent";
+    buildCommand = ''
+      echo "building a dependent derivation"
+      mkdir -p $out
+      echo ${rootCA}/hello > $out/dep
+    '';
+    __contentAddressed = true;
+    outputHashMode = "recursive";
+    outputHashAlgo = "sha256";
+  };
+  transitivelyDependentCA = mkDerivation {
+    name = "transitively-dependent";
+    buildCommand = ''
+      echo "building transitively-dependent"
+      cat ${dependentCA}/dep
+      echo ${dependentCA} > $out
+    '';
+    __contentAddressed = true;
+    outputHashMode = "recursive";
+    outputHashAlgo = "sha256";
+  };
 }

tests/content-addressed.sh

@@ -2,19 +2,26 @@
 
 source common.sh
 
-clearStore
-clearCache
-
-export REMOTE_STORE=file://$cacheDir
-
 drv=$(nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 1)
 nix --experimental-features 'nix-command ca-derivations' show-derivation --derivation "$drv" --arg seed 1
 
-commonArgs=("--experimental-features" "ca-derivations" "./content-addressed.nix" "-A" "rootCA" "--no-out-link")
-out1=$(nix-build "${commonArgs[@]}" ./content-addressed.nix --arg seed 1)
-out2=$(nix-build "${commonArgs[@]}" ./content-addressed.nix --arg seed 2)
+testDerivation () {
+    local derivationPath=$1
+    local commonArgs=("--experimental-features" "ca-derivations" "./content-addressed.nix" "-A" "$derivationPath" "--no-out-link")
+    local out1 out2
+    out1=$(nix-build "${commonArgs[@]}" --arg seed 1)
+    out2=$(nix-build "${commonArgs[@]}" --arg seed 2 "${secondSeedArgs[@]}")
+    test "$out1" == "$out2"
+}
 
-test $out1 == $out2
+testDerivation rootCA
+# The seed only changes the root derivation, and not it's output, so the
+# dependent derivations should only need to be built once.
+secondSeedArgs=(-j0)
+# Don't directly build depenentCA, that way we'll make sure we dodn't rely on
+# dependent derivations always being already built.
+#testDerivation dependentCA
+testDerivation transitivelyDependentCA
 
 nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 5
 nix-collect-garbage --experimental-features ca-derivations --option keep-derivations true