From cbeb4e6d1a2f9487e3e12062b653a71662ff72f4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 4 Feb 2023 12:03:47 -0500 Subject: [PATCH] Make command infra less stateful and more regular Already, we had classes like `BuiltPathsCommand` and `StorePathsCommand` which provided alternative `run` virtual functions providing the implementation with more arguments. This was a very nice and easy way to make writing command; just fill in the virtual functions and it is fairly clear what to do. However, exception to this pattern were `Installable{,s}Command`. These two classes instead just had a field where the installables would be stored, and various side-effecting `prepare` and `load` machinery too fill them in. Command would wish out those fields. This isn't so clear to use. What this commit does is make those command classes like the others, with richer run functions. Not only does this restore the pattern making commands easier to write, it has a number of other benefits: - `prepare` and `load` are gone entirely! One command just hands just hands off to the next. - We can use `ref` instead of `std::shared_ptr`. The former must be initialized (so it is like Rust's `Box` rather than `Option`, This expresses the invariant that the installable are in fact initialized much better. This is possible because since we just have local variables not fields, we can stop worrying about the not-yet-initialized case. - Fewer lines of code! (Finally I have a large refactor that makes the number go down not up...) - `nix repl` is now implemented in a clearer way. The last item deserves further mention. `nix repl` is not like the other installable commands because instead working from once-loaded installables, it needs to be able to load them again and again. To properly support this, we make a new superclass `RawInstallablesCommand`. This class has the argument parsing and completion logic, but does *not* hand off parsed installables but instead just the raw string arguments. This is exactly what `nix repl` needs, and allows us to instead of having the logic awkwardly split between `prepare`, `useDefaultInstallables,` and `load`, have everything right next to each other. I think this will enable future simplifications of that argument defaulting logic, but I am saving those for a future PR --- best to keep code motion and more complicated boolean expression rewriting separate steps. Finally do note that I stopped overloading the `run` functions. The reason was we are liable to get `-Woverloaded-virtual` warnings from this. Yes, there is a workaround in putting `using BaseClass::virtual_method;` next to overrides, but since I made the `run`-delegation chain cheaper that would have required *far* more `using` statements. I figured it was less obnoxious to just slightly vary the names than force all the commands to do that. Helps with https://github.com/NixOS/rfcs/pull/134 --- src/libcmd/command.cc | 14 +++--- src/libcmd/command.hh | 54 +++++++++++----------- src/libcmd/installables.cc | 74 +++++++++++++++++-------------- src/libcmd/installables.hh | 19 ++++---- src/libutil/args.hh | 1 - src/nix/app.cc | 4 +- src/nix/build.cc | 2 +- src/nix/bundle.cc | 2 +- src/nix/copy.cc | 4 +- src/nix/develop.cc | 19 ++++---- src/nix/dump-path.cc | 2 +- src/nix/edit.cc | 2 +- src/nix/eval.cc | 2 +- src/nix/flake.cc | 1 - src/nix/hash.cc | 1 - src/nix/log.cc | 2 +- src/nix/main.cc | 1 - src/nix/make-content-addressed.cc | 3 +- src/nix/nar.cc | 1 - src/nix/path-info.cc | 2 +- src/nix/profile.cc | 15 +++---- src/nix/realisation.cc | 3 +- src/nix/registry.cc | 1 - src/nix/repl.cc | 32 +++++++------ src/nix/run.cc | 4 +- src/nix/search.cc | 2 +- src/nix/show-derivation.cc | 2 +- src/nix/sigs.cc | 5 +-- src/nix/store-copy-log.cc | 2 +- src/nix/store-delete.cc | 2 +- src/nix/store-repair.cc | 2 +- src/nix/store.cc | 1 - src/nix/verify.cc | 2 +- 33 files changed, 140 insertions(+), 143 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index ab51c229dd6b..af50d28f33e7 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -165,7 +165,7 @@ BuiltPathsCommand::BuiltPathsCommand(bool recursive) }); } -void BuiltPathsCommand::run(ref store) +void BuiltPathsCommand::runIs(ref store, Installables && installables) { BuiltPaths paths; if (all) { @@ -190,7 +190,7 @@ void BuiltPathsCommand::run(ref store) } } - run(store, std::move(paths)); + runBPs(store, std::move(paths)); } StorePathsCommand::StorePathsCommand(bool recursive) @@ -198,7 +198,7 @@ StorePathsCommand::StorePathsCommand(bool recursive) { } -void StorePathsCommand::run(ref store, BuiltPaths && paths) +void StorePathsCommand::runBPs(ref store, BuiltPaths && paths) { StorePathSet storePaths; for (auto & builtPath : paths) @@ -208,15 +208,15 @@ void StorePathsCommand::run(ref store, BuiltPaths && paths) auto sorted = store->topoSortPaths(storePaths); std::reverse(sorted.begin(), sorted.end()); - run(store, std::move(sorted)); + runSPs(store, std::move(sorted)); } -void StorePathCommand::run(ref store, std::vector && storePaths) +void StorePathCommand::runSPs(ref store, StorePaths && storePaths) { if (storePaths.size() != 1) throw UsageError("this command requires exactly one store path"); - run(store, *storePaths.begin()); + runSP(store, *storePaths.begin()); } MixProfile::MixProfile() @@ -246,7 +246,7 @@ void MixProfile::updateProfile(const BuiltPaths & buildables) { if (!profile) return; - std::vector result; + StorePaths result; for (auto & buildable : buildables) { std::visit(overloaded { diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index b8116b1511c4..c485972b5065 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -97,10 +97,10 @@ struct SourceExprCommand : virtual Args, MixFlakeOptions SourceExprCommand(); - std::vector> parseInstallables( + Installables parseInstallables( ref store, std::vector ss); - std::shared_ptr parseInstallable( + ref parseInstallable( ref store, const std::string & installable); virtual Strings getDefaultFlakeAttrPaths(); @@ -115,16 +115,14 @@ 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> installables; + RawInstallablesCommand(); - InstallablesCommand(); + virtual void runRIs(ref store, std::vector && rawInstallables) = 0; - void prepare() override; - Installables load(); + void run(ref store) override; virtual bool useDefaultInstallables() { return true; } @@ -132,19 +130,29 @@ struct InstallablesCommand : virtual Args, SourceExprCommand std::vector getFlakesForCompletion() override; -protected: +private: - std::vector _installables; + std::vector 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 runIs(ref store, Installables && installables) = 0; + + void runRIs(ref store, std::vector && rawInstallables) override; + + std::vector getFlakesForCompletion() override; }; /* A command that operates on exactly one "installable" */ struct InstallableCommand : virtual Args, SourceExprCommand { - std::shared_ptr installable; - InstallableCommand(); - void prepare() override; + virtual void runI(ref store, ref installable) = 0; + + void run(ref store) override; std::vector getFlakesForCompletion() override { @@ -179,11 +187,9 @@ public: BuiltPathsCommand(bool recursive = false); - using StoreCommand::run; + virtual void runBPs(ref store, BuiltPaths && paths) = 0; - virtual void run(ref store, BuiltPaths && paths) = 0; - - void run(ref store) override; + void runIs(ref store, Installables && installables) override; bool useDefaultInstallables() override { return !all; } }; @@ -192,21 +198,17 @@ struct StorePathsCommand : public BuiltPathsCommand { StorePathsCommand(bool recursive = false); - using BuiltPathsCommand::run; + virtual void runSPs(ref store, StorePaths && storePaths) = 0; - virtual void run(ref store, std::vector && storePaths) = 0; - - void run(ref store, BuiltPaths && paths) override; + void runBPs(ref 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, const StorePath & storePath) = 0; + virtual void runSP(ref store, const StorePath & storePath) = 0; - void run(ref store, std::vector && storePaths) override; + void runSPs(ref store, StorePaths && storePaths) override; }; /* A helper class for registering commands globally. */ diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 5ecf6293ffb9..c79928f58eda 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -422,10 +422,10 @@ ref openEvalCache( }); } -std::vector> SourceExprCommand::parseInstallables( +Installables SourceExprCommand::parseInstallables( ref store, std::vector ss) { - std::vector> result; + Installables result; if (file || expr) { if (file && expr) @@ -451,7 +451,7 @@ std::vector> SourceExprCommand::parseInstallables( for (auto & s : ss) { auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(s); result.push_back( - std::make_shared( + make_ref( InstallableAttrPath::parse( state, *this, vFile, prefix, extendedOutputsSpec))); } @@ -468,7 +468,7 @@ std::vector> SourceExprCommand::parseInstallables( if (prefix.find('/') != std::string::npos) { try { - result.push_back(std::make_shared( + result.push_back(make_ref( InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec))); continue; } catch (BadStorePath &) { @@ -480,7 +480,7 @@ std::vector> SourceExprCommand::parseInstallables( try { auto [flakeRef, fragment] = parseFlakeRefWithFragment(std::string { prefix }, absPath(".")); - result.push_back(std::make_shared( + result.push_back(make_ref( this, getEvalState(), std::move(flakeRef), @@ -501,7 +501,7 @@ std::vector> SourceExprCommand::parseInstallables( return result; } -std::shared_ptr SourceExprCommand::parseInstallable( +ref SourceExprCommand::parseInstallable( ref store, const std::string & installable) { auto installables = parseInstallables(store, {installable}); @@ -513,7 +513,7 @@ std::vector Installable::build( ref evalStore, ref store, Realise mode, - const std::vector> & installables, + const Installables & installables, BuildMode bMode) { std::vector res; @@ -522,11 +522,11 @@ std::vector Installable::build( return res; } -std::vector, BuiltPathWithResult>> Installable::build2( +std::vector, BuiltPathWithResult>> Installable::build2( ref evalStore, ref store, Realise mode, - const std::vector> & installables, + const Installables & installables, BuildMode bMode) { if (mode == Realise::Nothing) @@ -535,7 +535,7 @@ std::vector, BuiltPathWithResult>> Instal struct Aux { ExtraPathInfo info; - std::shared_ptr installable; + ref installable; }; std::vector pathsToBuild; @@ -548,7 +548,7 @@ std::vector, BuiltPathWithResult>> Instal } } - std::vector, BuiltPathWithResult>> res; + std::vector, BuiltPathWithResult>> res; switch (mode) { @@ -620,7 +620,7 @@ BuiltPaths Installable::toBuiltPaths( ref store, Realise mode, OperateOn operateOn, - const std::vector> & installables) + const Installables & installables) { if (operateOn == OperateOn::Output) { BuiltPaths res; @@ -642,7 +642,7 @@ StorePathSet Installable::toStorePaths( ref evalStore, ref store, Realise mode, OperateOn operateOn, - const std::vector> & installables) + const Installables & installables) { StorePathSet outPaths; for (auto & path : toBuiltPaths(evalStore, store, mode, operateOn, installables)) { @@ -656,7 +656,7 @@ StorePath Installable::toStorePath( ref evalStore, ref store, Realise mode, OperateOn operateOn, - std::shared_ptr installable) + ref installable) { auto paths = toStorePaths(evalStore, store, mode, operateOn, {installable}); @@ -668,7 +668,7 @@ StorePath Installable::toStorePath( StorePathSet Installable::toDerivations( ref store, - const std::vector> & 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,48 @@ 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::run(ref store) { - if (_installables.empty() && useDefaultInstallables() && !readFromStdIn) + if (rawInstallables.empty() && useDefaultInstallables() && !readFromStdIn) { // FIXME: commands like "nix profile install" should not have a // default, probably. - _installables.push_back("."); + rawInstallables.push_back("."); + } 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); + runRIs(store, std::move(rawInstallables)); +} + +std::vector RawInstallablesCommand::getFlakesForCompletion() +{ + return rawInstallables; +} + +void InstallablesCommand::runRIs(ref store, std::vector && rawInstallables) +{ + auto installables = parseInstallables(store, rawInstallables); + runIs(store, std::move(installables)); } std::vector InstallablesCommand::getFlakesForCompletion() { - if (_installables.empty() && useDefaultInstallables()) - return {"."}; - return _installables; + auto res = RawInstallablesCommand::getFlakesForCompletion(); + if (res.empty() && useDefaultInstallables()) + res.push_back("."); + return res; } InstallableCommand::InstallableCommand() @@ -752,9 +759,10 @@ InstallableCommand::InstallableCommand() }); } -void InstallableCommand::prepare() +void InstallableCommand::run(ref store) { - installable = parseInstallable(getStore(), _installable); + auto installable = parseInstallable(store, _installable); + runI(store, std::move(installable)); } } diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index be77fdc81acf..6c2922d89d7e 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -79,6 +79,9 @@ struct BuiltPathWithResult typedef std::vector DerivedPathsWithInfo; +struct Installable; +typedef std::vector> Installables; + struct Installable { virtual ~Installable() { } @@ -122,14 +125,14 @@ struct Installable ref evalStore, ref store, Realise mode, - const std::vector> & installables, + const Installables & installables, BuildMode bMode = bmNormal); - static std::vector, BuiltPathWithResult>> build2( + static std::vector, BuiltPathWithResult>> build2( ref evalStore, ref store, Realise mode, - const std::vector> & installables, + const Installables & installables, BuildMode bMode = bmNormal); static std::set toStorePaths( @@ -137,18 +140,18 @@ struct Installable ref store, Realise mode, OperateOn operateOn, - const std::vector> & installables); + const Installables & installables); static StorePath toStorePath( ref evalStore, ref store, Realise mode, OperateOn operateOn, - std::shared_ptr installable); + ref installable); static std::set toDerivations( ref store, - const std::vector> & installables, + const Installables & installables, bool useDeriver = false); static BuiltPaths toBuiltPaths( @@ -156,9 +159,7 @@ struct Installable ref store, Realise mode, OperateOn operateOn, - const std::vector> & installables); + const Installables & installables); }; -typedef std::vector> Installables; - } diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 84866f12b4a2..7211ee307b11 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -198,7 +198,6 @@ struct Command : virtual public Args virtual ~Command() { } - virtual void prepare() { }; virtual void run() = 0; typedef int Category; diff --git a/src/nix/app.cc b/src/nix/app.cc index 5cd65136f53f..bfd75e278894 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -119,11 +119,11 @@ App UnresolvedApp::resolve(ref evalStore, ref store) { auto res = unresolved; - std::vector> installableContext; + Installables installableContext; for (auto & ctxElt : unresolved.context) installableContext.push_back( - std::make_shared(store, DerivedPath { ctxElt })); + make_ref(store, DerivedPath { ctxElt })); auto builtContext = Installable::build(evalStore, store, Realise::Outputs, installableContext); res.program = resolveString(*store, unresolved.program, builtContext); diff --git a/src/nix/build.cc b/src/nix/build.cc index f4f2ec81dbba..5b9a2dd538fa 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -89,7 +89,7 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile ; } - void run(ref store) override + void runIs(ref store, Installables && installables) override { if (dryRun) { std::vector pathsToBuild; diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index dcf9a6f2d1ff..ad34bdad1e99 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -70,7 +70,7 @@ struct CmdBundle : InstallableCommand return res; } - void run(ref store) override + void runI(ref store, ref installable) override { auto evalState = getEvalState(); diff --git a/src/nix/copy.cc b/src/nix/copy.cc index 8730a9a5c339..913cd57f2099 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -10,8 +10,6 @@ struct CmdCopy : virtual CopyCommand, virtual BuiltPathsCommand SubstituteFlag substitute = NoSubstitute; - using BuiltPathsCommand::run; - CmdCopy() : BuiltPathsCommand(true) { @@ -45,7 +43,7 @@ struct CmdCopy : virtual CopyCommand, virtual BuiltPathsCommand Category category() override { return catSecondary; } - void run(ref srcStore, BuiltPaths && paths) override + void runBPs(ref srcStore, BuiltPaths && paths) override { auto dstStore = getDstStore(); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 9d07a7a8506a..a0ed1cf671be 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -374,7 +374,7 @@ struct Common : InstallableCommand, MixProfile return res; } - StorePath getShellOutPath(ref store) + StorePath getShellOutPath(ref store, ref installable) { auto path = installable->getStorePath(); if (path && hasSuffix(path->to_string(), "-env")) @@ -392,9 +392,10 @@ struct Common : InstallableCommand, MixProfile } } - std::pair getBuildEnvironment(ref store) + std::pair + getBuildEnvironment(ref store, ref installable) { - auto shellOutPath = getShellOutPath(store); + auto shellOutPath = getShellOutPath(store, installable); auto strPath = store->printStorePath(shellOutPath); @@ -480,9 +481,9 @@ struct CmdDevelop : Common, MixEnvironment ; } - void run(ref store) override + void runI(ref store, ref installable) override { - auto [buildEnvironment, gcroot] = getBuildEnvironment(store); + auto [buildEnvironment, gcroot] = getBuildEnvironment(store, installable); auto [rcFileFd, rcFilePath] = createTempFile("nix-shell"); @@ -537,7 +538,7 @@ struct CmdDevelop : Common, MixEnvironment nixpkgsLockFlags.inputOverrides = {}; nixpkgsLockFlags.inputUpdates = {}; - auto bashInstallable = std::make_shared( + auto bashInstallable = make_ref( this, state, installable->nixpkgsFlakeRef(), @@ -573,7 +574,7 @@ struct CmdDevelop : Common, MixEnvironment // Need to chdir since phases assume in flake directory if (phase) { // chdir if installable is a flake of type git+file or path - auto installableFlake = std::dynamic_pointer_cast(installable); + auto installableFlake = installable.dynamic_pointer_cast(); if (installableFlake) { auto sourcePath = installableFlake->getLockedFlake()->flake.resolvedRef.input.getSourcePath(); if (sourcePath) { @@ -604,9 +605,9 @@ struct CmdPrintDevEnv : Common, MixJSON Category category() override { return catUtility; } - void run(ref store) override + void runI(ref store, ref installable) override { - auto buildEnvironment = getBuildEnvironment(store).first; + auto buildEnvironment = getBuildEnvironment(store, installable).first; stopProgressBar(); diff --git a/src/nix/dump-path.cc b/src/nix/dump-path.cc index c4edc894b896..1513537be447 100644 --- a/src/nix/dump-path.cc +++ b/src/nix/dump-path.cc @@ -18,7 +18,7 @@ struct CmdDumpPath : StorePathCommand ; } - void run(ref store, const StorePath & storePath) override + void runSP(ref store, const StorePath & storePath) override { FdSink sink(STDOUT_FILENO); store->narFromPath(storePath, sink); diff --git a/src/nix/edit.cc b/src/nix/edit.cc index dfe75fbdf92d..f434b975f536 100644 --- a/src/nix/edit.cc +++ b/src/nix/edit.cc @@ -25,7 +25,7 @@ struct CmdEdit : InstallableCommand Category category() override { return catSecondary; } - void run(ref store) override + void runI(ref store, ref installable) override { auto state = getEvalState(); diff --git a/src/nix/eval.cc b/src/nix/eval.cc index 209fd3ed2e2c..4e8f3c81822e 100644 --- a/src/nix/eval.cc +++ b/src/nix/eval.cc @@ -54,7 +54,7 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption Category category() override { return catSecondary; } - void run(ref store) override + void runI(ref store, ref installable) override { if (raw && json) throw UsageError("--raw and --json are mutually exclusive"); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 3fe093fc7fd0..0a66163962c8 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1329,7 +1329,6 @@ struct CmdFlake : NixMultiCommand if (!command) throw UsageError("'nix flake' requires a sub-command."); settings.requireExperimentalFeature(Xp::Flakes); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 60d9593a772a..6d95c04553d6 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -151,7 +151,6 @@ struct CmdHash : NixMultiCommand { if (!command) throw UsageError("'nix hash' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/log.cc b/src/nix/log.cc index 0c9f778f0e08..62cdb26fefbe 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -23,7 +23,7 @@ struct CmdLog : InstallableCommand Category category() override { return catSecondary; } - void run(ref store) override + void runI(ref store, ref installable) override { settings.readOnlyMode = true; diff --git a/src/nix/main.cc b/src/nix/main.cc index 53bf649d445b..7b715f281015 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -394,7 +394,6 @@ void mainWrapped(int argc, char * * argv) if (args.command->second->forceImpureByDefault() && !evalSettings.pureEval.overridden) { evalSettings.pureEval = false; } - args.command->second->prepare(); args.command->second->run(); } diff --git a/src/nix/make-content-addressed.cc b/src/nix/make-content-addressed.cc index 6693c55acec0..cc816d278f3d 100644 --- a/src/nix/make-content-addressed.cc +++ b/src/nix/make-content-addressed.cc @@ -28,8 +28,7 @@ struct CmdMakeContentAddressed : virtual CopyCommand, virtual StorePathsCommand, ; } - using StorePathsCommand::run; - void run(ref srcStore, StorePaths && storePaths) override + void runSPs(ref srcStore, StorePaths && storePaths) override { auto dstStore = dstUri.empty() ? openStore() : openStore(dstUri); diff --git a/src/nix/nar.cc b/src/nix/nar.cc index dbb043d9b9d8..9815410cfc00 100644 --- a/src/nix/nar.cc +++ b/src/nix/nar.cc @@ -25,7 +25,6 @@ struct CmdNar : NixMultiCommand { if (!command) throw UsageError("'nix nar' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/path-info.cc b/src/nix/path-info.cc index 613c5b1918dc..e12011961bac 100644 --- a/src/nix/path-info.cc +++ b/src/nix/path-info.cc @@ -80,7 +80,7 @@ struct CmdPathInfo : StorePathsCommand, MixJSON std::cout << fmt("\t%6.1f%c", res, idents.at(power)); } - void run(ref store, StorePaths && storePaths) override + void runSPs(ref store, StorePaths && storePaths) override { size_t pathLen = 0; for (auto & storePath : storePaths) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index eef33b3d9169..8be8023209b4 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -256,11 +256,11 @@ struct ProfileManifest static std::map> builtPathsPerInstallable( - const std::vector, BuiltPathWithResult>> & builtPaths) + const std::vector, BuiltPathWithResult>> & builtPaths) { std::map> res; for (auto & [installable, builtPath] : builtPaths) { - auto & r = res[installable.get()]; + auto & r = res[&*installable]; /* Note that there could be conflicting info (e.g. meta.priority fields) if the installable returned multiple derivations. So pick one arbitrarily. FIXME: @@ -296,7 +296,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile ; } - void run(ref store) override + void runIs(ref store, Installables && installables) override { ProfileManifest manifest(*getEvalState(), *profile); @@ -307,7 +307,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile for (auto & installable : installables) { ProfileElement element; - auto & [res, info] = builtPaths[installable.get()]; + auto & [res, info] = builtPaths[&*installable]; if (info.originalRef && info.resolvedRef && info.attrPath && info.extendedOutputsSpec) { element.source = ProfileElementSource { @@ -513,7 +513,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf auto matchers = getMatchers(store); - std::vector> installables; + Installables installables; std::vector indices; auto upgradedCount = 0; @@ -529,7 +529,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf Activity act(*logger, lvlChatty, actUnknown, fmt("checking '%s' for updates", element.source->attrPath)); - auto installable = std::make_shared( + auto installable = make_ref( this, getEvalState(), FlakeRef(element.source->originalRef), @@ -582,7 +582,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf for (size_t i = 0; i < installables.size(); ++i) { auto & installable = installables.at(i); auto & element = manifest.elements[indices.at(i)]; - element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()].first); + element.updateStorePaths(getEvalStore(), store, builtPaths[&*installable].first); } updateProfile(manifest.build(store)); @@ -798,7 +798,6 @@ struct CmdProfile : NixMultiCommand { if (!command) throw UsageError("'nix profile' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/realisation.cc b/src/nix/realisation.cc index 0d34665154d9..7485b31c544f 100644 --- a/src/nix/realisation.cc +++ b/src/nix/realisation.cc @@ -21,7 +21,6 @@ struct CmdRealisation : virtual NixMultiCommand { if (!command) throw UsageError("'nix realisation' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; @@ -44,7 +43,7 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON Category category() override { return catSecondary; } - void run(ref store, BuiltPaths && paths) override + void runBPs(ref store, BuiltPaths && paths) override { settings.requireExperimentalFeature(Xp::CaDerivations); RealisedPath::Set realisations; diff --git a/src/nix/registry.cc b/src/nix/registry.cc index b5bdfba95174..1f4f820aca58 100644 --- a/src/nix/registry.cc +++ b/src/nix/registry.cc @@ -227,7 +227,6 @@ struct CmdRegistry : virtual NixMultiCommand settings.requireExperimentalFeature(Xp::Flakes); if (!command) throw UsageError("'nix registry' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 679bdea7781c..278f8e922cab 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -5,26 +5,12 @@ namespace nix { -struct CmdRepl : InstallablesCommand +struct CmdRepl : RawInstallablesCommand { CmdRepl() { evalSettings.pureEval = false; } - void prepare() override - { - if (!settings.isExperimentalFeatureEnabled(Xp::ReplFlake) && !(file) && this->_installables.size() >= 1) { - warn("future versions of Nix will require using `--file` to load a file"); - if (this->_installables.size() > 1) - warn("more than one input file is not currently supported"); - auto filePath = this->_installables[0].data(); - file = std::optional(filePath); - _installables.front() = _installables.back(); - _installables.pop_back(); - } - installables = InstallablesCommand::load(); - } - std::vector files; Strings getDefaultFlakeAttrPaths() override @@ -54,11 +40,23 @@ struct CmdRepl : InstallablesCommand ; } - void run(ref store) override + void runRIs(ref store, std::vector && rawInstallables) override { + if (!settings.isExperimentalFeatureEnabled(Xp::ReplFlake) && !(file) && rawInstallables.size() >= 1) { + warn("future versions of Nix will require using `--file` to load a file"); + if (rawInstallables.size() > 1) + warn("more than one input file is not currently supported"); + auto filePath = rawInstallables[0].data(); + file = std::optional(filePath); + rawInstallables.front() = rawInstallables.back(); + rawInstallables.pop_back(); + } + if (rawInstallables.empty() && (file.has_value() or expr.has_value())) + rawInstallables.push_back("."); + auto state = getEvalState(); auto getValues = [&]()->AbstractNixRepl::AnnotatedValues{ - auto installables = load(); + auto installables = parseInstallables(store, rawInstallables); AbstractNixRepl::AnnotatedValues values; for (auto & installable: installables){ auto what = installable->what(); diff --git a/src/nix/run.cc b/src/nix/run.cc index 6fca6804794a..4d6c5e684195 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -97,7 +97,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment ; } - void run(ref store) override + void runIs(ref store, Installables && installables) override { auto outPaths = Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, installables); @@ -183,7 +183,7 @@ struct CmdRun : InstallableCommand return res; } - void run(ref store) override + void runI(ref store, ref installable) override { auto state = getEvalState(); diff --git a/src/nix/search.cc b/src/nix/search.cc index 2e38f7e4b9a7..a808c9e6b4c3 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -61,7 +61,7 @@ struct CmdSearch : InstallableCommand, MixJSON }; } - void run(ref store) override + void runI(ref store, ref installable) override { settings.readOnlyMode = true; evalSettings.enableImportFromDerivation.setDefault(false); diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 520e8b1ce4bc..4f9ad8eaac7c 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -39,7 +39,7 @@ struct CmdShowDerivation : InstallablesCommand Category category() override { return catUtility; } - void run(ref store) override + void runIs(ref store, Installables && installables) override { auto drvPaths = Installable::toDerivations(store, installables, true); diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index 1431652e01ce..109caafc73f3 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -27,7 +27,7 @@ struct CmdCopySigs : StorePathsCommand return "copy store path signatures from substituters"; } - void run(ref store, StorePaths && storePaths) override + void runSPs(ref store, StorePaths && storePaths) override { if (substituterUris.empty()) throw UsageError("you must specify at least one substituter using '-s'"); @@ -113,7 +113,7 @@ struct CmdSign : StorePathsCommand return "sign store paths"; } - void run(ref store, StorePaths && storePaths) override + void runSPs(ref store, StorePaths && storePaths) override { if (secretKeyFile.empty()) throw UsageError("you must specify a secret key file using '-k'"); @@ -219,7 +219,6 @@ struct CmdKey : NixMultiCommand { if (!command) throw UsageError("'nix key' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/store-copy-log.cc b/src/nix/store-copy-log.cc index d5fab5f2f384..7b6da76397ed 100644 --- a/src/nix/store-copy-log.cc +++ b/src/nix/store-copy-log.cc @@ -26,7 +26,7 @@ struct CmdCopyLog : virtual CopyCommand, virtual InstallablesCommand Category category() override { return catUtility; } - void run(ref srcStore) override + void runIs(ref srcStore, Installables && installables) override { auto & srcLogStore = require(*srcStore); diff --git a/src/nix/store-delete.cc b/src/nix/store-delete.cc index ca43f1530eef..e81788639e0f 100644 --- a/src/nix/store-delete.cc +++ b/src/nix/store-delete.cc @@ -32,7 +32,7 @@ struct CmdStoreDelete : StorePathsCommand ; } - void run(ref store, std::vector && storePaths) override + void runSPs(ref store, StorePaths && storePaths) override { auto & gcStore = require(*store); diff --git a/src/nix/store-repair.cc b/src/nix/store-repair.cc index 8fcb3639a43f..e0e7acc29590 100644 --- a/src/nix/store-repair.cc +++ b/src/nix/store-repair.cc @@ -17,7 +17,7 @@ struct CmdStoreRepair : StorePathsCommand ; } - void run(ref store, std::vector && storePaths) override + void runSPs(ref store, StorePaths && storePaths) override { for (auto & path : storePaths) store->repairPath(path); diff --git a/src/nix/store.cc b/src/nix/store.cc index 44e53c7c7586..2879e03b350a 100644 --- a/src/nix/store.cc +++ b/src/nix/store.cc @@ -18,7 +18,6 @@ struct CmdStore : virtual NixMultiCommand { if (!command) throw UsageError("'nix store' requires a sub-command."); - command->second->prepare(); command->second->run(); } }; diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 0b306cc11597..48f504fba5a7 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -59,7 +59,7 @@ struct CmdVerify : StorePathsCommand ; } - void run(ref store, StorePaths && storePaths) override + void runSPs(ref store, StorePaths && storePaths) override { std::vector> substituters; for (auto & s : substituterUris)