Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell

From: Jeff King <peff@peff.net>
Date: Feb 11, 2013, 04:33 UTC
Message-ID: <20130211043322.GA12735@sigill.intra.peff.net>
In-Reply-To: <20130211042609.GC15329@elie.Belkin>

On Sun, Feb 10, 2013 at 08:26:09PM -0800, Jonathan Nieder wrote:

Show 11 quoted lines

> Jeff King wrote:
> > On Sun, Feb 10, 2013 at 08:14:04PM -0800, Jonathan Nieder wrote:
> 
> >> Only interactive connections.  That's the existing behavior.
> >
> > Ah, sorry. I misread the patch. I see now that we already run help, and
> > this is just making the exit value significant. In that case, yeah, I
> > think it's fine.
> 
> No problem --- the description was unclear.  Would retitling the patch
> to "shell: pay attention to exit status from 'help' command" work?

I think what threw me off was reading the documentation part of the patch, which adds a note that we run "help" on startup, and then elaborates on the exit value. I didn't realize that the first half was documenting what already happened.

Tweaking the third paragraph of the commit message to:

  An appropriate greeting might even include more complex information,
  like a list of repositories the user has access to.  If the
  git-shell-commands directory exists and contains a "help" script, we
  already run it when the shell is run without any commands, giving the
  server a chance to provide a custom message. Unfortunately, the
  presence of the git-shell-commands directory means we also enter an
  interactive mode, prompting and accepting commands (of which there may
  be none) from the user, which many servers would not want. To solve
  this, we abort the interactive shell on a non-zero exit code from the
  "help" script. This lets the server say whatever it likes, and then
  hangup.

makes it more clear to me. But once you explained it, I realize that I also could have just read the C code part of the patch more carefully. :)

So I'm fine with or without that change.

-Peff