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
the term was hard to discover, as its definition and explanation were in
a very long document lacking an overview section.
search did not help because it occurs so often.
- clarify wording in the definition
- add an overview of installable types
- add "installable" to glossary
- link to definition from occurrences of the term
- be more precise about where store derivation outputs are processed
- installable Nix expressions must evaluate to a derivation
Co-authored-by: Adam Joseph <54836058+amjoseph-nixpkgs@users.noreply.github.com>
The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.
> Motivation
>> Plumbing CLI should be simple
Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)
The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.
>> Scripting
This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!
>> Surprisingly low impact
Only two lines in the tests need changing, showing that the impact of
this is pretty light.
Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:
- Special case the drv path and replace it with it's outputs (what this
gets rid of).
- Turn those output path *back* into the original drv path.
Now we just skip that entire round trip!
> Context
Issue #7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.
`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.
When testing older daemons, the post-build-hook will be run against the
old CLI, so we need the old version of the post-build-hook to support
that use-case.
Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Resolves#7437 for new `nix` commands only by adding a `--stdin` flag.
If paths are also passed on the cli they will be combined with the ones
from standard input.
It is independent of SourceExprCommand, which is about parsing
installables, except for the fact that parsing installables is one of
the many things influenced by read-only mode.
Per the old FIXME, this flag was on too many commands, and mostly
ignored. Now it is just on the commands where it actually has an effect.
Per https://github.com/NixOS/nix/issues/7261, I would still like to get
rid of it entirely, but that is a separate project. This change should
be good with or without doing that.
`nix app` had something called `InstallableDerivedPath` which is
actually the same thing. We go with the later's name because it has
become more correct.
I originally did this change (more hurriedly) as part of #6225 --- a
mini store-only Nix and a full Nix need to share this code. In the first
RFC meeting for https://github.com/NixOS/rfcs/pull/134 we discussed how
some splitting of the massive `installables.cc` could begin prior, as
that is a good thing anyways. (@edolstra's words, not mine!) This would
be one such PR.
- Clarify doc comments, Installables::getCursors returns non-empty
vector
- Use vector::at in Installable::getCursor instead of checking for empty
vector and throwing an exception with error message.
Handle the case where none of getActualAttrPaths() actually exists,
in which case instead of returning an empty vector.
This fixes the case where the user misspells the attribute name in nix
search. Instead of getting no search results, now it shows an error with
suggestions.
Also remove InstallableFlake::getCursor() override since it's now
equivalent to the base class version.
This should be a non-empty set, and so we don't want people doing this
by accident. We remove the zero-0 constructor with a little inheritance
trickery.
`DerivedPath::Built` and `DerivationGoal` were previously using a
regular set with the convention that the empty set means all outputs.
But it is easy to forget about this rule when processing those sets.
Using `OutputSpec` forces us to get it right.
This has the same goal as b13fd4c58e81b2b2b0d72caa5ce80de861622610,but
achieves it in a different way in order to not break
`nix why-depends --derivation`.
This makes 'nix build' work on paths (which will be copied to the
store) and store paths (returned as is). E.g. the following flake
output attributes can be built using 'nix build .#foo':
foo = ./src;
foo = self.outPath;
foo = builtins.fetchTarball { ... };
foo = (builtins.fetchTree { .. }).outPath;
foo = builtins.fetchTree { .. } + "/README.md";
foo = builtins.storePath /nix/store/...;
Note that this is potentially risky, e.g.
foo = /.;
will cause Nix to try to copy the entire file system to the store.
What doesn't work yet:
foo = self;
foo = builtins.fetchTree { .. };
because we don't handle attrsets with an outPath attribute in it yet,
and
foo = builtins.storePath /nix/store/.../README.md;
since result symlinks have to point to a store path currently (rather
than a file inside a store path).
Fixes#7417.
