Re: [PATCH 0/3] fix ^C killing pager when running alias

[Jacob Keller]

On Sat, Jan 7, 2017 at 3:26 PM, Jacob Keller  wrote:
> On Fri, Jan 6, 2017 at 5:14 PM, Jeff King  wrote:
>> On Fri, Jan 06, 2017 at 06:20:42PM -0500, Jeff King wrote:
>>
>>> > In general, I think it is wrong to wait for child processes when a signal
>>> > was received. After all, it is the purpose of a (deadly) signal to have 
>>> > the
>>> > process go away. There may be programs that know it better, like less, but
>>> > git should not attempt to know better in general.
>>> >
>>> > We do apply some special behavior for certain cases like we do for the
>>> > pager. And now the case with aliases is another special situation. The
>>> > parent git process only delegates to the child, and as such it is 
>>> > reasonable
>>> > that it binds its life time to the first child, which executes the 
>>> > expanded
>>> > alias.
>>>
>>> Yeah, I think I agree. That binding is something you want in many cases,
>>> but not necessarily all. The original purpose of clean_on_exit was to
>>> create a binding like that, but of course it can be (and with the
>>> smudge-filter stuff, arguably has been) used for other cases, too.
>>>
>>> I'll work up a patch that makes it a separate option, which should be
>>> pretty easy.
>>
>> Yeah, this did turn out to be really easy. I spent most of the time
>> trying to explain the issue in the commit message in a sane way.
>> Hopefully it didn't end up _too_ long. :)
>>
>> The interesting bit is in the third one. The first is a necessary
>> preparatory step, and the second is a cleanup I noticed in the
>> neighborhood.
>>
>>   [1/3]: execv_dashed_external: use child_process struct
>>   [2/3]: execv_dashed_external: stop exiting with negative code
>>   [3/3]: execv_dashed_external: wait for child on signal death
>>
>>  git.c | 36 +++-
>>  run-command.c | 19 +++
>>  run-command.h |  1 +
>>  3 files changed, 35 insertions(+), 21 deletions(-)
>>
>> -Peff
>
> I don't see the rest of the patches on the list..?
>
> Thanks,
> Jake

They showed up on public inbox so I assume it must be some spam filter
on my end..

Hmm,
Jake

Re: [PATCH 0/3] fix ^C killing pager when running alias

[Jacob Keller]

On Fri, Jan 6, 2017 at 5:14 PM, Jeff King  wrote:
> On Fri, Jan 06, 2017 at 06:20:42PM -0500, Jeff King wrote:
>
>> > In general, I think it is wrong to wait for child processes when a signal
>> > was received. After all, it is the purpose of a (deadly) signal to have the
>> > process go away. There may be programs that know it better, like less, but
>> > git should not attempt to know better in general.
>> >
>> > We do apply some special behavior for certain cases like we do for the
>> > pager. And now the case with aliases is another special situation. The
>> > parent git process only delegates to the child, and as such it is 
>> > reasonable
>> > that it binds its life time to the first child, which executes the expanded
>> > alias.
>>
>> Yeah, I think I agree. That binding is something you want in many cases,
>> but not necessarily all. The original purpose of clean_on_exit was to
>> create a binding like that, but of course it can be (and with the
>> smudge-filter stuff, arguably has been) used for other cases, too.
>>
>> I'll work up a patch that makes it a separate option, which should be
>> pretty easy.
>
> Yeah, this did turn out to be really easy. I spent most of the time
> trying to explain the issue in the commit message in a sane way.
> Hopefully it didn't end up _too_ long. :)
>
> The interesting bit is in the third one. The first is a necessary
> preparatory step, and the second is a cleanup I noticed in the
> neighborhood.
>
>   [1/3]: execv_dashed_external: use child_process struct
>   [2/3]: execv_dashed_external: stop exiting with negative code
>   [3/3]: execv_dashed_external: wait for child on signal death
>
>  git.c | 36 +++-
>  run-command.c | 19 +++
>  run-command.h |  1 +
>  3 files changed, 35 insertions(+), 21 deletions(-)
>
> -Peff

I don't see the rest of the patches on the list..?

Thanks,
Jake

[PATCH 0/3] fix ^C killing pager when running alias

[Jeff King]

On Fri, Jan 06, 2017 at 06:20:42PM -0500, Jeff King wrote:

> > In general, I think it is wrong to wait for child processes when a signal
> > was received. After all, it is the purpose of a (deadly) signal to have the
> > process go away. There may be programs that know it better, like less, but
> > git should not attempt to know better in general.
> > 
> > We do apply some special behavior for certain cases like we do for the
> > pager. And now the case with aliases is another special situation. The
> > parent git process only delegates to the child, and as such it is reasonable
> > that it binds its life time to the first child, which executes the expanded
> > alias.
> 
> Yeah, I think I agree. That binding is something you want in many cases,
> but not necessarily all. The original purpose of clean_on_exit was to
> create a binding like that, but of course it can be (and with the
> smudge-filter stuff, arguably has been) used for other cases, too.
> 
> I'll work up a patch that makes it a separate option, which should be
> pretty easy.

Yeah, this did turn out to be really easy. I spent most of the time
trying to explain the issue in the commit message in a sane way.
Hopefully it didn't end up _too_ long. :)

The interesting bit is in the third one. The first is a necessary
preparatory step, and the second is a cleanup I noticed in the
neighborhood.

  [1/3]: execv_dashed_external: use child_process struct
  [2/3]: execv_dashed_external: stop exiting with negative code
  [3/3]: execv_dashed_external: wait for child on signal death

 git.c | 36 +++-
 run-command.c | 19 +++
 run-command.h |  1 +
 3 files changed, 35 insertions(+), 21 deletions(-)

-Peff