Leading whitespace after `nix-shell` used to produce an empty argument,
while an empty argument at the end of the line was ignored.
Fix the first issue by consuming the initial whitespace before calling
shellwords; fix the second issue by returning immediately if whitespace
is found at the end of the string instead of checking for an empty
string.
Also throw if quotes aren't terminated.
Single quotes are a basic feature of shell syntax that people expect to
work. They are also more convenient for writing literal code expressions
with less escaping.
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>
It does not belong with the data type itself.
This also materializes the fact that `copyPath` does not do any version
negotiation just just hard-codes "16".
The non-standard interface of these serializers makes it harder to test,
but this is fixed in the next commit which then adds those tests.
Worker Protocol:
Note that the worker protocol already had a serialization for
`BuildResult`; this was added in
a4604f1928. It didn't have any versioning
support because at that time reusable seralizers were not away for the protocol
version. It could thus only be used for new messages also introduced in
that commit.
Now that we do support versioning in reusable serializers, we can expand
it to support all known versions and use it in many more places.
The exist test data becomes the version 1.29 tests: note that those
files' contents are unchanged. 1.28 and 1.27 tests are added to cover
the older code-paths.
The keyered build result test only has 1.29 because the keying was also
added in a4604f19284254ac98f19a13ff7c2216de7fe176; the older
serializations are always used unkeyed.
Serve Protocol:
Conversely, no attempt was made to factor out such a serializer for the
serve protocol, so our work there in this commit for that protocol
proceeds from scratch.
It was some ad-hoc functions to account for versions, while the already
factored-out serializer just supported the latest version.
Now, we can fold that version-specific logic into the factored out one,
and so we do.
It is dead code. It was added in
8e0946e8df as part of the repeated /
enforce-determinism feature, but that was removed in
8fdd156a65.
It is not good because it skips many fields. For testing purposes we
will soon want to add a new one that doesn't skip fields, but we want to
make sure making == sensitive to those fields won't change how Nix
works. Proving in this commit that the old version is dead code achieves
that.
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.
MemoryInputAccessor is an in-memory virtual filesystem that returns
files like <nix/fetchurl.nix>. This removes the need for special hacks
to handle those files.
This will allow us to factor out logic, which is currently scattered
inline, into several reusable instances
The tests are also updated to support versioning. Currently all Worker
and Serve protocol tests are using the minimum version, since no
version-specific serialisers have been created yet. But in subsequent
commits when that changes, we will test individual versions to ensure
complete coverage.
16591eb3cc (diff-19f999107b609d37cfb22c58e7f0bc1cf76edf1180e238dd6389e03cc279b604) (2013) added support for files to doBind
This is work towards allowing users to change the location of chrootRootDir, to, for example, a tmpfs.
inspired by trofi on matrix
> It looks like build sandbox created by nix-daemon runs on the same filesystem, as /nix/store including things like /tmp which makes all small temporary files hit the disk. Is it intentional? If it is is there an easy way to redirect chroot's root to be tmpfs?
dirsInChroot -> pathsInChroot
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.
To start, it is just a clone of the common protocol. But now that we
have the separate protocol implementations, we can add versioning
information without the versions of one protocol leaking into another.
Using the infrastructure from the previous commit, we don't have to
duplicate code for shared behavior.
Motivation: No more perverse incentives. [0] did some awkward things
because the serialisers did not store the version. I don't want anyone
making changes to be pushed towards keeping the serialization logic with
the core data types just because it's easier or the alternative is
tedious.
The actual versioning of the Worker and Serve protocol serialisers
(Common remains unversioned as the underlying mini-protocols are not
versioned) will happen in subsequent commits / PRs.
[0]: fe1f34fa60
Copy the relevant tests to ensure the new interfaces added in the last
commit are tested.
Perhaps I should try to deduplicat these tests some more. However its
not clear how to do that outside of a big ugly C++ macro.
https://github.com/google/googletest/blob/main/docs/advanced.md has some
stuff but it is cumbersome and I didn't figure it out yet.
This is done in a separate commit in order to be sure that the first
commit really didn't change any behavior; if we changed the
implementation and the tests at once, it would be harder to tell whether
or not some behavioral changes slipped in what is supposed to be a "pure
refactor".
Co-Authored-By: Valentin Gagarin <valentin.gagarin@tweag.io>
This introduces some shared infrastructure for our notion of protocols.
We can then define multiple protocols in terms of that notion.
We an also express how particular protocols depend on each other.
For example, we can define a common protocol and a worker protocol,
where the second depends on the first in terms of the data types it can
read and write.
The "serve" protocol can just use the common one for now, but will
eventually need its own machinary just like the worker protocol for
version-aware serialisers
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/
```
the `term` output mode leaves inline HTML around verbatim, while `nroff`
mode (used for `man` pages) does not.
the correct solution would be to pre-render all output with a more
benign tool so we have less liabilities in our own code, but this has to
do for now.
This has been the behaviour before Nix 2.4. It was dropped in a rewrite
in 759947bf72, allowing the creation of
store paths that aren't considered valid by older Nix versions or other
Nix tooling.
Nix 2.4 didn't ship in NixOS until 22.05, and stdenv.mkDerivation in
nixpkgs drops leading periods since April 2022, so it's unlikely anyone
is relying on the current lax behaviour.
Closes#9091.
Change-Id: I4a57bd9899e1b0dba56870ae5a1b680918a18ce9
This was somewhat of a false alarm. The problem was not that the
protocol implementation actually failed to round trip, but that two of
the fields were ignored entirely --- not serialized and deserialized at
all.
For reference, those fields were added in
fa68eb367e.
This reverts commit 5e3986f59c. This
un-implements RFC 92 but fixes the critical bug #9052 which many people
are hitting. This is a decent stop-gap until a minimal reproduction of
that bug is found and a proper fix can be made.
Mostly fixed#9052, but I would like to leave that issue open until we
have a regression test, so I can then properly fix the bug (unbreaking
RFC 92) later.
In #4770 I implemented proper `nix-shell(1)` support for derivations
using `__structuredAttrs = true;`. Back then we decided to introduce two
new environment variables, `NIX_ATTRS_SH_FILE` for `.attrs.sh` and
`NIX_ATTRS_JSON_FILE` for `.attrs.json`. This was to avoid having to
copy these files to `$NIX_BUILD_TOP` in a `nix-shell(1)` session which
effectively meant copying these files to the project dir without
cleaning up afterwords[1].
On last NixCon I resumed hacking on `__structuredAttrs = true;` by
default for `nixpkgs` with a few other folks and getting back to it,
I identified a few problems with the how it's used in `nixpkgs`:
* A lot of builders in `nixpkgs` don't care about the env vars and
assume that `.attrs.sh` and `.attrs.json` are in `$NIX_BUILD_TOP`.
The sole reason why this works is that `nix-shell(1)` sources
the contents of `.attrs.sh` and then sources `$stdenv/setup` if it
exists. This may not be pretty, but it mostly works. One notable
difference when using nixpkgs' stdenv as of now is however that
`$__structuredAttrs` is set to `1` on regular builds, but set to
an empty string in a shell session.
Also, `.attrs.json` cannot be used in shell sessions because
it can only be accessed by `$NIX_ATTRS_JSON_FILE` and not by
`$NIX_BUILD_TOP/.attrs.json`.
I considered changing Nix to be compatible with what nixpkgs
effectively does, but then we'd have to either move $NIX_BUILD_TOP for
shell sessions to a temporary location (and thus breaking a lot of
assumptions) or we'd reintroduce all the problems we solved back then
by using these two env vars.
This is partly because I didn't document these variables back
then (mea culpa), so I decided to drop all mentions of
`.attrs.{json,sh}` in the manual and only refer to `$NIX_ATTRS_SH_FILE`
and `$NIX_ATTRS_JSON_FILE`. The same applies to all our integration tests.
Theoretically we could deprecated using `"$NIX_BUILD_TOP"/.attrs.sh` in
the future now.
* `nix develop` and `nix print-dev-env` don't support this environment
variable at all even though they're supposed to be part of the replacement
for `nix-shell` - for the drv debugging part to be precise.
This isn't a big deal for the vast majority of derivations, i.e.
derivations relying on nixpkgs' `stdenv` wiring things together
properly. This is because `nix develop` effectively "clones" the
derivation and replaces the builder with a script that dumps all of
the environment, shell variables, functions etc, so the state of
structured attrs being "sourced" is transmitted into the dev shell and
most of the time you don't need to worry about `.attrs.sh` not
existing because the shell is correctly configured and the
if [ -e .attrs.sh ]; then source .attrs.sh; fi
is simply omitted.
However, this will break when having a derivation that reads e.g. from
`.attrs.json` like
with import <nixpkgs> {};
runCommand "foo" { __structuredAttrs = true; foo.bar = 23; } ''
cat $NIX_ATTRS_JSON_FILE # doesn't work because it points to /build/.attrs.json
''
To work around this I employed a similar approach as it exists for
`nix-shell`: the `NIX_ATTRS_{JSON,SH}_FILE` vars are replaced with
temporary locations.
The contents of `.attrs.sh` and `.attrs.json` are now written into the
JSON by `get-env.sh`, the builder that `nix develop` injects into the
derivation it's debugging. So finally the exact file contents are
present and exported by `nix develop`.
I also made `.attrs.json` a JSON string in the JSON printed by
`get-env.sh` on purpose because then it's not necessary to serialize
the object structure again. `nix develop` only needs the JSON
as string because it's only written into the temporary file.
I'm not entirely sure if it makes sense to also use a temporary
location for `nix print-dev-env` (rather than just skipping the
rewrite in there), but this would probably break certain cases where
it's relied upon `$NIX_ATTRS_SH_FILE` to exist (prime example are the
`nix print-dev-env` test-cases I wrote in this patch using
`tests/shell.nix`, these would fail because the env var exists, but it
cannot read from it).
[1] https://github.com/NixOS/nix/pull/4770#issuecomment-836799719
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
* document "Import From Derivation"
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: John Ericson <git@JohnEricson.me>
Detected by `gcc` as:
CXX src/libstore/profiles.o
src/libstore/profiles.cc: In function 'void nix::deleteGenerationsGreaterThan(const Path&, GenerationNumber, bool)':
src/libstore/profiles.cc:186:50: warning: comparison of integer expressions of different signedness: 'int' and 'nix::GenerationNumber' {aka 'long unsigned int'} [-Wsign-compare]
186 | for (auto keep = 0; i != gens.rend() && keep < max; ++i, ++keep);
| ~~~~~^~~~~
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>
Without this change, nix build processes will not drop the locks for derivation goals
which have already been built by another process when the current process gets
round to building them. This means the locks are held until the process
terminates.
If there are other nix build processes in a similar state, they will also try to
acquire the same locks when they try to build the same derivation, and so will
wait until the lock holder terminates (which might be a very long time if it has
a lot to build). In some pathological cases, those processes might be holding
their own locks on other derivations due to the same issue, and this can lead to
deadlock.
Resolves#6468
The `-c` flag belongs to `sh` not `nix shell`. As it stands, the command errors with:
```
$ nix shell nixpkgs#gnumake --command sh --command "cd src && make"
sh: --command: invalid option
```
https://github.com/NixOS/nix/pull/8276 was good for readability, but it missed this since that PR used a find/replace script.
The Derivation parser and old ATerm unfortunately leaves few ways to get
nice errors when an old version of Nix encounters a new version of the
format. The most likely scenario for this to occur is with a new client
making a derivation that the old daemon it is communicating with cannot
understand.
The extensions we just created for dynamic derivation deps will add a
version field, solving the problem going forward, but there is still the
issue of what to do about old versions of Nix up to now.
The solution here is to carefully catch the bad error from the daemon
that is likely to indicate this problem, and add some extra context to
it.
There is another "Ugly backwards compatibility hack" in
`remote-store.cc` that also works by transforming an error.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
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>
this removes a lot of noise from the web search, which precludes finding
the actual documentation.
some configuration settings have enough documentation to warrant
individual pages, so the alternative of including full setting
documentation in each command page doesn't make much sense here.
this change technically means that the command line flags to override
settings are "invisible", and not exported as JSON. this may or may not
be desirable. a more explicit approach would be adding a `hidden` field
to the flag's JSON output, but would also require adjusting
post-processing of that JSON for manual rendering.
- Don't assert: Derivation ATerms are not necessarily produced by Nix,
and parsers should always throw graceful errors
- Improve error message from `static void except(..)`, shows both what
we expected and what we actually got.
The intention is that we backport it, and then hopefully a few people
might get slightly better errors if they try out new experimental drv
files (for RFC 92) with an old version of Nix.
Continue with the characterization testing idioms begun in
c70484454f, but this time for unit tests.
Co-authored-by: Andreas Rammhold <andreas@rammhold.de>
Solves 1/3 of the infinite recursion at unknown location meme.
See #8879 for ensuring we always have a trace (for stack overflows)
We might want to re-add this for finding missing location info
*while hacking on that problem only*.
To avoid dealing with an optional `drvPath` (because we might not know
it yet) everywhere, make an `CreateDerivationAndRealiseGoal`. This goal
just builds/substitutes the derivation file, and then kicks of a build
for that obtained derivation; in other words it does the chaining of
goals when the drv file is missing (as can already be the case) or
computed (new case).
This also means the `getDerivation` state can be removed from
`DerivationGoal`, which makes the `BasicDerivation` / in memory case and
`Derivation` / drv file file case closer together.
The map type is factored out for clarity, and because we will soon hvae
a second use for it (`Derivation` itself).
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
We're about to split up `DerivationGoal` a bit. At that point
`makeDerivationGoal` will mean something more specific than it does
today. (Perhaps a future rename will make this clearer.)
On the other hand, the more public `Worker::makeGoal` function will
continue to work exactly as before. So by moving some call sites to use
that instead, we preemptively avoid issues in the next step.
An attrPath prefix of "." indicates no need to try default attrPath prefixes. For example 1nixpkgs#legacyPackages.x86_64-linux.ERROR` searches through
```
trying flake output attribute 'packages.x86_64-linux.legacyPackages.x86_64-linux.ERROR'
using cached attrset attribute ''
trying flake output attribute 'legacyPackages.x86_64-linux.legacyPackages.x86_64-linux.ERROR'
using cached attrset attribute 'legacyPackages.x86_64-linux'
trying flake output attribute 'legacyPackages.x86_64-linux.ERROR'
using cached attrset attribute 'legacyPackages.x86_64-linux'
```
And there is no way to specify that one does not want the automatic
search behavior. Now one can specify
`nixpkgs#.legacyPackages.x86_64-linux.ERROR` to only refer to the rooted
attribute path without any default injection of attribute search path or
system.
This function is now trivial enough that it doesn't need to exist.
`EvalState` can still be initialized with a custom search path, but we
don't have a need to mutate the search path after it has been
constructed, and I don't see why we would need to in the future.
Fixes#8229
Types converted:
- `NixStringContextElem`
- `OutputsSpec`
- `ExtendedOutputsSpec`
- `DerivationOutput`
- `DerivationType`
Existing ones mostly conforming the pattern cleaned up:
- `ContentAddressMethod`
- `ContentAddressWithReferences`
The `DerivationGoal::derivationType` field had a bogus initialization,
now caught, so I made it `std::optional`. I think #8829 can make it
non-optional again because it will ensure we always have the derivation
when we construct a `DerivationGoal`.
See that issue (#7479) for details on the general goal.
`git grep 'Raw::Raw'` indicates the two types I didn't yet convert
`DerivedPath` and `BuiltPath` (and their `Single` variants) . This is
because @roberth and I (can't find issue right now...) plan on reworking
them somewhat, so I didn't want to churn them more just yet.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
If you have a URL that needs to be percent-encoded, such as
`http://localhost:8181/test/+3d.tar.gz`, and try to lock that in a Nix
flake such as the following:
{
inputs.test = { url = "http://localhost:8181/test/+3d.tar.gz"; flake = false; };
outputs = { test, ... }: {
t = builtins.readFile test;
};
}
running `nix flake metadata` shows that the input URL has been
incorrectly double-encoded (despite the flake.lock being correctly
encoded only once):
[...snip...]
Inputs:
└───test: http://localhost:8181/test/%252B3d.tar.gz?narHash=sha256-EFUdrtf6Rn0LWIJufrmg8q99aT3jGfLvd1//zaJEufY%3D
(Notice the `%252B`? That's just `%2B` but percent-encoded again)
With this patch, the double-encoding is gone; running `nix flake
metadata` will show the proper URL:
[...snip...]
Inputs:
└───test: http://localhost:8181/test/%2B3d.tar.gz?narHash=sha256-EFUdrtf6Rn0LWIJufrmg8q99aT3jGfLvd1//zaJEufY%3D
---
As far as I can tell, this happens because Nix already percent-encodes
the URL and stores this as the value of `inputs.asdf.url`.
However, when Nix later tries to read this out of the eval state as a
string (via `getStrAttr`), it has to run it through `parseURL` again to
get the `ParsedURL` structure.
Now, this itself isn't a problem -- the true problem arises when using
`ParsedURL::to_string` later, which then _re-escapes the path_. It is
at this point that what would have been `%2B` (`+`) becomes `%252B`
(`%2B`).
Without the change build with `-D_GLIBCXX_ASSERTIONS` exposes testsuite
assertion:
$ gdb src/libexpr/tests/libnixexpr-tests
Reading symbols from src/libexpr/tests/libnixexpr-tests...
(gdb) break __glibcxx_assert_fail
(gdb) run
(gdb) bt
in std::__glibcxx_assert_fail(char const*, int, char const*, char const*)@plt () from /mnt/archive/big/git/nix/src/libexpr/libnixexpr.so
in std::basic_string_view<char, std::char_traits<char> >::operator[] (this=0x7fffffff56c0, __pos=4)
at /nix/store/r74fw2j8rx5idb0w8s1s6ynwwgs0qmh9-gcc-14.0.0/include/c++/14.0.0/string_view:258
in nix::SearchPath::Prefix::suffixIfPotentialMatch (this=0x7fffffff5780, path=...) at src/libexpr/search-path.cc:15
in nix::SearchPathElem_suffixIfPotentialMatch_partialPrefix_Test::TestBody (this=0x555555a17540) at src/libexpr/tests/search-path.cc:62
As string sizes are usigned types `(a - b) > 0` effectively means
`a != b`. While the intention should be `a > b`.
The change fixes test suite pass.
In the Nix language, given a drv path, we should be able to construct
another string referencing to one of its output. We can do this today
with `(import drvPath).output`, but this only works for derivations we
already have.
With dynamic derivations, however, that doesn't work well because the
`drvPath` isn't yet built: importing it like would need to trigger IFD,
when the whole point of this feature is to do "dynamic build graph"
without IFD!
Instead, what we want to do is create a placeholder value with the right
string context to refer to the output of the as-yet unbuilt derivation.
A new primop in the language, analogous to `builtins.placeholder` can be
used to create one. This will achieve all the right properties. The
placeholder machinery also will match out the `outPath` attribute for CA
derivations works.
In 60b7121d2c we added that type of
placeholder, and the derived path and string holder changes necessary to
support it. Then in the previous commit we cleaned up the code
(inspiration finally hit me!) to deduplicate the code and expose exactly
what we need. Now, we can wire up the primop trivally!
Part of RFC 92: dynamic derivations (tracking issue #6316)
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
`EvalState::mkSingleDerivedPathString` previously contained its own
inverse (printing, rather than parsing) in order to validate what was
parsed. Now that is pulled out into its own separate function:
`EvalState::coerceToSingleDerivedPath`.
In additional that pulled out logic is deduplicated with
`EvalState::mkOutputString` via `EvalState::mkOutputStringRaw`, which is
itself deduplicated (and generalized) with
`DownstreamPlaceholder::mkOutputStringRaw`.
All these changes make the unit tests simpler.
(We would ideally write more unit tests for `mkSingleDerivedPathString`
`coerceToSingleDerivedPath` directly, but we cannot yet do that because
the IO in reading the store path won't work when the dummy store cannot
hold anything. Someday we'll have a proper in-memory store which will
work for this.)
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
std::move(state->data) and data.empty() were called in a loop, and
could run with no other threads intervening. Accessing moved objects
is undefined behavior, and could cause a crash.
Virtual methods are no longer valid once the derived destructor has
run. This means the compiler is free to optimize them to be
non-virtual.
Found using clang-tidy
We want to be able to write down `foo.drv^bar.drv^baz`:
`foo.drv^bar.drv` is the dynamic derivation (since it is itself a
derivation output, `bar.drv` from `foo.drv`).
To that end, we create `Single{Derivation,BuiltPath}` types, that are
very similar except instead of having multiple outputs (in a set or
map), they have a single one. This is for everything to the left of the
rightmost `^`.
`NixStringContextElem` has an analogous change, and now can reuse
`SingleDerivedPath` at the top level. In fact, if we ever get rid of
`DrvDeep`, `NixStringContextElem` could be replaced with
`SingleDerivedPath` entirely!
Important note: some JSON formats have changed.
We already can *produce* dynamic derivations, but we can't refer to them
directly. Today, we can merely express building or example at the top
imperatively over time by building `foo.drv^bar.drv`, and then with a
second nix invocation doing `<result-from-first>^baz`, but this is not
declarative. The ethos of Nix of being able to write down the full plan
everything you want to do, and then execute than plan with a single
command, and for that we need the new inductive form of these types.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
This enables nix to correctly report what will be fetched in the case
that everything is a cache hit.
Note however that if an intermediate build of something which is not
cached could still cause products to end up being substituted if the
intermediate build results in a CA path which is in the cache.
Fixes#8615.
Signed-off-by: Peter Waller <p@pwaller.net>
When receiving a stream of NARs through the ssh-ng protocol, an already
existing path would cause the NAR archive to not be read in the stream,
resulting in trying to parse the NAR as a ValidPathInfo. This results in
the error message:
error: not an absolute path: 'nix-archive-1'
Fixes#6253
Usually this problem is avoided by running QueryValidPaths before
AddMultipleToStore, but can arise when two parallel nix processes gets
the same response from QueryValidPaths. This makes the problem more
prominent when running builds in parallel.
When loading a derivation from a JSON, malformed input would trigger
cryptic "assertion failed" errors. Simply replacing calls to `operator []`
with calls to `.at()` was not enough, as this would cause json.execptions
to be printed verbatim.
Display nice error messages instead and give some indication where the
error happened.
*Before:*
```
$ echo 4 | nix derivation add
error: [json.exception.type_error.305] cannot use operator[] with a string argument with number
$ nix derivation show nixpkgs#hello | nix derivation add
Assertion failed: (it != m_value.object->end()), function operator[], file /nix/store/8h9pxgq1776ns6qi5arx08ifgnhmgl22-nlohmann_json-3.11.2/include/nlohmann/json.hpp, line 2135.
$ nix derivation show nixpkgs#hello | jq '.[] | .name = 5' | nix derivation add
error: [json.exception.type_error.302] type must be string, but is object
$ nix derivation show nixpkgs#hello | jq '.[] | .outputs = { out: "/nix/store/8j3f8j-hello" }' | nix derivation add
error: [json.exception.type_error.302] type must be object, but is string
```
*After:*
```
$ echo 4 | nix derivation add
error: Expected JSON of derivation to be of type 'object', but it is of type 'number'
$ nix derivation show nixpkgs#hello | nix derivation add
error: Expected JSON object to contain key 'name' but it doesn't
$ nix derivation show nixpkgs#hello | jq '.[] | .name = 5' | nix derivation add
error: Expected JSON value to be of type 'string' but it is of type 'number'
$ nix derivation show nixpkgs#hello | jq '.[] | .outputs = { out: "/nix/store/8j3f8j-hello" }' | nix derivation add
error:
… while reading key 'outputs'
error: Expected JSON value to be of type 'object' but it is of type 'string'
```
I haven't checked when this was exactly introduced, but on Nix 2.16 I
realized that the additional lines inserted when using `--precise` are
completely separated from the tree:
nix why-depends /nix/store/ccgr4faaxys39s091qridxg1947lggh4-evcxr-0.14.2 /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0 --precise --extra-experimental-features nix-command
/nix/store/ccgr4faaxys39s091qridxg1947lggh4-evcxr-0.14.2
→ /nix/store/lcf37pgp3rgww67v9x2990hbfwx96c1w-gcc-wrapper-12.2.0
→ /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0
└───bin/evcxr: …':'}.PATH=${PATH/':''/nix/store/lcf37pgp3rgww67v9x2990hbfwx96c1w-gcc-wrapper-12.2.0/bin'':'/':'}…
└───bin/cpp: …k disable=SC2193.[[ "/nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/bin/cpp" = *++ ]] &&…
This is apparently because `std::cout` is buffered and flushed in the
end whereas the rest of the output isn't. The fix is rather simple, just
use `logger->cout` as it's already the case for the rest of the code.
This way we also don't need to insert additional newlines in the `hits`
map since that's something the logger takes care of.
Also added a small test to make sure that the layout of this is somehow
tested to reduce the risk of further regressions here.
Avoid duplicated code, and also avoid "on the fly" path construction
(which makes it harder to keep track of which paths we use).
The factored out code doesn't create the Nix state dir anymore, but this
is fine because other in nix-env and nix-channel does:
- nix-channel: Line 158 in this commit
- nix-env: Line 1407 in this commit
Special-casing the file name is rather ugly, so we shouldn't do
that. So now any {file,http,https} URL is handled by
TarballInputScheme, except for non-flake inputs (i.e. inputs that have
the attribute `flake = false`).
It was initially unclear to me which of these are temporary state for
the verify paths computation, and which of these are the results of that
computation to be used in the rest of the function. Now, it is clear,
and enforced.
We don't care about non-store-paths in there (things like `.links`, are,
in fact, allowed). So let's just skip them up front and be more strongly
typed.
Over the last year or so I've run into several use cases where I need to
parse and/or serialize URLs for use by `builtins.fetchTree` or
`builtins.getFlake`, largely in order to produce _lockfile-like_ files
for lang2nix frameworks or tools which use `nix` internally to drive
builds.
I've gone through the painstaking process of emulating
`nix::FlakeRef::fromAttrs` and `nix::parseFlakeRef` several times with
mixed success; but these are difficult to create and even harder to
maintain if I hope to stay aligned with changes to the real
parser/serializer.
I understand why adding new `builtins` isn't something we want to do
flagrantly. I'm recommending this addition simply because I keep
encountering use cases where I need to parse/serialize these URIs in
`nix` expressions, and I want a reliable solution.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: John Ericson <git@JohnEricson.me>
Will need to do subclass-specific implementations in the next commit.
This isn't because there will be multiple variations of the daemon
protocol (whew!) but because different clients pick and choose different
parts to use.
This makes it more useful. In general, the derivation will be in one
store, and the realisation info is in another.
This also helps us avoid duplication. See how `resolveDerivedPath` is
now simpler because it uses `queryPartialDerivationOutputMap`. In #8369
we get more flavors of derived path, and need more code to resolve them
all, and this problem only gets worse.
The fact that we need a new method to deal with the multiple dispatch is
unfortunate, but this generally relates to the fact that `Store` is a
sub-par interface, too bulky/unwieldy and conflating separate concerns.
Solving that is out of scope of this PR.
This is part of the RFC 92 work. See tracking issue #6316
the original change broke many pre-existing anchor links.
also change formatting of the constants listing slightly:
- the type should not be part of the anchor
- add highlight to the "impure only" note
We were bedeviled by sandboxing issues when working on the layered
store. The problem ended up being that when we have nested nix builds,
and the inner store is inside the build dir (e.g. store is
`/build/nix-test/$name/store`, build dir is `/build`) bind mounts
clobber each other and store paths cannot be found.
After thoroughly cleaning up `local-derivation-goal.cc`, we might be
able to make that work. But that is a lot of work. For now, we just fail
earlier with a proper error message.
Finally, test this: nested sandboxing without the problematic store dir
should work, and with should fail with the expected error message.
Co-authored-by: Dylan Green <67574902+cidkidnix@users.noreply.github.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
- Better types
- Own header / C++ file pair
- Test factored out methods
- Pass parsed thing around more than strings
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Whereas `ContentAddressWithReferences` is a sum type complex because different
varieties support different notions of reference, and
`ContentAddressMethod` is a nested enum to support that,
`ContentAddress` can be a simple pair of a method and hash.
`ContentAddress` does not need to be a sum type on the outside because
the choice of method doesn't effect what type of hashes we can use.
Co-Authored-By: Cale Gibbard <cgibbard@gmail.com>
'resolvedRef' was incorrect, since a resolved ref is one after
registry resolution, which may still be unlocked (e.g. 'nixpkgs' ->
'github:NixOS/nixpkgs').
If we call `adjustLoc`, the global variable `prev_yylloc` is shared
between threads and racy.
Currently, nix itself does not concurrently parsing files, but this is
helpful for libexpr users. (The parser is thread-safe except this.)
When explicitly requested by the caller, as suggested in the meeting
(https://github.com/NixOS/nix/pull/8090#issuecomment-1531139324)
> @edolstra: { toPath } vs { fromPath } is too implicit
I've opted for the `inputAddressed = true` requirement, because it
we did not agree on renaming the path attributes.
> @roberth: more explicit
> @edolstra: except for the direction; not immediately clear in which direction the rewriting happens
This is in fact the most explicit syntax and a bit redundant, which is
good, because that redundancy lets us deliver an error message that
reminds expression authors that CA provides a better experience to
their users.
This is done in roughly the same way builtin functions are documented.
Also auto-link experimental features for primops, subsuming PR #8371.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
* nix flake check: improve error message if overlay is not a lambda
Suppose you have an overlay like this
{
inputs = { /* ... */ };
outputs = { flake-utils, ... }: flake-utils.lib.eachDefaultSystem
(system: {
overlays.default = final: prev: {
};
});
}
then `nix flake check` (correctly) fails because `overlays` are supposed
to have the structure `overlays.<name> = final: prev: exp`. However, the
error-message is a little bit counter-intuitive:
error: overlay does not take an argument named 'final'
While one might guess where the error actually comes from because the
trace above says `… while checking the overlay 'overlays.x86_64-linux'`
this is still pretty confusing because it complains about an argument
not being named `final` even though that's evidently the case.
With this change, the error-message actually makes it clear what's
wrong:
[ma27@carsten:~/Projects/nix/tmp]$ nix flake check --extra-experimental-features 'nix-command flakes' path:$(pwd)
error:
… while checking flake output 'overlays'
at /nix/store/clgblnxx003hyrq8qkz5ab6kgqkck6qc-source/flake.nix:4:5:
3| outputs = { ... }: {
4| overlays.x86_64-linux.snens = final: prev: {
| ^
5| kek = throw "snens";
… while checking the overlay 'overlays.x86_64-linux'
at /nix/store/clgblnxx003hyrq8qkz5ab6kgqkck6qc-source/flake.nix:4:5:
3| outputs = { ... }: {
4| overlays.x86_64-linux.snens = final: prev: {
| ^
5| kek = throw "snens";
error: overlay is not a lambda, but a set instead
I got very confused trying to keep all the `first` and `second` straight
reading the code, *especially* as there is also another `(boolean,
string)` pair type also being used.
Named fields is much better.
There are other cleanups that we can do (for example, the existing
TODO), but we can do them later. Doing them now would just make this
harder to review.
- Improved API docs from comment
- Exit codes are for `nix-build`, not just `nix-store --release`
- Make note in tests so the magic numbers are not surprising
Picking up where #8387 left off.
Previously it was not possible to open a local store when its database is on a read-only filesystem. Obviously a store on a read-only filesystem cannot be modified, but it would still be useful to be able to query it.
This change adds a new read-only setting to LocalStore. When set to true, Nix will skip operations that fail when the database is on a read-only filesystem (acquiring big-lock, schema migration, etc), and the store database will be opened in immutable mode.
Co-authored-by: Ben Radford <benradf@users.noreply.github.com>
Co-authored-by: cidkidnix <cidkidnix@protonmail.com>
Co-authored-by: Dylan Green <67574902+cidkidnix@users.noreply.github.com>
Co-authored-by: John Ericson <git@JohnEricson.me>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
If the garbage collector has acquired the global GC lock, but hasn't
created the GC socket yet, then a client attempting to connect would
get ENOENT. Note that this only happens when the GC runs for the first
time on a machine. Subsequently clients will get ECONNREFUSED which
was already handled.
Fixes#7370.
Pass this around instead of `Source &` and `Sink &` directly. This will
give us something to put the protocol version on once the time comes.
To do this ergonomically, we need to expose `RemoteStore::Connection`,
so do that too. Give it some more API docs while we are at it.
The motivation is exactly the same as for the last commit. In addition,
this anticipates us formally defining separate serialisers for the serve
protocol.
See API docs on that struct for why. The pasing as as template argument
doesn't yet happen in that commit, but will instead happen in later
commit.
Also make `WorkerOp` (now `Op`) and enum struct. This led us to catch
that two operations were not handled!
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This is generally a fine practice: Putting implementations in headers
makes them harder to read and slows compilation. Unfortunately it is
necessary for templates, but we can ameliorate that by putting them in a
separate header. Only files which need to instantiate those templates
will need to include the header with the implementation; the rest can
just include the declaration.
This is now documenting in the contributing guide.
Also, it just happens that these polymorphic serializers are the
protocol agnostic ones. (Worker and serve protocol have the same logic
for these container types.) This means by doing this general template
cleanup, we are also getting a head start on better indicating which
code is protocol-specific and which code is shared between protocols.
- Greatly expand API docs
- Clean up code in misc ways
- Instead of a complicated single loop on generations, do different
operations in successive subsequent steps.
- Avoid `ref` in one place where `&` is fine
- Just return path instead of mutating an argument in `makeName`
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Rather than doing `allowEmpty` as boolean, have separate types and use
`std::optional`. This makes it harder to forget the possibility of an
empty path.
The `build-hook` setting was categorized as a `PathSetting`, but
actually it was split into arguments. No good! Now, it is
`Setting<Strings>` which actually reflects what it means and how it is
used.
Because of the subtyping, we now also have support for
`Setting<std::optional<String>>` in general. I imagine this can be used
to clean up many more settings also.
The code accidentally conflated `std::string::size_type` and `long unsigned int`.
This was fine on 64bits machines where they are apparently the same in
practice, but not on 32bits. Fix that by using `std::string::size_type`
everywhere.
A library shouldn't require changes to the caller's argument handling,
especially if it doesn't have to, and indeed we don't have to.
This changes the lookup order to prioritize the hardcoded path to nix
if it exists. The static executable still finds itself through /proc
and the like.
Introduce what substituters "are" in the configuration option entry.
Remove arbitrary line breaks for easier editing in the future.
Link glossary some more.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: John Ericson <git@JohnEricson.me>
The remaining constructor RegisterPrimOp::RegisterPrimOp(Info && info)
allows specifying the documentation in .args and .doc members of the
Info structure.
Commit 8ec1ba0210 removed all uses of the removed constructor in the
nix binary. Here, we remove the constructor completely as well as its
use in a plugin test. According to #8515, we didn't promis to maintain
compatibility with external plugins.
Fixes#8515
`filesystem.cc` is the only place where `createSymlink()` is used with three arguments:
in the definition of `replaceSymlink()` with three parameters that _is not used at all_.
Closes#8495
Previously, for tarball flakes, we recorded the original URL of the
tarball flake, rather than the URL to which it ultimately
redirects. Thus, a flake URL like
http://example.org/patchelf-latest.tar that redirects to
http://example.org/patchelf-<revision>.tar was not really usable. We
couldn't record the redirected URL, because sites like GitHub redirect
to CDN URLs that we can't rely on to be stable.
So now we use the redirected URL only if the server returns the
`x-nix-is-immutable` or `x-amz-meta-nix-is-immutable` headers in its
response.
This will allow documenting them (in later commits).
Note that we keep the old constructor even if it is no longer used by
Nix code, because it is used in tests/plugins/plugintest.cc, which
suggests that it might be used by some external plugin.
This is necessary when we're in a chroot environment, where the
process root is not the same as the root of the mount namespace
(e.g. in nixos-enter).
Fixes#7602.
Currently `fromTOML` throws an exception when encountering a timestamp
since the Nix language lacks a way to represent them.
This patch changes this beaviour and makes `fromTOML` parse timestamps as
attrsets of the format
{ _type = "timestamp"; value = "1979-05-27T07:32:00Z"; }
This is guarded by an experimental feature flag to leave room for iterating on the representation.
* Document manual migration for use-xdg-base-directories
As there's currently no automatic migration for use-xdg-base-directories
option, add instructions for manual migration to the option's
description.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Add support to --list-generations
as another way to say
nix-env --profile /nix/var/nix/profiles/per-user/$USER/channels --list-generations
the way we did for nix-channel --rollback [generation id]
And fix a test failure in the sandbox due to /home
existing on Darwin but not being accessible in the sandbox since it's a
symlink to /System/Volumes/Data/home, see
https://github.com/NixOS/nix/actions/runs/4205378453/jobs/7297384658#step:6:2127:
C++ exception with description "error: getting status of /home/schnitzel/darmstadt/pommes: Operation not permitted" thrown in the test body.
On Linux this wasn't a problem because there /home doesn't exist in the sandbox
The primop `builtins.replaceStrings` currently always strictly evaluates the
replacement strings, however time and space are wasted for their computation
if the corresponding pattern do not occur in the input string. This commit
makes the evaluation of the replacement strings lazy by deferring their
evaluation to when the corresponding pattern are matched and memoize the result
for efficient retrieval on subsequent matches.
The testcases for replaceStrings was updated to check for lazy evaluation
of the replacements. A note was also added in the release notes to
document the behavior change.
When encountering a build error, Nix moves the output paths out of the
chroot into their final location (for “easier debugging of build
failures”). However this was broken for chroot stores as it was moving
it to the _logical_ location, not the _physical_ one.
Fix it by moving to the physical (_real_) location.
Fix https://github.com/NixOS/nix/issues/8395
The `hashed-mirrors` option did use to have this default value,
but it was removed and re-added with an empty default value.
As the autogenerated docs show the (actual) default values from code,
remove this incorrect reference from the docs.
I was updating my nix.conf settings after a few years and noticed this.
This adds a new configuration option to Nix, `always-allow-substitutes`,
whose effect is simple: it causes the `allowSubstitutes` attribute in
derivations to be ignored, and for substituters to always be used.
This is extremely valuable for users of Nix in CI, where usually
`nix-build-uncached` is used. There, derivations which disallow
substitutes cause headaches as the inputs for building already-cached
derivations need to be fetched to spuriously rebuild some simple text
file.
This option should be a good middle-ground, since it doesn't imply
rebuilding the world, such as the approach I took in
https://github.com/NixOS/nixpkgs/pull/221048
Using abstract types like can help cut down on compilation time, both
from scratch, and especially incremental builds during development. The
idea is that `worker-protocol.hh` can declare all the (de)serializers, but
only again abstract types; when code needs to use some (de)serializers, it can
include headers just for the data types it needs to (de)serialize.
`store-api.hh` in particular is a bit of a sledgehammer, and the data
types we want to serialize have their own headers.
This is the more typically way to do [Argument-dependent
lookup](https://en.cppreference.com/w/cpp/language/adl)-leveraging
generic serializers in C++. It makes the relationship between the `read`
and `write` methods more clear and rigorous, and also looks more
familiar to users coming from other languages that do not have C++'s
libertine ad-hoc overloading.
I am returning to this because during the review in
https://github.com/NixOS/nix/pull/6223, it came up as something that
would make the code easier to read --- easier today hopefully already,
but definitely easier if we were have multiple codified protocols with
code sharing between them as that PR seeks to accomplish.
If I recall correctly, the main criticism of this the first time around
(in 2020) was that having to specify the type when writing, e.g.
`WorkerProto<MyType>::write`, was too verbose and cumbersome. This is
now addressed with the `workerProtoWrite` wrapper function.
This method is also the way `nlohmann::json`, which we have used for a
number of years now, does its serializers, for what its worth.
This reverts commit 45a0ed82f0. That
commit in turn reverted 9ab07e99f5.
This is good in general, but in particular ensures when we heavily
refactor it in the next commit there is less likelihood for an
unintentional change in behavior to sneak in.
These items are not templates, and they declared in
`worker-protocol.hh`; therefore they should live in a
`worker-protocol.cc`.
Anything else needlessly diverges from convention. After all, it is not
like this code is only used in `remote-store.cc`; it is also used in
`daemon.cc`. There is no good reason to place it with the client
implementation or the server implementation when it used equally by
both.
- If the element comes from a flake, print the full flakeref (with the
fragment part) and not just the reference to the flake itself
- If the element doesn't come from a flake, print its store path(s)
This is a bit too verbose, but has the advantages of being correct (and
not crashing), so it's strictly better than the previous situation
Fix https://github.com/NixOS/nix/issues/8284
They were improperly added in 8a93b5a551.
They were not `.gitignore`d because they were stale in that commit --
build artifacts no longer used that name by then and so `.gitignore` was
updated accordingly.
As discussed in #7417, it would be good to make more string values work
as installables. That is to say, if an installable refers to a value,
and the value is a string, it used to not work at all, since #7484, it
works somewhat, and this PR make it work some more.
The new cases that are added for `BuiltPath` contexts:
- Fixed input- or content-addressed derivation:
```
nix-repl> hello.out.outPath
"/nix/store/jppfl2bp1zhx8sgs2mgifmsx6dv16mv2-hello-2.12"
nix-repl> :p builtins.getContext hello.out.outPath
{ "/nix/store/c7jrxqjhdda93lhbkanqfs07x2bzazbm-hello-2.12.drv" = { outputs = [ "out" ]; }; }
The string matches the specified single output of that derivation, so
it should also be valid.
- Floating content-addressed derivation:
```
nix-repl> (hello.overrideAttrs (_: { __contentAddressed = true; })).out.outPath
"/1a08j26xqc0zm8agps8anxpjji410yvsx4pcgyn4bfan1ddkx2g0"
nix-repl> :p builtins.getContext (hello.overrideAttrs (_: { __contentAddressed = true; })).out.outPath
{ "/nix/store/qc645pyf9wl37c6qvqzaqkwsm1gp48al-hello-2.12.drv" = { outputs = [ "out" ]; }; }
```
The string is not a path but a placeholder, however it also matches
the context, and because it is a CA derivation we have no better
option. This should also be valid.
We may also want to think about richer attrset based values (also
discussed in that issue and #6507), but this change "completes" our
string-based building blocks, from which the others can be desugared
into or at least described/document/taught in terms of.
Progress towards #7417
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Fixes#8309
This regression was because both `CmdDevelop` and `CmdPrintDevEnv` were
switched to be `InstallableValueCommand` subclasses, but actually
neither should have been.
The `nixpkgsFlakeRef` method should indeed not be on the base
installable class, because "flake refs" and "nixpkgs" are not
installable-wide notions, but that doesn't mean these commands should
only accept installable values.
This fixes a bug in commands like `nix eval' which would emit invalid attribute
sets if they contained reserved keywords such as "assert", "let", etc.
These keywords will not be quoted when printed, making them valid expressions.
All keywords recognized by the lexer are quoted except "or", which does not
require quotation.
it's probably better not to show the manifest file documentation in the
command-specific pages, because these are implementation details that are not really practically useful.
this means no additional hassle for building the manual, but clutters
the table of contents a bit.
Previously, we relied on the `shutdown()` function to terminate `accept()`
calls on a listening socket. However, this approach did not work on macOS as
the waiting `accept()` call is not considered a connected socket, resulting in
an `ENOTCONN` error. Instead, we now close the listening socket to terminate
the `accept()` call.
Additionally, we fixed a resource management issue where we set the
`daemonSocket` variable to -1, triggering resource cleanup and causing the
`stopDaemon` function to be called twice. This resulted in errors as the socket
was already closed by the time the second `stopDaemon` call was made. Instead of
setting `daemonSocket` to -1, we now release the socket using the `release()`
method on a unique pointer. This properly transfers ownership and allows for
correct resource cleanup.
These changes ensure proper behavior and resource management for the
recursive-nix feature on macOS.
Motivation
`PathSet` is not correct because string contexts have other forms
(`Built` and `DrvDeep`) that are not rendered as plain store paths.
Instead of wrongly using `PathSet`, or "stringly typed" using
`StringSet`, use `std::std<StringContextElem>`.
-----
In support of this change, `NixStringContext` is now defined as
`std::std<StringContextElem>` not `std:vector<StringContextElem>`. The
old definition was just used by a `getContext` method which was only
used by the eval cache. It can be deleted altogether since the types are
now unified and the preexisting `copyContext` function already suffices.
Summarizing the previous paragraph:
Old:
- `value/context.hh`: `NixStringContext = std::vector<StringContextElem>`
- `value.hh`: `NixStringContext Value::getContext(...)`
- `value.hh`: `copyContext(...)`
New:
- `value/context.hh`: `NixStringContext = std::set<StringContextElem>`
- `value.hh`: `copyContext(...)`
----
The string representation of string context elements no longer contains
the store dir. The diff of `src/libexpr/tests/value/context.cc` should
make clear what the new representation is, so we recommend reviewing
that file first. This was done for two reasons:
Less API churn:
`Value::mkString` and friends did not take a `Store` before. But if
`NixStringContextElem::{parse, to_string}` *do* take a store (as they
did before), then we cannot have the `Value` functions use them (in
order to work with the fully-structured `NixStringContext`) without
adding that argument.
That would have been a lot of churn of threading the store, and this
diff is already large enough, so the easier and less invasive thing to
do was simply make the element `parse` and `to_string` functions not
take the `Store` reference, and the easiest way to do that was to simply
drop the store dir.
Space usage:
Dropping the `/nix/store/` (or similar) from the internal representation
will safe space in the heap of the Nix programming being interpreted. If
the heap contains many strings with non-trivial contexts, the saving
could add up to something significant.
----
The eval cache version is bumped.
The eval cache serialization uses `NixStringContextElem::{parse,
to_string}`, and since those functions are changed per the above, that
means the on-disk representation is also changed.
This is simply done by changing the name of the used for the eval cache
from `eval-cache-v4` to eval-cache-v5`.
----
To avoid some duplication `EvalCache::mkPathString` is added to abstract
over the simple case of turning a store path to a string with just that
string in the context.
Context
This PR picks up where #7543 left off. That one introduced the fully
structured `NixStringContextElem` data type, but kept `PathSet context`
as an awkward middle ground between internal `char[][]` interpreter heap
string contexts and `NixStringContext` fully parsed string contexts.
The infelicity of `PathSet context` was specifically called out during
Nix team group review, but it was agreeing that fixing it could be left
as future work. This is that future work.
A possible follow-up step would be to get rid of the `char[][]`
evaluator heap representation, too, but it is not yet clear how to do
that. To use `NixStringContextElem` there we would need to get the STL
containers to GC pointers in the GC build, and I am not sure how to do
that.
----
PR #7543 effectively is writing the inverse of a `mkPathString`,
`mkOutputString`, and one more such function for the `DrvDeep` case. I
would like that PR to have property tests ensuring it is actually the
inverse as expected.
This PR sets things up nicely so that reworking that PR to be in that
more elegant and better tested way is possible.
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
In other words, use a plain `ContentAddress` not
`ContentAddressWithReferences` for `DerivationOutput::CAFixed`.
Supporting fixed output derivations with (fixed) references would be a
cool feature, but it is out of scope at this moment.
Recently, I encountered the "NAR info file 'xxxx' is corrupt" error
with my binary cache. The message is not helpful in determining, which
kind of corruption happened. The file, fetched with curl, looked
reasonably.
This commit adds more information to the error message, which should
allow debugging and hopefully fixing the problem.
