wroclaw-git-backups/nix
1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-27 00:11:17 +02:00
Code Activity

Make command infra less stateful and more regular

Browse source 
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.

- `useDefaultInstallables` because `defaultInstallables`. This takes
  over `prepare` for the one case that needs it, and provides enough
  flexiblity to handle `nix repl`'s idiosyncratic migration.

- We can use `ref` instead of `std::shared_ptr`. The former must be
  initialized (so it is like Rust's `Box` rather than `Option<Box>`,
  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.

The "diagnostic ignored `-Woverloaded-virtual`" pragma helps because C++
doesn't like our many `run` methods. In our case, we don't mind the
shadowing it all --- it is *intentional* that the derived class only
provides a `run` method, and doesn't call any of the overridden `run`
methods.

Helps with https://github.com/NixOS/rfcs/pull/134
This commit is contained in:
John Ericson 2023-02-04 12:03:47 -05:00
parent 1b8c13cbba
commit bc23a44c54
30 changed files with 137 additions and 136 deletions
Download patch file Download diff file Expand all files Collapse all files

79
src/libcmd/installables.cc
View file

@ -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()
void RawInstallablesCommand::applyDefaultInstallables(std::vector<std::string> & rawInstallables)
{
installables = load();
}
Installables InstallablesCommand::load()
{
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)
{
installable = parseInstallable(getStore(), _installable);
auto installable = parseInstallable(store, _installable);
run(store, std::move(installable));
}
void BuiltPathsCommand::applyDefaultInstallables(std::vector<std::string> & rawInstallables)
{
if (rawInstallables.empty() && !all)
rawInstallables.push_back(".");
}
}