Enfore the use of properly built paths in libcmd

Replace `DerivedPathWithHints` by a new `BuiltPath` type that serves as
a proof that the corresponding path has been built.

4 files changed, 108 insertions, 102 deletions

diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc
index 25e4873e8..569c4b9e4 100644
--- a/src/libcmd/command.cc
+++ b/src/libcmd/command.cc
@@ -54,7 +54,7 @@ void StoreCommand::run()
     run(getStore());
 }
 
-RealisedPathsCommand::RealisedPathsCommand(bool recursive)
+BuiltPathsCommand::BuiltPathsCommand(bool recursive)
     : recursive(recursive)
 {
     if (recursive)
@@ -81,39 +81,45 @@ RealisedPathsCommand::RealisedPathsCommand(bool recursive)
     });
 }
 
-void RealisedPathsCommand::run(ref<Store> store)
+void BuiltPathsCommand::run(ref<Store> store)
 {
-    std::vector<RealisedPath> paths;
+    BuiltPaths paths;
     if (all) {
         if (installables.size())
             throw UsageError("'--all' does not expect arguments");
         // XXX: Only uses opaque paths, ignores all the realisations
         for (auto & p : store->queryAllValidPaths())
-            paths.push_back(p);
+            paths.push_back(BuiltPath::Opaque{p});
     } else {
-        auto pathSet = toRealisedPaths(store, realiseMode, operateOn, installables);
+        paths = toBuiltPaths(store, realiseMode, operateOn, installables);
         if (recursive) {
-            auto roots = std::move(pathSet);
-            pathSet = {};
-            RealisedPath::closure(*store, roots, pathSet);
+            // XXX: This only computes the store path closure, ignoring
+            // intermediate realisations
+            StorePathSet pathsRoots, pathsClosure;
+            for (auto & root: paths) {
+                auto rootFromThis = root.outPaths();
+                pathsRoots.insert(rootFromThis.begin(), rootFromThis.end());
+            }
+            store->computeFSClosure(pathsRoots, pathsClosure);
+            for (auto & path : pathsClosure)
+                paths.push_back(BuiltPath::Opaque{path});
         }
-        for (auto & path : pathSet)
-            paths.push_back(path);
     }
 
     run(store, std::move(paths));
 }
 
 StorePathsCommand::StorePathsCommand(bool recursive)
-    : RealisedPathsCommand(recursive)
+    : BuiltPathsCommand(recursive)
 {
 }
 
-void StorePathsCommand::run(ref<Store> store, std::vector<RealisedPath> paths)
+void StorePathsCommand::run(ref<Store> store, BuiltPaths paths)
 {
     StorePaths storePaths;
-    for (auto & p : paths)
-        storePaths.push_back(p.path());
+    for (auto& builtPath : paths)
+        for (auto& p : builtPath.outPaths())
+            storePaths.push_back(p);
 
     run(store, std::move(storePaths));
 }
@@ -175,10 +181,7 @@ void MixProfile::updateProfile(const BuiltPaths & buildables)
             },
             [&](BuiltPath::Built bfd) {
                 for (auto & output : bfd.outputs) {
-                    /* Output path should be known because we just tried to
-                       build it. */
-                    assert(output.second);
-                    result.push_back(*output.second);
+                    result.push_back(output.second);
                 }
             },
         }, buildable.raw());
diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh
index 952279f7b..35b3a384b 100644
--- a/src/libcmd/command.hh
+++ b/src/libcmd/command.hh
@@ -143,7 +143,7 @@ private:
 };
 
 /* A command that operates on zero or more store paths. */
-struct RealisedPathsCommand : public InstallablesCommand
+struct BuiltPathsCommand : public InstallablesCommand
 {
 private:
 
@@ -156,26 +156,26 @@ protected:
 
 public:
 
-    RealisedPathsCommand(bool recursive = false);
+    BuiltPathsCommand(bool recursive = false);
 
     using StoreCommand::run;
 
-    virtual void run(ref<Store> store, std::vector<RealisedPath> paths) = 0;
+    virtual void run(ref<Store> store, BuiltPaths paths) = 0;
 
     void run(ref<Store> store) override;
 
     bool useDefaultInstallables() override { return !all; }
 };
 
-struct StorePathsCommand : public RealisedPathsCommand
+struct StorePathsCommand : public BuiltPathsCommand
 {
     StorePathsCommand(bool recursive = false);
 
-    using RealisedPathsCommand::run;
+    using BuiltPathsCommand::run;
 
     virtual void run(ref<Store> store, std::vector<StorePath> storePaths) = 0;
 
-    void run(ref<Store> store, std::vector<RealisedPath> paths) override;
+    void run(ref<Store> store, BuiltPaths paths) override;
 };
 
 /* A command that operates on exactly one store path. */
@@ -231,7 +231,7 @@ std::set<StorePath> toDerivations(ref<Store> store,
     std::vector<std::shared_ptr<Installable>> installables,
     bool useDeriver = false);
 
-std::set<RealisedPath> toRealisedPaths(
+BuiltPaths toBuiltPaths(
     ref<Store> store,
     Realise mode,
     OperateOn operateOn,
diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc
index 36d7ecc39..fe52912cf 100644
--- a/src/libcmd/installables.cc
+++ b/src/libcmd/installables.cc
@@ -285,9 +285,9 @@ void completeFlakeRef(ref<Store> store, std::string_view prefix)
     }
 }
 
-BuiltPath Installable::toBuiltPath()
+DerivedPath Installable::toDerivedPath()
 {
-    auto buildables = toBuiltPaths();
+    auto buildables = toDerivedPaths();
     if (buildables.size() != 1)
         throw Error("installable '%s' evaluates to %d derivations, where only one is expected", what(), buildables.size());
     return std::move(buildables[0]);
@@ -321,22 +321,19 @@ struct InstallableStorePath : Installable
 
     std::string what() override { return store->printStorePath(storePath); }
 
-    BuiltPaths toBuiltPaths() override
+    DerivedPaths toDerivedPaths() override
     {
         if (storePath.isDerivation()) {
-            std::map<std::string, std::optional<StorePath>> outputs;
             auto drv = store->readDerivation(storePath);
-            for (auto & [name, output] : drv.outputsAndOptPaths(*store))
-                outputs.emplace(name, output.second);
             return {
-                BuiltPath::Built {
+                DerivedPath::Built {
                     .drvPath = storePath,
-                    .outputs = std::move(outputs)
+                    .outputs = drv.outputNames(),
                 }
             };
         } else {
             return {
-                BuiltPath::Opaque {
+                DerivedPath::Opaque {
                     .path = storePath,
                 }
             };
@@ -349,22 +346,22 @@ struct InstallableStorePath : Installable
     }
 };
 
-BuiltPaths InstallableValue::toBuiltPaths()
+DerivedPaths InstallableValue::toDerivedPaths()
 {
-    BuiltPaths res;
+    DerivedPaths res;
 
-    std::map<StorePath, std::map<std::string, std::optional<StorePath>>> drvsToOutputs;
+    std::map<StorePath, std::set<std::string>> drvsToOutputs;
 
     // Group by derivation, helps with .all in particular
     for (auto & drv : toDerivations()) {
         auto outputName = drv.outputName;
         if (outputName == "")
             throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(drv.drvPath));
-        drvsToOutputs[drv.drvPath].insert_or_assign(outputName, drv.outPath);
+        drvsToOutputs[drv.drvPath].insert(outputName);
     }
 
     for (auto & i : drvsToOutputs)
-        res.push_back(BuiltPath::Built { i.first, i.second });
+        res.push_back(DerivedPath::Built { i.first, i.second });
 
     return res;
 }
@@ -675,32 +672,67 @@ std::shared_ptr<Installable> SourceExprCommand::parseInstallable(
     return installables.front();
 }
 
+BuiltPaths getBuiltPaths(ref<Store> store, DerivedPaths hopefullyBuiltPaths)
+{
+    BuiltPaths res;
+    for (auto& b : hopefullyBuiltPaths)
+        std::visit(
+            overloaded{
+                [&](DerivedPath::Opaque bo) {
+                    res.push_back(BuiltPath::Opaque{bo.path});
+                },
+                [&](DerivedPath::Built bfd) {
+                    OutputPathMap outputs;
+                    auto drv = store->readDerivation(bfd.drvPath);
+                    auto outputHashes = staticOutputHashes(*store, drv);
+                    auto drvOutputs = drv.outputsAndOptPaths(*store);
+                    for (auto& output : bfd.outputs) {
+                        if (!outputHashes.count(output))
+                            throw Error(
+                                "the derivation '%s' doesn't have an output "
+                                "named '%s'",
+                                store->printStorePath(bfd.drvPath), output);
+                        if (settings.isExperimentalFeatureEnabled(
+                                "ca-derivations")) {
+                            auto outputId =
+                                DrvOutput{outputHashes.at(output), output};
+                            auto realisation =
+                                store->queryRealisation(outputId);
+                            if (!realisation)
+                                throw Error(
+                                    "cannot operate on an output of unbuilt "
+                                    "content-addresed derivation '%s'",
+                                    outputId.to_string());
+                            outputs.insert_or_assign(
+                                output, realisation->outPath);
+                        } else {
+                            // If ca-derivations isn't enabled, assume that
+                            // the output path is statically known.
+                            assert(drvOutputs.count(output));
+                            assert(drvOutputs.at(output).second);
+                            outputs.insert_or_assign(
+                                output, *drvOutputs.at(output).second);
+                        }
+                    }
+                    res.push_back(BuiltPath::Built{bfd.drvPath, outputs});
+                },
+            },
+            b.raw());
+
+    return res;
+}
+
 BuiltPaths build(ref<Store> store, Realise mode,
     std::vector<std::shared_ptr<Installable>> installables, BuildMode bMode)
 {
     if (mode == Realise::Nothing)
         settings.readOnlyMode = true;
 
-    BuiltPaths buildables;
-
     std::vector<DerivedPath> pathsToBuild;
 
     for (auto & i : installables) {
-        for (auto & b : i->toBuiltPaths()) {
-            std::visit(overloaded {
-                [&](BuiltPath::Opaque bo) {
-                    pathsToBuild.push_back(bo);
-                },
-                [&](BuiltPath::Built bfd) {
-                    StringSet outputNames;
-                    for (auto & output : bfd.outputs)
-                        outputNames.insert(output.first);
-                    pathsToBuild.push_back(
-                        DerivedPath::Built{bfd.drvPath, outputNames});
-                },
-            }, b.raw());
-            buildables.push_back(std::move(b));
-        }
+        auto b = i->toDerivedPaths();
+        pathsToBuild.insert(pathsToBuild.end(), b.begin(), b.end());
     }
 
     if (mode == Realise::Nothing)
@@ -708,57 +740,26 @@ BuiltPaths build(ref<Store> store, Realise mode,
     else if (mode == Realise::Outputs)
         store->buildPaths(pathsToBuild, bMode);
 
-    return buildables;
+    return getBuiltPaths(store, pathsToBuild);
 }
 
-std::set<RealisedPath> toRealisedPaths(
+BuiltPaths toBuiltPaths(
     ref<Store> store,
     Realise mode,
     OperateOn operateOn,
     std::vector<std::shared_ptr<Installable>> installables)
 {
-    std::set<RealisedPath> res;
     if (operateOn == OperateOn::Output) {
-        for (auto & b : build(store, mode, installables))
-            std::visit(overloaded {
-                [&](BuiltPath::Opaque bo) {
-                    res.insert(bo.path);
-                },
-                [&](BuiltPath::Built bfd) {
-                    auto drv = store->readDerivation(bfd.drvPath);
-                    auto outputHashes = staticOutputHashes(*store, drv);
-                    for (auto & output : bfd.outputs) {
-                        if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
-                            if (!outputHashes.count(output.first))
-                                throw Error(
-                                    "the derivation '%s' doesn't have an output named '%s'",
-                                    store->printStorePath(bfd.drvPath),
-                                    output.first);
-                            auto outputId = DrvOutput{outputHashes.at(output.first), output.first};
-                            auto realisation = store->queryRealisation(outputId);
-                            if (!realisation)
-                                throw Error("cannot operate on an output of unbuilt content-addresed derivation '%s'", outputId.to_string());
-                            res.insert(RealisedPath{*realisation});
-                        }
-                        else {
-                            // If ca-derivations isn't enabled, behave as if
-                            // all the paths are opaque to keep the default
-                            // behavior
-                            assert(output.second);
-                            res.insert(*output.second);
-                        }
-                    }
-                },
-            }, b.raw());
+        return build(store, mode, installables);
     } else {
         if (mode == Realise::Nothing)
             settings.readOnlyMode = true;
 
-        auto drvPaths = toDerivations(store, installables, true);
-        res.insert(drvPaths.begin(), drvPaths.end());
+        BuiltPaths res;
+        for (auto & drvPath : toDerivations(store, installables, true))
+            res.push_back(BuiltPath::Opaque{drvPath});
+        return res;
     }
-
-    return res;
 }
 
 StorePathSet toStorePaths(ref<Store> store,
@@ -766,8 +767,10 @@ StorePathSet toStorePaths(ref<Store> store,
     std::vector<std::shared_ptr<Installable>> installables)
 {
     StorePathSet outPaths;
-    for (auto & path : toRealisedPaths(store, mode, operateOn, installables))
-            outPaths.insert(path.path());
+    for (auto & path : toBuiltPaths(store, mode, operateOn, installables)) {
+        auto thisOutPaths = path.outPaths();
+        outPaths.insert(thisOutPaths.begin(), thisOutPaths.end());
+    }
     return outPaths;
 }
 
@@ -789,9 +792,9 @@ StorePathSet toDerivations(ref<Store> store,
     StorePathSet drvPaths;
 
     for (auto & i : installables)
-        for (auto & b : i->toBuiltPaths())
+        for (auto & b : i->toDerivedPaths())
             std::visit(overloaded {
-                [&](BuiltPath::Opaque bo) {
+                [&](DerivedPath::Opaque bo) {
                     if (!useDeriver)
                         throw Error("argument '%s' did not evaluate to a derivation", i->what());
                     auto derivers = store->queryValidDerivers(bo.path);
@@ -800,7 +803,7 @@ StorePathSet toDerivations(ref<Store> store,
                     // FIXME: use all derivers?
                     drvPaths.insert(*derivers.begin());
                 },
-                [&](BuiltPath::Built bfd) {
+                [&](DerivedPath::Built bfd) {
                     drvPaths.insert(bfd.drvPath);
                 },
             }, b.raw());
diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh
index 7d79efaad..16bc4ae68 100644
--- a/src/libcmd/installables.hh
+++ b/src/libcmd/installables.hh
@@ -29,9 +29,9 @@ struct Installable
 
     virtual std::string what() = 0;
 
-    virtual BuiltPaths toBuiltPaths() = 0;
+    virtual DerivedPaths toDerivedPaths() = 0;
 
-    BuiltPath toBuiltPath();
+    DerivedPath toDerivedPath();
 
     App toApp(EvalState & state);
 
@@ -74,7 +74,7 @@ struct InstallableValue : Installable
 
     virtual std::vector<DerivationInfo> toDerivations() = 0;
 
-    BuiltPaths toBuiltPaths() override;
+    DerivedPaths toDerivedPaths() override;
 };
 
 struct InstallableFlake : InstallableValue