From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Sitaram Chamarty <sitaramc@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Ethan Reesor <firelizzard@gmail.com>,
git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>,
Greg Brockman <gdb@mit.edu>
Subject: Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
Date: Sun, 10 Feb 2013 23:33:22 -0500 [thread overview]
Message-ID: <20130211043322.GA12735@sigill.intra.peff.net> (raw)
In-Reply-To: <20130211042609.GC15329@elie.Belkin>
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
next prev parent reply other threads:[~2013-02-11 4:33 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-10 21:05 Git prompt Ethan Reesor
2013-02-10 21:25 ` Jonathan Nieder
2013-02-10 21:54 ` Ethan Reesor
2013-02-10 22:43 ` Jeff King
2013-02-10 22:54 ` Junio C Hamano
2013-02-11 0:43 ` Sitaram Chamarty
2013-02-11 1:20 ` [RFC/PATCH] shell: allow 'help' command to disable interactive shell Jonathan Nieder
2013-02-11 3:44 ` Junio C Hamano
2013-02-11 4:17 ` Jonathan Nieder
2013-02-11 4:30 ` Junio C Hamano
2013-02-11 4:32 ` Jonathan Nieder
2013-02-11 4:36 ` Jeff King
2013-02-11 5:22 ` Junio C Hamano
2013-02-11 5:57 ` Ethan Reesor
2013-02-11 6:07 ` Ethan Reesor
2013-02-11 6:09 ` Jonathan Nieder
2013-02-11 6:11 ` Ethan Reesor
2013-02-11 6:15 ` Jonathan Nieder
2013-02-11 6:22 ` Ethan Reesor
2013-02-11 6:14 ` Jonathan Nieder
2013-02-11 7:01 ` Junio C Hamano
2013-02-11 7:12 ` Jonathan Nieder
2013-02-11 7:17 ` Junio C Hamano
2013-02-11 7:21 ` Jonathan Nieder
2013-02-11 7:44 ` Junio C Hamano
2013-02-11 8:13 ` Jonathan Nieder
2013-02-11 16:17 ` Junio C Hamano
2013-02-11 16:00 ` Jeff King
2013-02-11 17:18 ` Junio C Hamano
2013-02-11 17:27 ` Jeff King
2013-02-11 7:18 ` Ethan Reesor
2013-02-11 7:15 ` Ethan Reesor
2013-02-11 7:22 ` Junio C Hamano
2013-02-11 7:26 ` Ethan Reesor
2013-02-11 7:28 ` Junio C Hamano
2013-02-11 3:59 ` Jeff King
2013-02-11 4:14 ` Jonathan Nieder
2013-02-11 4:17 ` Jeff King
2013-02-11 4:26 ` Jonathan Nieder
2013-02-11 4:33 ` Jeff King [this message]
2013-02-11 5:56 ` [PATCH 0/2 v2] " Jonathan Nieder
2013-02-11 5:57 ` [PATCH 1/2] shell doc: emphasize purpose and security model Jonathan Nieder
2013-02-11 7:10 ` Junio C Hamano
2013-02-11 7:13 ` Jonathan Nieder
2013-02-11 18:32 ` Junio C Hamano
2013-02-11 5:58 ` [PATCH 2/2] shell: pay attention to exit status from 'help' command Jonathan Nieder
2013-02-11 6:06 ` Ethan Reesor
2013-02-11 7:15 ` Junio C Hamano
2013-02-11 7:52 ` Jonathan Nieder
2013-02-11 16:28 ` Junio C Hamano
2013-02-11 4:45 ` [RFC/PATCH] shell: allow 'help' command to disable interactive shell Jeff King
2013-03-09 21:52 ` [PATCH v3 0/2] shell: allow 'no-interactive-login' " Jonathan Nieder
2013-03-09 21:55 ` [PATCH 1/2] shell doc: emphasize purpose and security model Jonathan Nieder
2013-03-09 22:00 ` [PATCH 2/2] shell: new no-interactive-login command to print a custom message Jonathan Nieder
2013-03-10 5:04 ` Junio C Hamano
2013-03-10 5:21 ` Jonathan Nieder
2013-03-10 10:49 ` Ramkumar Ramachandra
2013-03-11 22:48 ` Jonathan Nieder
2013-03-12 10:47 ` [PATCH v3 0/2] shell: allow 'no-interactive-login' command to disable interactive shell 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=20130211043322.GA12735@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=artagnon@gmail.com \
--cc=firelizzard@gmail.com \
--cc=gdb@mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=sitaramc@gmail.com \
/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).