Fix some of the issues raised by @edolstra

- More and better comments

 - The easier renames

src/libstore/build.cc

@@ -718,7 +718,7 @@ typedef enum {rpAccept, rpDecline, rpPostpone} HookReply;
 
 class SubstitutionGoal;
 
-struct KnownInitialOutputStatus {
+struct InitialOutputStatus {
     StorePath path;
     /* The output optional indicates whether it's already valid; i.e. exists
        and is registered. If we're repairing, inner bool indicates whether the
@@ -731,9 +731,9 @@ struct KnownInitialOutputStatus {
     }
 };
 
-struct InitialOutputStatus {
+struct InitialOutput {
     bool wanted;
-    std::optional<KnownInitialOutputStatus> known;
+    std::optional<InitialOutputStatus> known;
 };
 
 class DerivationGoal : public Goal
@@ -770,7 +770,7 @@ private:
        immediate input paths). */
     StorePathSet inputPaths;
 
-    std::map<std::string, InitialOutputStatus> initialOutputs;
+    std::map<std::string, InitialOutput> initialOutputs;
 
     /* User selected for running the builder. */
     std::unique_ptr<UserLock> buildUser;
@@ -1050,11 +1050,14 @@ private:
     /* Forcibly kill the child process, if any. */
     void killChild();
 
-    /* Map a path to another (reproducably) so we can avoid overwriting outputs
+    /* Create alternative path calculated from but distinct from the
+       input, so we can avoid overwriting outputs (or other store paths)
        that already exist. */
     StorePath makeFallbackPath(const StorePath & path);
-    /* Make a path to another based on the output name alone, if one doesn't
-       want to use a random path for CA builds. */
+    /* Make a path to another based on the output name along with the
+       derivation hash. */
+    /* FIXME add option to randomize, so we can audit whether our
+       rewrites caught everything */
     StorePath makeFallbackPath(std::string_view outputName);
 
     void repairClosure();
@@ -2141,8 +2144,6 @@ void DerivationGoal::startBuilder()
            with the actual hashes. */
         auto scratchPath =
             !status.known
-                /* FIXME add option to randomize, so we can audit whether our
-                 * rewrites caught everything */
                 ? makeFallbackPath(outputName)
             : !needsHashRewrite()
                 /* Can always use original path in sandbox */
@@ -4582,19 +4583,19 @@ void DerivationGoal::checkPathValidity()
 {
     bool checkHash = buildMode == bmRepair;
     for (auto & i : queryPartialDerivationOutputMap()) {
-        InitialOutputStatus status {
+        InitialOutput info {
             .wanted = wantOutput(i.first, wantedOutputs),
         };
         if (i.second) {
             auto outputPath = *i.second;
-            status.known = {
+            info.known = {
                 .path = outputPath,
                 .valid = !worker.store.isValidPath(outputPath)
                     ? std::optional<bool> {}
                     : !checkHash || worker.pathContentsGood(outputPath),
             };
         }
-        initialOutputs.insert_or_assign(i.first, status);
+        initialOutputs.insert_or_assign(i.first, info);
     }
 }
 

src/libstore/derivations.hh

@@ -145,6 +145,11 @@ Derivation parseDerivation(const Store & store, std::string && s, std::string_vi
 // FIXME: remove
 bool isDerivation(const string & fileName);
 
+/* Calculate the name that will be used for the store path for this
+   output.
+
+   This is usually <drv-name>-<output-name>, but is just <drv-name> when
+   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
@@ -194,8 +199,21 @@ struct Sink;
 Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, std::string_view name);
 void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv);
 
+/* This creates an opaque and almost certainly unique string
+   deterministically from the output name.
+
+   It is used as a placeholder to allow derivations to refer to their
+   own outputs without needing to use the hash of a derivation in
+   itself, making the hash near-impossible to calculate. */
 std::string hashPlaceholder(const std::string & outputName);
 
+/* This creates an opaque and almost certainly unique string
+   deterministically from a derivation path and output name.
+
+   It is used as a placeholder to allow derivations to refer to
+   content-addressed paths whose content --- and thus the path
+   themselves --- isn't yet known. This occurs when a derivation has a
+   dependency which is a CA derivation. */
 std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName);
 
 }

src/libstore/local-store.hh

@@ -279,9 +279,8 @@ private:
        specified by the ‘secret-key-files’ option. */
     void signPathInfo(ValidPathInfo & info);
 
-    /* Add a mapping from the deriver of the path info (if specified) to its
-     * out path
-     */
+    /* Register the store path 'output' as the output named 'outputName' of
+       derivation 'deriver'. */
     void linkDeriverToPath(const StorePath & deriver, const string & outputName, const StorePath & output);
     void linkDeriverToPath(State & state, uint64_t deriver, const string & outputName, const StorePath & output);
 

src/nix/command.cc

@@ -150,8 +150,9 @@ void MixProfile::updateProfile(const Buildables & buildables)
             },
             [&](BuildableFromDrv bfd) {
                 for (auto & output : bfd.outputs) {
-                    if (!output.second)
-                        throw Error("output path should be known because we just tried to build it");
+                    /* Output path should be known because we just tried to
+                       build it. */
+                    assert(!output.second);
                     result.push_back(*output.second);
                 }
             },