After the removal of the InputAccessor::fetchToStore() method, the
only remaining functionality in InputAccessor was `fingerprint` and
`getLastModified()`, and there is no reason to keep those in a
separate class.
This missing GC root wasn't much of a problem before, because the
heap would end up with a reference to the `baseEnv` pretty soon,
but when unit testing, the construction of `EvalState` doesn't
necessarily happen well before GC runs for the first time.
Found while unit testing the Rust bindings that currently reside
at https://github.com/nixops4/nixops4/tree/main/rust
Add a method to check if a value has been initialized. This helps avoid
segfaults when calling `type()`.
Useful in the context of the new C API.
Closes#10524
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>
Code operating on store objects (including creating them) should, in
general, use `ContentAddressMethod` rather than `FileIngestionMethod`.
See also dfc876531f which included some
similar refactors.
Now that we have a few things identifying content address methods by
name, we should be consistent about it.
Move up the `parseHashAlgoOpt` for tidiness too.
Discussed this change for consistency's sake as part of #8876
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
This was used in only one place, namely builtins.fetchurl with an
expected hash. Since this can cause similar issues as described
in #9814 and #9905 with the "locked" flag for fetchTarball and fetchTree,
let's just remove it.
Note that if an expected hash is given and the hash algorithm is
SHA-256, then we will never do a download anyway if the resulting
store path already exists. So removing the "locked" flag will only
cause potentially unnecessary HTTP requests (subject to the tarball
TTL) for non-SHA-256 hashes.
code blocks, if not surrounded by empty lines, have the language
tags (in these cases, always `nix`) show up in the output of :doc.
for example:
nix-repl> :doc builtins.parseFlakeRef
Synopsis: builtins.parseFlakeRef flake-ref
Parse a flake reference, and return its exploded form.
For example: nix builtins.parseFlakeRef
"github:NixOS/nixpkgs/23.05?dir=lib" evaluates to: nix { dir =
"lib"; owner = "NixOS"; ref = "23.05"; repo = "nixpkgs"; type =
"github"; }
is now instead:
nix-repl> :doc builtins.parseFlakeRef
Synopsis: builtins.parseFlakeRef flake-ref
Parse a flake reference, and return its exploded form.
For example:
| builtins.parseFlakeRef "github:NixOS/nixpkgs/23.05?dir=lib"
evaluates to:
| { dir = "lib"; owner = "NixOS"; ref = "23.05"; repo = "nixpkgs"; type = "github"; }
This was part of approved PR #10021. Unfortunately that one is stalled
on a peculiar Linux test timeout, so trying to get bits of it merged
first to bisect failure.
A possible use of them might have been to figure out the paths
(which can now be retrieved with maybePathsOut), but I have
not found evidence that it was used this way, and it would have
been broken, because non-CA outputs weren't recorded in the map.
Thunks are now overwritten by a helper function
`Value::finishValue(newType, payload)` (where `payload` is the
original anonymous union inside `Value`). This helps to ensure we
never update a value elsewhere, since that would be incompatible with
parallel evaluation (i.e. after a value has transitioned from being a
thunk to being a non-thunk, it should be immutable).
There were two places where this happened: `Value::mkString()` and
`ExprAttrs::eval()`.
This PR also adds a bunch of accessor functions for value contents,
like `Value::integer()` to access the integer field in the union.
Fixes
terminate called after throwing an instance of 'boost::wrapexcept<boost::io::too_few_args>'
what(): boost::too_few_args: format-string referred to more arguments than were passed
Aborted (core dumped)
for type errors in AttrCursor.
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);
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.
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.
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().
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
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.
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?
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).
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.
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
];
}
```
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`.
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.
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»}
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.
