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>
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.
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.
Robert Hensing
Maximilian Bosch
Emily Trau
Yorick