-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Better signals interface #10401
Better signals interface #10401
Conversation
1a1448b
to
2edc3c9
Compare
* some extra Unix-only interfaces. | ||
* | ||
* (The only reason everything about signals isn't Unix-only is some | ||
* no-op definitions are provided on Windows to avoid excess CPP in |
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.
Not present in this PR, that's coming later?
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.
Yes
src/libutil/signals.hh
Outdated
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.
Appreciate the comment notes on this. Looks good.
This avoids some CPP and accidentally using Unix stuff in client code.
2edc3c9
to
50f621b
Compare
/** | ||
* @note Does nothing on Windows | ||
*/ | ||
static inline void setInterruptCheck(std::function<bool()> interruptCheck); |
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.
GCC says
In file included from src/nix-collect-garbage/nix-collect-garbage.cc:2:
src/libutil/signals.hh:27:20: warning: ‘void nix::setInterruptCheck(std::function<bool()>)’ declared ‘static’ but never defined [-Wunused-function]
27 | static inline void setInterruptCheck(std::function<bool()> interruptCheck);
| ^~~~~~~~~~~~~~~~~
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.
Fixed in #10411.
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.
Thanks, sorry about that!
Motivation
This avoids some CPP and accidentally using Unix stuff in client code.
Context
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.