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`.
(cherry picked from commit c6a89c1a16)
Change-Id: Iced91ba4e00ca9e801518071fb43798936cbd05a
Catch `Error`, not `BaseError` in `ValuePrinter`
BaseError includes Interrupt. We probably don't want the value printer to tell you Ctrl-C was pressed while it was printing.
(cherry picked from commit c291d2d8dd)
Change-Id: I70b105bfb2f52a8f345ae0281d12f022aa36b14e
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.
(cherry picked from commit 09a1128d9e)
Change-Id: I73d9f66be4555168e048cb2d542277251580c2d1
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.
(cherry picked from commit b596cc9e79)
Change-Id: I481a7623afc783e9d28a6eb4627552cf8a780986
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.
(cherry picked from commit e1aa585964)
Change-Id: Ibc704567462bb40f37cda05d8fadd465519db5f5
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.
(cherry picked from commit 835a6c7bcf)
Change-Id: I17cab8a6cc14cac5b64624431957bfcf04140809
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.
(cherry picked from commit 0076056164)
Change-Id: I92feaec796530e1d4d0f7d4fba924229591cea95
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.
(cherry picked from commit e8d9de967f)
Change-Id: I560c50d11dceddc2d7cf9ed2c6c631a309ce574e
Print the value in `error: cannot coerce` messages
(cherry picked from commit 5b7bfd2d6b)
===
test taken from 6e8d5983143ae576e3f4b1d2954a5267f2943a49; it was added
previously (and not backported because its pr was a mostly-revert), but
it's useful to have around.
Change-Id: Icbd14b55e3610ce7b774667bf14b82e6dc717982
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`.
(cherry picked from commit 0fa08b4516, )
===
Restore ambiguous value printer for `nix-instantiate`
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.
(cherry picked from commit df84dd4d8d)
Change-Id: I7cca6b4b53cd0642f2d49af657d5676a8554c9f8
Don't attempt to `git add` ignored files
(cherry picked from commit 359990dfdc)
===
also added a regression test that isn't upstream to be sure we're
actually fixing the bug.
Change-Id: I8267a3d0ece9909d8008b7435b90e7b3eee366f6
Restore `builtins.pathExists` behavior on broken symlinks
(cherry picked from commit d53c8901ef7f2033855dd99063522e3d56a19dab)
===
note that this variant differs markedly from the source commit because
we haven't endured quite as much lazy trees.
Change-Id: I0facf282f21fe0db4134be5c65a8368c1b3a06fc
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).
Test that we can't leverage abstract unix domain sockets to leak file
descriptors out of the sandbox and modify the path after it has been
registered.
(cherry picked from commit 2dadfeb690e7f4b8f97298e29791d202fdba5ca6)
(tests cherry picked from commit c854ae5b3078ac5d99fa75fe148005044809e18c)
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Co-authored-by: Theophane Hufschmitt <theophane.hufschmitt@tweag.io>
Co-authored-by: Tom Bereknyei <tomberek@gmail.com>
Change-Id: I87cd58f1c0a4f7b7a610d354206b33301e47b1a4
Combine `AbstractPos`, `PosAdapter`, and `Pos`
(cherry picked from commit 113499d16f)
===
this is a bit cursed because originally it was based on InputAccessor
code that we don't have and moved/patched features we likewise don't
have (fetchToStore caching, all the individual accessors,
ContentAddressMethod). the commit is adjusted accordingly to
match (remove caching, ignore accessors, use FileIngestionMethod).
note that `state.rootPath . CanonPath == abs` and
computeStorePathForPath works relative to cwd, so the slight rewrite in
the moved fetchToStore is legal.
Change-Id: I05fd340c273f0bcc8ffabfebdc4a88b98083bce5
Add position information to `while evaluating the attribute` errors in the debugger
(cherry picked from commit ffe67c86a8ef3695e5c8b9c9800c192ac633dded)
Change-Id: I177ea5ec60898abe09fb9d80d9602b2a32ff8f44
Fix "Failed tcsetattr(TCSADRAIN)" when `nix repl` is not a TTY
(cherry picked from commit 864fc85fc88ff092725ba99907611b2b8d2205fb)
Change-Id: I8198674b935fabd741a349cc74544e61c53ea7b3
`nix`: Fix `haveInternet` to check for proxy
(cherry picked from commit accae60e7710a18f6f2bd7d2f4cd836bcd76b684)
Change-Id: I996dafdcd266f4bc5806386c86b19040120842bf
Say how many channels were unpacked in nix-channel
(cherry picked from commit 9ae665b9e1dc64c507ab6002fc5d7824208f3777)
Change-Id: Ie0950cf32123b550c5b83981a020e513f72a9b7c
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.
Upstream-PR: https://github.com/NixOS/nix/pull/10126
Original-Change-Id: I513cceaac1371decb3d96231e6ef9181c910c218
Change-Id: I531a51f6348a746e8e41d88203b08f614898356c
eldritch horrors
jade
puck