Merge pull request #3856 from obsidiansystems/buildable-variant

Make `Buildable` a `std::variant`

src/nix/build.cc

@@ -64,17 +64,24 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixProfile
 
         if (dryRun) return;
 
-        if (outLink != "") {
-            for (size_t i = 0; i < buildables.size(); ++i) {
-                for (auto & output : buildables[i].outputs)
-                    if (auto store2 = store.dynamic_pointer_cast<LocalFSStore>()) {
-                        std::string symlink = outLink;
-                        if (i) symlink += fmt("-%d", i);
-                        if (output.first != "out") symlink += fmt("-%s", output.first);
-                        store2->addPermRoot(output.second, absPath(symlink), true);
-                    }
-            }
-        }
+        if (outLink != "")
+            if (auto store2 = store.dynamic_pointer_cast<LocalFSStore>())
+                for (size_t i = 0; i < buildables.size(); ++i)
+                    std::visit(overloaded {
+                        [&](BuildableOpaque bo) {
+                            std::string symlink = outLink;
+                            if (i) symlink += fmt("-%d", i);
+                            store2->addPermRoot(bo.path, absPath(symlink), true);
+                        },
+                        [&](BuildableFromDrv bfd) {
+                            for (auto & output : bfd.outputs) {
+                                std::string symlink = outLink;
+                                if (i) symlink += fmt("-%d", i);
+                                if (output.first != "out") symlink += fmt("-%s", output.first);
+                                store2->addPermRoot(output.second, absPath(symlink), true);
+                            }
+                        },
+                    }, buildables[i]);
 
         updateProfile(buildables);
     }

src/nix/command.cc

@@ -128,20 +128,25 @@ void MixProfile::updateProfile(const Buildables & buildables)
 {
     if (!profile) return;
 
-    std::optional<StorePath> result;
+    std::vector<StorePath> result;
 
     for (auto & buildable : buildables) {
-        for (auto & output : buildable.outputs) {
-            if (result)
-                throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple");
-            result = output.second;
-        }
+        std::visit(overloaded {
+            [&](BuildableOpaque bo) {
+                result.push_back(bo.path);
+            },
+            [&](BuildableFromDrv bfd) {
+                for (auto & output : bfd.outputs) {
+                    result.push_back(output.second);
+                }
+            },
+        }, buildable);
     }
 
-    if (!result)
-        throw Error("'--profile' requires that the arguments produce a single store path, but there are none");
+    if (result.size() != 1)
+        throw Error("'--profile' requires that the arguments produce a single store path, but there are %d", result.size());
 
-    updateProfile(*result);
+    updateProfile(result[0]);
 }
 
 MixDefaultProfile::MixDefaultProfile()

src/nix/installables.cc

@@ -308,16 +308,15 @@ struct InstallableStorePath : Installable
             for (auto & [name, output] : drv.outputs)
                 outputs.emplace(name, output.path(*store, drv.name));
             return {
-                Buildable {
+                BuildableFromDrv {
                     .drvPath = storePath,
                     .outputs = std::move(outputs)
                 }
             };
         } else {
             return {
-                Buildable {
-                    .drvPath = {},
-                    .outputs = {{"out", storePath}}
+                BuildableOpaque {
+                    .path = storePath,
                 }
             };
         }
@@ -333,33 +332,20 @@ Buildables InstallableValue::toBuildables()
 {
     Buildables res;
 
-    StorePathSet drvPaths;
+    std::map<StorePath, OutputPathMap> drvsToOutputs;
 
+    // Group by derivation, helps with .all in particular
     for (auto & drv : toDerivations()) {
-        Buildable b{.drvPath = drv.drvPath};
-        drvPaths.insert(drv.drvPath);
-
         auto outputName = drv.outputName;
         if (outputName == "")
-            throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(*b.drvPath));
-
-        b.outputs.emplace(outputName, drv.outPath);
-
-        res.push_back(std::move(b));
+            throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(drv.drvPath));
+        drvsToOutputs[drv.drvPath].insert_or_assign(outputName, drv.outPath);
     }
 
-    // Hack to recognize .all: if all drvs have the same drvPath,
-    // merge the buildables.
-    if (drvPaths.size() == 1) {
-        Buildable b{.drvPath = *drvPaths.begin()};
-        for (auto & b2 : res)
-            for (auto & output : b2.outputs)
-                b.outputs.insert_or_assign(output.first, output.second);
-        Buildables bs;
-        bs.push_back(std::move(b));
-        return bs;
-    } else
-        return res;
+    for (auto & i : drvsToOutputs)
+        res.push_back(BuildableFromDrv { i.first, i.second });
+
+    return res;
 }
 
 struct InstallableAttrPath : InstallableValue
@@ -656,14 +642,17 @@ Buildables build(ref<Store> store, Realise mode,
 
     for (auto & i : installables) {
         for (auto & b : i->toBuildables()) {
-            if (b.drvPath) {
-                StringSet outputNames;
-                for (auto & output : b.outputs)
-                    outputNames.insert(output.first);
-                pathsToBuild.push_back({*b.drvPath, outputNames});
-            } else
-                for (auto & output : b.outputs)
-                    pathsToBuild.push_back({output.second});
+            std::visit(overloaded {
+                [&](BuildableOpaque bo) {
+                    pathsToBuild.push_back({bo.path});
+                },
+                [&](BuildableFromDrv bfd) {
+                    StringSet outputNames;
+                    for (auto & output : bfd.outputs)
+                        outputNames.insert(output.first);
+                    pathsToBuild.push_back({bfd.drvPath, outputNames});
+                },
+            }, b);
             buildables.push_back(std::move(b));
         }
     }
@@ -684,16 +673,23 @@ StorePathSet toStorePaths(ref<Store> store,
 
     if (operateOn == OperateOn::Output) {
         for (auto & b : build(store, mode, installables))
-            for (auto & output : b.outputs)
-                outPaths.insert(output.second);
+            std::visit(overloaded {
+                [&](BuildableOpaque bo) {
+                    outPaths.insert(bo.path);
+                },
+                [&](BuildableFromDrv bfd) {
+                    for (auto & output : bfd.outputs)
+                        outPaths.insert(output.second);
+                },
+            }, b);
     } else {
         if (mode == Realise::Nothing)
             settings.readOnlyMode = true;
 
         for (auto & i : installables)
             for (auto & b : i->toBuildables())
-                if (b.drvPath)
-                    outPaths.insert(*b.drvPath);
+                if (auto bfd = std::get_if<BuildableFromDrv>(&b))
+                    outPaths.insert(bfd->drvPath);
     }
 
     return outPaths;
@@ -717,20 +713,21 @@ StorePathSet toDerivations(ref<Store> store,
     StorePathSet drvPaths;
 
     for (auto & i : installables)
-        for (auto & b : i->toBuildables()) {
-            if (!b.drvPath) {
-                if (!useDeriver)
-                    throw Error("argument '%s' did not evaluate to a derivation", i->what());
-                for (auto & output : b.outputs) {
-                    auto derivers = store->queryValidDerivers(output.second);
+        for (auto & b : i->toBuildables())
+            std::visit(overloaded {
+                [&](BuildableOpaque bo) {
+                    if (!useDeriver)
+                        throw Error("argument '%s' did not evaluate to a derivation", i->what());
+                    auto derivers = store->queryValidDerivers(bo.path);
                     if (derivers.empty())
                         throw Error("'%s' does not have a known deriver", i->what());
                     // FIXME: use all derivers?
                     drvPaths.insert(*derivers.begin());
-                }
-            } else
-                drvPaths.insert(*b.drvPath);
-        }
+                },
+                [&](BuildableFromDrv bfd) {
+                    drvPaths.insert(bfd.drvPath);
+                },
+            }, b);
 
     return drvPaths;
 }

src/nix/installables.hh

@@ -14,12 +14,20 @@ struct SourceExprCommand;
 
 namespace eval_cache { class EvalCache; class AttrCursor; }
 
-struct Buildable
-{
-    std::optional<StorePath> drvPath;
+struct BuildableOpaque {
+    StorePath path;
+};
+
+struct BuildableFromDrv {
+    StorePath drvPath;
     std::map<std::string, StorePath> outputs;
 };
 
+typedef std::variant<
+    BuildableOpaque,
+    BuildableFromDrv
+> Buildable;
+
 typedef std::vector<Buildable> Buildables;
 
 struct App

src/nix/log.cc

@@ -45,11 +45,14 @@ struct CmdLog : InstallableCommand
 
         RunPager pager;
         for (auto & sub : subs) {
-            auto log = b.drvPath ? sub->getBuildLog(*b.drvPath) : nullptr;
-            for (auto & output : b.outputs) {
-                if (log) break;
-                log = sub->getBuildLog(output.second);
-            }
+            auto log = std::visit(overloaded {
+                [&](BuildableOpaque bo) {
+                    return sub->getBuildLog(bo.path);
+                },
+                [&](BuildableFromDrv bfd) {
+                    return sub->getBuildLog(bfd.drvPath);
+                },
+            }, b);
             if (!log) continue;
             stopProgressBar();
             printInfo("got build log for '%s' from '%s'", installable->what(), sub->getUri());