-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix repl test for buildReadlineNoMarkdown
#11167
Conversation
Slightly custom because the automated formatting messes up the braced initializer with named fields.
b86a7f5
to
f35d1a1
Compare
For testing only.
... as well as match buildReadlineNoMarkdown. Unfortunately it doesn't support long inputs or multiline inputs for now. This needs to make better use of the interacter interface.
f35d1a1
to
7d4d34a
Compare
The CI failure on e48e0cb is similar to #11144 (comment), so I've cherry-picked a commit that logs more before aborting. |
* @note: This assumes that the logger is operational | ||
*/ | ||
[[noreturn]] | ||
void panic(std::string_view msg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer a macro similar to printError()
and debug()
that wraps fmt()
so we can write stuff like
panic("bla %s", arg);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to keep it somewhat simple, because it might have to perform in a somewhat broken process state, though probably it shouldn't even use the logger then, and just write
to fd 2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made them signal safe.
For non-critical errors, we can throw Error
as we did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if signal safety is worth pursuing, since it's really too hard to ensure that nothing ever calls malloc()
or whatever. It's better to keep signal handlers trivial (e.g. they should only wake up a thread by writing to a file descriptor).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed any mention of that. The goal is just to be robust; signal safety would have been a nice property, but it doesn't have to be a goal.
src/libutil/error.hh
Outdated
* | ||
* @note: This assumes that the logger is operational | ||
*/ | ||
#define NIX_UNREACHABLE() (panic(__FILE__, __LINE__, __func__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define NIX_UNREACHABLE() (panic(__FILE__, __LINE__, __func__)) | |
#define unreachable() (panic(__FILE__, __LINE__, __func__)) |
04c54b5
to
3172e88
Compare
Plus one or two tweaks.
ba99ca0
to
55a654a
Compare
Merging because it's blocking
|
Motivation
buildReadlineNoMarkdown
buildContext
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.