From: Jeff King <peff@peff.net>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
"Johannes Schindelin" <johannes.schindelin@gmx.de>,
"Torsten Bögershausen" <tboegi@web.de>
Subject: Re: [PATCH v3 3/3] connect: improve check for plink to reduce false positives
Date: Fri, 26 Jun 2015 09:15:24 -0400 [thread overview]
Message-ID: <20150626131524.GA2626@peff.net> (raw)
In-Reply-To: <1430080212-396370-4-git-send-email-sandals@crustytoothpaste.net>
On Sun, Apr 26, 2015 at 08:30:12PM +0000, brian m. carlson wrote:
> The git_connect function has code to handle plink and tortoiseplink
> specially, as they require different command line arguments from
> OpenSSH (-P instead of -p for ports; tortoiseplink additionally requires
> -batch). However, the match was done by checking for "plink" anywhere
> in the string, which led to a GIT_SSH value containing "uplink" being
> treated as an invocation of putty's plink.
>
> Improve the check by looking for "plink" or "tortoiseplink" (or those
> names suffixed with ".exe") only in the final component of the path.
> This has the downside that a program such as "plink-0.63" would no
> longer be recognized, but the increased robustness is likely worth it.
> Add tests to cover these cases to avoid regressions.
FYI, this ended up biting me today. We have some integration tests that
make sure we can clone over putty, and we wrap plink in a
"plink-wrapper.sh" script that tweaks a few extra options. That used to
match under the old scheme, but not the new. It would also match if we
looked for "plink" anywhere in the basename (but not in leading
directories).
I was able to work around it pretty easily by changing our test setup,
but I thought I would include it here as a data point. It's probably not
that representative of real-world users.
-Peff
next prev parent reply other threads:[~2015-06-26 13:15 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-22 14:36 [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true Patrick Sharp
2015-04-22 17:46 ` Johannes Schindelin
2015-04-22 19:12 ` Patrick Sharp
2015-04-22 20:29 ` Jeff King
2015-04-22 21:19 ` brian m. carlson
2015-04-22 21:29 ` Jeff King
2015-04-22 21:44 ` brian m. carlson
2015-04-22 22:00 ` Jeff King
2015-04-22 22:24 ` brian m. carlson
2015-04-22 23:23 ` Jeff King
2015-04-23 0:06 ` [PATCH 1/2] connect: simplify SSH connection code path brian m. carlson
2015-04-23 0:06 ` [PATCH 2/2] connect: improve check for plink to reduce false positives brian m. carlson
2015-04-23 6:50 ` Johannes Schindelin
2015-04-23 15:53 ` Jeff King
2015-04-23 23:14 ` brian m. carlson
2015-04-24 6:41 ` Johannes Schindelin
2015-04-24 22:28 ` [PATCH v2 1/2] connect: simplify SSH connection code path brian m. carlson
2015-04-24 22:28 ` [PATCH v2 2/2] connect: improve check for plink to reduce false positives brian m. carlson
2015-04-24 22:46 ` Pete Harlan
2015-04-24 22:48 ` brian m. carlson
2015-04-25 16:03 ` Torsten Bögershausen
2015-04-26 18:52 ` brian m. carlson
2015-04-26 20:30 ` [PATCH v3 0/3] Improve robustness of putty detection brian m. carlson
2015-04-26 20:30 ` [PATCH v3 1/3] connect: simplify SSH connection code path brian m. carlson
2015-04-26 20:30 ` [PATCH v3 2/3] t5601: fix quotation error leading to skipped tests brian m. carlson
2015-04-26 20:30 ` [PATCH v3 3/3] connect: improve check for plink to reduce false positives brian m. carlson
2015-04-27 7:57 ` Johannes Schindelin
2015-04-28 3:53 ` Jeff King
2015-06-26 13:15 ` Jeff King [this message]
2015-06-26 16:16 ` Junio C Hamano
2015-06-26 16:27 ` Jeff King
2015-06-26 17:13 ` Johannes Schindelin
2015-06-26 17:23 ` Jeff King
2015-06-26 20:43 ` brian m. carlson
2015-04-26 22:04 ` [PATCH v3 0/3] Improve robustness of putty detection Junio C Hamano
2015-04-27 15:46 ` Torsten Bögershausen
2015-04-28 4:15 ` Jeff King
2015-04-29 1:38 ` brian m. carlson
2015-04-24 6:37 ` [PATCH 2/2] connect: improve check for plink to reduce false positives Johannes Schindelin
2015-04-23 5:08 ` [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true Torsten Bögershausen
2015-04-23 13:15 ` Patrick Sharp
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=20150626131524.GA2626@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=sandals@crustytoothpaste.net \
--cc=tboegi@web.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).