Need to decouple parsing from actually opening a store for Machine
configs.
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
In particular `local://<path>` and `unix://` (without any path) now
work, and mean the same things as `local` and `daemon`, respectively. We
thus now have the opportunity to desguar `local` and `daemon` early.
This will allow me to make a change to
https://github.com/NixOS/nix/pull/9839 requested during review to
desugar those earlier.
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
1. Hydra currently queries for multiple path infos at once, so let us
make a connection item for that.
2. The minimum of the two versions should always be used, see #9584.
(The issue remains open because the daemon protocol needs to be
likewise updated.)
The JSON format no longer uses the legacy ATerm `r:` prefixing nonsese,
but separate fields.
Progress on #9866
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
On several occasions I've found myself confused when trying to delete
a store path, because I am told it's still alive, but
nix-store --query --roots doesn't show anything. Let's save future
users this confusion by mentioning that a path might be alive due to
having referrers, not just roots.
In addition:
- Take the opportunity to add a bunch more missing hyperlinks, too.
- Remove some glossary entries that are now subsumed by dedicated pages.
We used to not be able to do this without breaking link fragments, but
now we can, so pick up where we left off.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This is useful for diagnosing whether an evaluation is copying large
paths to the store. Example:
$ nix build .#packages.x86_64-linux.default --large-path-warning-threshold 1000000
warning: copied large path '/home/eelco/Dev/nix-master/' to the store (6271792 bytes)
warning: copied large path '«github:NixOS/nixpkgs/b550fe4b4776908ac2a861124307045f8e717c8e?narHash=sha256-7kkJQd4rZ%2BvFrzWu8sTRtta5D1kBG0LSRYAfhtmMlSo%3D»/' to the store (155263768 bytes)
warning: copied large path '«github:libgit2/libgit2/45fd9ed7ae1a9b74b957ef4f337bc3c8b3df01b5?narHash=sha256-oX4Z3S9WtJlwvj0uH9HlYcWv%2Bx1hqp8mhXl7HsLu2f0%3D»/' to the store (22175416 bytes)
warning: copied large path '/nix/store/z985088mcd6w23qwdlirsinnyzayagki-source' to the store (5885872 bytes)
the old `copyFile` was just a wrapper that was calling the `copy`
function. This wrapper function is removed and the `copy` function is
renamed to `copyFile`.
Building derivations is a lot harder, but the downloading goals is
portable enough.
The "common channel" code is due to Volth. I wonder if there is a way we
can factor it out into separate functions / files to avoid some
within-function CPP.
Co-authored-by: volth <volth@volth.com>
Sometimes we read a directory with children we cannot stat. It's a pitty
we even try to stat at all (wasteful) in the `DT_UNKNOWN` case, but at
least this should get rid of the failure.
This makes for shorter and more portable code.
The only tricky part is catching exceptions: I just searched for near by
`catch (Error &)` or `catch (SysError &)` and adjusted them to `catch
(std::filesystem::filesystem_error &)` according to my human judgement.
Good for windows portability; will help @siddhantk232 with his GSOC
project.
Now that SourcePath uses a SourceAccessor instead of an InputAccessor,
we can use it in function signatures instead of passing a
SourceAccessor and CanonPath separately.
Make sure that `extraSandboxProfile` is set before we check whether it's
empty or not (in the `sandbox=true` case).
Also adds a test case for this.
Co-Authored-By: Artemis Tosini <lix@artem.ist>
Co-Authored-By: Eelco Dolstra <edolstra@gmail.com>
Fix formatting violations, update blacklist to reflect moved files.
PR #10556 passed CI before the new formating rules were added, and our
CI has the race condition of allowing old results, resulting in master
getting broken.
Windows now has some basic Unix Domain Socket support, see
https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
Building `nix daemon` on Windows I've left for later, because the daemon
currently forks per connection but this is not an option on Windows. But
we can get the client part working right away.
With Linux kernel >=6.6 & glibc 2.39 a `fchmodat2(2)` is available that
isn't filtered away by the libseccomp sandbox.
Being able to use this to bypass that restriction has surprising results
for some builds such as lxc[1]:
> With kernel ≥6.6 and glibc 2.39, lxc's install phase uses fchmodat2,
> which slips through 9b88e52846/src/libstore/build/local-derivation-goal.cc (L1650-L1663).
> The fixupPhase then uses fchmodat, which fails.
> With older kernel or glibc, setting the suid bit fails in the
> install phase, which is not treated as fatal, and then the
> fixup phase does not try to set it again.
Please note that there are still ways to bypass this sandbox[2] and this is
mostly a fix for the breaking builds.
This change works by creating a syscall filter for the `fchmodat2`
syscall (number 452 on most systems). The problem is that glibc 2.39
and seccomp 2.5.5 are needed to have the correct syscall number available
via `__NR_fchmodat2` / `__SNR_fchmodat2`, but this flake is still on
nixpkgs 23.11. To have this change everywhere and not dependent on the
glibc this package is built against, I added a header
"fchmodat2-compat.hh" that sets the syscall number based on the
architecture. On most platforms its 452 according to glibc with a few
exceptions:
$ rg --pcre2 'define __NR_fchmodat2 (?!452)'
sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h
58:#define __NR_fchmodat2 1073742276
sysdeps/unix/sysv/linux/mips/mips64/n32/arch-syscall.h
67:#define __NR_fchmodat2 6452
sysdeps/unix/sysv/linux/mips/mips64/n64/arch-syscall.h
62:#define __NR_fchmodat2 5452
sysdeps/unix/sysv/linux/mips/mips32/arch-syscall.h
70:#define __NR_fchmodat2 4452
sysdeps/unix/sysv/linux/alpha/arch-syscall.h
59:#define __NR_fchmodat2 562
I tested the change by adding the diff below as patch to
`pkgs/tools/package-management/nix/common.nix` & then built a VM from
the following config using my dirty nixpkgs master:
{
vm = { pkgs, ... }: {
virtualisation.writableStore = true;
virtualisation.memorySize = 8192;
virtualisation.diskSize = 12 * 1024;
nix.package = pkgs.nixVersions.nix_2_21;
};
}
The original issue can be triggered via
nix build -L github:nixos/nixpkgs/d6dc19adbda4fd92fe9a332327a8113eaa843894#lxc \
--extra-experimental-features 'nix-command flakes'
however the problem disappears with this patch applied.
Closes#10424
[1] https://github.com/NixOS/nixpkgs/issues/300635#issuecomment-2031073804
[2] https://github.com/NixOS/nixpkgs/issues/300635#issuecomment-2030844251
With Nix 2.3, it was possible to pass a subpath of a store path to
exportReferencesGraph:
with import <nixpkgs> {};
let
hello = writeShellScriptBin "hello" ''
echo ${toString builtins.currentTime}
'';
in
writeClosure [ "${hello}/bin/hello" ]
This regressed with Nix 2.4, with a very confusing error message, that
presumably indicates it was unintentional:
error: path '/nix/store/3gl7kgjr4pwf03f0x70dgx9ln3bhl7zc-hello/bin/hello' is not in the Nix store
At this point many features are stripped out, but this works:
- Can run libnix{util,store,expr} unit tests
- Can run some Nix commands
Co-Authored-By volth <volth@volth.com>
Co-Authored-By Brian McKenna <brian@brianmckenna.org>
* add intermediate variables and clarifying comments
Co-authored-by: Alexander Groleau <alex@proof.construction>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
https://github.com/NixOS/nix/pull/10456 fixed the addition of symlink
store paths to the sandbox, but also made it so that the hardcoded
sandbox paths (like `/etc/hosts`) were now bind-mounted without
following the possible symlinks. This made these files unreadable if
there were symlinks (because the sandbox would now contain a symlink to
an unreachable file rather than the underlying file).
In particular, this broke FOD derivations on NixOS as `/etc/hosts` is a
symlink there.
Fix that by canonicalizing all these hardcoded sandbox paths before
adding them to the sandbox.
Like always declining; local builds only, as can be inferred from the
docs. (Not worth spending too many words on this pretty obvious
behavior, I think. Also, plans to remove it? https://github.com/NixOS/nix/issues/1221)
This requires moving resolveSymlinks() into SourceAccessor. Also, it
requires LocalStoreAccessor::maybeLstat() to work on parents of the
store (to avoid an error like "/nix is not in the store").
Fixes#10375.
Bind-mounting symlinks is apparently not possible, which is why the
thing was failing.
Fortunately, symlinks are small, so we can fallback to copy them at no cost.
Fix https://github.com/NixOS/nix/issues/9579
Co-authored-by: Artturin <Artturin@artturin.com>
Forcing a conditional include, vs making the headers content
conditional, I think is more maintainable.
It is also how the other platform-specific headers (like
`namespaces.hh`) have been adapted.
This introduces new utility functions to get elements from JSON — in an ergonomic way and with nice error messages if the expected type does not match.
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
This splits files and adds new identifiers in preperation for supporting
windows, but no Windows-specific code is actually added yet.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This function is nice for more than `PosixSourceAccessor`. We can make a
few things simpler with it.
Note that the error logic slightly changes in some of the call sites, in
that we also count `ENOTDIR` and not just `ENOENT` as not having the
file, but that should be fine.
Fixes an instance of
nix: src/libutil/util.cc:139: nix::Path nix::canonPath(PathView, bool): Assertion `path != ""' failed.
... which I've been getting in one of my shells for some reason.
I have yet to find out why TMPDIR was empty, but it's no reason for
Nix to break.
In a daemon-based Nix setup, some options cannot be overridden by a
client unless the client's user is considered trusted.
Currently, if an untrusted user tries to override one of those
options, we are silently ignoring it.
This can be pretty confusing in certain situations.
e.g. a user thinks he disabled the sandbox when in reality he did not.
We are now sending a warning message letting know the user some options
have been ignored.
Related to #1761.
This is a cherry-pick of 9e0f5f803f.
The above commit has been reverted by
a59e77d9e5 to prevent spamming warnings
with experimental features, but these are now totally ignored on the
daemon side, so there's no reason for the revert any more.
It is possible to exfiltrate a file descriptor out of the build sandbox
of FODs, and use it to modify the store path after it has been
registered.
To avoid that issue, don't register the output of the build, but a copy
of it (that will be free of any leaked file descriptor).
Instead, serialize as NAR and send that over, then rehash sever side.
This is alorithmically simpler, but comes at the cost of a newer
parameter to `Store::addToStoreFromDump`.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Part of RFC 133
Extracted from our old IPFS branches.
Co-Authored-By: Matthew Bauer <mjbauer95@gmail.com>
Co-Authored-By: Carlo Nucera <carlo.nucera@protonmail.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Florian Klink <flokli@flokli.de>
"hash type" -> "hash algorithm" in all comments, documentation, and
messages.
ht -> ha, [Hh]ashType -> [HhashAlgo] for all local variables and
function arguments. No API change is made.
Continuation of 5334c9c792 and 837b889c41.
The sandbox rule `(allow network* (local ip))` doesn't do what it
implies. Adding this rule permits all network traffic. We should be
matching on (remote ip "localhost:*")` instead.
No outward facing behavior is changed.
Older methods with same names that operate on on method + algo pair (for
old-style `<method>:algo`) are renamed to `*WithAlgo`.)
The functions are unit-tested in the same way the names for the hash
algorithms are tested.
* reword description of the `cores` setting
- be precise about the `builder` executable
- clearly distinguish between `builder` and job parallelism
- clarify the role of `mkDerivation` in the example
- remove prose for the default, it's shown programmatically
- mention relation to `max-jobs`
- move all reference documentation to the `builders` configuration setting
- reword documentation on machine specification, add examples
- disable showing the default value, as it rendered as `@/dummy/machines`, which is wrong
- highlight the examples
- link to the configuration docs for distributed builds
- builder -> build machine
Co-authored-by: Janik H <janik@aq0.de>
When I started contributing to Nix, I found the mix of definitions and
names in `fmt.hh` to be rather confusing, especially the small
difference between `hintfmt` and `hintformat`. I've renamed many classes
and added documentation to most definitions.
- `formatHelper` is no longer exported.
- `fmt`'s documentation is now with `fmt` rather than (misleadingly)
above `formatHelper`.
- `yellowtxt` is renamed to `Magenta`.
`yellowtxt` wraps its value with `ANSI_WARNING`, but `ANSI_WARNING`
has been equal to `ANSI_MAGENTA` for a long time. Now the name is
updated.
- `normaltxt` is renamed to `Uncolored`.
- `hintfmt` has been merged into `hintformat` as extra constructor
functions.
- `hintformat` has been renamed to `hintfmt`.
- The single-argument `hintformat(std::string)` constructor has been
renamed to a static member `hintformat::interpolate` to avoid pitfalls
with using user-generated strings as format strings.
While preparing PRs like #9753, I've had to change error messages in
dozens of code paths. It would be nice if instead of
EvalError("expected 'boolean' but found '%1%'", showType(v))
we could write
TypeError(v, "boolean")
or similar. Then, changing the error message could be a mechanical
refactor with the compiler pointing out places the constructor needs to
be changed, rather than the error-prone process of grepping through the
codebase. Structured errors would also help prevent the "same" error
from having multiple slightly different messages, and could be a first
step towards error codes / an error index.
This PR reworks the exception infrastructure in `libexpr` to
support exception types with different constructor signatures than
`BaseError`. Actually refactoring the exceptions to use structured data
will come in a future PR (this one is big enough already, as it has to
touch every exception in `libexpr`).
The core design is in `eval-error.hh`. Generally, errors like this:
state.error("'%s' is not a string", getAttrPathStr())
.debugThrow<TypeError>()
are transformed like this:
state.error<TypeError>("'%s' is not a string", getAttrPathStr())
.debugThrow()
The type annotation has moved from `ErrorBuilder::debugThrow` to
`EvalState::error`.
As discussed in the maintainer meeting on 2024-01-29.
Mainly this is to avoid a situation where the name is parsed and
treated as a file name, mostly to protect users.
.-* and ..-* are also considered invalid because they might strip
on that separator to remove versions. Doesn't really work, but that's
what we decided, and I won't argue with it, because .-* probably
doesn't seem to have a real world application anyway.
We do still permit a 1-character name that's just "-", which still
poses a similar risk in such a situation. We can't start disallowing
trailing -, because a non-zero number of users will need it and we've
seen how annoying and painful such a change is.
What matters most is preventing a situation where . or .. can be
injected, and to just get this done.
To quote the method doc:
Non-impure derivations can still behave impurely, to the degree permitted
by the sandbox. Hence why this method isn't `isPure`: impure derivations
are not the negation of pure derivations. Purity can not be ascertained
except by rather heavy tools.
This is more conceptually correct (the order does not matter), and also
matches what Hydra already does.
(Nix and Hydra matching is needed for dedup
https://github.com/NixOS/hydra/issues/1164)
More invariants are enforced in the type, and less state needs to be
stored in the main sink itself. The method here is roughly that known as
"session types".
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This avoids split-on-whitespace errors:
- No more `bash -c` needed
- No more `shellEscape` needed
- `remote-program` ssh store setting also cleanly supports args (e.g.
`nix daemon`)
- `ssh` uses `--` to separate args for SSH from args for the command to
run.
and will help with Hydra dedup.
Some code taken from #6628.
Co-Authored-By: Alexander Bantyev <balsoft@balsoft.ru>
Factor out `ServeProto::BasicClientConnection` for Hydra to share
- `queryValidPaths`: Hydra uses the lock argument differently than Nix,
so we un-hard-code it.
- `buildDerivationRequest`: Just the request half, as Hydra does some
things between requesting and responding.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Good to document these formats separately from commands that happen to
use them.
Eventually I would like this and `builtins.derivation` to refer to a
store section on derivations that is authoritative, but that doesn't yet
exist, and will take some time to make. So I think we're just best off
merging this now as is.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Return a value instead of throwing.
Rather than the more trivial refactor of wrapping the return value in
another std::optional, we retain the meaning of the outer optional:
"we know at least something."
So we have changed:
return nullopt -> return nullopt
throw InvalidPath -> return make_optional(nullptr)
return vpi -> return make_optional(vpi)
In rare cases (e.g. when using allowSubstitutes = false), it's
possible that we simultaneously have a DerivationGoal *and* a
SubstitutionGoal building the same path. So if a DerivationGoal
already built the path while the SubstitutionGoal was waiting for a
download slot, it saves us a superfluous download to exit early.
In the "discard" case (i.e. when the store path already exists
locally), when we call parseDump() from a Finally and it throws an
exception (e.g. if the download of the NAR fails), Nix crashes:
terminate called after throwing an instance of 'nix::SubstituteGone'
what(): error: file 'nar/06br3254rx4gz4cvjzxlv028jrx80zg5i4jr62vjmn416dqihgr7.nar.xz' does not exist in binary cache 'http://localhost'
Aborted (core dumped)
Instead of having it be the default method in `Store` itself, have it be
the implementation in `DummyStore` and `LegacySSHStore`. Then just the
implementations which fail to provide the method pay the "penalty" of
dealing with the icky `unimplemented` function for non-compliance.
Picks up where #8217. Getting close to no `unsupported` in the `Store`
interface itself!
More progress on issue #5729.
This is useful for determining quickly which substituters to query.
An alternative would be for users to invoke the narinfo cache db directly,
so why do we need this change?
- It is easier to use. I believe Nix itself should also use it.
- This way, the narinfo cache db remains an implementation detail.
- Callers get to use the in-memory cache as well.
The data was (accidentally?) copied into a std::string,
even though the string is immediately converted into a std::string_view.
The code has been changed to construct a std::string_view directly,
such that one copy less happens.
Most of this is a `catch SysError` -> `catch SystemError` sed. This
is a rather pure-churn change I would like to get out of the way. **The
intersting part is `src/libutil/error.hh`.**
On Unix, we will only throw the `SysError` concrete class, which has
the same constructors that `SystemError` used to have.
On Windows, we will throw `WinError` *and* `SysError`. `WinError`
(which will be created in a later PR), will use a `DWORD` instead of
`int` error value, and `GetLastError()`, which is the Windows equivalent
of the `errno` machinery. Windows will *also* use `SysError` because
Window's "libc" (MSVCRT) implements the POSIX interface, and we use it
too.
As the docs describe, while we *throw* one of the 3 choices above (2
concrete classes or the alias), we should always *catch* `SystemError`.
This ensures no matter how the implementation changes for Windows (e.g.
between `SysError` and `WinError`) the catching logic stays the same
and stays correct.
Co-Authored-By volth <volth@volth.com>
Co-Authored-By Eugene Butler <eugene@eugene4.com>
Also move `SourcePath` into `libutil`.
These changes allow `error.hh` and `error.cc` to access source path and
position information, which we can use to produce better error messages
(for example, we could consider omitting filenames when two or more
consecutive stack frames originate from the same file).
This sets up infrastructure in libutil to allow for signing other than
by a secret key in memory. #9076 uses this to implement remote signing.
(Split from that PR to allow reviewing in smaller chunks.)
Co-Authored-By: Raito Bezarius <masterancpp@gmail.com>
many paths need not be heap-allocated, and derivation env name/valye
pairs can be moved into the map.
before:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.883 s ± 0.016 s [User: 5.250 s, System: 1.424 s]
Range (min … max): 6.860 s … 6.905 s 10 runs
after:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.868 s ± 0.027 s [User: 5.194 s, System: 1.466 s]
Range (min … max): 6.828 s … 6.913 s 10 runs
the table is very small compared to cache sizes and a single indexed
load is faster than three comparisons.
before:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.907 s ± 0.012 s [User: 5.272 s, System: 1.429 s]
Range (min … max): 6.893 s … 6.926 s 10 runs
after:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.883 s ± 0.016 s [User: 5.250 s, System: 1.424 s]
Range (min … max): 6.860 s … 6.905 s 10 runs
a bunch of derivation strings contain no escape sequences. we can
optimize for this fact by first scanning for the end of a derivation
string and simply returning the contents unmodified if no escape
sequences were found. to make this even more efficient we can also use
BackedStringViews to avoid copies, avoiding heap allocations for
transient data.
before:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.952 s ± 0.015 s [User: 5.294 s, System: 1.452 s]
Range (min … max): 6.926 s … 6.974 s 10 runs
after:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.907 s ± 0.012 s [User: 5.272 s, System: 1.429 s]
Range (min … max): 6.893 s … 6.926 s 10 runs
istream sentry objects are very expensive for single-character
operations, and since we don't configure exception masks for the
istreams used here they don't even do anything. all we need is
end-of-string checks and an advancing position in an immutable memory
buffer, both of which can be had for much cheaper than istreams allow.
the effect of this change is most apparent on empty stores.
before:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 7.167 s ± 0.013 s [User: 5.528 s, System: 1.431 s]
Range (min … max): 7.147 s … 7.182 s 10 runs
after:
Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
Time (mean ± σ): 6.963 s ± 0.011 s [User: 5.330 s, System: 1.421 s]
Range (min … max): 6.943 s … 6.974 s 10 runs
as written the comparisons generate copies, even though it looks as
though they shouldn't.
before:
Time (mean ± σ): 4.396 s ± 0.002 s [User: 3.894 s, System: 0.501 s]
Range (min … max): 4.393 s … 4.399 s 10 runs
after:
Time (mean ± σ): 4.260 s ± 0.003 s [User: 3.754 s, System: 0.505 s]
Range (min … max): 4.257 s … 4.266 s 10 runs
resizing a std::string clears the newly added bytes, which is not
necessary here and comes with a ~1.4% slowdown on our test nixos config.
〉 nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
before:
Time (mean ± σ): 4.486 s ± 0.003 s [User: 3.978 s, System: 0.507 s]
Range (min … max): 4.482 s … 4.492 s 10 runs
after:
Time (mean ± σ): 4.429 s ± 0.002 s [User: 3.929 s, System: 0.500 s]
Range (min … max): 4.427 s … 4.433 s 10 runs
`eval-system` option overrides just the value of `builtins.currentSystem`.
This is more useful than overriding `system` since you can build these
derivations on remote builders which can work on the given system.
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
I wrote the `configure.ac` wrong, and so we just got no builds
supporting ACLs.
Also, it needs to be more precise because Darwin puts other stuff in
that same header, evidently.
I don't love the way this code looks. There are two larger problems:
- eval, build/scratch, destination stores (#5025) should have different
types to reflect the fact that they are used for different purposes
and those purposes correspond to different operations. It should be
impossible to "use the wrong store" in my cases.
- Since drvs can end up in both the eval and build/scratch store, we
should have some sort of union/layered store (not on the file sytem
level, just conceptual level) that allows accessing both. This would
get rid of the ugly "check both" boilerplate in this PR.
Still, it might be better to land this now / soon after minimal cleanup,
so we have a concrete idea of what problem better abstractions are
supposed to solve.
Below the comment added by this commit is a much longer comment
followed by a trust check, both of which have confused me on at
least two occasions. I figured it out once, forgot it, then had to
ask @Ericson2314 to explain it, at which point I understood it
again. I think this might confuse other people too, or maybe I will
just forget it a third time. So let's add a comment.
Farther down in the function is the following check:
```
if (!(drvType.isCA() || trusted))
throw Error("you are not privileged to build input-addressed derivations");
```
This seems really strange at first. A key property of Nix is that
you can compute the outpath of a derivation using the derivation
(and its references-closure) without trusting anybody!
The missing insight is that at this point in the code the builder
doesn't necessarily have the references-closure of the derivation
being built, and therefore needs to trust that the derivation's
outPath is honest. It's incredibly easy to overlook this, because
the only difference between these two cases is which of these
identically-named functions we used:
- `readDerivation(Source,Store)`
- `Store::readDerivation()`
These functions have different trust models (except in the special
case where the first function is used on the local store). We
should call the reader's attention to this fact.
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
In https://github.com/NixOS/nix/pull/6134#issuecomment-1079199888,
@thuffschmitt proposed exposing `LegacySSHStore` in Nix for
deduplication with Hydra, at least temporarily. I think that is a good
idea.
Note that the diff will look bad unless one ignores whitespace! Also try
this locally:
```shell-session
git diff --ignore-all-space HEAD^:src/libstore/legacy-ssh-store.cc HEAD:src/libstore/legacy-ssh-store.cc
git diff --ignore-all-space HEAD^:src/libstore/legacy-ssh-store.cc HEAD:src/libstore/legacy-ssh-store.hh
```
This is needed for building CA deriations with a src store / dest store
split. In particular it is needed for Hydra.
https://github.com/NixOS/hydra/issues/838 currently puts realizations,
and thus build outputs, in the local store, but it should not.
In the process, partially undo e89b5bd0bf
in that the ancient < 2.4 version is now supported again by the
serializer again. `LegacySSHStore`, instead of also asserting that the
version is at least 4, just checks that `narHash` is set.
This allows us to better test the serializer in isolation for both
versions (< 4 and >= 4).
- remove prose for the default value, which is shown programmatically
- add note on how this relates to `cores`
- add link to mentioned derivation attribute
Today, with the tests inside a `tests` intermingled with the
corresponding library's source code, we have a few problems:
- We have to be careful that wildcards don't end up with tests being
built as part of Nix proper, or test headers being installed as part
of Nix proper.
- Tests in libraries but not executables is not right:
- It means each executable runs the previous unit tests again, because
it needs the libraries.
- It doesn't work right on Windows, which doesn't want you to load a
DLL just for the side global variable . It could be made to work
with the dlopen equivalent, but that's gross!
This reorg solves these problems.
There is a remaining problem which is that sibbling headers (like
`hash.hh` the test header vs `hash.hh` the main `libnixutil` header) end
up shadowing each other. This PR doesn't solve that. That is left as
future work for a future PR.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
