git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] git_connect: clarify conn->use_shell flag

From
Jeff King <peff@peff.net>
Date
Sep 8, 2015, 21:40 UTC
Message-ID
<20150908214019.GA24159@sigill.intra.peff.net>
In-Reply-To
<xmqqwpw096dq.fsf@gitster.mtv.corp.google.com>
On Tue, Sep 08, 2015 at 10:18:41AM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> > To make the flow easier to follow, let's just set
> > conn->use_shell when we're setting up the "conn" struct, and
> > unset it (with a comment) in the historical GIT_SSH case.
> 
> Makes perfect sense.  I think another thing that falls into the same
> class wrt readability is 'putty'; if the code does putty=0 at the
> beginning of "various flavours of SSH", and sets it only when it
> checks with "various flavours of plink" when GIT_SSH_COMMAND is not
> set, the logic would be even clearer, I suspect.
Yeah, I think so.
Show 8 quoted lines
> > Note that for clarity we leave "use_shell" on in the case
> > that we fall back to our default "ssh" This looks like a
> > behavior change, but in practice run-command.c optimizes
> > shell invocations without metacharacters into a straight
> > execve anyway.
> 
> Hmm, interesting.  I am not sure if that has to be the way, though.
> Wouldn't the resulting code become simpler if you do not do that?

Heh, I originally wrote it that way, and waffled between sending one or the other.

Show 5 quoted lines
> That is, is is what I have in mind on top of your patch.  Did I
> break something?
> 
>  connect.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)

I think both changes are correct, and the result looks nice to read. Feel free to squash them in as you apply.

-Peff
Previous: Junio C Hamano
Message 12 of 12 in “[RFC] a tale of Git 2.5, ssh transport and GIT_* environment variables”
  1. Giuseppe BilottaSep 4, 2015
  2. Jeff KingSep 4, 2015
  3. Giuseppe BilottaSep 4, 2015
  4. Jeff KingSep 4, 2015
  5. Junio C HamanoSep 4, 2015
  6. Giuseppe BilottaSep 4, 2015
  7. Jeff KingSep 4, 2015
  8. git_connect: clear GIT_* environment for sshJeff King, Sep 4, 2015
  9. Giuseppe BilottaSep 5, 2015
  10. git_connect: clarify conn->use_shell flagJeff King, Sep 8, 2015
  11. Junio C HamanoSep 8, 2015
  12. Jeff KingSep 8, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.