I wouldn't call it *good* yet, but this will do for now.
- `RetrieveRegularNARSink` renamed to `RegularFileSink` and moved
accordingly because it actually has nothing to do with NARs in
particular.
- its `fd` field is also marked private
- `copyRecursive` introduced to dump a `SourceAccessor` into a
`ParseSink`.
- `NullParseSink` made so `ParseSink` no longer has sketchy default
methods.
This was done while updating #8918 to work with the new
`SourceAccessor`.
Adding the inputPath as a positional feature uncovered this bug.
As positional argument forms were discarded from the `expectedArgs`
list, their closures were not. When the `.completer` closure was then
called, part of the surrounding object did not exist anymore.
This didn't cause an issue before, but with the new call to
`getEvalState()` in the "inputs" completer in nix/flake.cc, a segfault
was triggered reproducibly on invalid memory access to the `this`
pointer, which was always 0.
The solution of splicing the argument forms into a new list to extend
their lifetime is a bit of a hack, but I was unable to get the "nicer"
iterator-based solution to work.
As I complained in
https://github.com/NixOS/nix/pull/6784#issuecomment-1421777030 (a
comment on the wrong PR, sorry again!), #6693 introduced a second
completions mechanism to fix a bug. Having two completion mechanisms
isn't so nice.
As @thufschmitt also pointed out, it was a bummer to go from `FlakeRef`
to `std::string` when collecting flake refs. Now it is `FlakeRefs`
again.
The underlying issue that sought to work around was that completion of
arguments not at the end can still benefit from the information from
latter arguments.
To fix this better, we rip out that change and simply defer all
completion processing until after all the (regular, already-complete)
arguments have been passed.
In addition, I noticed the original completion logic used some global
variables. I do not like global variables, because even if they save
lines of code, they also obfuscate the architecture of the code.
I got rid of them moved them to a new `RootArgs` class, which now has
`parseCmdline` instead of `Args`. The idea is that we have many argument
parsers from subcommands and what-not, but only one root args that owns
the other per actual parsing invocation. The state that was global is
now part of the root args instead.
This did, admittedly, add a bunch of new code. And I do feel bad about
that. So I went and added a lot of API docs to try to at least make the
current state of things clear to the next person.
--
This is needed for RFC 134 (tracking issue #7868). It was very hard to
modularize `Installable` parsing when there were two completion
arguments. I wouldn't go as far as to say it is *easy* now, but at least
it is less hard (and the completions test finally passed).
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
hashBase is ambiguous, since it's not about the digital bases, but about
the format of hashes. Base16, Base32 and Base64 are all character maps
for binary encoding.
Rename the enum Base to HashFormat.
Rename variables of type HashFormat from [hash]Base to hashFormat,
including CmdHashBase::hashFormat and CmdToBase::hashFormat.
Two changes:
* The (probably unintentional) hack to handle paths as tarballs has
been removed. This is almost certainly not what users expect and is
inconsistent with flakeref handling everywhere else.
* The hack to support scp-style Git URLs has been moved to the Git
fetcher, so it's now supported not just by fetchTree but by flake
inputs.
Add a new experimental `impure-env` setting that is a key-value list of
environment variables to inject into FOD derivations that specify the
corresponding `impureEnvVars`.
This allows clients to make use of this feature (without having to change the
environment of the daemon itself) and might eventually deprecate the current
behaviour (pick whatever is in the environment of the daemon) as it's more
principled and might prevent information leakage.
I think it is bad for these reasons when `tests/` contains a mix of
functional and integration tests
- Concepts is harder to understand, the documentation makes a good
unit vs functional vs integration distinction, but when the
integration tests are just two subdirs within `tests/` this is not
clear.
- Source filtering in the `flake.nix` is more complex. We need to
filter out some of the dirs from `tests/`, rather than simply pick
the dirs we want and take all of them. This is a good sign the
structure of what we are trying to do is not matching the structure
of the files.
With this change we have a clean:
```shell-session
$ git show 'HEAD:tests'
tree HEAD:tests
functional/
installer/
nixos/
```
While `nix` has always been respectful towards requests for `NO_COLOR=1`, this change asks represents a new stage of maturity for `nix` - making it also respect quests for `NOCOLOR=1`.
This ideally makes the tool more accessible to folks like me, who are exhausted by guessing whether `NO_COLOR` or `NOCOLOR` is the right environment variable to set.
<3
Behavior change:
Before we only showed uption if the command-specific options were
non-empty. But that is somewhat odd since we also show common options.
Now, we do everything based on the union of both sorts of options (with
hidden-categories filtered, as before).
Implementation change:
The JSON dumping once again includes all options; the filtering of
hidden categories is done in the Nix instead. This is better separation
of "content" vs "presentation", and prepare the way for the HTML manual
vs manpages / `--help` doing different things.
We will soon add a new implemenation so the one for NARs in `archive.cc`
isn't the only one.
Co-Authored-By: Matthew Bauer <mjbauer95@gmail.com>
Co-Authored-By: Carlo Nucera <carlo.nucera@protonmail.com>
Support using nix flakes in paths with spaces or abitrary unicode characters.
This introduces the convention that the path part of the URL should be
percent-encoded when dealing with `path:` urls and not when using
filepaths (following the convention of firefox).
Co-authored-by: Rendal <rasmus@rend.al>
We use the same nested map representation we used for goals, again in
order to save space. We might someday want to combine with `inputDrvs`,
by doing `V = bool` instead of `V = std::set<OutputName>`, but we are
not doing that yet for sake of a smaller diff.
The ATerm format for Derivations also needs to be extended, in addition
to the in-memory format. To accomodate this, we added a new basic
versioning scheme, so old versions of Nix will get nice errors. (And
going forward, if the ATerm format changes again the errors will be even
better.)
`parsedStrings`, an internal function used as part of parsing
derivations in A-Term format, used to consume the final `]` but expect
the initial `[` to already be consumed. This made for what looked like
unbalanced brackets at callsites, which was confusing. Now it consumes
both which is hopefully less confusing.
As part of testing, we also created a unit test for the A-Term format for
regular non-experimental derivations too.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Apply suggestions from code review
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
