git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Sven Strickroth <sven.strickroth@tu-clausthal.de>,
	Neal Groothuis <ngroot@lo-cal.org>,
	Feanil Patel <feanil@gmail.com>,
	git@vger.kernel.org
Subject: Re: Push from an SSH Terminal
Date: Sat, 4 Feb 2012 03:09:10 -0500	[thread overview]
Message-ID: <20120204080910.GA28317@sigill.intra.peff.net> (raw)
In-Reply-To: <7vwr83hwg0.fsf@alter.siamese.dyndns.org>

On Fri, Feb 03, 2012 at 11:47:11PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Fri, Feb 03, 2012 at 12:10:27PM -0500, Neal Groothuis wrote:
> > ...
> >> Check to see if the GIT_ASKPASS and/or SSH_ASKPASS environment variables
> >> are set, and if the core.askpass config variable is set.  If any of these
> >> are set, unset them.  Git should fall back to a simple password prompt.
> >
> > Hmm, yeah that is likely the problem. I was thinking git would fall back
> > to asking on the terminal, but it does not. We probably should.
> 
> How well would it mesh with the goal of the ss/git-svn-prompt-sans-terminal
> topic, which is now stalled [*1*]?  I do not mean this change and the other
> topic textually conflict with each other---but the philosophies of this
> topic and the other one seem to conflict.  Not falling back to the terminal
> that is not available and failing the command outright might make more
> sense.

I don't see a conflict in the two series. That one seems to do two
things for perl programs:

  1. respect SSH_ASKPASS along with GIT_ASKPASS

  2. prefer askpass over asking on the terminal

But both of those are already the case in the C code.

If you look into the original complaint mentioned in the commit
messages, though, you will see that the some GUIs will appear to hang
when the terminal is prompted (because the prompt is reading from some
location invisible to the user). So in that sense, my patches could be a
regression for those users, as outright failing is better for them.

But I would argue that the bug is not prompting on the terminal, but
rather that the terminal-prompting code does not recognize when there is
no terminal connection to the user (and AFAICT, this is a Windows
problem). Any solution that doesn't fix that is really just papering
over the problem, and hurting people[1] on sane systems.

So I'd rather see the version of getpass() in compat/mingw.c better
learn to realize when we aren't actually connected to a console.

-Peff

[1] The amount of hurt is relatively small, though. It only hurts people
    who set GIT_ASKPASS but can't use it (e.g., you set it in your
    .bashrc because you connect via "ssh -X", but this time you happen
    to be ssh-ing from a Windows box). And you can generally fix that
    outside of git (e.g., by checking $DISPLAY before setting the
    variable).

    So one one hand, I don't want to make a decision on behavior for
    Unix users because we have to cater to Windows shortcomings. On the
    other hand, while fixing the root problem is preferable, if
    for whatever reason we can't reliably find out whether the user is
    actually going to see and respond to the prompt on Windows, it may
    be practical to just paper over the issue. On the gripping hand,
    after the Sven's series, TortoiseGit users would see the hang
    (instead of a failure) _only_ if their askpass command failed. Which
    is also perhaps not that big a deal.

  reply	other threads:[~2012-02-04  8:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-03 15:50 Push from an SSH Terminal Feanil Patel
2012-02-03 16:21 ` Neal Groothuis
2012-02-03 16:40   ` Feanil Patel
2012-02-03 17:10     ` Neal Groothuis
2012-02-03 21:36       ` Jeff King
2012-02-03 22:13         ` Jeff King
2012-02-03 22:14         ` [PATCH 1/2] prompt: clean up strbuf usage Jeff King
2012-02-03 22:16         ` [PATCH 2/2] prompt: fall back to terminal if askpass fails Jeff King
2012-02-04  7:47         ` Push from an SSH Terminal Junio C Hamano
2012-02-04  8:09           ` Jeff King [this message]
2012-02-04  8:16             ` Junio C Hamano
2012-02-03 21:35 ` Jeff King

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=20120204080910.GA28317@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=feanil@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ngroot@lo-cal.org \
    --cc=sven.strickroth@tu-clausthal.de \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).