Use BuildableReq for buildPaths and ensurePath

This avoids an ambiguity where the `StorePathWithOutputs { drvPath, {}
}` could mean "build `brvPath`" or "substitute `drvPath`" depending on
context.

It also brings the internals closer in line to the new CLI, by
generalizing the `Buildable` type is used there and makes that
distinction already.

In doing so, relegate `StorePathWithOutputs` to being a type just for
backwards compatibility (CLI and RPC).

src/libcmd/installables.cc

@@ -679,19 +679,20 @@ Buildables build(ref<Store> store, Realise mode,
 
     Buildables buildables;
 
-    std::vector<StorePathWithOutputs> pathsToBuild;
+    std::vector<BuildableReq> pathsToBuild;
 
     for (auto & i : installables) {
         for (auto & b : i->toBuildables()) {
             std::visit(overloaded {
                 [&](BuildableOpaque bo) {
-                    pathsToBuild.push_back({bo.path});
+                    pathsToBuild.push_back(bo);
                 },
                 [&](BuildableFromDrv bfd) {
                     StringSet outputNames;
                     for (auto & output : bfd.outputs)
                         outputNames.insert(output.first);
-                    pathsToBuild.push_back({bfd.drvPath, outputNames});
+                    pathsToBuild.push_back(
+                        BuildableReqFromDrv{bfd.drvPath, outputNames});
                 },
             }, b);
             buildables.push_back(std::move(b));

src/libexpr/get-drvs.cc

@@ -2,6 +2,7 @@
 #include "util.hh"
 #include "eval-inline.hh"
 #include "store-api.hh"
+#include "path-with-outputs.hh"
 
 #include <cstring>
 #include <regex>

src/libexpr/primops.cc

@@ -35,30 +35,32 @@ InvalidPathError::InvalidPathError(const Path & path) :
 
 void EvalState::realiseContext(const PathSet & context)
 {
-    std::vector<StorePathWithOutputs> drvs;
+    std::vector<BuildableReqFromDrv> drvs;
 
     for (auto & i : context) {
         auto [ctxS, outputName] = decodeContext(i);
         auto ctx = store->parseStorePath(ctxS);
         if (!store->isValidPath(ctx))
             throw InvalidPathError(store->printStorePath(ctx));
         if (!outputName.empty() && ctx.isDerivation()) {
-            drvs.push_back(StorePathWithOutputs{ctx, {outputName}});
+            drvs.push_back({ctx, {outputName}});
         }
     }
 
     if (drvs.empty()) return;
 
     if (!evalSettings.enableImportFromDerivation)
         throw EvalError("attempted to realize '%1%' during evaluation but 'allow-import-from-derivation' is false",
-            store->printStorePath(drvs.begin()->path));
+            store->printStorePath(drvs.begin()->drvPath));
 
     /* For performance, prefetch all substitute info. */
     StorePathSet willBuild, willSubstitute, unknown;
     uint64_t downloadSize, narSize;
-    store->queryMissing(drvs, willBuild, willSubstitute, unknown, downloadSize, narSize);
+    std::vector<BuildableReq> buildReqs;
+    for (auto & d : drvs) buildReqs.emplace_back(BuildableReq { d });
+    store->queryMissing(buildReqs, willBuild, willSubstitute, unknown, downloadSize, narSize);
 
-    store->buildPaths(drvs);
+    store->buildPaths(buildReqs);
 
     /* Add the output of this derivations to the allowed
        paths. */

src/libmain/shared.cc

@@ -36,7 +36,7 @@ void printGCWarning()
 }
 
 
-void printMissing(ref<Store> store, const std::vector<StorePathWithOutputs> & paths, Verbosity lvl)
+void printMissing(ref<Store> store, const std::vector<BuildableReq> & paths, Verbosity lvl)
 {
     uint64_t downloadSize, narSize;
     StorePathSet willBuild, willSubstitute, unknown;

src/libmain/shared.hh

@@ -4,6 +4,7 @@
 #include "args.hh"
 #include "common-args.hh"
 #include "path.hh"
+#include "buildable.hh"
 
 #include <signal.h>
 
@@ -42,7 +43,7 @@ struct StorePathWithOutputs;
 
 void printMissing(
     ref<Store> store,
-    const std::vector<StorePathWithOutputs> & paths,
+    const std::vector<BuildableReq> & paths,
     Verbosity lvl = lvlInfo);
 
 void printMissing(ref<Store> store, const StorePathSet & willBuild,

src/libstore/build/derivation-goal.cc

@@ -73,7 +73,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath,
     state = &DerivationGoal::getDerivation;
     name = fmt(
         "building of '%s' from .drv file",
-        StorePathWithOutputs { drvPath, wantedOutputs }.to_string(worker.store));
+        to_string(worker.store, BuildableReqFromDrv { drvPath, wantedOutputs }));
     trace("created");
 
     mcExpectedBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.expectedBuilds);
@@ -94,7 +94,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation
     state = &DerivationGoal::haveDerivation;
     name = fmt(
         "building of '%s' from in-memory derivation",
-        StorePathWithOutputs { drvPath, drv.outputNames() }.to_string(worker.store));
+        to_string(worker.store, BuildableReqFromDrv { drvPath, drv.outputNames() }));
     trace("created");
 
     mcExpectedBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.expectedBuilds);

src/libstore/build/entry-points.cc

@@ -6,16 +6,20 @@
 
 namespace nix {
 
-void Store::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode)
+void Store::buildPaths(const std::vector<BuildableReq> & reqs, BuildMode buildMode)
 {
     Worker worker(*this);
 
     Goals goals;
-    for (auto & path : drvPaths) {
-        if (path.path.isDerivation())
-            goals.insert(worker.makeDerivationGoal(path.path, path.outputs, buildMode));
-        else
-            goals.insert(worker.makePathSubstitutionGoal(path.path, buildMode == bmRepair ? Repair : NoRepair));
+    for (auto & br : reqs) {
+        std::visit(overloaded {
+            [&](BuildableReqFromDrv bfd) {
+                goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode));
+            },
+            [&](BuildableOpaque bo) {
+                goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair));
+            },
+        }, br);
     }
 
     worker.run(goals);

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

@@ -1190,6 +1190,26 @@ void LocalDerivationGoal::writeStructuredAttrs()
     chownToBuilder(tmpDir + "/.attrs.sh");
 }
 
+
+static StorePath pathPartOfReq(const BuildableReq & req)
+{
+    return std::visit(overloaded {
+        [&](BuildableOpaque bo) {
+            return bo.path;
+        },
+        [&](BuildableReqFromDrv bfd)  {
+            return bfd.drvPath;
+        },
+    }, req);
+}
+
+
+bool LocalDerivationGoal::isAllowed(const BuildableReq & req)
+{
+    return this->isAllowed(pathPartOfReq(req));
+}
+
+
 struct RestrictedStoreConfig : virtual LocalFSStoreConfig
 {
     using LocalFSStoreConfig::LocalFSStoreConfig;
@@ -1312,25 +1332,27 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
     // an allowed derivation
     { throw Error("queryRealisation"); }
 
-    void buildPaths(const std::vector<StorePathWithOutputs> & paths, BuildMode buildMode) override
+    void buildPaths(const std::vector<BuildableReq> & paths, BuildMode buildMode) override
     {
         if (buildMode != bmNormal) throw Error("unsupported build mode");
 
         StorePathSet newPaths;
 
-        for (auto & path : paths) {
-            if (!goal.isAllowed(path.path))
-                throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path));
+        for (auto & req : paths) {
+            if (!goal.isAllowed(req))
+                throw InvalidPath("cannot build '%s' in recursive Nix because path is unknown", to_string(*next, req));
         }
 
         next->buildPaths(paths, buildMode);
 
         for (auto & path : paths) {
-            if (!path.path.isDerivation()) continue;
-            auto outputs = next->queryDerivationOutputMap(path.path);
-            for (auto & output : outputs)
-                if (wantOutput(output.first, path.outputs))
-                    newPaths.insert(output.second);
+            auto p =  std::get_if<BuildableReqFromDrv>(&path);
+            if (!p) continue;
+            auto & bfd = *p;
+            auto outputs = next->queryDerivationOutputMap(bfd.drvPath);
+            for (auto & [outputName, outputPath] : outputs)
+                if (wantOutput(outputName, bfd.outputs))
+                    newPaths.insert(outputPath);
         }
 
         StorePathSet closure;
@@ -1358,20 +1380,20 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
     void addSignatures(const StorePath & storePath, const StringSet & sigs) override
     { unsupported("addSignatures"); }
 
-    void queryMissing(const std::vector<StorePathWithOutputs> & targets,
+    void queryMissing(const std::vector<BuildableReq> & targets,
         StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
         uint64_t & downloadSize, uint64_t & narSize) override
     {
         /* This is slightly impure since it leaks information to the
            client about what paths will be built/substituted or are
            already present. Probably not a big deal. */
 
-        std::vector<StorePathWithOutputs> allowed;
-        for (auto & path : targets) {
-            if (goal.isAllowed(path.path))
-                allowed.emplace_back(path);
+        std::vector<BuildableReq> allowed;
+        for (auto & req : targets) {
+            if (goal.isAllowed(req))
+                allowed.emplace_back(req);
             else
-                unknown.insert(path.path);
+                unknown.insert(pathPartOfReq(req));
         }
 
         next->queryMissing(allowed, willBuild, willSubstitute,

src/libstore/build/local-derivation-goal.hh

@@ -116,6 +116,7 @@ struct LocalDerivationGoal : public DerivationGoal
     {
         return inputPaths.count(path) || addedPaths.count(path);
     }
+    bool isAllowed(const BuildableReq & req);
 
     friend struct RestrictedStore;
 

src/libstore/build/worker.cc

@@ -226,14 +226,14 @@ void Worker::waitForAWhile(GoalPtr goal)
 
 void Worker::run(const Goals & _topGoals)
 {
-    std::vector<nix::StorePathWithOutputs> topPaths;
+    std::vector<nix::BuildableReq> topPaths;
 
     for (auto & i : _topGoals) {
         topGoals.insert(i);
         if (auto goal = dynamic_cast<DerivationGoal *>(i.get())) {
-            topPaths.push_back({goal->drvPath, goal->wantedOutputs});
+            topPaths.push_back(BuildableReqFromDrv{goal->drvPath, goal->wantedOutputs});
         } else if (auto goal = dynamic_cast<PathSubstitutionGoal *>(i.get())) {
-            topPaths.push_back({goal->storePath});
+            topPaths.push_back(BuildableOpaque{goal->storePath});
         }
     }
 

src/libstore/buildable.cc

@@ -11,6 +11,7 @@ nlohmann::json BuildableOpaque::toJSON(ref<Store> store) const {
     return res;
 }
 
+template<>
 nlohmann::json BuildableFromDrv::toJSON(ref<Store> store) const {
     nlohmann::json res;
     res["drvPath"] = store->printStorePath(drvPath);
@@ -30,4 +31,50 @@ nlohmann::json buildablesToJSON(const Buildables & buildables, ref<Store> store)
     return res;
 }
 
+
+std::string BuildableOpaque::to_string(const Store & store) const {
+    return store.printStorePath(path);
+}
+
+template<>
+std::string BuildableReqFromDrv::to_string(const Store & store) const {
+    return store.printStorePath(drvPath)
+        + "!"
+        + (outputs.empty() ? std::string { "*" } : concatStringsSep(",", outputs));
+}
+
+std::string to_string(const Store & store, const BuildableReq & req)
+{
+    return std::visit(
+        [&](const auto & req) { return req.to_string(store); },
+        req);
+}
+
+
+BuildableOpaque BuildableOpaque::parse(const Store & store, std::string_view s)
+{
+    return {store.parseStorePath(s)};
+}
+
+template<>
+BuildableReqFromDrv BuildableReqFromDrv::parse(const Store & store, std::string_view s)
+{
+    size_t n = s.find("!");
+    assert(n != s.npos);
+    auto drvPath = store.parseStorePath(s.substr(0, n));
+    auto outputsS = s.substr(n + 1);
+    std::set<string> outputs;
+    if (outputsS != "*")
+        outputs = tokenizeString<std::set<string>>(outputsS);
+    return {drvPath, outputs};
+}
+
+BuildableReq parseBuildableReq(const Store & store, std::string_view s)
+{
+    size_t n = s.find("!");
+    return n == s.npos
+        ? (BuildableReq) BuildableOpaque::parse(store, s)
+        : (BuildableReq) BuildableReqFromDrv::parse(store, s);
+}
+
 }

src/libstore/buildable.hh

@@ -2,6 +2,7 @@
 
 #include "util.hh"
 #include "path.hh"
+#include "path.hh"
 
 #include <optional>
 
@@ -13,19 +14,37 @@ class Store;
 
 struct BuildableOpaque {
     StorePath path;
+
     nlohmann::json toJSON(ref<Store> store) const;
+    std::string to_string(const Store & store) const;
+    static BuildableOpaque parse(const Store & store, std::string_view);
 };
 
-struct BuildableFromDrv {
+template<typename Outputs>
+struct BuildableForFromDrv {
     StorePath drvPath;
-    std::map<std::string, std::optional<StorePath>> outputs;
+    Outputs outputs;
+
     nlohmann::json toJSON(ref<Store> store) const;
+    std::string to_string(const Store & store) const;
+    static BuildableForFromDrv<Outputs> parse(const Store & store, std::string_view);
 };
 
-typedef std::variant<
+template <typename Outputs>
+using BuildableFor = std::variant<
     BuildableOpaque,
-    BuildableFromDrv
-> Buildable;
+    BuildableForFromDrv<Outputs>
+>;
+
+typedef BuildableForFromDrv<std::set<std::string>> BuildableReqFromDrv;
+typedef BuildableFor<std::set<std::string>> BuildableReq;
+
+std::string to_string(const Store & store, const BuildableReq &);
+
+BuildableReq parseBuildableReq(const Store & store, std::string_view);
+
+typedef BuildableForFromDrv<std::map<std::string, std::optional<StorePath>>> BuildableFromDrv;
+typedef BuildableFor<std::map<std::string, std::optional<StorePath>>> Buildable;
 
 typedef std::vector<Buildable> Buildables;