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.
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
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).
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
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.
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.
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
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`.
Use `diff --color=always` to print colored output for language test
failures. I've also flipped the arguments so that expected lines missing
from the actual output will be marked with a red `-` and additional
lines found in the actual output will be marked with a green `+`.
Previously it was the other way around, which was very confusing.
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.
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).
Otherwise we get a stray `tests/functional/result`, which can cause
spurious failures later.
(I got a failure because the test temp dir effecting the store dir
changed. This caused a test later because Nix didn't want to remove the
old `result` because it wasn't pointing inside the new Nix store.)
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`.
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)
| ^
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.
On macOS in the `nix develop` shell, `make
tests/functional/logging.sh.test` errors:
++(logging.sh:18) mktemp
+(logging.sh:18) builder=/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/tmp.StuabKUhMh
+(logging.sh:19) echo -e '#!/bin/sh\nmkdir $out'
+++(logging.sh:22) mktemp -d
++(logging.sh:22) nix-build -E 'with import ./config.nix; mkDerivation { name = "fnord"; builder = /var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/tmp.StuabKUhMh; }' --out-link /var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/tmp.oaKcy0NXqC/result
error:
… while calling the 'derivationStrict' builtin
at <nix/derivation-internal.nix>:9:12:
8|
9| strict = derivationStrict drvAttrs;
| ^
10|
… while evaluating derivation 'fnord'
whose name attribute is located at «string»:1:42
… while evaluating attribute 'args' of derivation 'fnord'
at /Users/wiggles/nix/tests/functional/config.nix:23:7:
22| builder = shell;
23| args = ["-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
| ^
24| if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
error: path '/var' is a symlink
+(logging.sh:22) outp=
++(logging.sh:22) onError
++(/Users/wiggles/nix/tests/functional/common/vars-and-functions.sh:237) set +x
logging.sh: test failed at:
main in logging.sh:22
This is because `mktemp` returns a path like
`/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/tmp.qDY24l6bIM`,
where `/var` is a symlink to `/private/var`.
Then, we attempt to use that path as a `builder`, which errors because
symlinks are impure or whatever.
Anyways, we can fix this by using `realpath "$(mktemp)"` instead of
`mktemp` directly.
NB: This error doesn't seem to happen when I run the tests through `nix
flake check`. I'm not sure if Nix does something to `TMP` in that case.
As part of the CLI stabilization effort, the last remaining checkbox (at
the moment) for `nix daemon` is that it "needs testing". This implements
the proposal of using `nix daemon` in place of `nix-daemon` in the test
suite.
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|
It works with both `ssh://` and `ssh-ng://` now since #9600 (and
`ssh-ng:// didn't work before that).
Also, by making the two tests share code, we nudge ourselves towards
making sure there is feature parity.
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.
* 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 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
```
`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>
The basic idea here is to separate a few intertwined notions:
1. Not all "run bash tests" are "install tests"
2. Not all "run bash tests" use `tests/functional/init.sh`, or any
pre-test initialization at all.
This will used in the next commit when we have a test that check unit
test golden master data.
Also, move our custom `PS4` from the test to the test runner, as it is
part of how we want to display the tests, not the test themselves.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
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.
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.
This adds simple tests of the commit signature verification mechanism of
fetchGit and its flake input wrapper.
OpenSSH is added to the build dependencies since it's needed to create
a key when testing the functionality. It is neither a built- nor a
runtime dependency.
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.
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
- Remove some stray saved error messages that didn't correspond to any
test, because they were renamed in
d11faa01b5.
- Need `--eval` in test failure test in order to get in "read-only" mode
where we don't try to write to the store. (The other tests already do
this.)
- Need `--strict` so top-level attribute sets are still forced, like
they are without `--eval`.
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.
Additionally this skipping of the building is reimplemented to be a bit
more robust and use the same idioms as the functionality for skipping
the tests. In particular, it will now work even if the source files
exist, so we can do this during development too.
I think it is bad for these reasons when `tests/` contains a mix of
functional and integration tests
- Concepts is harder to understand, the documentation makes a good
unit vs functional vs integration distinction, but when the
integration tests are just two subdirs within `tests/` this is not
clear.
- Source filtering in the `flake.nix` is more complex. We need to
filter out some of the dirs from `tests/`, rather than simply pick
the dirs we want and take all of them. This is a good sign the
structure of what we are trying to do is not matching the structure
of the files.
With this change we have a clean:
```shell-session
$ git show 'HEAD:tests'
tree HEAD:tests
functional/
installer/
nixos/
```
