public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Benjamin Quorning <bquorning@zendesk•com>, git@vger•kernel.org
Subject: Re: [PATCH] add--interactive: leave main loop on read error
Date: Mon, 15 Dec 2014 14:49:24 -0800	[thread overview]
Message-ID: <xmqqvblcbkaz.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20141215163527.GA15136@peff.net> (Jeff King's message of "Mon, 15 Dec 2014 11:35:27 -0500")

Jeff King <peff@peff•net> writes:

> On Mon, Dec 15, 2014 at 03:41:45PM +0100, Benjamin Quorning wrote:
>
>> Reproduction steps:
>> 
>> 1. A repository with a changed file, but no staged changes.
>> 2. Execute `git checkout --patch`
>> 3. When asked, press `e` to edit a chunk (opens an external editor in my case)
>> 4. With the editor still open, click ctrl-C in the terminal.
>> 5. The diff that was being edited, and the command prompt ("discard
>> this hunk from worktree" etc) is printed to the screen, over and over
>> again.
>> 6. I have to grep and kill this process: /usr/bin/perl
>> /usr/local/Cellar/git/2.2.0/libexec/git-core/git-add--interactive
>> --patch=checkout --
>
> Thanks, I could reproduce this pretty easily with:
>
>   GIT_EDITOR='f() { sleep 60; }; f' git checkout -p
>
> (and then hit 'e', and ^C). Explanation and fix are below.
>
> -- >8 --
> The main hunk loop for add--interactive will loop if it does
> not get a known input. This is a good thing if the user
> typed some invalid input. However, if we have an
> uncorrectable read error, we'll end up looping infinitely.
> We can fix this by noticing read errors (i.e., <STDIN>
> returns undef) and breaking out of the loop.
>
> One easy way to trigger this is if you have an editor that
> does not take over the terminal (e.g., one that spawns a
> window in an existing process and waits), start the editor
> with the hunk-edit command, and hit ^C to send SIGINT. The
> editor process dies due to SIGINT, but the perl
> add--interactive process does not (perl suspends SIGINT for
> the duration of our system() call).
>
> We return to the main loop, but further reads from stdin
> don't work. The SIGINT _also_ killed our parent git process,
> which orphans our process group, meaning that further reads
> from the terminal will always fail. We loop infinitely,
> getting EIO on each read.
>
> Note that there are several other spots where we read from
> stdin, too. However, in each of those cases, we do something
> sane when the read returns undef (breaking out of the loop,
> taking the input as "no", etc). They don't need similar
> treatment.
>
> Signed-off-by: Jeff King <peff@peff•net>
> ---

Thanks.

>  git-add--interactive.perl | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 1fadd69..c725674 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1356,6 +1356,7 @@ sub patch_update_file {
>  		  $patch_mode_flavour{TARGET},
>  		  " [y,n,q,a,d,/$other,?]? ";
>  		my $line = prompt_single_character;
> +		last unless defined $line;
>  		if ($line) {
>  			if ($line =~ /^y/i) {
>  				$hunk[$ix]{USE} = 1;

     prev parent reply	other threads:[~2014-12-15 22:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-15 14:41 But in `git checkout --patch` Benjamin Quorning
2014-12-15 16:35 ` [PATCH] add--interactive: leave main loop on read error Jeff King
2014-12-15 22:49   ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvblcbkaz.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=bquorning@zendesk$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox