Simplify the handling of the hash modulo

Rather than having four different but very similar types of hashes, make
only one, with a tag indicating whether it corresponds to a regular of
deferred derivation.

This implies a slight logical change: The original Nix+multiple-outputs
model assumed only one hash-modulo per derivation. Adding
multiple-outputs CA derivations changed this as these have one
hash-modulo per output. This change is now treating each derivation as
having one hash modulo per output.
This obviously means that we internally loose the guaranty that
all the outputs of input-addressed derivations have the same hash
modulo. But it turns out that it doesn’t matter because there’s nothing
in the code taking advantage of that fact (and it probably shouldn’t
anyways).

The upside is that it is now much easier to work with these hashes, and
we can get rid of a lot of useless `std::visit{ overloaded`.

Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>

5 files changed, 56 insertions, 118 deletions

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index f3eb5e925..9f549e52f 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -1222,34 +1222,26 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
                 DerivationOutput::Deferred { });
         }
 
-        // Regular, non-CA derivation should always return a single hash and not
-        // hash per output.
-        auto hashModulo = hashDerivationModulo(*state.store, drv, true);
-        std::visit(overloaded {
-            [&](const DrvHash & drvHash) {
-                auto & h = drvHash.hash;
-                switch (drvHash.kind) {
-                case DrvHash::Kind::Deferred:
-                    /* Outputs already deferred, nothing to do */
-                    break;
-                case DrvHash::Kind::Regular:
-                    for (auto & [outputName, output] : drv.outputs) {
-                        auto outPath = state.store->makeOutputPath(outputName, h, drvName);
-                        drv.env[outputName] = state.store->printStorePath(outPath);
-                        output = DerivationOutput::InputAddressed {
-                            .path = std::move(outPath),
-                        };
-                    }
-                    break;
-                }
-            },
-            [&](const CaOutputHashes &) {
-                // Shouldn't happen as the toplevel derivation is not CA.
-                assert(false);
-            },
-        },
-        hashModulo.raw());
-
+        auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true);
+        switch (hashModulo.kind) {
+        case DrvHash::Kind::Regular:
+            for (auto & i : outputs) {
+                auto h = hashModulo.hashes.at(i);
+                auto outPath = state.store->makeOutputPath(i, h, drvName);
+                drv.env[i] = state.store->printStorePath(outPath);
+                drv.outputs.insert_or_assign(
+                    i,
+                    DerivationOutputInputAddressed {
+                        .path = std::move(outPath),
+                    });
+            }
+            break;
+            ;
+        case DrvHash::Kind::Deferred:
+            for (auto & i : outputs) {
+                drv.outputs.insert_or_assign(i, DerivationOutputDeferred {});
+            }
+        }
     }
 
     /* Write the resulting term into the Nix store directory. */
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 7fed80387..85d75523f 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -474,7 +474,7 @@ Sync<DrvHashes> drvHashes;
 /* Look up the derivation by value and memoize the
    `hashDerivationModulo` call.
  */
-static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & drvPath)
+static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPath)
 {
     {
         auto hashes = drvHashes.lock();
@@ -509,7 +509,7 @@ static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath &
    don't leak the provenance of fixed outputs, reducing pointless cache
    misses as the build itself won't know this.
  */
-DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs)
+DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs)
 {
     auto type = drv.type();
 
@@ -524,7 +524,10 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m
                 + store.printStorePath(dof.path(store, drv.name, i.first)));
             outputHashes.insert_or_assign(i.first, std::move(hash));
         }
-        return outputHashes;
+        return DrvHash{
+            .hashes = outputHashes,
+            .kind = DrvHash::Kind::Regular,
+        };
     }
 
     auto kind = std::visit(overloaded {
@@ -540,65 +543,36 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m
         },
     }, drv.type().raw());
 
-    /* For other derivations, replace the inputs paths with recursive
-       calls to this function. */
     std::map<std::string, StringSet> inputs2;
     for (auto & [drvPath, inputOutputs0] : drv.inputDrvs) {
         // Avoid lambda capture restriction with standard / Clang
         auto & inputOutputs = inputOutputs0;
         const auto & res = pathDerivationModulo(store, drvPath);
-        std::visit(overloaded {
-            // Regular non-CA derivation, replace derivation
-            [&](const DrvHash & drvHash) {
-                kind |= drvHash.kind;
-                inputs2.insert_or_assign(drvHash.hash.to_string(Base16, false), inputOutputs);
-            },
-            // CA derivation's output hashes
-            [&](const CaOutputHashes & outputHashes) {
-                std::set<std::string> justOut = { "out" };
-                for (auto & output : inputOutputs) {
-                    /* Put each one in with a single "out" output.. */
-                    const auto h = outputHashes.at(output);
-                    inputs2.insert_or_assign(
-                        h.to_string(Base16, false),
-                        justOut);
-                }
-            },
-        }, res.raw());
+        if (res.kind == DrvHash::Kind::Deferred)
+            kind = DrvHash::Kind::Deferred;
+        for (auto & outputName : inputOutputs) {
+            const auto h = res.hashes.at(outputName);
+            inputs2[h.to_string(Base16, false)].insert(outputName);
+        }
     }
 
     auto hash = hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2));
 
-    return DrvHash { .hash = hash, .kind = kind };
-}
-
-
-void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept
-{
-    switch (other) {
-    case DrvHash::Kind::Regular:
-        break;
-    case DrvHash::Kind::Deferred:
-        self = other;
-        break;
+    std::map<std::string, Hash> outputHashes;
+    for (const auto & [outputName, _] : drv.outputs) {
+        outputHashes.insert_or_assign(outputName, hash);
     }
+
+    return DrvHash {
+        .hashes = outputHashes,
+        .kind = kind,
+    };
 }
 
 
 std::map<std::string, Hash> staticOutputHashes(Store & store, const Derivation & drv)
 {
-    std::map<std::string, Hash> res;
-    std::visit(overloaded {
-        [&](const DrvHash & drvHash) {
-            for (auto & outputName : drv.outputNames()) {
-                res.insert({outputName, drvHash.hash});
-            }
-        },
-        [&](const CaOutputHashes & outputHashes) {
-            res = outputHashes;
-        },
-    }, hashDerivationModulo(store, drv, true).raw());
-    return res;
+    return hashDerivationModulo(store, drv, true).hashes;
 }
 
 
@@ -747,7 +721,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String
     auto hashModulo = hashDerivationModulo(store, Derivation(drv), true);
     for (auto & [outputName, output] : drv.outputs) {
         if (std::holds_alternative<DerivationOutput::Deferred>(output.raw())) {
-            auto & h = hashModulo.requireNoFixedNonDeferred();
+            auto & h = hashModulo.hashes.at(outputName);
             auto outPath = store.makeOutputPath(outputName, h, drv.name);
             drv.env[outputName] = store.printStorePath(outPath);
             output = DerivationOutput::InputAddressed {
@@ -758,13 +732,6 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String
 
 }
 
-const Hash & DrvHashModulo::requireNoFixedNonDeferred() const {
-    auto * drvHashOpt = std::get_if<DrvHash>(&raw());
-    assert(drvHashOpt);
-    assert(drvHashOpt->kind == DrvHash::Kind::Regular);
-    return drvHashOpt->hash;
-}
-
 static bool tryResolveInput(
     Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites,
     const StorePath & inputDrv, const StringSet & inputOutputs)
diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
index 8dea90abf..63ea5ef76 100644
--- a/src/libstore/derivations.hh
+++ b/src/libstore/derivations.hh
@@ -202,12 +202,14 @@ bool isDerivation(const std::string & fileName);
    the output name is "out". */
 std::string outputPathName(std::string_view drvName, std::string_view outputName);
 
-// known CA drv's output hashes, current just for fixed-output derivations
-// whose output hashes are always known since they are fixed up-front.
-typedef std::map<std::string, Hash> CaOutputHashes;
 
+// The hashes modulo of a derivation.
+//
+// Each output is given a hash, although in practice only the content-addressed
+// derivations (fixed-output or not) will have a different hash for each
+// output.
 struct DrvHash {
-    Hash hash;
+    std::map<std::string, Hash> hashes;
 
     enum struct Kind: bool {
         // Statically determined derivations.
@@ -222,28 +224,6 @@ struct DrvHash {
 
 void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept;
 
-typedef std::variant<
-    // Regular normalized derivation hash, and whether it was deferred (because
-    // an ancestor derivation is a floating content addressed derivation).
-    DrvHash,
-    // Fixed-output derivation hashes
-    CaOutputHashes
-> _DrvHashModuloRaw;
-
-struct DrvHashModulo : _DrvHashModuloRaw {
-    using Raw = _DrvHashModuloRaw;
-    using Raw::Raw;
-
-    /* Get hash, throwing if it is per-output CA hashes or a
-       deferred Drv hash.
-     */
-    const Hash & requireNoFixedNonDeferred() const;
-
-    inline const Raw & raw() const {
-        return static_cast<const Raw &>(*this);
-    }
-};
-
 /* Returns hashes with the details of fixed-output subderivations
    expunged.
 
@@ -267,7 +247,7 @@ struct DrvHashModulo : _DrvHashModuloRaw {
    ATerm, after subderivations have been likewise expunged from that
    derivation.
  */
-DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs);
+DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs);
 
 /*
    Return a map associating each output to a hash that uniquely identifies its
@@ -276,7 +256,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m
 std::map<std::string, Hash> staticOutputHashes(Store& store, const Derivation& drv);
 
 /* Memoisation of hashDerivationModulo(). */
-typedef std::map<StorePath, DrvHashModulo> DrvHashes;
+typedef std::map<StorePath, DrvHash> DrvHashes;
 
 // FIXME: global, though at least thread-safe.
 extern Sync<DrvHashes> drvHashes;
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 46a547db1..60fe53af1 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -695,16 +695,15 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat
     // combinations that are currently prohibited.
     drv.type();
 
-    std::optional<Hash> h;
+    std::optional<DrvHash> hashesModulo;
     for (auto & i : drv.outputs) {
         std::visit(overloaded {
             [&](const DerivationOutput::InputAddressed & doia) {
-                if (!h) {
+                if (!hashesModulo) {
                     // somewhat expensive so we do lazily
-                    auto h0 = hashDerivationModulo(*this, drv, true);
-                    h = h0.requireNoFixedNonDeferred();
+                    hashesModulo = hashDerivationModulo(*this, drv, true);
                 }
-                StorePath recomputed = makeOutputPath(i.first, *h, drvName);
+                StorePath recomputed = makeOutputPath(i.first, hashesModulo->hashes.at(i.first), drvName);
                 if (doia.path != recomputed)
                     throw Error("derivation '%s' has incorrect output '%s', should be '%s'",
                         printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed));
diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index d2f9b5a6a..7fc74d34e 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -204,10 +204,10 @@ static StorePath getDerivationEnvironment(ref<Store> store, ref<Store> evalStore
             output.second = DerivationOutput::Deferred { };
             drv.env[output.first] = "";
         }
-        auto h0 = hashDerivationModulo(*evalStore, drv, true);
-        const Hash & h = h0.requireNoFixedNonDeferred();
+        auto hashesModulo = hashDerivationModulo(*evalStore, drv, true);
 
         for (auto & output : drv.outputs) {
+            Hash h = hashesModulo.hashes.at(output.first);
             auto outPath = store->makeOutputPath(output.first, h, drv.name);
             output.second = DerivationOutput::InputAddressed {
                 .path = outPath,