{"thread":{"id":"24885","subject":"[PATCH 2/2] Allow core.askpass to override SSH_ASKPASS.","startedAt":"2010-08-27T10:51:21Z","lastAt":"2010-08-27T17:28:43Z","messageCount":4,"participants":["Knut Franke","Erik Faye-Lund","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"149116","messageId":"201008271251.23136.k.franke@science-computing.de","threadId":"24885","inReplyTo":null,"subject":"[PATCH 2/2] Allow core.askpass to override SSH_ASKPASS.","fromName":"Knut Franke","fromEmail":"k.franke@science-computing.de","sentAt":"2010-08-27T10:51:21Z","receivedAt":"2010-08-27T10:51:21Z","isPatch":true,"sender":{"key":"knut.franke@gmx.de","avatar":null},"body":"From: Knut Franke \n\nModify handling of the 'core.askpass' option so that it has the same effect as\nGIT_ASKPASS also if SSH_ASKPASS is set.\n\nSigned-off-by: Knut Franke \n---\nI'd say this precedence order is more intuitive; and more useful.\n\n connect.c | 4 ++++\n git.c | 3 ---\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex e296bfc..f97b84c 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -627,9 +627,13 @@ char *git_getpass(const char *prompt)\n \tstatic struct strbuf buffer = STRBUF_INIT;\n \n \taskpass = getenv(\"GIT_ASKPASS\");\n+\n \tif (!askpass)\n \t\taskpass = askpass_program;\n \n+\tif (!askpass)\n+\t\taskpass = getenv(\"SSH_ASKPASS\");\n+\n \tif (!askpass || !(*askpass))\n \t\treturn getpass(prompt);\n \ndiff --git a/git.c b/git.c\nindex 12d2952..1545257 100644\n--- a/git.c\n+++ b/git.c\n@@ -55,9 +55,6 @@ static int handle_options(const char ***argv, int *argc, int \n*envchanged)\n {\n \tint handled = 0;\n \n-\tif (!getenv(\"GIT_ASKPASS\") && getenv(\"SSH_ASKPASS\"))\n-\t\tsetenv(\"GIT_ASKPASS\", getenv(\"SSH_ASKPASS\"), 1);\n-\n \twhile (*argc > 0) {\n \t\tconst char *cmd = (*argv)[0];\n \t\tif (cmd[0] != '-')\n-- \n1.7.2.1\n\n\n-- \nVorstand/Board of Management:\nDr. Bernd Finkbeiner, Dr. Roland Niemeier, \nDr. Arno Steitz, Dr. Ingrid Zech\nVorsitzender des Aufsichtsrats/\nChairman of the Supervisory Board:\nMichel Lepert\nSitz/Registered Office: Tuebingen\nRegistergericht/Registration Court: Stuttgart\nRegisternummer/Commercial Register No.: HRB 382196 \n"},{"id":"149118","messageId":"AANLkTik5eMLCKK7muBW9k8-0ZAxraATCs5hHVcX-Zutn@mail.gmail.com","threadId":"24885","inReplyTo":"201008271251.23136.k.franke@science-computing.de","subject":"Re: [PATCH 2/2] Allow core.askpass to override SSH_ASKPASS.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-08-27T12:00:16Z","receivedAt":"2010-08-27T12:00:16Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Aug 27, 2010 at 12:51 PM, Knut Franke\n wrote:\n> From: Knut Franke \n>\n\nYou don't need this line unless you're forwarding a patch for someone\nelse (or using a different e-mail account).\n\n> Modify handling of the 'core.askpass' option so that it has the same effect as\n> GIT_ASKPASS also if SSH_ASKPASS is set.\n>\n\nWhy are you sending a two-patch series where the second patch tries to\ncorrect the first one? Wouldn't it make more sense to squash the two?\n\n> Signed-off-by: Knut Franke \n> ---\n> I'd say this precedence order is more intuitive; and more useful.\n>\n>  connect.c |    4 ++++\n>  git.c     |    3 ---\n>  2 files changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index e296bfc..f97b84c 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -627,9 +627,13 @@ char *git_getpass(const char *prompt)\n>        static struct strbuf buffer = STRBUF_INIT;\n>\n>        askpass = getenv(\"GIT_ASKPASS\");\n> +\n\nWhy?\n\nApart from these nits, I think the patch does the right thing. IMO it\ndoesn't make sense for SSH_ASKPASS to trump core.askpass.\n"},{"id":"149119","messageId":"201008271445.26098.k.franke@science-computing.de","threadId":"24885","inReplyTo":"AANLkTik5eMLCKK7muBW9k8-0ZAxraATCs5hHVcX-Zutn@mail.gmail.com","subject":"Re: [PATCH 2/2] Allow core.askpass to override SSH_ASKPASS.","fromName":"Knut Franke","fromEmail":"k.franke@science-computing.de","sentAt":"2010-08-27T12:45:24Z","receivedAt":"2010-08-27T12:45:24Z","isPatch":true,"sender":{"key":"knut.franke@gmx.de","avatar":null},"body":"On Friday 27 August 2010 14:00:16 Erik Faye-Lund wrote:\n> > From: Knut Franke \n>\n> You don't need this line unless you're forwarding a patch for someone\n> else (or using a different e-mail account).\n\nACK.\n\n> > Modify handling of the 'core.askpass' option so that it has the same\n> > effect as GIT_ASKPASS also if SSH_ASKPASS is set.\n>\n> Why are you sending a two-patch series where the second patch tries to\n> correct the first one? Wouldn't it make more sense to squash the two?\n\na) The author of the first patch is on holiday, and I didn't want to pass off \nhis work as mine nor my mistakes as his. ;-)\n\nb) Maybe not everyone agrees about the correct order of precedence.\n\n> > diff --git a/connect.c b/connect.c\n> > index e296bfc..f97b84c 100644\n> > --- a/connect.c\n> > +++ b/connect.c\n> > @@ -627,9 +627,13 @@ char *git_getpass(const char *prompt)\n> >        static struct strbuf buffer = STRBUF_INIT;\n> >\n> >        askpass = getenv(\"GIT_ASKPASS\");\n> > +\n>\n> Why?\n\nConsidering the check for SSH_ASKPASS added below, I believe this improves \nlegibilty since all of the \"if (!askpass...\" blocks are similar in intention.\n\n-- \nVorstand/Board of Management:\nDr. Bernd Finkbeiner, Dr. Roland Niemeier, \nDr. Arno Steitz, Dr. Ingrid Zech\nVorsitzender des Aufsichtsrats/\nChairman of the Supervisory Board:\nMichel Lepert\nSitz/Registered Office: Tuebingen\nRegistergericht/Registration Court: Stuttgart\nRegisternummer/Commercial Register No.: HRB 382196 \n"},{"id":"149133","messageId":"7v4oeghsms.fsf@alter.siamese.dyndns.org","threadId":"24885","inReplyTo":"201008271445.26098.k.franke@science-computing.de","subject":"Re: [PATCH 2/2] Allow core.askpass to override SSH_ASKPASS.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-27T17:28:43Z","receivedAt":"2010-08-27T17:28:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Knut Franke writes:\n\n>> > diff --git a/connect.c b/connect.c\n>> > index e296bfc..f97b84c 100644\n>> > --- a/connect.c\n>> > +++ b/connect.c\n>> > @@ -627,9 +627,13 @@ char *git_getpass(const char *prompt)\n>> > static struct strbuf buffer = STRBUF_INIT;\n>> >\n>> > askpass = getenv(\"GIT_ASKPASS\");\n>> > +\n>>\n>> Why?\n>\n> Considering the check for SSH_ASKPASS added below, I believe this improves \n> legibilty since all of the \"if (!askpass...\" blocks are similar in intention.\n\nI think Erik is asking about the extra blank line. You could have\nachieved the same consistency by removing, no?\n\ndiff --git a/connect.c b/connect.c\nindex e296bfc..f97b84c 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -627,9 +627,10 @@ char *git_getpass(const char *prompt)\n \tstatic struct strbuf buffer = STRBUF_INIT;\n \n \taskpass = getenv(\"GIT_ASKPASS\");\n \tif (!askpass)\n \t\taskpass = askpass_program;\n-\n+\tif (!askpass)\n+\t\taskpass = getenv(\"SSH_ASKPASS\");\n \tif (!askpass || !(*askpass))\n \t\treturn getpass(prompt);\n \n"}]}