{"thread":{"id":"38177","subject":"But in `git checkout --patch`","startedAt":"2014-12-15T14:41:45Z","lastAt":"2014-12-15T22:49:24Z","messageCount":3,"participants":["Benjamin Quorning","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"253690","messageId":"CAN9HoQH5=z-d=J1HCA2UwGuFek21X6qCd_jFEkNpE6GiE50oNg@mail.gmail.com","threadId":"38177","inReplyTo":null,"subject":"But in `git checkout --patch`","fromName":"Benjamin Quorning","fromEmail":"bquorning@zendesk.com","sentAt":"2014-12-15T14:41:45Z","receivedAt":"2014-12-15T14:41:45Z","isPatch":false,"sender":{"key":"bquorning@zendesk.com","avatar":null},"body":"I'm not entirely sure how you like your bug reports, so I'll just the\nbest I can :-)\n\n$ git --version\ngit version 2.2.0\n\n$ uname -a\nDarwin buzz.local 13.4.0 Darwin Kernel Version 13.4.0: Sun Aug 17\n19:50:11 PDT 2014; root:xnu-2422.115.4~1/RELEASE_X86_64 x86_64\n\nReproduction steps:\n\n1. A repository with a changed file, but no staged changes.\n2. Execute `git checkout --patch`\n3. When asked, press `e` to edit a chunk (opens an external editor in my case)\n4. With the editor still open, click ctrl-C in the terminal.\n5. The diff that was being edited, and the command prompt (\"discard\nthis hunk from worktree\" etc) is printed to the screen, over and over\nagain.\n6. I have to grep and kill this process: /usr/bin/perl\n/usr/local/Cellar/git/2.2.0/libexec/git-core/git-add--interactive\n--patch=checkout --\n\n--\nBenjamin Quorning\n"},{"id":"253693","messageId":"20141215163527.GA15136@peff.net","threadId":"38177","inReplyTo":"CAN9HoQH5=z-d=J1HCA2UwGuFek21X6qCd_jFEkNpE6GiE50oNg@mail.gmail.com","subject":"[PATCH] add--interactive: leave main loop on read error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-15T16:35:27Z","receivedAt":"2014-12-15T16:35:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 15, 2014 at 03:41:45PM +0100, Benjamin Quorning wrote:\n\n> Reproduction steps:\n> \n> 1. A repository with a changed file, but no staged changes.\n> 2. Execute `git checkout --patch`\n> 3. When asked, press `e` to edit a chunk (opens an external editor in my case)\n> 4. With the editor still open, click ctrl-C in the terminal.\n> 5. The diff that was being edited, and the command prompt (\"discard\n> this hunk from worktree\" etc) is printed to the screen, over and over\n> again.\n> 6. I have to grep and kill this process: /usr/bin/perl\n> /usr/local/Cellar/git/2.2.0/libexec/git-core/git-add--interactive\n> --patch=checkout --\n\nThanks, I could reproduce this pretty easily with:\n\n  GIT_EDITOR='f() { sleep 60; }; f' git checkout -p\n\n(and then hit 'e', and ^C). Explanation and fix are below.\n\n-- >8 --\nThe main hunk loop for add--interactive will loop if it does\nnot get a known input. This is a good thing if the user\ntyped some invalid input. However, if we have an\nuncorrectable read error, we'll end up looping infinitely.\nWe can fix this by noticing read errors (i.e., <STDIN>\nreturns undef) and breaking out of the loop.\n\nOne easy way to trigger this is if you have an editor that\ndoes not take over the terminal (e.g., one that spawns a\nwindow in an existing process and waits), start the editor\nwith the hunk-edit command, and hit ^C to send SIGINT. The\neditor process dies due to SIGINT, but the perl\nadd--interactive process does not (perl suspends SIGINT for\nthe duration of our system() call).\n\nWe return to the main loop, but further reads from stdin\ndon't work. The SIGINT _also_ killed our parent git process,\nwhich orphans our process group, meaning that further reads\nfrom the terminal will always fail. We loop infinitely,\ngetting EIO on each read.\n\nNote that there are several other spots where we read from\nstdin, too. However, in each of those cases, we do something\nsane when the read returns undef (breaking out of the loop,\ntaking the input as \"no\", etc). They don't need similar\ntreatment.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-add--interactive.perl | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 1fadd69..c725674 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1356,6 +1356,7 @@ sub patch_update_file {\n \t\t  $patch_mode_flavour{TARGET},\n \t\t  \" [y,n,q,a,d,/$other,?]? \";\n \t\tmy $line = prompt_single_character;\n+\t\tlast unless defined $line;\n \t\tif ($line) {\n \t\t\tif ($line =~ /^y/i) {\n \t\t\t\t$hunk[$ix]{USE} = 1;\n-- \n2.2.0.465.ge0d36f1.dirty\n"},{"id":"253717","messageId":"xmqqvblcbkaz.fsf@gitster.dls.corp.google.com","threadId":"38177","inReplyTo":"20141215163527.GA15136@peff.net","subject":"Re: [PATCH] add--interactive: leave main loop on read error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-15T22:49:24Z","receivedAt":"2014-12-15T22:49:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Dec 15, 2014 at 03:41:45PM +0100, Benjamin Quorning wrote:\n>\n>> Reproduction steps:\n>> \n>> 1. A repository with a changed file, but no staged changes.\n>> 2. Execute `git checkout --patch`\n>> 3. When asked, press `e` to edit a chunk (opens an external editor in my case)\n>> 4. With the editor still open, click ctrl-C in the terminal.\n>> 5. The diff that was being edited, and the command prompt (\"discard\n>> this hunk from worktree\" etc) is printed to the screen, over and over\n>> again.\n>> 6. I have to grep and kill this process: /usr/bin/perl\n>> /usr/local/Cellar/git/2.2.0/libexec/git-core/git-add--interactive\n>> --patch=checkout --\n>\n> Thanks, I could reproduce this pretty easily with:\n>\n>   GIT_EDITOR='f() { sleep 60; }; f' git checkout -p\n>\n> (and then hit 'e', and ^C). Explanation and fix are below.\n>\n> -- >8 --\n> The main hunk loop for add--interactive will loop if it does\n> not get a known input. This is a good thing if the user\n> typed some invalid input. However, if we have an\n> uncorrectable read error, we'll end up looping infinitely.\n> We can fix this by noticing read errors (i.e., <STDIN>\n> returns undef) and breaking out of the loop.\n>\n> One easy way to trigger this is if you have an editor that\n> does not take over the terminal (e.g., one that spawns a\n> window in an existing process and waits), start the editor\n> with the hunk-edit command, and hit ^C to send SIGINT. The\n> editor process dies due to SIGINT, but the perl\n> add--interactive process does not (perl suspends SIGINT for\n> the duration of our system() call).\n>\n> We return to the main loop, but further reads from stdin\n> don't work. The SIGINT _also_ killed our parent git process,\n> which orphans our process group, meaning that further reads\n> from the terminal will always fail. We loop infinitely,\n> getting EIO on each read.\n>\n> Note that there are several other spots where we read from\n> stdin, too. However, in each of those cases, we do something\n> sane when the read returns undef (breaking out of the loop,\n> taking the input as \"no\", etc). They don't need similar\n> treatment.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nThanks.\n\n>  git-add--interactive.perl | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 1fadd69..c725674 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1356,6 +1356,7 @@ sub patch_update_file {\n>  \t\t  $patch_mode_flavour{TARGET},\n>  \t\t  \" [y,n,q,a,d,/$other,?]? \";\n>  \t\tmy $line = prompt_single_character;\n> +\t\tlast unless defined $line;\n>  \t\tif ($line) {\n>  \t\t\tif ($line =~ /^y/i) {\n>  \t\t\t\t$hunk[$ix]{USE} = 1;\n"}]}