Migrate Unix implementation to signal-hook-registry#98
Migrate Unix implementation to signal-hook-registry#98notgull wants to merge 2 commits intoDetegr:masterfrom
Conversation
|
@Detegr Poke, any chance you can merge this? CI is failing because of spurious networks failures in AppVeyor. |
|
I still don't know how I feel about this. But I feel the crate does too much and using it would defeat the purpose of this crate. I get that it's not ideal that having the both crates in a dependency chain just breaks things, that's something I'd like to fix and proposed a fix for that earlier. However, I'd like to see a concrete example of such dependency chain. From a brief look, uses of |
This is probably the best way of doing it, as this is what
Signals in Windows CRT are practically just a wrapper around console control handlers, so it doesn't have to be this way.
Ideally, the best way of doing this is to avoid stepping on others' toes is to just use this strategy.
It's less of "this does break something right now" and more of "this could cause things to break in an extremely hard-to-debug and potentially unsound way in the future". I'm not aware of any cases where ctrlc and signal hook are used in the same dependency graph, but if it does happen you could end up with a very messy situation. |
|
I'm with @notgull on this one. In my opinion there's little harm in following the structure of a largely accepted, solid implementation like |
|
I decided against using a multi-hundred line dependency for fixing a hyphotetical bug, and implemented a simple "see if we have signal handler installed, and error out if we do" strategy instead. If |
|
This change is breaking a lot of other people's stuff and I'm regretting implementing it. I moved the checking implementation to |
|
Good deal. Seems like that should do the trick for now. Too bad there isn't another, robust implementation hanging around. |
|
Would there be any interest in reviving this patch in some form? I have an actual project with both |
|
Possibly. As you might have noticed I've tried to keep the maintenance burden to a minimum and turned down almost every new feature, I think it would make sense to transition this crate to be a wrapper for |
Closes #97 by migrating the Unix backend to signal-hook-registry.
To avoid a breaking change the
signal-hook-registryerrors are translated toEINVAL, since that's what they usually are anyways.