libstore: don't return optionals from Derivation::path

output paths are always known now that CA and deferred outputs are gone.

Change-Id: I359d13ffb5141f1e07a5fc55425831af3332c22e

lix/legacy/build-remote.cc

@@ -347,11 +347,10 @@ connected:
 
 
         StorePathSet missingPaths;
-        auto outputPaths = drv.outputsAndOptPaths(*store);
-        for (auto & [outputName, hopefullyOutputPath] : outputPaths) {
-            assert(hopefullyOutputPath.second);
-            if (!aio.blockOn(store->isValidPath(*hopefullyOutputPath.second)))
-                missingPaths.insert(*hopefullyOutputPath.second);
+        auto outputPaths = drv.outputsAndPaths(*store);
+        for (auto & [outputName, outputPath] : outputPaths) {
+            if (!aio.blockOn(store->isValidPath(outputPath.second)))
+                missingPaths.insert(outputPath.second);
         }
 
         if (!missingPaths.empty()) {

lix/legacy/nix-store.cc

@@ -254,10 +254,8 @@ try {
         StorePathSet outputs;
         if (forceRealise)
             co_return TRY_AWAIT(store->queryDerivationOutputs(storePath));
-        for (auto & i : drv.outputsAndOptPaths(*store)) {
-            if (!i.second.second)
-                throw UsageError("Cannot use output path of floating content-addressed derivation until we know what it is (e.g. by building it)");
-            outputs.insert(*i.second.second);
+        for (auto & i : drv.outputsAndPaths(*store)) {
+            outputs.insert(i.second.second);
         }
         co_return outputs;
     }

lix/libexpr/eval.cc

@@ -925,8 +925,8 @@ std::string EvalState::mkSingleDerivedPathStringRaw(
             auto i = drv.outputs.find(b.output);
             if (i == drv.outputs.end())
                 throw Error("derivation '%s' does not have output '%s'", b.drvPath.to_string(*ctx.store), b.output);
-            auto optStaticOutputPath = i->second.path(*ctx.store, drv.name, b.output);
-            return mkOutputStringRaw(b, optStaticOutputPath);
+            auto staticOutputPath = i->second.path(*ctx.store, drv.name, b.output);
+            return mkOutputStringRaw(b, staticOutputPath);
         }
     }, p.raw());
 }

lix/libstore/build/derivation-goal.cc

@@ -219,9 +219,8 @@ try {
 
     parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv);
 
-    for (auto & i : drv->outputsAndOptPaths(worker.store))
-        if (i.second.second)
-            TRY_AWAIT(worker.store.addTempRoot(*i.second.second));
+    for (auto & i : drv->outputsAndPaths(worker.store))
+        TRY_AWAIT(worker.store.addTempRoot(i.second.second));
 
     auto outputHashes = TRY_AWAIT(staticOutputHashes(worker.evalStore, *drv));
     for (auto & [outputName, outputHash] : outputHashes)
@@ -624,8 +623,6 @@ retry:
        other goal can start a build, and if not, the main loop will sleep a few
        seconds and then retry this goal. */
     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. */
@@ -634,13 +631,8 @@ retry:
         /* 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));
-            else
-                lockFiles.insert(
-                    worker.store.Store::toRealPath(drvPath) + "." + i.first
-                );
+        for (auto & i : drv->outputsAndPaths(worker.store)) {
+            lockFiles.insert(worker.store.Store::toRealPath(i.second.second));
         }
     }
 
@@ -1438,8 +1430,8 @@ try {
 kj::Promise<Result<OutputPathMap>> DerivationGoal::queryDerivationOutputMap()
 try {
     OutputPathMap res;
-    for (auto & [name, output] : drv->outputsAndOptPaths(worker.store))
-        res.insert_or_assign(name, *output.second);
+    for (auto & [name, output] : drv->outputsAndPaths(worker.store))
+        res.insert_or_assign(name, output.second);
     co_return res;
 } catch (...) {
     co_return result::current_exception();

lix/libstore/build/local-derivation-goal.cc

@@ -1893,10 +1893,8 @@ try {
            floating CA derivations and hash-mismatching fixed-output
            derivations. */
         std::optional<PathLock> dynamicOutputLock;
-        auto optFixedPath = output->path(worker.store, drv->name, outputName);
-        if (!optFixedPath ||
-            worker.store.printStorePath(*optFixedPath) != finalDestPath)
-        {
+        auto fixedPath = output->path(worker.store, drv->name, outputName);
+        if (worker.store.printStorePath(fixedPath) != finalDestPath) {
             assert(newInfo.ca);
             dynamicOutputLock = TRY_AWAIT(lockPathAsync(worker.store.toRealPath(finalDestPath)));
         }

lix/libstore/derivations.cc

@@ -14,16 +14,14 @@
 
 namespace nix {
 
-std::optional<StorePath> DerivationOutput::path(const Store & store, std::string_view drvName, OutputNameView outputName) const
+StorePath DerivationOutput::path(const Store & store, std::string_view drvName, OutputNameView outputName) const
 {
     return std::visit(overloaded {
-        [](const DerivationOutput::InputAddressed & doi) -> std::optional<StorePath> {
-            return { doi.path };
+        [](const DerivationOutput::InputAddressed & doi) -> StorePath {
+            return doi.path;
         },
-        [&](const DerivationOutput::CAFixed & dof) -> std::optional<StorePath> {
-            return {
-                dof.path(store, drvName, outputName)
-            };
+        [&](const DerivationOutput::CAFixed & dof) -> StorePath {
+            return dof.path(store, drvName, outputName);
         },
     }, raw);
 }
@@ -645,16 +643,16 @@ StringSet BasicDerivation::outputNames() const
     return names;
 }
 
-DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const
+DerivationOutputsAndPaths BasicDerivation::outputsAndPaths(const Store & store) const
 {
-    DerivationOutputsAndOptPaths outsAndOptPaths;
+    DerivationOutputsAndPaths outsAndPaths;
     for (auto & [outputName, output] : outputs)
-        outsAndOptPaths.insert(std::make_pair(
+        outsAndPaths.insert(std::make_pair(
             outputName,
             std::make_pair(output, output.path(store, name, outputName))
             )
         );
-    return outsAndOptPaths;
+    return outsAndPaths;
 }
 
 std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath)

lix/libstore/derivations.hh

@@ -83,9 +83,9 @@ struct DerivationOutput
      * \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
+     * BasicDerivation::outputsAndPaths
      */
-    std::optional<StorePath> path(const Store & store, std::string_view drvName, OutputNameView outputName) const;
+    StorePath path(const Store & store, std::string_view drvName, OutputNameView outputName) const;
 
     JSON toJSON(
         const Store & store,
@@ -106,12 +106,12 @@ typedef std::map<std::string, DerivationOutput> DerivationOutputs;
 
 /**
  * These are analogues to the previous DerivationOutputs data type,
- * but they also contains, for each output, the (optional) store
+ * but they also contains, for each output, the store
  * path in which it would be written. To calculate values of these
  * types, see the corresponding functions in BasicDerivation.
  */
-typedef std::map<std::string, std::pair<DerivationOutput, std::optional<StorePath>>>
-  DerivationOutputsAndOptPaths;
+typedef std::map<std::string, std::pair<DerivationOutput, StorePath>>
+  DerivationOutputsAndPaths;
 
 /**
  * For inputs that are sub-derivations, we specify exactly which
@@ -207,7 +207,7 @@ struct BasicDerivation
      * augmented with knowledge of the Store paths they would be written
      * into.
      */
-    DerivationOutputsAndOptPaths outputsAndOptPaths(const Store & store) const;
+    DerivationOutputsAndPaths outputsAndPaths(const Store & store) const;
 
     static std::string_view nameFromPath(const StorePath & storePath);
 

lix/libstore/local-store.cc

@@ -813,11 +813,8 @@ try {
            registration above is undone. */
         if (checkOutputs) TRY_AWAIT(drv.checkInvariants(*this, info.path));
 
-        for (auto & i : drv.outputsAndOptPaths(*this)) {
-            /* Floating CA derivations have indeterminate output paths until
-               they are built, so don't register anything in that case */
-            if (i.second.second)
-                cacheDrvOutputMapping(state, id, i.first, *i.second.second);
+        for (auto & i : drv.outputsAndPaths(*this)) {
+            cacheDrvOutputMapping(state, id, i.first, i.second.second);
         }
     }
 

lix/libstore/platform/linux.cc

@@ -816,14 +816,8 @@ void LinuxLocalDerivationGoal::prepareSandbox()
        rebuilding a path that is in settings.sandbox-paths
        (typically the dependencies of /bin/sh).  Throw them
        out. */
-    for (auto & i : drv->outputsAndOptPaths(worker.store)) {
-        /* If the name isn't known a priori (i.e. floating
-           content-addressed derivation), the temporary location we use
-           should be fresh.  Freshness means it is impossible that the path
-           is already in the sandbox, so we don't need to worry about
-           removing it.  */
-        if (i.second.second)
-            pathsInChroot.erase(worker.store.printStorePath(*i.second.second));
+    for (auto & i : drv->outputsAndPaths(worker.store)) {
+        pathsInChroot.erase(worker.store.printStorePath(i.second.second));
     }
 
     if (cgroup) {

lix/libstore/store-api.cc

@@ -513,7 +513,7 @@ Store::queryStaticPartialDerivationOutputMap(const StorePath & path)
 try {
     std::map<std::string, std::optional<StorePath>> outputs;
     auto drv = TRY_AWAIT(readInvalidDerivation(path));
-    for (auto & [outputName, output] : drv.outputsAndOptPaths(*this)) {
+    for (auto & [outputName, output] : drv.outputsAndPaths(*this)) {
         outputs.emplace(outputName, output.second);
     }
     co_return outputs;
@@ -928,14 +928,8 @@ try {
     for (auto & j : paths2) {
         if (j.isDerivation()) {
             Derivation drv = TRY_AWAIT(derivationFromPath(j));
-            for (auto & k : drv.outputsAndOptPaths(*this)) {
-                if (!k.second.second)
-                    /* FIXME: I am confused why we are calling
-                       `computeFSClosure` on the output path, rather than
-                       derivation itself. That doesn't seem right to me, so I
-                       won't try to implemented this for CA derivations. */
-                    throw UnimplementedError("exportReferences on CA derivations is not yet implemented");
-                TRY_AWAIT(computeFSClosure(*k.second.second, paths));
+            for (auto & k : drv.outputsAndPaths(*this)) {
+                TRY_AWAIT(computeFSClosure(k.second.second, paths));
             }
         }
     }

perl/lib/Nix/Store.xs

@@ -328,12 +328,10 @@ SV * derivationFromPath(char * drvPath)
             hash = newHV();
 
             HV * outputs = newHV();
-            for (auto & i : drv.outputsAndOptPaths(*store())) {
+            for (auto & i : drv.outputsAndPaths(*store())) {
                 hv_store(
                     outputs, i.first.c_str(), i.first.size(),
-                    !i.second.second
-                        ? newSV(0) /* null value */
-                        : newSVpv(store()->printStorePath(*i.second.second).c_str(), 0),
+                    newSVpv(store()->printStorePath(i.second.second).c_str(), 0),
                     0);
             }
             hv_stores(hash, "outputs", newRV((SV *) outputs));