Make command infra less stateful and more regular

src/libcmd/command.cc

@@ -165,7 +165,7 @@ BuiltPathsCommand::BuiltPathsCommand(bool recursive)
     });
 }
 
-void BuiltPathsCommand::run(ref<Store> store)
+void BuiltPathsCommand::run(ref<Store> store, Installables && installables)
 {
     BuiltPaths paths;
     if (all) {
@@ -211,7 +211,7 @@ void StorePathsCommand::run(ref<Store> store, BuiltPaths && paths)
     run(store, std::move(sorted));
 }
 
-void StorePathCommand::run(ref<Store> store, std::vector<StorePath> && storePaths)
+void StorePathCommand::run(ref<Store> store, StorePaths && storePaths)
 {
     if (storePaths.size() != 1)
         throw UsageError("this command requires exactly one store path");
@@ -246,7 +246,7 @@ void MixProfile::updateProfile(const BuiltPaths & buildables)
 {
     if (!profile) return;
 
-    std::vector<StorePath> result;
+    StorePaths result;
 
     for (auto & buildable : buildables) {
         std::visit(overloaded {

src/libcmd/command.hh

@@ -29,6 +29,9 @@ struct NixMultiCommand : virtual MultiCommand, virtual Command
     nlohmann::json toJSON() override;
 };
 
+// For the overloaded run methods
+#pragma GCC diagnostic ignored "-Woverloaded-virtual"
+
 /* A command that requires a Nix store. */
 struct StoreCommand : virtual Command
 {
@@ -97,10 +100,10 @@ struct SourceExprCommand : virtual Args, MixFlakeOptions
 
     SourceExprCommand();
 
-    std::vector<std::shared_ptr<Installable>> parseInstallables(
+    Installables parseInstallables(
         ref<Store> store, std::vector<std::string> ss);
 
-    std::shared_ptr<Installable> parseInstallable(
+    ref<Installable> parseInstallable(
         ref<Store> store, const std::string & installable);
 
     virtual Strings getDefaultFlakeAttrPaths();
@@ -115,36 +118,43 @@ struct MixReadOnlyOption : virtual Args
     MixReadOnlyOption();
 };
 
-/* A command that operates on a list of "installables", which can be
-   store paths, attribute paths, Nix expressions, etc. */
-struct InstallablesCommand : virtual Args, SourceExprCommand
+/* Like InstallablesCommand but the installables are not loaded */
+struct RawInstallablesCommand : virtual Args, SourceExprCommand
 {
-    std::vector<std::shared_ptr<Installable>> installables;
+    RawInstallablesCommand();
 
-    InstallablesCommand();
+    virtual void run(ref<Store> store, std::vector<std::string> && rawInstallables) = 0;
 
-    void prepare() override;
-    Installables load();
+    void run(ref<Store> store) override;
 
-    virtual bool useDefaultInstallables() { return true; }
+    // FIXME make const after CmdRepl's override is fixed up
+    virtual void applyDefaultInstallables(std::vector<std::string> & rawInstallables);
 
     bool readFromStdIn = false;
 
     std::vector<std::string> getFlakesForCompletion() override;
 
-protected:
+private:
+
+    std::vector<std::string> rawInstallables;
+};
+/* A command that operates on a list of "installables", which can be
+   store paths, attribute paths, Nix expressions, etc. */
+struct InstallablesCommand : RawInstallablesCommand
+{
+    virtual void run(ref<Store> store, Installables && installables) = 0;
 
-    std::vector<std::string> _installables;
+    void run(ref<Store> store, std::vector<std::string> && rawInstallables) override;
 };
 
 /* A command that operates on exactly one "installable" */
 struct InstallableCommand : virtual Args, SourceExprCommand
 {
-    std::shared_ptr<Installable> installable;
-
     InstallableCommand();
 
-    void prepare() override;
+    virtual void run(ref<Store> store, ref<Installable> installable) = 0;
+
+    void run(ref<Store> store) override;
 
     std::vector<std::string> getFlakesForCompletion() override
     {
@@ -179,34 +189,28 @@ public:
 
     BuiltPathsCommand(bool recursive = false);
 
-    using StoreCommand::run;
-
     virtual void run(ref<Store> store, BuiltPaths && paths) = 0;
 
-    void run(ref<Store> store) override;
+    void run(ref<Store> store, Installables && installables) override;
 
-    bool useDefaultInstallables() override { return !all; }
+    void applyDefaultInstallables(std::vector<std::string> & rawInstallables) override;
 };
 
 struct StorePathsCommand : public BuiltPathsCommand
 {
     StorePathsCommand(bool recursive = false);
 
-    using BuiltPathsCommand::run;
-
-    virtual void run(ref<Store> store, std::vector<StorePath> && storePaths) = 0;
+    virtual void run(ref<Store> store, StorePaths && storePaths) = 0;
 
     void run(ref<Store> store, BuiltPaths && paths) override;
 };
 
 /* A command that operates on exactly one store path. */
 struct StorePathCommand : public StorePathsCommand
 {
-    using StorePathsCommand::run;
-
     virtual void run(ref<Store> store, const StorePath & storePath) = 0;
 
-    void run(ref<Store> store, std::vector<StorePath> && storePaths) override;
+    void run(ref<Store> store, StorePaths && storePaths) override;
 };
 
 /* A helper class for registering commands globally. */

src/libcmd/installables.cc

@@ -422,10 +422,10 @@ ref<eval_cache::EvalCache> openEvalCache(
         });
 }
 
-std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
+Installables SourceExprCommand::parseInstallables(
     ref<Store> store, std::vector<std::string> ss)
 {
-    std::vector<std::shared_ptr<Installable>> result;
+    Installables result;
 
     if (file || expr) {
         if (file && expr)
@@ -451,7 +451,7 @@ std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
         for (auto & s : ss) {
             auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(s);
             result.push_back(
-                std::make_shared<InstallableAttrPath>(
+                make_ref<InstallableAttrPath>(
                     InstallableAttrPath::parse(
                         state, *this, vFile, prefix, extendedOutputsSpec)));
         }
@@ -468,7 +468,7 @@ std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
 
             if (prefix.find('/') != std::string::npos) {
                 try {
-                    result.push_back(std::make_shared<InstallableDerivedPath>(
+                    result.push_back(make_ref<InstallableDerivedPath>(
                         InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec)));
                     continue;
                 } catch (BadStorePath &) {
@@ -480,7 +480,7 @@ std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
 
             try {
                 auto [flakeRef, fragment] = parseFlakeRefWithFragment(std::string { prefix }, absPath("."));
-                result.push_back(std::make_shared<InstallableFlake>(
+                result.push_back(make_ref<InstallableFlake>(
                         this,
                         getEvalState(),
                         std::move(flakeRef),
@@ -501,7 +501,7 @@ std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
     return result;
 }
 
-std::shared_ptr<Installable> SourceExprCommand::parseInstallable(
+ref<Installable> SourceExprCommand::parseInstallable(
     ref<Store> store, const std::string & installable)
 {
     auto installables = parseInstallables(store, {installable});
@@ -513,7 +513,7 @@ std::vector<BuiltPathWithResult> Installable::build(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode,
-    const std::vector<std::shared_ptr<Installable>> & installables,
+    const Installables & installables,
     BuildMode bMode)
 {
     std::vector<BuiltPathWithResult> res;
@@ -522,11 +522,11 @@ std::vector<BuiltPathWithResult> Installable::build(
     return res;
 }
 
-std::vector<std::pair<std::shared_ptr<Installable>, BuiltPathWithResult>> Installable::build2(
+std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build2(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode,
-    const std::vector<std::shared_ptr<Installable>> & installables,
+    const Installables & installables,
     BuildMode bMode)
 {
     if (mode == Realise::Nothing)
@@ -535,7 +535,7 @@ std::vector<std::pair<std::shared_ptr<Installable>, BuiltPathWithResult>> Instal
     struct Aux
     {
         ExtraPathInfo info;
-        std::shared_ptr<Installable> installable;
+        ref<Installable> installable;
     };
 
     std::vector<DerivedPath> pathsToBuild;
@@ -548,7 +548,7 @@ std::vector<std::pair<std::shared_ptr<Installable>, BuiltPathWithResult>> Instal
         }
     }
 
-    std::vector<std::pair<std::shared_ptr<Installable>, BuiltPathWithResult>> res;
+    std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> res;
 
     switch (mode) {
 
@@ -620,7 +620,7 @@ BuiltPaths Installable::toBuiltPaths(
     ref<Store> store,
     Realise mode,
     OperateOn operateOn,
-    const std::vector<std::shared_ptr<Installable>> & installables)
+    const Installables & installables)
 {
     if (operateOn == OperateOn::Output) {
         BuiltPaths res;
@@ -642,7 +642,7 @@ StorePathSet Installable::toStorePaths(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode, OperateOn operateOn,
-    const std::vector<std::shared_ptr<Installable>> & installables)
+    const Installables & installables)
 {
     StorePathSet outPaths;
     for (auto & path : toBuiltPaths(evalStore, store, mode, operateOn, installables)) {
@@ -656,7 +656,7 @@ StorePath Installable::toStorePath(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode, OperateOn operateOn,
-    std::shared_ptr<Installable> installable)
+    ref<Installable> installable)
 {
     auto paths = toStorePaths(evalStore, store, mode, operateOn, {installable});
 
@@ -668,7 +668,7 @@ StorePath Installable::toStorePath(
 
 StorePathSet Installable::toDerivations(
     ref<Store> store,
-    const std::vector<std::shared_ptr<Installable>> & installables,
+    const Installables & installables,
     bool useDeriver)
 {
     StorePathSet drvPaths;
@@ -692,9 +692,8 @@ StorePathSet Installable::toDerivations(
     return drvPaths;
 }
 
-InstallablesCommand::InstallablesCommand()
+RawInstallablesCommand::RawInstallablesCommand()
 {
-
     addFlag({
         .longName = "stdin",
         .description = "Read installables from the standard input.",
@@ -703,40 +702,45 @@ InstallablesCommand::InstallablesCommand()
 
     expectArgs({
         .label = "installables",
-        .handler = {&_installables},
+        .handler = {&rawInstallables},
         .completer = {[&](size_t, std::string_view prefix) {
             completeInstallable(prefix);
         }}
     });
 }
 
-void InstallablesCommand::prepare()
-{
-    installables = load();
-}
-
-Installables InstallablesCommand::load()
+void RawInstallablesCommand::applyDefaultInstallables(std::vector<std::string> & rawInstallables)
 {
-    if (_installables.empty() && useDefaultInstallables() && !readFromStdIn)
+    if (rawInstallables.empty()) {
         // FIXME: commands like "nix profile install" should not have a
         // default, probably.
-        _installables.push_back(".");
+        rawInstallables.push_back(".");
+    }
+}
 
+void RawInstallablesCommand::run(ref<Store> store)
+{
     if (readFromStdIn && !isatty(STDIN_FILENO)) {
         std::string word;
         while (std::cin >> word) {
-            _installables.emplace_back(std::move(word));
+            rawInstallables.emplace_back(std::move(word));
         }
     }
 
-    return parseInstallables(getStore(), _installables);
+    applyDefaultInstallables(rawInstallables);
+    run(store, std::move(rawInstallables));
 }
 
-std::vector<std::string> InstallablesCommand::getFlakesForCompletion()
+std::vector<std::string> RawInstallablesCommand::getFlakesForCompletion()
 {
-    if (_installables.empty() && useDefaultInstallables())
-        return {"."};
-    return _installables;
+    applyDefaultInstallables(rawInstallables);
+    return rawInstallables;
+}
+
+void InstallablesCommand::run(ref<Store> store, std::vector<std::string> && rawInstallables)
+{
+    auto installables = parseInstallables(store, rawInstallables);
+    run(store, std::move(installables));
 }
 
 InstallableCommand::InstallableCommand()
@@ -752,9 +756,16 @@ InstallableCommand::InstallableCommand()
     });
 }
 
-void InstallableCommand::prepare()
+void InstallableCommand::run(ref<Store> store)
+{
+    auto installable = parseInstallable(store, _installable);
+    run(store, std::move(installable));
+}
+
+void BuiltPathsCommand::applyDefaultInstallables(std::vector<std::string> & rawInstallables)
 {
-    installable = parseInstallable(getStore(), _installable);
+    if (rawInstallables.empty() && !all)
+        rawInstallables.push_back(".");
 }
 
 }