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.

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);
+            // XXX: This only computes the store path closure, ignoring
-            pathSet = {};
+            // intermediate realisations
-            RealisedPath::closure(*store, roots, pathSet);
+            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)
+    for (auto& builtPath : paths)
-        storePaths.push_back(p.path());
+        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
+                    result.push_back(output.second);
-                       build it. */
-                    assert(output.second);
-                    result.push_back(*output.second);
                 }
             },
         }, buildable.raw());

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,

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()) {
+        auto b = i->toDerivedPaths();
-            std::visit(overloaded {
+        pathsToBuild.insert(pathsToBuild.end(), b.begin(), b.end());
-                [&](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));
-        }
     }
 
     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))
+        return 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());
     } else {
         if (mode == Realise::Nothing)
             settings.readOnlyMode = true;
 
-        auto drvPaths = toDerivations(store, installables, true);
+        BuiltPaths res;
-        res.insert(drvPaths.begin(), drvPaths.end());
+        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))
+    for (auto & path : toBuiltPaths(store, mode, operateOn, installables)) {
-            outPaths.insert(path.path());
+        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());

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

src/libstore/derived-path.cc

@@ -15,11 +15,26 @@ nlohmann::json BuiltPath::Built::toJSON(ref<Store> store) const {
     nlohmann::json res;
     res["drvPath"] = store->printStorePath(drvPath);
     for (const auto& [output, path] : outputs) {
-        res["outputs"][output] = path ? store->printStorePath(*path) : "";
+        res["outputs"][output] = store->printStorePath(path);
     }
     return res;
 }
 
+StorePathSet BuiltPath::outPaths() const
+{
+    return std::visit(
+        overloaded{
+            [](BuiltPath::Opaque p) { return StorePathSet{p.path}; },
+            [](BuiltPath::Built b) {
+                StorePathSet res;
+                for (auto & [_, path] : b.outputs)
+                    res.insert(path);
+                return res;
+            },
+        }, raw()
+    );
+}
+
 nlohmann::json derivedPathsWithHintsToJSON(const BuiltPaths & buildables, ref<Store> store) {
     auto res = nlohmann::json::array();
     for (const BuiltPath & buildable : buildables) {
@@ -74,4 +89,30 @@ DerivedPath DerivedPath::parse(const Store & store, std::string_view s)
         : (DerivedPath) DerivedPath::Built::parse(store, s);
 }
 
+RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const
+{
+    RealisedPath::Set res;
+    std::visit(
+        overloaded{
+            [&](BuiltPath::Opaque p) { res.insert(p.path); },
+            [&](BuiltPath::Built p) {
+                auto drvHashes =
+                    staticOutputHashes(store, store.readDerivation(p.drvPath));
+                for (auto& [outputName, outputPath] : p.outputs) {
+                    if (settings.isExperimentalFeatureEnabled(
+                            "ca-derivations")) {
+                        auto thisRealisation = store.queryRealisation(
+                            DrvOutput{drvHashes.at(outputName), outputName});
+                        assert(thisRealisation);  // We’ve built it, so we must h
+                                                  // ve the realisation
+                        res.insert(*thisRealisation);
+                    } else {
+                        res.insert(outputPath);
+                    }
+                }
+            },
+        },
+        raw());
+    return res;
+}
 }

src/libstore/derived-path.hh

@@ -2,6 +2,7 @@
 
 #include "util.hh"
 #include "path.hh"
+#include "realisation.hh"
 
 #include <optional>
 
@@ -83,7 +84,7 @@ struct DerivedPath : _DerivedPathRaw {
  */
 struct BuiltPathBuilt {
     StorePath drvPath;
-    std::map<std::string, std::optional<StorePath>> outputs;
+    std::map<std::string, StorePath> outputs;
 
     nlohmann::json toJSON(ref<Store> store) const;
     static BuiltPathBuilt parse(const Store & store, std::string_view);
@@ -95,20 +96,9 @@ using _BuiltPathRaw = std::variant<
 >;
 
 /**
- * A derived path with hints in the form of optional concrete output paths in the built case.
+ * A built path. Similar to a `DerivedPath`, but enriched with the corresponding
- *
+ * output path(s).
- * This type is currently just used by the CLI. The paths are filled in
- * during evaluation for derivations that know what paths they will
- * produce in advanced, i.e. input-addressed or fixed-output content
- * addressed derivations.
- *
- * That isn't very good, because it puts floating content-addressed
- * derivations "at a disadvantage". It would be better to never rely on
- * the output path of unbuilt derivations, and exclusively use the
- * realizations types to work with built derivations' concrete output
- * paths.
  */
-// FIXME Stop using and delete this, or if that is not possible move out of libstore to libcmd.
 struct BuiltPath : _BuiltPathRaw {
     using Raw = _BuiltPathRaw;
     using Raw::Raw;
@@ -120,8 +110,12 @@ struct BuiltPath : _BuiltPathRaw {
         return static_cast<const Raw &>(*this);
     }
 
+    StorePathSet outPaths() const;
+    RealisedPath::Set toRealisedPaths(Store & store) const;
+
 };
 
+typedef std::vector<DerivedPath> DerivedPaths;
 typedef std::vector<BuiltPath> BuiltPaths;
 
 nlohmann::json derivedPathsWithHintsToJSON(const BuiltPaths & buildables, ref<Store> store);

src/nix/copy.cc

@@ -8,7 +8,7 @@
 
 using namespace nix;
 
-struct CmdCopy : RealisedPathsCommand
+struct CmdCopy : BuiltPathsCommand
 {
     std::string srcUri, dstUri;
 
@@ -16,10 +16,10 @@ struct CmdCopy : RealisedPathsCommand
 
     SubstituteFlag substitute = NoSubstitute;
 
-    using RealisedPathsCommand::run;
+    using BuiltPathsCommand::run;
 
     CmdCopy()
-        : RealisedPathsCommand(true)
+        : BuiltPathsCommand(true)
     {
         addFlag({
             .longName = "from",
@@ -75,16 +75,22 @@ struct CmdCopy : RealisedPathsCommand
         if (srcUri.empty() && dstUri.empty())
             throw UsageError("you must pass '--from' and/or '--to'");
 
-        RealisedPathsCommand::run(store);
+        BuiltPathsCommand::run(store);
     }
 
-    void run(ref<Store> srcStore, std::vector<RealisedPath> paths) override
+    void run(ref<Store> srcStore, BuiltPaths paths) override
     {
         ref<Store> dstStore = dstUri.empty() ? openStore() : openStore(dstUri);
 
+        RealisedPath::Set stuffToCopy;
+
+        for (auto & builtPath : paths) {
+            auto theseRealisations = builtPath.toRealisedPaths(*srcStore);
+            stuffToCopy.insert(theseRealisations.begin(), theseRealisations.end());
+        }
+
         copyPaths(
-            srcStore, dstStore, RealisedPath::Set(paths.begin(), paths.end()),
+            srcStore, dstStore, stuffToCopy, NoRepair, checkSigs, substitute);
-            NoRepair, checkSigs, substitute);
     }
 };
 

src/nix/develop.cc

@@ -265,9 +265,9 @@ struct Common : InstallableCommand, MixProfile
         for (auto & [installable_, dir_] : redirects) {
             auto dir = absPath(dir_);
             auto installable = parseInstallable(store, installable_);
-            auto buildable = installable->toBuiltPath();
+            auto builtPaths = toStorePaths(
-            auto doRedirect = [&](const StorePath & path)
+                store, Realise::Nothing, OperateOn::Output, {installable});
-            {
+            for (auto & path: builtPaths) {
                 auto from = store->printStorePath(path);
                 if (script.find(from) == std::string::npos)
                     warn("'%s' (path '%s') is not used by this build environment", installable->what(), from);
@@ -275,16 +275,7 @@ struct Common : InstallableCommand, MixProfile
                     printInfo("redirecting '%s' to '%s'", from, dir);
                     rewrites.insert({from, dir});
                 }
-            };
+            }
-            std::visit(overloaded {
-                [&](const BuiltPath::Opaque & bo) {
-                    doRedirect(bo.path);
-                },
-                [&](const BuiltPath::Built & bfd) {
-                    for (auto & [outputName, path] : bfd.outputs)
-                        if (path) doRedirect(*path);
-                },
-            }, buildable.raw());
         }
 
         return rewriteStrings(script, rewrites);

src/nix/log.cc

@@ -30,15 +30,15 @@ struct CmdLog : InstallableCommand
 
         subs.push_front(store);
 
-        auto b = installable->toBuiltPath();
+        auto b = installable->toDerivedPath();
 
         RunPager pager;
         for (auto & sub : subs) {
             auto log = std::visit(overloaded {
-                [&](BuiltPath::Opaque bo) {
+                [&](DerivedPath::Opaque bo) {
                     return sub->getBuildLog(bo.path);
                 },
-                [&](BuiltPath::Built bfd) {
+                [&](DerivedPath::Built bfd) {
                     return sub->getBuildLog(bfd.drvPath);
                 },
             }, b.raw());

src/nix/realisation.cc

@@ -28,7 +28,7 @@ struct CmdRealisation : virtual NixMultiCommand
 
 static auto rCmdRealisation = registerCommand<CmdRealisation>("realisation");
 
-struct CmdRealisationInfo : RealisedPathsCommand, MixJSON
+struct CmdRealisationInfo : BuiltPathsCommand, MixJSON
 {
     std::string description() override
     {
@@ -44,12 +44,19 @@ struct CmdRealisationInfo : RealisedPathsCommand, MixJSON
 
     Category category() override { return catSecondary; }
 
-    void run(ref<Store> store, std::vector<RealisedPath> paths) override
+    void run(ref<Store> store, BuiltPaths paths) override
     {
         settings.requireExperimentalFeature("ca-derivations");
+        RealisedPath::Set realisations;
+
+        for (auto & builtPath : paths) {
+            auto theseRealisations = builtPath.toRealisedPaths(*store);
+            realisations.insert(theseRealisations.begin(), theseRealisations.end());
+        }
+
         if (json) {
             nlohmann::json res = nlohmann::json::array();
-            for (auto & path : paths) {
+            for (auto & path : realisations) {
                 nlohmann::json currentPath;
                 if (auto realisation = std::get_if<Realisation>(&path.raw))
                     currentPath = realisation->toJSON();
@@ -61,7 +68,7 @@ struct CmdRealisationInfo : RealisedPathsCommand, MixJSON
             std::cout << res.dump();
         }
         else {
-            for (auto & path : paths) {
+            for (auto & path : realisations) {
                 if (auto realisation = std::get_if<Realisation>(&path.raw)) {
                     std::cout <<
                         realisation->id.to_string() << " " <<