Previously, `state.mkList()` would set the type of the value to tList
and allocate the list vector, but it would not initialize the values
in the list. This has two problems:
* If an exception occurs, the list is left in an undefined state.
* More importantly, for multithreaded evaluation, if a value
transitions from thunk to non-thunk, it should be final (i.e. other
threads should be able to access the value safely).
To address this, there now is a `ListBuilder` class (analogous to
`BindingsBuilder`) to build the list vector prior to the call to
`Value::mkList()`. Typical usage:
auto list = state.buildList(size);
for (auto & v : list)
v = ... set value ...;
vRes.mkList(list);
* Add regression test
* Fix 'no repo' test so it doesn't succeed if the data is still in cache
* Use git_revparse_single inside git-utils instead of reimplementing the same logic.
Previously, errors while printing values in `nix repl` would be printed
in `«error: ...»` brackets rather than displayed normally:
```
nix-repl> legacyPackages.aarch64-darwin.pythonPackages.APScheduler
«error: Package ‘python-2.7.18.7’ in /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/pkgs/development/interpreters/python/cpython/2.7/default.nix:335 is marked as insecure, refusing to evaluate.»
```
Now, errors will be displayed normally if they're emitted at the
top-level of an expression:
```
nix-repl> legacyPackages.aarch64-darwin.pythonPackages.APScheduler
error:
… in the condition of the assert statement
at /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/lib/customisation.nix:268:17:
267| in commonAttrs // {
268| drvPath = assert condition; drv.drvPath;
| ^
269| outPath = assert condition; drv.outPath;
… in the left operand of the OR (||) operator
at /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/pkgs/development/interpreters/python/passthrufun.nix:28:45:
27| if lib.isDerivation value then
28| lib.extendDerivation (valid value || throw "${name} should use `buildPythonPackage` or `toPythonModule` if it is to be part of the Python packages set.") {} value
| ^
29| else
(stack trace truncated; use '--show-trace' to show the full trace)
error: Package ‘python-2.7.18.7’ in /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/pkgs/development/interpreters/python/cpython/2.7/default.nix:335 is marked as insecure, refusing to evaluate.
```
Errors emitted in nested structures (like e.g. when printing `nixpkgs`)
will still be printed in brackets.
Strings are now printed directly when evaluated by `:print`, rather than
escaped. This makes it easier to debug multi-line strings or strings
containing quotes, like the results of `builtins.readFile`,
`lib.toShellArg`, and so on.
```
nix-repl> "cuppy\ndog\ncity"
"cuppy\ndog\ncity"
nix-repl> :p "cuppy\ndog\ncity"
cuppy
dog
city
```
Fixes this very long warning, which I'll only include the first line of:
/nix/store/8wrjhrycpshhc3b41xmjwvgqr2m3yajq-libcxx-16.0.6-dev/include/c++/v1/__memory/construct_at.h:66:5: warning: destructor called on non-final 'RegexMatcher' that has virtual functions but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]
__loc->~_Tp();
`nix eval` forces values and prints derivations as attribute sets, so
commands that print derivations (e.g. `nix eval nixpkgs#bash`) will
infinitely loop and segfault.
Printing derivations as `.drv` paths makes `nix eval` complete as
expected. Further work is needed, but this is better than a segfault.
This wasn't caught by CI because #10149 and #10152 pass
individually... It doesn't happen on lazy-trees either because we
never try to fetch relative path flakes (#10089).
we now keep not a table of all positions, but a table of all origins and
their sizes. position indices are now direct pointers into the virtual
concatenation of all parsed contents. this slightly reduces memory usage
and time spent in the parser, at the cost of not being able to report
positions if the total input size exceeds 4GiB. this limit is not unique
to nix though, rustc and clang also limit their input to 4GiB (although
at least clang refuses to process inputs that are larger, we will not).
this new 4GiB limit probably will not cause any problems for quite a
while, all of nixpkgs together is less than 100MiB in size and already
needs over 700MiB of memory and multiple seconds just to parse. 4GiB
worth of input will easily take multiple minutes and over 30GiB of
memory without even evaluating anything. if problems *do* arise we can
probably recover the old table-based system by adding some tracking to
Pos::Origin (or increasing the size of PosIdx outright), but for time
being this looks like more complexity than it's worth.
since we now need to read the entire input again to determine the
line/column of a position we'll make unsafeGetAttrPos slightly lazy:
mostly the set it returns is only used to determine the file of origin
of an attribute, not its exact location. the thunks do not add
measurable runtime overhead.
notably this change is necessary to allow changing the parser since
apparently nothing supports nix's very idiosyncratic line ending choice
of "anything goes", making it very hard to calculate line/column
positions in the parser (while byte offsets are very easy).
this needs a string comparison because there seems to be no other way to
get that information out of bison. usually the location info is going to
be correct (pointing at a bad token), but since EOF isn't a token as
such it'll be wrong in that this case.
this hasn't shown up much so far because a single line ending *is* a
token, so any file formatted in the usual manner (ie, ending in a line
ending) would have its EOF position reported correctly.
the parser treats a plain \r as a newline, error reports do not. this
can lead to interesting divergences if anything makes use of this
feature, with error reports pointing to wrong locations in the input (or
even outside the input altogether).
previously we reported the error at the beginning of the binding
block (for plain inherits) or the beginning of the attr list (for
inherit-from), effectively hiding where exactly the error happened.
this also carries over to runtime positions of attributes in sets as
reported by unsafeGetAttrPos. we're not worried about this changing
observable eval behavior because it *is* marked unsafe, and the new
behavior is much more useful.
we already normalize attr order to lexicographic, doing the same for
formals makes sense. doubly so because the order of formals would
otherwise depend on the context of the expression, which is not quite as
useful as one might expect.
the parser modifies its inputs, which means that sharing them between
the error context reporting system and the parser itself can confuse the
reporting system. usually this led to early truncation of error context
reports which, while not dangerous, can be quite confusing.
* Convert all InputScheme::fetch() methods to getAccessor().
* Add checkLocks() method for checking lock attributes.
* Rename fetch() to fetchToStore().
This is the case for e.g. dirty Git workdirs, where we would get
$ nix flake metadata
Resolved URL: git+file:///home/eelco/Dev/nix-master
Locked URL: git+file:///home/eelco/Dev/nix-master
The "lockedRef" field is a misnomer, since it can be unlocked
(e.g. for a dirty Git workdir). In that case, `nix profile upgrade`
needs to assume that the package can have changed, and perform an
upgrade.
When reviewing old PRs, I found that #9997 adds some code to ensure one
particular assert is always present. But, removing asserts isn't
something we do in our own release builds either in the flake here or in
nixpkgs, and is plainly a bad idea that increases support burden,
especially if other distros make bad choices of build flags in their Nix
packaging.
For context, the assert macro in the C standard is defined to do nothing
if NDEBUG is set.
There is no way in our build system to set -DNDEBUG without manually
adding it to CFLAGS, so this is simply a configuration we do not use.
Let's ban it at compile time.
I put this preprocessor directive in src/libutil.cc because it is not
obvious where else to put it, and it seems like the most logical file
since you are not getting a usable nix without it.
Directly fail if a flakeref points to something that isn't a directory
instead of falling back to the logic of trying to look up the hierarchy
to find a valid flake root.
Fix https://github.com/NixOS/nix/issues/9868
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>
desugaring inherit-from to syntactic duplication of the source expr also
duplicates side effects of the source expr (such as trace calls) and
expensive computations (such as derivationStrict).
Flakes still reside in the Nix store (so there shouldn't be any change
in behaviour), but they are now accessed via the rootFS
accessor. Since rootFS implements access checks, we no longer have to
worry about flake.{nix,lock} or their parents being symlinks that
escape from the flake.
Extracted from the lazy-trees branch.
"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.
This seems to have found one actual bug in fs-sink.cc: the symlink case
was falling into the regular file case, which can't possibly be
intentional, right?
When a file conflict arises during a package install a suggestion is
made to remove the old entry. This was previously done using the
installable URLs of the old entry. These URLs are quite verbose and
often do not equal the URL of the existing entry.
This change uses the recently introduced profile entry name for the
suggestion, resulting in a simpler output.
The improvement is easily seen in the change to the functional test.
- `nix store add` supports text hashing
With functional test ensuring it matches `builtins.toFile`.
- Factored-out flags for both commands
- Move all common reusable flags to `libcmd`
- They are not part of the *definition* of the CLI infra, just a usag
of it.
- The `libstore` flag couldn't go in `args.hh` in libutil anyways,
would be awkward for it to live alone
- Shuffle around `Cmd*` hierarchy so flags for deprecated commands don't
end up on the new ones
This PR reduces the creation of short-lived basic_json objects while
parsing flake.lock files. For large flake.lock files (~1.5MB) I was
observing ~60s being spent for trivial nix build operations while
after this change it is now taking ~1.6s.
It's better to just check whether the input has all the attributes
needed to consider itself locked (e.g. whether a Git input has an
'rev' attribute).
Also, the 'locked' field was actually incorrect for Git inputs: it
would be set to true even for dirty worktrees. As a result, we got
away with using fetchTree() internally even though fetchTree()
requires a locked input in pure mode. In particular, this allowed
'--override-input' to work by accident.
The fix is to pass a set of "overrides" to call-flake.nix for all the
unlocked inputs (i.e. the top-level flake and any --override-inputs).
This fixes warnings like
warning: Ignoring setting 'auto-allocate-uids' because experimental feature 'auto-allocate-uids' is not enabled
warning: Ignoring setting 'impure-env' because experimental feature 'configurable-impure-env' is not enabled
when using the daemon and the user didn't actually set those settings.
Note: this also hides those settings from `nix config show`, but that
seems a good thing.
`canonPath` and `absPath` work on native paths, and so should switch
between supporting Unix paths and Windows paths accordingly.
The templating is because `CanonPath`, which shares the implementation,
should always be Unix style. It is the pure "nix-native" path type for
virtual file operations --- it is part of Nix's "business logic", and
should not vary with the host OS accordingly.
The core `CanonPath` constructors were using `absPath`, but `absPath` in
some situations does IO which is not appropriate. It turns out that
these constructors avoided those situations, and thus were pure, but it
was far from obvious this was the case.
To remedy the situation, abstract the core algorithm from `canonPath` to
use separately in `CanonPath` without any IO. No we know by-construction
that those constructors are pure.
That leaves `CanonPath::fromCWD` as the only operation which uses IO /
is impure. Add docs on it, and `CanonPath` as a whole, explaining the
situation.
This is also necessary to support Windows paths on windows without
messing up `CanonPath`. But, I think it is good even without that.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
There is no longer an `importTarball` method. Instead, there is a
`unpackTarfileToSink` function (back in libutil). The caller can use
thisw with the `getParseSink` method we added in the last commit easily
enough.
In addition, tarball cache functionality is separated from `git-utils`
and moved into `tarball-cache`. This ensures we are separating mechanism
and policy.
There is now a separation of:
1. A `FileSystemObjectSink` for writing to git repos
2. Adapting libarchive to use that parse sink.
The prepares a proper separation of concerns.
A command like
rm -rf ~/.cache/nix/tarball-cache/ ~/.cache/nix/fetcher-cache-v1.sqlite*; nix flake metadata 'git+file:///home/eelco/Dev/nixpkgs?rev=9463103069725474698139ab10f17a9d125da859'
was spending about 84% of its runtime in lookup(), specifically in
git_tree_entry_bypath(). (The reading of blobs is less than 3%.)
It appears libgit2 doesn't do a lot of caching of trees, so we now
make sure that when we look up a path, we add all its parents, and all
the immediate children of the parents (since we have them in memory
anyway), to our own cache.
This speed up the command above from 17.2s to 7.8s on my machine.
Fixes (or at least should improve a lot) #9684.
Commit 83c067c0fa changed `builtins.pathExists`
to resolve symlinks before checking for existence. Consequently, if the path
refers to a symlink itself, existence of the target of the symlink (instead of
the symlink itself) was checked. Restore the previous behavior by skipping
symlink resolution in the last component.
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>
for plain inherits this is really just a stylistic choice, but for
inherit-from it actually fixes an exponential size increase problem
during expr printing (as may happen during assertion failure reporting,
on during duplicate attr detection in the parser)
this also has the effect of sorting let bindings lexicographically
rather than by symbol creation order as was previously done, giving a
better canonicalization in the process.
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.
As discussed in the last Nix team meeting (2024-02-95), this method
doesn't belong because `CanonPath` is a virtual/ideal absolute path
format, not used in file systems beyond the native OS format for which a
"current working directory" is defined.
Progress towards #9205
Pretty-print values in the REPL by printing each item in a list or
attrset on a separate line. When possible, single-item lists and
attrsets are printed on one line, as long as they don't contain a nested
list, attrset, or thunk.
Before:
```
{ attrs = { a = { b = { c = { }; }; }; }; list = [ 1 ]; list' = [ 1 2 3 ]; }
```
After:
```
{
attrs = {
a = {
b = {
c = { };
};
};
};
list = [ 1 ];
list' = [
1
2
3
];
}
```
Some tools which consume the "nix print-dev-env" rc script (such as
"nix-direnv") are sensitive to the use of unbound variables. They use
"set -u".
The "nix print-dev-env" rc script initially unsets "shellHook", then
loads variables from the derivation, and then evaluates "shellHook".
However, most derivations don't have a "shellHook" attribute.
So users get the error "shellHook: unbound variable". This can be
demonstrated with the command:
nix print-dev-env nixpkgs#hello | bash -u
This commit changes the rc script to provide an empty fallback value
for the "shellHook" variable.
Closes: #7951#8253
It is entirely possible for the path to be an empty string and many
unit tests actually pass it as an empty string (e.g. both_roundrip or
turnsEmptyPathIntoCWD). In this case, without this patch, absPath will
perform a one-byte out-of-bounds access.
This was discovered while enabling the nix test suite on Alpine where
we compile all software with `-D_GLIBCXX_ASSERTIONS=1`, thus resulting
in a test failure on Alpine.
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`.
Previously, the "file:./" prefix was not correctly recognized in
fixGitURL; instead, it was mistaken as a file path, which resulted in a
parsed url of the form "file://file:./".
This commit fixes the issue by properly detecting the "file:" prefix.
Note, however, that unlike "file://", the "file:./" URI is _not_
standardized, but has been widely used to referred to relative file
paths. In particular, the "git+file:./" did work for nix<=2.18, and was
broken since nix 2.19.0.
Finally, this commit fixes the issue completely for the 2.19 series, but
is still inadequate for the 2.20 series due to new behaviors from the
switch to libgit2. However, it does improve the correctness of parsing
even though it is not yet a complete solution.
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.
The code works fine on macOS, but the default stack size we attempt to
set is larger than what my system will allow (Nix attempts to set the
stack size to 67108864, but the maximum allowed is 67092480), so I've
instead used the requested stack size or the maximum allowed, whichever
is smaller.
I've also added an error message if setting the stack size fails. It
looks like this:
> Failed to increase stack size from 8372224 to 67108864 (maximum
> allowed stack size: 67092480): Invalid argument
This extends the `error: cannot coerce a TYPE to a string` message
to print the value that could not be coerced. This helps with debugging
by making it easier to track down where the value is being produced
from, especially in errors with deep or unhelpful stack traces.
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>
Low-hanging fruit in the spirit of #9753 and #9754 (means 9999years did
all the hard work already).
This basically prints out what was attempted to be called as function,
i.e.
map (import <nixpkgs> {}) [ 1 2 3 ]
now gives the following error message:
error:
… while calling the 'map' builtin
at «string»:1:1:
1| map (import <nixpkgs> {}) [ 1 2 3 ]
| ^
… while evaluating the first argument passed to builtins.map
error: expected a function but found a set: { _type = "pkgs"; AAAAAASomeThingsFailToEvaluate = «thunk»; AMB-plugins = «thunk»; ArchiSteamFarm = «thunk»; BeatSaberModManager = «thunk»; CHOWTapeModel = «thunk»; ChowCentaur = «thunk»; ChowKick = «thunk»; ChowPhaser = «thunk»; CoinMP = «thunk»; «18783 attributes elided»}
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>
Do this if we want to do `--hash-algo` everywhere, and not `--algo` for
hash commands.
The new `nix hash convert` is updated. Deprecated new CLI commands are
left as-is (`nix hash path` needs to be redone and is also left as-is).
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)
Add several tests for git fetching:
- shallow-cache-separation: can fetch the same repo shallowly and non-shallowly
- shallow-ignore-ref: ensure that ref gets ignored when shallow=true is set
- ssh-shallow: can fetch a git repo via ssh using shallow=1
libgit2 is not capable of using git-credentials helpers yet.
This prevents private repositories from being used.
Based on code that was replaced in https://github.com/NixOS/nix/pull/9240
(Introduce libgit2); hence:
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
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.
It is good to propagate the underlying error so whether or not we use a
process to deal with path length issues is not observable.
Also, as these wrapper functions got more and more complex, the code
duplication got worse and worse. The new `bindConnectProcHelper`
function deduplicates them.
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.
This does not yet resolve the coupling between packages and
derivations, but it makes the code more consistent with the
terminology, and it accentuates places where the coupling is
obvious, such as
auto drvPath = packageInfo.queryDrvPath();
if (!drvPath)
throw Error("'%s' is not a derivation", what());
... which isn't wrong, and in my opinion, doesn't even look
wrong, because it just reflects the current logic.
However, I do like that we can now start to see in the code that
this coupling is perhaps a bit arbitrary.
After this rename, we can bring the DerivingPath concept into type
and start to lift this limitation.
these symbols are used a *lot*, so it makes sense to cache them. this
mostly increases clarity of the code (however clear one may wish to call
the parser desugaring here), but it also provides a small performance
benefit.
there's no reason the parser itself should be doing semantic analysis
like bindVars. split this bit apart (retaining the previous name in
EvalState) and have the parser really do *only* parsing, decoupled from
EvalState.
most EvalState and Expr members defined here could be elsewhere, where
they'd be easier to maintain (not being embedded in a file with arcane
syntax) and *somewhat* more faithfully placed according to the path of
the file they're defined in.
most instances of this being used do not refer to the "current"
position, sometimes not even to one reasonably close by. it could also
be called `makePos` instead, but `at` seems clear in context.
ParserState better describes what this struct really is. the parser
really does modify its state (most notably position and symbol tables),
so calling it that rather than obliquely "data" (which implies being
input only) makes sense.
since nix doesn't use the bison `error` terminal anywhere any invocation
of yyerror will immediately cause a failure. since we're *already*
leaking tons of memory whatever little bit bison allocates internally
doesn't much matter any more, and we'll be replacing the parser soon anyway.
coincidentally this now also matches the error behavior of URIs when
they are disabled or ~/ paths in pure eval mode, duplicate attr
detection etc.
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.
`FLOAT`, `INT`, and `IN` are identifers taken by macros.
The name `IN_KW` is chosen to match `OR_KW`, which is presumably named
that way for the same reason of dodging macros.
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>
When returning a 0-length substring, avoid calling coerceToString,
since it returns a string_view with the string's length, which is
expensive to compute for large strings.
Also fingerprint and some preparatory improvements.
Testing is still not up to scratch because lots of logic is duplicated
between the workdir and commit cases.
Enabled for fetchGit, which historically had this behavior,
among other behaviors we do not want in fetchGit.
fetchTree disables this parameter by default. It can choose the
simpler behavior, as it is still experimental.
I am not confident that the filtering implementation is future
proof. It should reuse a source filtering wrapper, which I believe
Eelco has already written, but not merged yet.
The Nix team has requested that this output format remain unchanged.
I've added a warning to the man page explaining that `nix-instantiate
--eval` output will not parse correctly in many situations.
Previously, there were two mostly-identical value printers -- one in
`libexpr/eval.cc` (which didn't force values) and one in
`libcmd/repl.cc` (which did force values and also printed ANSI color
codes).
This PR unifies both of these printers into `print.cc` and provides a
`PrintOptions` struct for controlling the output, which allows for
toggling whether values are forced, whether repeated values are tracked,
and whether ANSI color codes are displayed.
Additionally, `PrintOptions` allows tuning the maximum number of
attributes, list items, and bytes in a string that will be displayed;
this makes it ideal for contexts where printing too much output (e.g.
all of Nixpkgs) is distracting. (As requested by @roberth in
https://github.com/NixOS/nix/pull/9554#issuecomment-1845095735)
Please read the tests for example output.
Future work:
- It would be nice to provide this function as a builtin, perhaps
`builtins.toStringDebug` -- a printing function that never fails would
be useful when debugging Nix code.
- It would be nice to support customizing `PrintOptions` members on the
command line, e.g. `--option to-string-max-attrs 1000`.
Changes:
- CPP variable is now `USE_READLINE` not `READLINE`
- `configure.ac` supports with new CLI flag
- `package.nix` supports with new configuration option
- `flake.nix` CIs this (along with no markdown)
Remove old Ubuntu 16.04 stop-gap too, as that is now quite old.
Motivation:
- editline does not build for Windows, but readline *should*. (I am
still working on this in Nixpkgs at this time, however. So there will
be a follow-up Nix PR removing the windows-only skipping of the
readline library once I am done.)
- Per
https://salsa.debian.org/debian/nix/-/blob/master/debian/rules?ref_type=heads#L27
and #2551, Debian builds Nix with readline. Now we better support and
CI that build configuration.
This is picking up where #2551 left off, ensuring we test a few more
things not merely have CPP for them.
Co-authored-by: Weijia Wang <9713184+wegank@users.noreply.github.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>
This avoids a Value allocation for empty list constants. During a `nix
search nixpkgs`, about 82% of all thunked lists are empty, so this
removes about 3 million Value allocations.
Performance comparison on `nix search github:NixOS/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 --no-eval-cache`:
maximum RSS: median = 3845432.0000 mean = 3845432.0000 stddev = 0.0000 min = 3845432.0000 max = 3845432.0000 [rejected?, p=0.00000, Δ=-70084.00000±0.00000]
soft page faults: median = 965395.0000 mean = 965394.6667 stddev = 1.1181 min = 965392.0000 max = 965396.0000 [rejected?, p=0.00000, Δ=-17929.77778±38.59610]
system CPU time: median = 1.8029 mean = 1.7702 stddev = 0.0621 min = 1.6749 max = 1.8417 [rejected, p=0.00064, Δ=-0.12873±0.09905]
user CPU time: median = 14.1022 mean = 14.0633 stddev = 0.1869 min = 13.8118 max = 14.3190 [not rejected, p=0.03006, Δ=-0.18248±0.24928]
elapsed time: median = 15.8205 mean = 15.8618 stddev = 0.2312 min = 15.5033 max = 16.1670 [not rejected, p=0.00558, Δ=-0.28963±0.29434]
since `up` and `values` are both pointer-aligned the type field will
also be pointer-aligned, wasting 48 bits of space on most machines. we
can get away with removing the type field altogether by encoding some
information into the `with` expr that created the env to begin with,
reducing the GC load for the absolutely massive amount of single-entry
envs we create for lambdas. this reduces memory usage of system eval by
quite a bit (reducing heap size of our system eval from 8.4GB to 8.23GB)
and gives similar savings in eval time.
running `nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'`
before:
Time (mean ± σ): 5.576 s ± 0.003 s [User: 5.197 s, System: 0.378 s]
Range (min … max): 5.572 s … 5.581 s 10 runs
after:
Time (mean ± σ): 5.408 s ± 0.002 s [User: 5.019 s, System: 0.388 s]
Range (min … max): 5.405 s … 5.411 s 10 runs
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
This fixes a segfault on infinite function call recursion (rather than
infinite thunk recursion) by tracking the function call depth in
`EvalState`.
Additionally, to avoid printing extremely long stack traces, stack
frames are now deduplicated, with a `(19997 duplicate traces omitted)`
message. This should only really be triggered in infinite recursion
scenarios.
Before:
$ nix-instantiate --eval --expr '(x: x x) (x: x x)'
Segmentation fault: 11
After:
$ nix-instantiate --eval --expr '(x: x x) (x: x x)'
error: stack overflow
at «string»:1:14:
1| (x: x x) (x: x x)
| ^
$ nix-instantiate --eval --expr '(x: x x) (x: x x)' --show-trace
error:
… from call site
at «string»:1:1:
1| (x: x x) (x: x x)
| ^
… while calling anonymous lambda
at «string»:1:2:
1| (x: x x) (x: x x)
| ^
… from call site
at «string»:1:5:
1| (x: x x) (x: x x)
| ^
… while calling anonymous lambda
at «string»:1:11:
1| (x: x x) (x: x x)
| ^
… from call site
at «string»:1:14:
1| (x: x x) (x: x x)
| ^
(19997 duplicate traces omitted)
error: stack overflow
at «string»:1:14:
1| (x: x x) (x: x x)
| ^
more buffers that can be uninitialized and on the stack. small
difference, but still worth doing.
before:
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
after:
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
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
Previously, IFDs would be built within the eval store, even though one
is typically using `--eval-store` precisely to *avoid* local builds.
Because the resulting Nix expression must be copied back to the eval
store in order to be imported, this requires the eval store to trust
the build store's signatures.
The profile manifest is now an object keyed on the name returned by
getNameFromURL() at installation time, instead of an array. This
ensures that the names of profile elements don't change when other
elements are added/removed.
Prior to this change, Nix would prepend every installable to the PATH
list in order to ensure that installables appeared before the current
PATH from the ambient environment.
With this change, all the installables are still prepended to the PATH,
but in the same order as they appear on the command line. This means
that the first of two packages that expose an executable `hello` would
appear in the PATH first, and thus be executed first.
See the test in the prior commit for a more concrete example.
There's no good reason to deprecate it:
- For consistency reasons it should continue to exist, such that all
primitive types have a corresponding `builtins.is*` primop.
- There's no implementation cost to continuing to have this function
- It costs users time to try to migrate away from it, e.g.
https://github.com/NixOS/nixpkgs/pull/219747 and https://github.com/NixOS/nixpkgs/pull/275548
- Using it can give easier-to-read code like `all isNull list`
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
this also reduces forceValue code size and removes the need for
hideInDiagnostics. coopting thunk forcing like this has the additional
benefit of clarifying how these errors can happen in the first place.
forceValue is extremely hot. interestingly adding likeliness annotations
to the branches does not seem to make a difference.
before:
Time (mean ± σ): 4.224 s ± 0.005 s [User: 3.711 s, System: 0.512 s]
Range (min … max): 4.218 s … 4.234 s 10 runs
after:
Time (mean ± σ): 4.140 s ± 0.009 s [User: 3.647 s, System: 0.492 s]
Range (min … max): 4.130 s … 4.152 s 10 runs
almost all uses of this are interactive, except for deepSeq. deepSeq is
going to be expensive and rare enough to not care much about, and
Value::determinePos should usually be cheap enough to not be too much of
a burden in any case.
~1% parser speedup from not using TLS indirections, less on system eval.
this could have also gone in flex yyextra data, but that's significantly
slower for some reason (albeit still faster than thread locals).
before:
Time (mean ± σ): 4.231 s ± 0.004 s [User: 3.725 s, System: 0.504 s]
Range (min … max): 4.226 s … 4.240 s 10 runs
after:
Time (mean ± σ): 4.224 s ± 0.005 s [User: 3.711 s, System: 0.512 s]
Range (min … max): 4.218 s … 4.234 s 10 runs
~2% speedup on parsing without eval, less (but still significant) on
system eval. having flex generate faster parsers leads to very strange
misparses. maybe re2c is worth investigating.
before:
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
after:
Time (mean ± σ): 4.231 s ± 0.004 s [User: 3.725 s, System: 0.504 s]
Range (min … max): 4.226 s … 4.240 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
checking for isBlackhole in the forceValue hot path is rather more
expensive than necessary, and with a little bit of trickery we can move
such handling into the isApp case. small performance benefit, but under
some circumstances we've seen 2% improvement as well.
〉 nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
before:
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
after:
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
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
This keeps hint messages, source location information, and source code
snippets grouped together, while making stack traces shorter (so that
more stack frames can be viewed on the same terminal).
Before:
error:
… while evaluating the attribute 'body'
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:4:3:
3|
4| body = x "x";
| ^
5| }
… from call site
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:4:10:
3|
4| body = x "x";
| ^
5| }
… while calling 'x'
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:2:7:
1| let {
2| x = arg: assert arg == "y"; 123;
| ^
3|
error: assertion '(arg == "y")' failed
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:2:12:
1| let {
2| x = arg: assert arg == "y"; 123;
| ^
3|
After:
error:
… while evaluating the attribute 'body'
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:4:3:
3|
4| body = x "x";
| ^
5| }
… from call site
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:4:10:
3|
4| body = x "x";
| ^
5| }
… while calling 'x'
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:2:7:
1| let {
2| x = arg: assert arg == "y"; 123;
| ^
3|
error: assertion '(arg == "y")' failed
at /Users/wiggles/nix/tests/functional/lang/eval-fail-assert.nix:2:12:
1| let {
2| x = arg: assert arg == "y"; 123;
| ^
3|
`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
```
* document `fetchTree`
* display experimental feature note at the top
we have to enable the new `fetchTree` experimental feature to render it
at all. this was a bug introduced when adding that new feature flag.
Co-authored-by: tomberek <tomberek@users.noreply.github.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Silvan Mosberger <github@infinisil.com>
The code has already been fixed (yay!) so what is left of this commit is
just updating the API docs.
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
According https://en.cppreference.com/w/cpp/io/strstream, it has been
deprecated since C++98! The Clang + Linux build systems to not have it
at all, or at least be hiding it.
We can just use `std::stringstream` instead, I think.
* Print the value in `error: cannot coerce` messages
This extends the `error: cannot coerce a TYPE to a string` message
to print the value that could not be coerced. This helps with debugging
by making it easier to track down where the value is being produced
from, especially in errors with deep or unhelpful stack traces.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
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.
This includes position information in more places, making debugging
easier.
Before:
```
$ nix-instantiate --show-trace --eval tests/functional/lang/eval-fail-using-set-as-attr-name.nix
error:
… while evaluating an attribute name
at «none»:0: (source not available)
error: value is a set while a string was expected
```
After:
```
error:
… while evaluating an attribute name
at /pwd/lang/eval-fail-using-set-as-attr-name.nix:5:10:
4| in
5| attr.${key}
| ^
6|
error: value is a set while a string was expected
```
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).
AppleDouble files were extracted differently on macOS machines than on other
UNIX's.
Setting `archive_read_set_format_option(this->archive, NULL ,"mac-ext",NULL)`
fixes this problem, since it just ignores the AppleDouble file and treats it as
a normal one.
This was a problem since it caused source archives to be different between macOS
and Linux.
Ref: nixos/nix#9290
* Factor out the default `MultiCommand` behavior
All the `MultiCommand`s had (nearly) the same behavior when called
without a subcommand.
Factor out this behavior into the `NixMultiCommand` class.
* Display the list of available subcommands when none is specified
Whenever a user runs a command that excepts a subcommand, add the list
of available subcommands to the error message.
* Print the multi-command lists as Markdown lists
This takes more screen real estate, but is also much more readable than
a comma-separated list
without knowing a lot of context, it's not clear who "we" are in that
text. I'm also strongly opposed to adding procedural notes into
a reference manual; it just won't age well.
this change leaves a factual description of the experimental feature and
its purpose.
- 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>
This makes for more useful manual table of contents, that displays the
information at a glance.
The `nix help-stores` command is kept as-is, even though it will show up
in the manual with the same information as these pages due to the way it
is written as a "`--help`-style" command. Deciding what to do with that
command is left for a later PR.
This change also lists all store types at the top of the respective overview page.
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems
- helps navigating the code as it highlights which files are generated
- makes it less error prone when working incrementally
(although this should be just fixed by building out of tree)
This uses `git check-ignore` to determine if files are ignored before
attempting to add them in `putFile`.
We also add a condition to the `fetchFromWorkdir` filter to always add
the `flake.lock` file, even if it's not tracked. This is necessary to
resolve inputs.
This fixes#8854 without `git add --force`.
The problem was that f880469173 forgot
that the `#include <sys/xattr.h>` was guarded by an `#ifdef __linux__`.
However, the build failure was only on FreeBSD --- turns out other
platforms have this header too!
The fix therefore uses a new configure check so we properly clear ACLs
on more platforms.
This allows templates such as `NLOHMANN_DEFINE_TYPE_*` templates and other generators with things like `std::vector<std::optional<T>>`.
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
`installcheck` doesn't yet work, but the rest of the build can now
happen mostly inside a separate build directory.
Progress on #9342
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
GitArchiveInputScheme now streams tarballs into a Git repository. This
deduplicates data a lot, e.g. when you're fetching different revisions
of the Nixpkgs repo. It also warns if the tree hash returned by GitHub
doesn't match the tree hash of the imported tarball.
When writing a shebang script, you expect your path to be relative to
the script, not the cwd. We previously handled this correctly for
relative file paths, but not for expressions.
This handles both -p & -E args. My understanding is this should be
what we want in any cases I can think of - people run scripts from
many different working directories. @edolstra is there any reason to
handle -p args differently in this case?
Fixes#4232
In commit 0d2163c6dc, the progress bar was hidden
in nix repl because of a regression that caused it to interfere with user
input. Several users like(d) seeing the progress bar in the repl during builds.
Only hiding it while waiting for user input gives us the best of both worlds,
so do just that.
Without the change build for `eval.o` fails occasionally as:
$ make src/libexpr/eval.o
GEN Makefile.config
GEN src/libexpr/primops/derivation.nix.gen.hh
GEN src/libexpr/fetchurl.nix.gen.hh
GEN src/libexpr/parser-tab.cc
GEN src/libexpr/lexer-tab.cc
src/libexpr/lexer.l:314: warning, -s option given but default rule can be matched
CXX src/libexpr/eval.o
src/libexpr/eval.cc:519:18: fatal error: flake/call-flake.nix.gen.hh: No such file or directory
519 | #include "flake/call-flake.nix.gen.hh"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [mk/patterns.mk:3: src/libexpr/eval.o] Error 1
Noticed in https://github.com/NixOS/nixpkgs/pull/269439
This builds on #8817, to add additional UX help for people with existing
muscle memory (or shell history) with --update-input and tries to gently
guide them towards the newly evolved CLI UI.
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
We occasionnally commit to git repositories (like with `nix flake update --commit-lock-file`).
This shells out to `git commit`, which might wait for user input (for a signing key passphrase for instance).
Disable the progress bar while this is running to make sure that the
user can enter it.
* doc: primops: add more info for foldl
From the existing doc it is not obvious whether the first or the
second argument is the accumulator. This is however relevant to
know, as for certain scenarios, this might change the behavior.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
https://github.com/NixOS/nixpkgs/pull/269064 makes rapidcheck be build
as a shared lib, but that broke Nix because the `-lrapidcheck` was
missing. This fixes that (and doesn't break Nix what the library is a
static archive as today).
It is not inherently tied to `LocalStore`, it could probably even go in
`libnixutil`. Functions not attached to `LocalStore` should not be
declared in `local-store.hh`.
I am moving it to facilitate experimenting for #9344. If
canonicalisation should be done client-side in client-side builds, there
wouldn't be a `LocalStore` at all so having to include that header to
get this freestanding function is cumbersome and wrong.
Perhaps canonicalisation should still be done server-side for security
reasons --- I don't mean to make that judgement call now --- but even if
so, this freestanding function still isn't connected to `LocalStore` so
while less urgent it is still better to move out of this header.
This avoids repeated copying of the same source tree between Nix
invocations. It requires the accessor to have a "fingerprint" (e.g. a
Git revision) that uniquely determines its contents.
getFlake currently calls lstat (via isLink via canonPath) before it
performs the sanity check that a flake.nix exists in the first place.
This commit moves the check to before path canonicalization, so that
failed symlink check operations don't throw before the check does.
Fixes:
warning: destructor called on non-final 'nix::ParseUnquoted' that has virtual functions but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]
**`Value` and `const`**
These two deserve some explanation. We'll get to lists later.
Values can normally be thought of as immutable, except they are
are also the vehicle for call by need, which must be implemented
using mutation.
This circumstance makes a `const Value` a rather useless thing:
- If it's a thunk, you can't evaluate it, except by copying, but
that would not be call by need.
- If it's not a thunk, you know the type, so the method that
acquired it for you should have returned something more specific,
such as a `const Bindings &` (which actually does make sense
because that's an immutable span of pointers to mutable `Value`s.
- If you don't care about the type yet, you might establish the
convention that `const Value` means `deepSeq`-ed data, but
this is hardly useful and not actually as safe as you would
supposedly want to trust it to be - just convention.
**Lists**
`std::span` is a tuple of pointer and size - just what we need.
We don't return them as `const Value`, because considering the
first bullet point we discussed before, we'd have to force all
the list values, which isn't what we want.
So what we end up with is a nice representation of a list in
weak head normal form: the spine is immutable, but the
items may need some evaluation later.
Closes#9343
See that issue for motivation.
Installing these is disabled by default, but we enable it (and the
additional output we want isntall these too so as not to clutter the
existing ones) to use in cross builds and dev shells.
Try to stay away from stack overflows.
These small vectors use stack space. Most instances will not need
to allocate because in general most things are small, and large
things are worth heap allocating.
16 * 3 * word = 384 bytes is still quite a bit, but these functions
tend not to be part of deep recursions.
This makes stack usage significantly more compact, allowing larger
amounts of data to be processed on the same stack.
PrimOp functions with more than 8 positional (curried) arguments
should use an attrset instead.
VLAs are a dangerous feature, and their usage triggers an undefined
behavior since theire size can be zero in some cases.
So replace them with `boost::small_vector`s which fit the same goal but
are safer.
It's also incidentally consistently 1% faster on the benchmarks.
SHELL was inherited from the system environment. This resulted in a new
shell being started, but with SHELL still referring to the system shell
and not the one used by nix-develop.
Applications like make, use SHELL to run commands, which meant that
top-level commands are run inside the nix-develop-shell, but
sub-commands are ran inside the system shell.
This setenv forces SHELL to always be set to the shell used by
nix-develop.
This is the core functionality but just unit-tested and not yet made
part of the store layer. This is because there is some tech debt around
(a) repeated boilerplate hashing objects (b) better integration of the
new `SourceAccessor` type that needs to be cleaned up first.
Part of RFC 133
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>
As discussed in our last meeting, we need a bit more time, but we are
"time boxing" the work left to do to ensure there is not unbounded
delay.
Rather than putting it back underneath `flakes`, though, put it
underneath its own `fetch-tree` experimental feature (which `flakes`
includes/implies). This signals our commitment to the plan to stabilize
it first without waiting to go through the rest of Flakes, and also will
give users a "release candidate" when we get closer to stabilization.
This reverts commit 4112dd1fc9.
These usages of the working directory are perhaps unlikely to
interact with shebangs, but the code is more consistent this way,
and we're less likely to miss usages that do interact.
Enables shebang usage of nix shell. All arguments with `#! nix` get
added to the nix invocation. This implementation does NOT set any
additional arguments other than placing the script path itself as the
first argument such that the interpreter can utilize it.
Example below:
```
#!/usr/bin/env nix
#! nix shell --quiet
#! nix nixpkgs#bash
#! nix nixpkgs#shellcheck
#! nix nixpkgs#hello
#! nix --ignore-environment --command bash
# shellcheck shell=bash
set -eu
shellcheck "$0" || exit 1
function main {
hello
echo 0:"$0" 1:"$1" 2:"$2"
}
"$@"
```
fix: include programName usage
EDIT: For posterity I've changed shellwords to shellwords2 in order
not to interfere with other changes during a rebase.
shellwords2 is removed in a later commit. -- roberth
Users may select specific outputs using the ^output syntax or selecting
any output using ^*.
URL parsing currently doesn't support these kinds of output references:
parsing will fail.
Currently `queryRegex` was reused for URL fragments, which didn't
include support for ^. Now queryRegex has been split from fragmentRegex,
where only the fragmentRegex supports ^.
* Fix boost::bad_format_string exception in builtins.addErrorContext
The message passed to addTrace was incorrectly being used as a format
string and this this would cause an exception when the string contained
a '%', which can be hit in places where arbitrary file paths are
interpolated.
* add test
Before it returned a list of JSON objects with store object information,
including the path in each object. Now, it maps the paths to JSON
objects with the metadata sans path.
This matches how `nix derivation show` works.
Quite hillariously, none of our existing functional tests caught this
change to `path-info --json` though they did use it. So just new
functional tests need to be added.
`Store::pathInfoToJSON` was a rather baroque functions, being full of
parameters to support both parsed derivations and `nix path-info`. The
common core of each, a simple `dValidPathInfo::toJSON` function, is
factored out, but the rest of the logic is just duplicated and then
specialized to its use-case (at which point it is no longer that
duplicated).
This keeps the human oriented CLI logic (which is currently unstable)
and the core domain logic (export reference graphs with structured
attrs, which is stable), separate, which I think is better.
Spent a while debugging why `nix-copy-closure` wasn't working anymore
and it was my shell RC printing something I added for debug.
Hopefully this can save someone else some time.
All OS and IO operations should be moved out, leaving only some misc
portable pure functions.
This is useful to avoid copious CPP when doing things like Windows and
Emscripten ports.
Newly exposed functions to break cycles:
- `restoreSignals`
- `updateWindowSize`
The new `MemorySourceAccessor` rather than being a slightly lossy flat
map is a complete in-memory model of file system objects.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
This adds publicKeys as an optional fetcher input attribute to flakes
and builtins.fetchGit to provide a nix interface for the json-encoded
`publicKeys` attribute of the git fetcher.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
This implements the git input attributes `verifyCommit`, `keytype`,
`publicKey` and `publicKeys` as experimental feature
`verified-fetches`. `publicKeys` should be a json string.
This representation was chosen because all attributes must be of type bool,
int or string so they can be included in flake uris (see definition of
fetchers::Attr).
When doing local builds, we get phase reporting lines in the log file,
they look like '@nix {"action":"setPhase","phase":"unpackPhase"}'.
With the ssh-ng protocol, we do have access to these messages, but since we
are only including messages of type resBuildLogLine in the logs, the phase
information does not end up in the log file.
The phase reporting could probably be improved altoghether (it looks like it
is kind of accidental that these JSON messages for phase reporting show up
but others don't, just because they are actually emitted by nixpkgs' stdenv),
but as a first step I propose to make ssh-ng behave in the same way as local builds do.
Deduplicating code moreover enforcing the pattern means:
- It is easier to write new characterization tests because less boilerplate
- It is harder to mess up new tests because there are fewer places to
make mistakes.
Co-authored-by: Jacek Galowicz <jacek@galowicz.de>
Noticed because of a warning during an rpm build:
*** WARNING: ./usr/src/debug/nix-2.18.1-1.fc40.x86_64/src/nix-copy-closure/nix-copy-closure.cc is executable but has no shebang, removing executable bit
*** WARNING: ./usr/src/debug/nix-2.18.1-1.fc40.x86_64/src/nix-channel/nix-channel.cc is executable but has no shebang, removing executable bit
I wouldn't call it *good* yet, but this will do for now.
- `RetrieveRegularNARSink` renamed to `RegularFileSink` and moved
accordingly because it actually has nothing to do with NARs in
particular.
- its `fd` field is also marked private
- `copyRecursive` introduced to dump a `SourceAccessor` into a
`ParseSink`.
- `NullParseSink` made so `ParseSink` no longer has sketchy default
methods.
This was done while updating #8918 to work with the new
`SourceAccessor`.
Adding the inputPath as a positional feature uncovered this bug.
As positional argument forms were discarded from the `expectedArgs`
list, their closures were not. When the `.completer` closure was then
called, part of the surrounding object did not exist anymore.
This didn't cause an issue before, but with the new call to
`getEvalState()` in the "inputs" completer in nix/flake.cc, a segfault
was triggered reproducibly on invalid memory access to the `this`
pointer, which was always 0.
The solution of splicing the argument forms into a new list to extend
their lifetime is a bit of a hack, but I was unable to get the "nicer"
iterator-based solution to work.
Instead of making a complete copy of the repo, fetching the
submodules, and writing the result to the store (which is all
superexpensive), we now fetch the submodules recursively using the Git
fetcher, and return a union accessor that "mounts" the accessors for
the submodules on top of the root accessor.
We now have `schemeName` and `allowedAttrs` functions for this purpose.
We look up the schema with the former; we restrict the set of input
attributes with the latter.
The brings a number of advantages, including:
- Easier to update test data if design changes (and I do think our
derivation JSON is not yet complaint with the guidelines).
- Easier to reuse test data in other implementations, inching closer to
compliance tests for Nix *the concept* rather than any one
implementation.
Before the change builder ID exhaustion printed the following message:
[0/1 built] waiting for UID to build '/nix/store/hiy9136x0iyib4ssh3w3r5m8pxjnad50-python3.11-breathe-4.35.0.drv'
After the change it should be:
[0/1 built] waiting for a free build user ID for '/nix/store/hiy9136x0iyib4ssh3w3r5m8pxjnad50-python3.11-breathe-4.35.0.drv'
Committing a lock file using markFileChanged() required the input to
be writable by the caller in the local filesystem (using the path
returned by getSourcePath()). putFile() abstracts over this.
Rather than having a misc tutorial page in the grab-bag "package management" section, this information should just be part of the S3 store docs.
---------
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
End goal: make `(mkDerivation x).drvPath` behave like a non-DrvDeep
context.
Problem: users won't be able to recover the DrvDeep behavior when
nixpkgs makes this change.
Solution: add this primop.
The new primop is fairly simple, and is supposed to complement other
existing ones (`builtins.storePath`, `builtins.outputOf`) so there are
simple ways to construct strings with every type of string context
element.
(It allows nothing we couldn't already do with `builtins.getContext` and `builtins.appendContext`, which is also true of those other two primops.)
This was originally in #8595, but then it was proposed to land some doc
changes separately. So now the code changes proper is just moved to
this, and the doc will be done in that.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.nore
github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io
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
