# [PATCH 3/3] fallback SSH_ASKPASS when GIT_ASKPASS not set 5 messages from 2010-02-24 to 2010-02-25. Participants: Frank Li, Junio C Hamano, Erik Faye-Lund. Thread: https://gitlist.dev/t/22793 ## Frank Li, 2010-02-24 15:15 Subject: [PATCH 3/3] fallback SSH_ASKPASS when GIT_ASKPASS not set Message-ID: <1267024554-3288-1-git-send-email-lznuaa@gmail.com> URL: https://gitlist.dev/e/1267024554-3288-1-git-send-email-lznuaa%40gmail.com ``` If GIT_ASKPASS is not set and SSH_ASKPASS set, GIT_ASKPASS will use SSH_ASKPASS. If GIT_ASKPASS set and SSH_ASKPASS is not set, SSH_ASKPASS will use GIT_ASKPASS. Signed-off-by: Frank Li --- git.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/git.c b/git.c index 90c6daf..39e1eba 100644 --- a/git.c +++ b/git.c @@ -54,6 +54,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) { int handled = 0; + if (getenv("GIT_ASKPASS") && !getenv("SSH_ASKPASS")) + setenv("SSH_ASKPASS", getenv("GIT_ASKPASS"), 1); + if (!getenv("GIT_ASKPASS") && getenv("SSH_ASKPASS")) + setenv("GIT_ASKPASS", getenv("SSH_ASKPASS"), 1); + while (*argc > 0) { const char *cmd = (*argv)[0]; if (cmd[0] != '-') -- 1.7.0.85.g37fda.dirty ``` ## Erik Faye-Lund, 2010-02-24 15:34 Subject: Re: [PATCH 3/3] fallback SSH_ASKPASS when GIT_ASKPASS not set Message-ID: <40aa078e1002240734u1ef6364fp16fbce50caf575bd@mail.gmail.com> URL: https://gitlist.dev/e/40aa078e1002240734u1ef6364fp16fbce50caf575bd%40mail.gmail.com In-Reply-To: <1267024554-3288-1-git-send-email-lznuaa@gmail.com> ``` On Wed, Feb 24, 2010 at 4:15 PM, Frank Li wrote: > If GIT_ASKPASS is not set and SSH_ASKPASS set, GIT_ASKPASS will > use SSH_ASKPASS. If GIT_ASKPASS set and SSH_ASKPASS is not set, > SSH_ASKPASS will use GIT_ASKPASS. > Since there's a maximum total size of the environment on Windows (64kib), perhaps it's better to not increase the environment-size more than necessary and instead check both variables when used? I must admit doing so gives me a kind of yuck-feeling, but at least it should also make non-builtins (like git-svn, which AFAICT is what you're fixing) work consistently even if it's not called through the git-wrapper... > Signed-off-by: Frank Li > --- >  git.c |    5 +++++ >  1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/git.c b/git.c > index 90c6daf..39e1eba 100644 > --- a/git.c > +++ b/git.c > @@ -54,6 +54,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) >  { >        int handled = 0; > > +       if (getenv("GIT_ASKPASS") && !getenv("SSH_ASKPASS")) > +               setenv("SSH_ASKPASS", getenv("GIT_ASKPASS"), 1); > +       if (!getenv("GIT_ASKPASS") && getenv("SSH_ASKPASS")) > +               setenv("GIT_ASKPASS", getenv("SSH_ASKPASS"), 1); > + >        while (*argc > 0) { >                const char *cmd = (*argv)[0]; >                if (cmd[0] != '-') > -- > 1.7.0.85.g37fda.dirty > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at  http://vger.kernel.org/majordomo-info.html > -- Erik "kusma" Faye-Lund ``` ## Junio C Hamano, 2010-02-24 16:24 Subject: Re: [PATCH 3/3] fallback SSH_ASKPASS when GIT_ASKPASS not set Message-ID: <7vocje38ll.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vocje38ll.fsf%40alter.siamese.dyndns.org In-Reply-To: <1267024554-3288-1-git-send-email-lznuaa@gmail.com> ``` Frank Li writes: > If GIT_ASKPASS is not set and SSH_ASKPASS set, GIT_ASKPASS will > use SSH_ASKPASS. If GIT_ASKPASS set and SSH_ASKPASS is not set, > SSH_ASKPASS will use GIT_ASKPASS. If both are set and to different values, what should happen? I think the basic idea is sound, but I am not sure if we should fallback on both ways. If both GIT_ASKPASS and SSH_ASKPASS are set, the user is probably telling us to use GIT_ASKPASS when git does something, similar to how GIT_EDITOR and EDITOR interacts. So probably the patch to implement the fallback should be more like if (SSH_ASKPASS is set but not GIT_ASKPASS) set GIT_ASKPASS from SSH_ASKPASS and then the password prompter should use GIT_ASKPASS exclusively. Of course, when we _do_ spawn ssh ourselves internally in connect.c, we may have to export the value of GIT_ASKPASS as SSH_ASKPASS at the call site to honor the user's request to use GIT_ASKPASS while in git (that is part of "use GIT_ASKPASS exclusively"). But it feels wrong if we exported SSH_ASKPASS set from GIT_ASKPASS when spawning processes in other codepaths, especially if SSH_ASKPASS is set (or unset) differently from GIT_ASKPASS. Shawn is CC'ed as git-gui currently honors only SSH_ASKPASS and this patch changes the behaviour slightly. ``` ## Frank Li, 2010-02-25 01:37 Subject: Re: [PATCH 3/3] fallback SSH_ASKPASS when GIT_ASKPASS not set Message-ID: <1976ea661002241737s74e0b9d3q30ba3f6632980fa2@mail.gmail.com> URL: https://gitlist.dev/e/1976ea661002241737s74e0b9d3q30ba3f6632980fa2%40mail.gmail.com In-Reply-To: <7vocje38ll.fsf@alter.siamese.dyndns.org> ``` > So probably the patch to implement the fallback should be more like > >        if (SSH_ASKPASS is set but not GIT_ASKPASS) >                set GIT_ASKPASS from SSH_ASKPASS > do you means just keep these? + if (!getenv("GIT_ASKPASS") && getenv("SSH_ASKPASS")) + setenv("GIT_ASKPASS", getenv("SSH_ASKPASS"), 1); best regards Frank Li ``` ## Frank Li, 2010-02-25 04:17 Subject: Re: [PATCH 3/3] fallback SSH_ASKPASS when GIT_ASKPASS not set Message-ID: <1976ea661002242017v318ec3davb62087006d37f5e4@mail.gmail.com> URL: https://gitlist.dev/e/1976ea661002242017v318ec3davb62087006d37f5e4%40mail.gmail.com In-Reply-To: <40aa078e1002240734u1ef6364fp16fbce50caf575bd@mail.gmail.com> ``` > > Since there's a maximum total size of the environment on Windows > (64kib), perhaps it's better to not increase the environment-size more > than necessary and instead check both variables when used? I must > admit doing so gives me a kind of yuck-feeling, but at least it should > also make non-builtins (like git-svn, which AFAICT is what you're > fixing) work consistently even if it's not called through the > git-wrapper... > Git-svn use the same way, please read first patch. ```