Re: [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
Jeff King p...@peff.net writes: Since this can be thought of as act more like system(3), I wondered whether the signal-ignore logic should be moved into run-command, or even used by default for blocking calls to run_command (which are basically our version of system(3)). But it is detrimental in the common case that the child is not taking control of the terminal, and is just an implementation detail (e.g., we call git update-ref behind the scenes, but the user does not know or care). If they hit ^C during such a run and we are ignoring SIGINT, then either: 1. we will notice the child died by signal and report an error in the subprocess rather than just dying; the end result is similar, but the error is unnecessarily confusing 2. we do not bother to check the child's return code (because we do not care whether the child succeeded or not, like a gc --auto); we end up totally ignoring the user's request to abort the operation So I do not think we care about this behavior except for launching the editor. And the signal-propagation behavior of 5/5 is really so weirdly editor-specific (because it is about behaving well whether the child blocks signals or not). Nicely explained. Very much appreciated. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
On Fri, Nov 30, 2012 at 05:39:43PM -0500, Jeff King wrote: This is a re-roll of the pf/editor-ignore-sigint series. People mentioned some buggy editors which go into an infinite EIO loop when their parent dies due to SIGQUIT. That should be a non-issue now, as we will be ignoring SIGQUIT. And even if you could replicate it (e.g., with another signal) those programs should be (and reportedly have been) fixed. It is not git's job to babysit its child processes. Also some good editors printed error message after they got EIO, confusing the user. Looks good to me. I've tested this with ed (always ignores SIGINT and SIGQUIT), vim (always ignores SIGINT, but dies after three SIGQUIT) and sleep (dies after SIGINT and SIGQUIT) and git works now as expected. Doing what editor does is probably the best thing to do. Tested-by: Krzysztof Mazur krzys...@podlesie.net Thanks, Krzysiek -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
jeff wrote: This is a re-roll of the pf/editor-ignore-sigint series. There are two changes from the original: 1. We ignore both SIGINT and SIGQUIT for least surprise compared to system(3). 2. We now use code + 128 to look for signal death (instead of WTERMSIG), as per run-command's documentation on how it munges the code. this series all looks good to me. thanks for re- and re-re-rolling. paul People mentioned some buggy editors which go into an infinite EIO loop when their parent dies due to SIGQUIT. That should be a non-issue now, as we will be ignoring SIGQUIT. And even if you could replicate it (e.g., with another signal) those programs should be (and reportedly have been) fixed. It is not git's job to babysit its child processes. The patches are: [1/5]: run-command: drop silent_exec_failure arg from wait_or_whine [2/5]: launch_editor: refactor to use start/finish_command [3/5]: launch_editor: ignore terminal signals while editor has control [4/5]: run-command: do not warn about child death from terminal [5/5]: launch_editor: propagate signals from editor to git Since this can be thought of as act more like system(3), I wondered whether the signal-ignore logic should be moved into run-command, or even used by default for blocking calls to run_command (which are basically our version of system(3)). But it is detrimental in the common case that the child is not taking control of the terminal, and is just an implementation detail (e.g., we call git update-ref behind the scenes, but the user does not know or care). If they hit ^C during such a run and we are ignoring SIGINT, then either: 1. we will notice the child died by signal and report an error in the subprocess rather than just dying; the end result is similar, but the error is unnecessarily confusing 2. we do not bother to check the child's return code (because we do not care whether the child succeeded or not, like a gc --auto); we end up totally ignoring the user's request to abort the operation So I do not think we care about this behavior except for launching the editor. And the signal-propagation behavior of 5/5 is really so weirdly editor-specific (because it is about behaving well whether the child blocks signals or not). -Peff =- paul fox, p...@foxharp.boston.ma.us (arlington, ma, where it's 24.8 degrees) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
This is a re-roll of the pf/editor-ignore-sigint series. There are two changes from the original: 1. We ignore both SIGINT and SIGQUIT for least surprise compared to system(3). 2. We now use code + 128 to look for signal death (instead of WTERMSIG), as per run-command's documentation on how it munges the code. People mentioned some buggy editors which go into an infinite EIO loop when their parent dies due to SIGQUIT. That should be a non-issue now, as we will be ignoring SIGQUIT. And even if you could replicate it (e.g., with another signal) those programs should be (and reportedly have been) fixed. It is not git's job to babysit its child processes. The patches are: [1/5]: run-command: drop silent_exec_failure arg from wait_or_whine [2/5]: launch_editor: refactor to use start/finish_command [3/5]: launch_editor: ignore terminal signals while editor has control [4/5]: run-command: do not warn about child death from terminal [5/5]: launch_editor: propagate signals from editor to git Since this can be thought of as act more like system(3), I wondered whether the signal-ignore logic should be moved into run-command, or even used by default for blocking calls to run_command (which are basically our version of system(3)). But it is detrimental in the common case that the child is not taking control of the terminal, and is just an implementation detail (e.g., we call git update-ref behind the scenes, but the user does not know or care). If they hit ^C during such a run and we are ignoring SIGINT, then either: 1. we will notice the child died by signal and report an error in the subprocess rather than just dying; the end result is similar, but the error is unnecessarily confusing 2. we do not bother to check the child's return code (because we do not care whether the child succeeded or not, like a gc --auto); we end up totally ignoring the user's request to abort the operation So I do not think we care about this behavior except for launching the editor. And the signal-propagation behavior of 5/5 is really so weirdly editor-specific (because it is about behaving well whether the child blocks signals or not). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
