On Sun, Feb 10, 2013 at 08:26:09PM -0800, Jonathan Nieder wrote: > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html