Clean up `DerivationOutput`, and headers

1. `DerivationOutput` now as the `std::variant` as a base class. And the
   variants are given hierarchical names under `DerivationOutput`.

   In 8e0d0689be797f9e42f9b43b06f50c1af7f20b4a @matthewbauer and I
   didn't know a better idiom, and so we made it a field. But this sort
   of "newtype" is anoying for literals downstream.

   Since then we leaned the base class, inherit the constructors trick,
   e.g. used in `DerivedPath`. Switching to use that makes this more
   ergonomic, and consistent.

2. `store-api.hh` and `derivations.hh` are now independent.

   In bcde5456cc3295061a0726881c3e441444dd6680 I swapped the dependency,
   but I now know it is better to just keep on using incomplete types as
   much as possible for faster compilation and good separation of
   concerns.

10 files changed, 84 insertions, 71 deletions

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index 4e763e570..8a301d4ac 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -2279,7 +2279,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
             return res;
         };
 
-        auto newInfoFromCA = [&](const DerivationOutputCAFloating outputHash) -> ValidPathInfo {
+        auto newInfoFromCA = [&](const DerivationOutput::CAFloating outputHash) -> ValidPathInfo {
             auto & st = outputStats.at(outputName);
             if (outputHash.method == FileIngestionMethod::Flat) {
                 /* The output path should be a regular file without execute permission. */
@@ -2346,7 +2346,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
 
         ValidPathInfo newInfo = std::visit(overloaded {
 
-            [&](const DerivationOutputInputAddressed & output) {
+            [&](const DerivationOutput::InputAddressed & output) {
                 /* input-addressed case */
                 auto requiredFinalPath = output.path;
                 /* Preemptively add rewrite rule for final hash, as that is
@@ -2366,8 +2366,8 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                 return newInfo0;
             },
 
-            [&](const DerivationOutputCAFixed & dof) {
-                auto newInfo0 = newInfoFromCA(DerivationOutputCAFloating {
+            [&](const DerivationOutput::CAFixed & dof) {
+                auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating {
                     .method = dof.hash.method,
                     .hashType = dof.hash.hash.type,
                 });
@@ -2389,17 +2389,17 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                 return newInfo0;
             },
 
-            [&](DerivationOutputCAFloating & dof) {
+            [&](const DerivationOutput::CAFloating & dof) {
                 return newInfoFromCA(dof);
             },
 
-            [&](DerivationOutputDeferred) -> ValidPathInfo {
+            [&](const DerivationOutput::Deferred &) -> ValidPathInfo {
                 // No derivation should reach that point without having been
                 // rewritten first
                 assert(false);
             },
 
-        }, output.output);
+        }, output.raw());
 
         /* FIXME: set proper permissions in restorePath() so
             we don't have to do another traversal. */
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 1fe45bd87..3dd3a78d8 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -11,25 +11,25 @@ namespace nix {
 std::optional<StorePath> DerivationOutput::path(const Store & store, std::string_view drvName, std::string_view outputName) const
 {
     return std::visit(overloaded {
-        [](const DerivationOutputInputAddressed & doi) -> std::optional<StorePath> {
+        [](const DerivationOutput::InputAddressed & doi) -> std::optional<StorePath> {
             return { doi.path };
         },
-        [&](const DerivationOutputCAFixed & dof) -> std::optional<StorePath> {
+        [&](const DerivationOutput::CAFixed & dof) -> std::optional<StorePath> {
             return {
                 dof.path(store, drvName, outputName)
             };
         },
-        [](const DerivationOutputCAFloating & dof) -> std::optional<StorePath> {
+        [](const DerivationOutput::CAFloating & dof) -> std::optional<StorePath> {
             return std::nullopt;
         },
-        [](const DerivationOutputDeferred &) -> std::optional<StorePath> {
+        [](const DerivationOutput::Deferred &) -> std::optional<StorePath> {
             return std::nullopt;
         },
-    }, output);
+    }, raw());
 }
 
 
-StorePath DerivationOutputCAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const {
+StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const {
     return store.makeFixedOutputPath(
         hash.method, hash.hash,
         outputPathName(drvName, outputName));
@@ -179,35 +179,27 @@ static DerivationOutput parseDerivationOutput(const Store & store,
         const auto hashType = parseHashType(hashAlgo);
         if (hash != "") {
             validatePath(pathS);
-            return DerivationOutput {
-                .output = DerivationOutputCAFixed {
-                    .hash = FixedOutputHash {
-                        .method = std::move(method),
-                        .hash = Hash::parseNonSRIUnprefixed(hash, hashType),
-                    },
+            return DerivationOutput::CAFixed {
+                .hash = FixedOutputHash {
+                    .method = std::move(method),
+                    .hash = Hash::parseNonSRIUnprefixed(hash, hashType),
                 },
             };
         } else {
             settings.requireExperimentalFeature(Xp::CaDerivations);
             assert(pathS == "");
-            return DerivationOutput {
-                .output = DerivationOutputCAFloating {
-                    .method = std::move(method),
-                    .hashType = std::move(hashType),
-                },
+            return DerivationOutput::CAFloating {
+                .method = std::move(method),
+                .hashType = std::move(hashType),
             };
         }
     } else {
         if (pathS == "") {
-            return DerivationOutput {
-                .output = DerivationOutputDeferred { }
-            };
+            return DerivationOutput::Deferred { };
         }
         validatePath(pathS);
-        return DerivationOutput {
-            .output = DerivationOutputInputAddressed {
-                .path = store.parseStorePath(pathS),
-            }
+        return DerivationOutput::InputAddressed {
+            .path = store.parseStorePath(pathS),
         };
     }
 }
@@ -335,27 +327,27 @@ std::string Derivation::unparse(const Store & store, bool maskOutputs,
         if (first) first = false; else s += ',';
         s += '('; printUnquotedString(s, i.first);
         std::visit(overloaded {
-            [&](const DerivationOutputInputAddressed & doi) {
+            [&](const DerivationOutput::InputAddressed & doi) {
                 s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(doi.path));
                 s += ','; printUnquotedString(s, "");
                 s += ','; printUnquotedString(s, "");
             },
-            [&](const DerivationOutputCAFixed & dof) {
+            [&](const DerivationOutput::CAFixed & dof) {
                 s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(dof.path(store, name, i.first)));
                 s += ','; printUnquotedString(s, dof.hash.printMethodAlgo());
                 s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false));
             },
-            [&](const DerivationOutputCAFloating & dof) {
+            [&](const DerivationOutput::CAFloating & dof) {
                 s += ','; printUnquotedString(s, "");
                 s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType));
                 s += ','; printUnquotedString(s, "");
             },
-            [&](const DerivationOutputDeferred &) {
+            [&](const DerivationOutput::Deferred &) {
                 s += ','; printUnquotedString(s, "");
                 s += ','; printUnquotedString(s, "");
                 s += ','; printUnquotedString(s, "");
             }
-        }, i.second.output);
+        }, i.second.raw());
         s += ')';
     }
 
@@ -423,13 +415,13 @@ DerivationType BasicDerivation::type() const
     std::optional<HashType> floatingHashType;
     for (auto & i : outputs) {
         std::visit(overloaded {
-            [&](const DerivationOutputInputAddressed &) {
+            [&](const DerivationOutput::InputAddressed &) {
                inputAddressedOutputs.insert(i.first);
             },
-            [&](const DerivationOutputCAFixed &) {
+            [&](const DerivationOutput::CAFixed &) {
                 fixedCAOutputs.insert(i.first);
             },
-            [&](const DerivationOutputCAFloating & dof) {
+            [&](const DerivationOutput::CAFloating & dof) {
                 floatingCAOutputs.insert(i.first);
                 if (!floatingHashType) {
                     floatingHashType = dof.hashType;
@@ -438,10 +430,10 @@ DerivationType BasicDerivation::type() const
                         throw Error("All floating outputs must use the same hash type");
                 }
             },
-            [&](const DerivationOutputDeferred &) {
+            [&](const DerivationOutput::Deferred &) {
                deferredIAOutputs.insert(i.first);
             },
-        }, i.second.output);
+        }, i.second.raw());
     }
 
     if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) {
@@ -516,7 +508,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m
     case DerivationType::CAFixed: {
         std::map<std::string, Hash> outputHashes;
         for (const auto & i : drv.outputs) {
-            auto & dof = std::get<DerivationOutputCAFixed>(i.second.output);
+            auto & dof = std::get<DerivationOutput::CAFixed>(i.second.raw());
             auto hash = hashString(htSHA256, "fixed:out:"
                 + dof.hash.printMethodAlgo() + ":"
                 + dof.hash.hash.to_string(Base16, false) + ":"
@@ -672,27 +664,27 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr
     for (auto & i : drv.outputs) {
         out << i.first;
         std::visit(overloaded {
-            [&](const DerivationOutputInputAddressed & doi) {
+            [&](const DerivationOutput::InputAddressed & doi) {
                 out << store.printStorePath(doi.path)
                     << ""
                     << "";
             },
-            [&](const DerivationOutputCAFixed & dof) {
+            [&](const DerivationOutput::CAFixed & dof) {
                 out << store.printStorePath(dof.path(store, drv.name, i.first))
                     << dof.hash.printMethodAlgo()
                     << dof.hash.hash.to_string(Base16, false);
             },
-            [&](const DerivationOutputCAFloating & dof) {
+            [&](const DerivationOutput::CAFloating & dof) {
                 out << ""
                     << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType))
                     << "";
             },
-            [&](const DerivationOutputDeferred &) {
+            [&](const DerivationOutput::Deferred &) {
                 out << ""
                     << ""
                     << "";
             },
-        }, i.second.output);
+        }, i.second.raw());
     }
     worker_proto::write(store, out, drv.inputSrcs);
     out << drv.platform << drv.builder << drv.args;
@@ -740,14 +732,12 @@ 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<DerivationOutputDeferred>(output.output)) {
+        if (std::holds_alternative<DerivationOutput::Deferred>(output.raw())) {
             auto & h = hashModulo.requireNoFixedNonDeferred();
             auto outPath = store.makeOutputPath(outputName, h, drv.name);
             drv.env[outputName] = store.printStorePath(outPath);
-            output = DerivationOutput {
-                .output = DerivationOutputInputAddressed {
-                    .path = std::move(outPath),
-                },
+            output = DerivationOutput::InputAddressed {
+                .path = std::move(outPath),
             };
         }
     }
diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
index 2fb18d7f7..6b92b2f21 100644
--- a/src/libstore/derivations.hh
+++ b/src/libstore/derivations.hh
@@ -4,6 +4,7 @@
 #include "types.hh"
 #include "hash.hh"
 #include "content-address.hh"
+#include "repair-flag.hh"
 #include "sync.hh"
 
 #include <map>
@@ -44,19 +45,31 @@ struct DerivationOutputCAFloating
  */
 struct DerivationOutputDeferred {};
 
-struct DerivationOutput
+typedef std::variant<
+    DerivationOutputInputAddressed,
+    DerivationOutputCAFixed,
+    DerivationOutputCAFloating,
+    DerivationOutputDeferred
+> _DerivationOutputRaw;
+
+struct DerivationOutput : _DerivationOutputRaw
 {
-    std::variant<
-        DerivationOutputInputAddressed,
-        DerivationOutputCAFixed,
-        DerivationOutputCAFloating,
-        DerivationOutputDeferred
-    > output;
+    using Raw = _DerivationOutputRaw;
+    using Raw::Raw;
+
+    using InputAddressed = DerivationOutputInputAddressed;
+    using CAFixed = DerivationOutputCAFixed;
+    using CAFloating = DerivationOutputCAFloating;
+    using Deferred = DerivationOutputDeferred;
 
     /* Note, when you use this function you should make sure that you're passing
        the right derivation name. When in doubt, you should use the safer
        interface provided by BasicDerivation::outputsAndOptPaths */
     std::optional<StorePath> path(const Store & store, std::string_view drvName, std::string_view outputName) const;
+
+    inline const Raw & raw() const {
+        return static_cast<const Raw &>(*this);
+    }
 };
 
 typedef std::map<std::string, DerivationOutput> DerivationOutputs;
@@ -150,8 +163,6 @@ struct Derivation : BasicDerivation
 
 class Store;
 
-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,
@@ -197,10 +208,10 @@ typedef std::variant<
     DrvHash,
     // Fixed-output derivation hashes
     CaOutputHashes
-> DrvHashModuloRaw;
+> _DrvHashModuloRaw;
 
-struct DrvHashModulo : DrvHashModuloRaw {
-    using Raw = DrvHashModuloRaw;
+struct DrvHashModulo : _DrvHashModuloRaw {
+    using Raw = _DrvHashModuloRaw;
     using Raw::Raw;
 
     /* Get hash, throwing if it is per-output CA hashes or a
diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc
index 0183bda35..319b1c790 100644
--- a/src/libstore/derived-path.cc
+++ b/src/libstore/derived-path.cc
@@ -1,4 +1,5 @@
 #include "derived-path.hh"
+#include "derivations.hh"
 #include "store-api.hh"
 
 #include <nlohmann/json.hpp>
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 9230be15a..75726a1ab 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -698,7 +698,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat
     std::optional<Hash> h;
     for (auto & i : drv.outputs) {
         std::visit(overloaded {
-            [&](const DerivationOutputInputAddressed & doia) {
+            [&](const DerivationOutput::InputAddressed & doia) {
                 if (!h) {
                     // somewhat expensive so we do lazily
                     auto h0 = hashDerivationModulo(*this, drv, true);
@@ -710,16 +710,16 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat
                         printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed));
                 envHasRightPath(doia.path, i.first);
             },
-            [&](const DerivationOutputCAFixed & dof) {
+            [&](const DerivationOutput::CAFixed & dof) {
                 StorePath path = makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName);
                 envHasRightPath(path, i.first);
             },
-            [&](const DerivationOutputCAFloating &) {
+            [&](const DerivationOutput::CAFloating &) {
                 /* Nothing to check */
             },
-            [&](const DerivationOutputDeferred &) {
+            [&](const DerivationOutput::Deferred &) {
             },
-        }, i.second.output);
+        }, i.second.raw());
     }
 }
 
diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc
index 6409874ff..1f0bae7fe 100644
--- a/src/libstore/misc.cc
+++ b/src/libstore/misc.cc
@@ -87,7 +87,7 @@ std::optional<ContentAddress> getDerivationCA(const BasicDerivation & drv)
 {
     auto out = drv.outputs.find("out");
     if (out != drv.outputs.end()) {
-        if (auto v = std::get_if<DerivationOutputCAFixed>(&out->second.output))
+        if (const auto * v = std::get_if<DerivationOutput::CAFixed>(&out->second.raw()))
             return v->hash;
     }
     return std::nullopt;
diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh
index effcf099d..95bec21e8 100644
--- a/src/libstore/parsed-derivations.hh
+++ b/src/libstore/parsed-derivations.hh
@@ -1,5 +1,6 @@
 #pragma once
 
+#include "derivations.hh"
 #include "store-api.hh"
 
 #include <nlohmann/json_fwd.hpp>
diff --git a/src/libstore/repair-flag.hh b/src/libstore/repair-flag.hh
new file mode 100644
index 000000000..a13cda312
--- /dev/null
+++ b/src/libstore/repair-flag.hh
@@ -0,0 +1,7 @@
+#pragma once
+
+namespace nix {
+
+enum RepairFlag : bool { NoRepair = false, Repair = true };
+
+}
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 86fa6a211..59937be4d 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1,6 +1,7 @@
 #include "crypto.hh"
 #include "fs-accessor.hh"
 #include "globals.hh"
+#include "derivations.hh"
 #include "store-api.hh"
 #include "util.hh"
 #include "nar-info-disk-cache.hh"
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 635a82a2a..0c8a4db56 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -10,8 +10,8 @@
 #include "sync.hh"
 #include "globals.hh"
 #include "config.hh"
-#include "derivations.hh"
 #include "path-info.hh"
+#include "repair-flag.hh"
 
 #include <atomic>
 #include <limits>
@@ -62,6 +62,8 @@ MakeError(BadStorePath, Error);
 
 MakeError(InvalidStoreURI, Error);
 
+struct BasicDerivation;
+struct Derivation;
 class FSAccessor;
 class NarInfoDiskCache;
 class Store;