{"thread":{"id":"45709","subject":"[PATCH] __git_ps1: Don't kill shell if user types `set -e`","startedAt":"2017-04-16T08:28:48Z","lastAt":"2017-05-04T09:35:42Z","messageCount":5,"participants":["Tom \"Ravi\" Hale","Junio C Hamano","Tom Hale","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"316939","messageId":"b344d0c3-c8b8-da27-79f6-ae750be6830a@hale.ee","threadId":"45709","inReplyTo":null,"subject":"[PATCH] __git_ps1: Don't kill shell if user types `set -e`","fromName":"Tom \"Ravi\" Hale","fromEmail":"tom@hale.ee","sentAt":"2017-04-16T08:28:33Z","receivedAt":"2017-04-16T08:28:48Z","isPatch":true,"sender":{"key":"tom@hale.ee","avatar":"https://gravatar.com/avatar/61d620fad9c6227a6f41dee4cdb68be18a7d8bf8c69c3fd70d6b3c65eb65eea8?d=mp&s=160"},"body":"If a user types `set -e` in an interactive shell, and is using __git_ps1\nto set\ntheir prompt, the shell will die if the current directory isn't inside a git\nrepository.\n\nThis is because `set -e` instructs the shell to exit upon a command\nreturning a non-zero exit status, and the following command exits with\nstatus 128:\n\n\trepo_info=\"$(git rev-parse --git-dir --is-inside-git-dir \\\n\t\t--is-bare-repository --is-inside-work-tree \\\n\t\t--short HEAD 2>/dev/null)\"\n\nI couldn't write a test to illustrate this - the test harness seems to\nbe preventing\n`set -e` from having an effect.\n\t\n<attachment>\n\nSigned-off-by: Tom \"Ravi\" Hale <tom@hale.ee>\n\n\nFrom 7867768f4516559c129ed9eb07ae29c36a6c9367 Mon Sep 17 00:00:00 2001\nFrom: \"Tom \\\"Ravi\\\" Hale\" <tom@hale.ee>\nDate: Sun, 16 Apr 2017 14:10:10 +0700\nSubject: [PATCH] __git_ps1: Don't kill shell if user types `set -e`\n\nEnsure that `set +e` is in effect inside __git_ps1 to prevent any commands\nexiting non-zero from killing an interactive shell in which the user has run\n`set -e`.\n---\n contrib/completion/git-prompt.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex 97eacd7..15a0b74 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -304,6 +304,10 @@ __git_ps1 ()\n \tlocal ps1pc_start='\\u@\\h:\\w '\n \tlocal ps1pc_end='\\$ '\n \tlocal printf_format=' (%s)'\n+\t# Prevent any non-zero exit statuses from killing an interactive shell in the\n+\t# case that a user types `set -e`\n+\tlocal -- - # Shadow shell options in $- (restored upon return)\n+\tset +e\n \n \tcase \"$#\" in\n \t\t2|3)\tpcmode=yes\n-- \n2.11.1\n\n"},{"id":"316986","messageId":"xmqq60i3k6ed.fsf@gitster.mtv.corp.google.com","threadId":"45709","inReplyTo":"b344d0c3-c8b8-da27-79f6-ae750be6830a@hale.ee","subject":"Re: [PATCH] __git_ps1: Don't kill shell if user types `set -e`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-17T04:24:58Z","receivedAt":"2017-04-17T04:25:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tom \\\"Ravi\\\" Hale\" <tom@hale.ee> writes:\n\n> If a user types `set -e` in an interactive shell, and is using __git_ps1\n> to set\n> their prompt, the shell will die if the current directory isn't inside a git\n> repository.\n>\n> This is because `set -e` instructs the shell to exit upon a command\n> returning a non-zero exit status, and the following command exits with\n> status 128:\n>\n> \trepo_info=\"$(git rev-parse --git-dir --is-inside-git-dir \\\n> \t\t--is-bare-repository --is-inside-work-tree \\\n> \t\t--short HEAD 2>/dev/null)\"\n\nHmph.  So the fix would be something like this?\n\n \trepo_info=\"$(git rev-parse --git-dir --is-inside-git-dir \\\n \t\t--is-bare-repository --is-inside-work-tree \\\n- \t\t--short HEAD 2>/dev/null)\"\n+ \t\t--short HEAD 2>/dev/null || :)\"\n\nI am too afraid to ask what a user would try to achieve by doing the\n\"set -e\" thing in an interactive session.  It is understandable (I\nam not saying I think it is necessarily a good idea) to do so in a\nscript, but for an interactive shell?\n"},{"id":"318731","messageId":"c8879806-4e43-d033-bd69-918e730e2d05@hale.ee","threadId":"45709","inReplyTo":"xmqq60i3k6ed.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] __git_ps1: Don't kill shell if user types `set -e`","fromName":"Tom Hale","fromEmail":"tom@hale.ee","sentAt":"2017-05-04T08:26:58Z","receivedAt":"2017-05-04T08:27:10Z","isPatch":true,"sender":{"key":"tom@hale.ee","avatar":"https://gravatar.com/avatar/61d620fad9c6227a6f41dee4cdb68be18a7d8bf8c69c3fd70d6b3c65eb65eea8?d=mp&s=160"},"body":"\nOn 17/04/17 11:24, Junio C Hamano wrote:\n> \"Tom \\\"Ravi\\\" Hale\" <tom@hale.ee> writes:\n>\n> > If a user types `set -e` in an interactive shell, and is using __git_ps1\n> > to set\n> > their prompt, the shell will die if the current directory isn't inside a git\n> > repository.\n> >\n> Hmph.  So the fix would be something like this?\n>\n>      repo_info=\"$(git rev-parse --git-dir --is-inside-git-dir \\\n>          --is-bare-repository --is-inside-work-tree \\\n> -         --short HEAD 2>/dev/null)\"\n> +         --short HEAD 2>/dev/null || :)\"\n\nNope, that would cause the next line\n    rev_parse_exit_code=\"$?\"\nto always be assigned 0.\n\nI believe the patch we're after is attached.\n\n-- \nCheers,\n\nTom\n\n\nFrom 74fa2e2fa058f89605cf0b2774ad8e9df981cf86 Mon Sep 17 00:00:00 2001\nFrom: \"Tom \\\"Ravi\\\" Hale\" <tom@hale.ee>\nDate: Sun, 16 Apr 2017 14:15:14 +0700\nSubject: [PATCH] Prevent non-zero exit if outside a repository\n\n---\n contrib/completion/git-prompt.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex 97eacd7..d0d890c 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -361,8 +361,8 @@ __git_ps1 ()\n \tlocal repo_info rev_parse_exit_code\n \trepo_info=\"$(git rev-parse --git-dir --is-inside-git-dir \\\n \t\t--is-bare-repository --is-inside-work-tree \\\n-\t\t--short HEAD 2>/dev/null)\"\n-\trev_parse_exit_code=\"$?\"\n+\t\t--short HEAD 2>/dev/null)\" && \n+\t\trev_parse_exit_code=\"$?\" || : # Don't die if \"set -e\"\n \n \tif [ -z \"$repo_info\" ]; then\n \t\treturn $exit\n-- \n2.12.2\n\n"},{"id":"318735","messageId":"CACBZZX6XdwND2-7cY0zp8+K_uh60tn-i_r9V7HAe2X-a-RJeQA@mail.gmail.com","threadId":"45709","inReplyTo":"c8879806-4e43-d033-bd69-918e730e2d05@hale.ee","subject":"Re: [PATCH] __git_ps1: Don't kill shell if user types `set -e`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-04T09:16:57Z","receivedAt":"2017-05-04T09:17:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, May 4, 2017 at 10:26 AM, Tom Hale <tom@hale.ee> wrote:\n>\n> On 17/04/17 11:24, Junio C Hamano wrote:\n>> \"Tom \\\"Ravi\\\" Hale\" <tom@hale.ee> writes:\n>>\n>> > If a user types `set -e` in an interactive shell, and is using __git_ps1\n>> > to set\n>> > their prompt, the shell will die if the current directory isn't inside a git\n>> > repository.\n>> >\n>> Hmph.  So the fix would be something like this?\n>>\n>>      repo_info=\"$(git rev-parse --git-dir --is-inside-git-dir \\\n>>          --is-bare-repository --is-inside-work-tree \\\n>> -         --short HEAD 2>/dev/null)\"\n>> +         --short HEAD 2>/dev/null || :)\"\n>\n> Nope, that would cause the next line\n>     rev_parse_exit_code=\"$?\"\n> to always be assigned 0.\n>\n> I believe the patch we're after is attached.\n\nI see how this would fix things, but this just seems like a rather\ncrazy thing for us to support, we can take this patch, but it's going\nto take quite some maintenance burden to ensure that this code is set\n-e clean going forward, and I don't think we should take a patch like\nthis without some general support in the regression tests to ensure\nthat the completion code is set -e clean, otherwise this is going to\nvery easily regress the next time someone not aware of this patches\nit.\n\nWhat's the real use-case here? If you do \"set -e\" in your shell you\nalso get e.g.:\n\n    $ ls -l blah\n    ls: cannot access blah: No such file or directory\n    === Command terminated with exit status 2 (Thu May  4 11:16:03 2017) ===\n\nI.e. any little failure will terminate your shell, are you actually\nrunning a shell like this? For what purpose?\n"},{"id":"318738","messageId":"60ea6f0e-aa27-9a2c-54fd-9c14db9f028c@hale.ee","threadId":"45709","inReplyTo":"CACBZZX6XdwND2-7cY0zp8+K_uh60tn-i_r9V7HAe2X-a-RJeQA@mail.gmail.com","subject":"Re: [PATCH] __git_ps1: Don't kill shell if user types `set -e`","fromName":"Tom Hale","fromEmail":"tom@hale.ee","sentAt":"2017-05-04T09:35:31Z","receivedAt":"2017-05-04T09:35:42Z","isPatch":true,"sender":{"key":"tom@hale.ee","avatar":"https://gravatar.com/avatar/61d620fad9c6227a6f41dee4cdb68be18a7d8bf8c69c3fd70d6b3c65eb65eea8?d=mp&s=160"},"body":"On 04/05/17 16:16, Ævar Arnfjörð Bjarmason wrote:\n\n> What's the real use-case here? If you do \"set -e\" in your shell you\n> also get e.g.:\n> \n>     $ ls -l blah\n>     ls: cannot access blah: No such file or directory\n>     === Command terminated with exit status 2 (Thu May  4 11:16:03 2017) ===\n> \n> I.e. any little failure will terminate your shell, are you actually\n> running a shell like this? For what purpose?\n\nIf I want to copy and paste a list of commands and have the execution\nstop at a failure, I type \"bash\" then \"set -e\", then paste the commands.\n\nIt saves me creating a script file and then removing it later.\n\n-- \nTom Hale\n"}]}