* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 16:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Sitaram Chamarty, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7v8v6vpbej.fsf@alter.siamese.dyndns.org>
On Sun, Feb 10, 2013 at 11:17:24PM -0800, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
> > Isn't that a criticism of the git-shell-commands facility in general?
> > If it is common to have a lot of users with distinct home directories
> > but all with git-shell as their login shell, then the
> > git-shell-commands should not go in their home directory to begin
> > with, no?
>
> You can give one set of commands to some users while restricting
> others, no?
But that seems to me to argue against /etc/git/shell-disabled or
similar, which would apply to every user. Or are you proposing that the
check be:
if -d ~/git-shell-commands; then
: ok, interactive
elif -x /etc/git/shell-disabled; then
exec /etc/git/shell-disabled
else
echo >&2 'go away'
exit 1
fi
That at least means you can apply _whether_ to disable the shell
selectively for each user (by providing or not a git-shell-commands
directory), but you cannot individually select the script that runs for
that user. But it's probably still flexible enough; you can, after all, run
arbitrary code in the shell-disabled script, so it can select which
class of user it was called on and dispatch to a sub-script.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #04; Sat, 9)
From: Junio C Hamano @ 2013-02-11 16:01 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqliavxlep.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> * mm/allow-contrib-build (2013-02-07) 2 commits
>> - perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other directories
>> - Makefile: extract perl-related rules to make them available from other dirs
>>
>> Will merge to 'next'.
>
> These two patches do not make much sense without the 3rd one:
>
> [PATCH 3/4] Makefile: factor common configuration in git-default-config.mak
>
> because perl.mak uses $(pathsep) that is defined in Makefile, hence
> unreachable from external callers.
>
> You can either drop the series (I still think it's a good thing to make
> the toplevel Makefile more modular, but I have no longer a personal
> interest in it since the original goal is already reached by
> mm/remote-mediawiki-build), or continue the discussion on [PATCH 3/4].
Ahh, you are absolutely right.
Thanks for reminding me.
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 16:17 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211081346.GP15329@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>
>> The purpose of the directory is to keep custom commands that are
>> allowed. If the site administrator does not want any command, it
>> would be more natural to expect that the way to disable them would
>> be _not_ to have that directory which is a collection of allowed
>> commands. Adding that directory and add a "help" that exits with
>> non-zero feels quite a roundabout and counter-intuitive way, no?
>
> I think it comes down to the reason the site admin doesn't want to
> allow interactive logins. That reason seems to be mostly that
> presenting a
>
> git>
>
> prompt at which you can only ask for "help" or "exit" is a bit
> confusing and pointless. I have sympathy for that, which is why I
> looked for a way for the admin to ask to avoid the prompt altogether
> in that case.
Yeah, the prompt does look pointless.
> I do not think the reason is "because I don't want a
> git-shell-commands directory". I think it's good to have basically
> one kind of setup instead of significantly different ones with and
> without that special directory --- and it means that starting from a
> setup like this, one can easily drop in additional commands like
> set-head or create-repo without changing anything basic. It's making
> the admin's later life easier.
I do not think I follow. If the admin wants to eventually have
extra commands supported at the site, but not yet ready to do so,
isn't it more natural to start with a less elaborate configuration
(i.e. without the directory) now and then add the directory when the
site is ready for offering extra commands later?
> Maybe a better test than "help exits with special exit code" is "there
> are no other custom commands than help". Would that be more sensible?
>
> From a "make it possible to emulate gitolite" point of view, that
> doesn't permit disabling the interactive mode when there are other
> commands available, so my hunch is that it wouldn't.
A paragraph I had in the message you are responding to before I sent
it out (but removed because it felt somewhat offtopic) said "if the
mechanism to disable weren't the magic 'help exited with failure'
but 'an interactive-disabled flag file exists there', I may find it
less strange to have the directory there", or something like that.
And that flag file could be a custom script that gives a custom
message.
^ permalink raw reply
* [PATCHv3 0/5] Add git-credential support to git-send-email
From: Michal Nazarewicz @ 2013-02-11 16:23 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
From: Michal Nazarewicz <mina86@mina86.com>
The third version of the patch with changes suggested by Jeff in the
4/5 patch. Also credential_read and credential_write are now public
functions in case someone wants to write a helper in perl.
Michal Nazarewicz (5):
Git.pm: allow command_close_bidi_pipe to be called as method
Git.pm: fix example in command_close_bidi_pipe documentation
Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
Git.pm: add interface for git credential command
git-send-email: use git credential to obtain password
Documentation/git-send-email.txt | 4 +-
git-send-email.perl | 59 ++++++++------
perl/Git.pm | 166 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 198 insertions(+), 31 deletions(-)
--
1.8.1.3.571.g3f8bed7
^ permalink raw reply
* [PATCHv3 1/5] Git.pm: allow command_close_bidi_pipe to be called as method
From: Michal Nazarewicz @ 2013-02-11 16:23 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git, Michal Nazarewicz
In-Reply-To: <cover.1360599057.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter. Using _maybe_self()
fixes this.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
- my ($pid, $in, $out, $ctx) = @_;
+ my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
--
1.8.1.3.571.g3f8bed7.dirty
^ permalink raw reply related
* [PATCHv3 2/5] Git.pm: fix example in command_close_bidi_pipe documentation
From: Michal Nazarewicz @ 2013-02-11 16:23 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git, Michal Nazarewicz
In-Reply-To: <cover.1360599057.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by C<command_bidi_pipe()>. The call idiom
is:
my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
- print "000000000\n" $out;
+ print $out "000000000\n";
while (<$in>) { ... }
$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
--
1.8.1.3.571.g3f8bed7.dirty
^ permalink raw reply related
* [PATCHv3 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz @ 2013-02-11 16:23 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git, Michal Nazarewicz
In-Reply-To: <cover.1360599057.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe(). With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..9dded54 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,13 +426,26 @@ Note that you should not rely on whatever actually is in C<CTX>;
currently it is simply the command name but in future the context might
have more complicated structure.
+C<PIPE_IN> and C<PIPE_OUT> may be C<undef> if they have been closed prior to
+calling this function. This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+ my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
+ print $out "000000000\n";
+ close $out;
+ while (<$in>) { ... }
+ $r->command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
=cut
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
- unless (close $fh) {
+ if (defined $fh && !close $fh) {
if ($!) {
carp "error closing pipe: $!";
} elsif ($? >> 8) {
--
1.8.1.3.571.g3f8bed7.dirty
^ permalink raw reply related
* [PATCHv3 5/5] git-send-email: use git credential to obtain password
From: Michal Nazarewicz @ 2013-02-11 16:23 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git, Michal Nazarewicz
In-Reply-To: <cover.1360599057.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59 +++++++++++++++++++++++-----------------
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=<host>::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL->import(qw(Perl));
+ };
+
+ # TODO: Authentication may fail not because credentials were
+ # invalid but due to other reasons, in which we should not
+ # reject credentials.
+ $auth = Git::credential({
+ 'protocol' => 'smtp',
+ 'host' => join(':', $smtp_server, $smtp_server_port),
+ 'username' => $smtp_authuser,
+ # if there's no password, "git credential fill" will
+ # give us one, otherwise it'll just pass this one.
+ 'password' => $smtp_authpass
+ }, sub {
+ my $cred = shift;
+ return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ? " port=$smtp_server_port" : "";
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL->import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system "stty -echo";
-
- do {
- print "Password: ";
- $_ = <STDIN>;
- print "\n";
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system "stty echo";
- }
-
- $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
- }
+ smtp_auth_maybe or die $smtp->message;
$smtp->mail( $raw_from ) or die $smtp->message;
$smtp->to( @recipients ) or die $smtp->message;
--
1.8.1.3.571.g3f8bed7.dirty
^ permalink raw reply related
* [PATCHv3 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-11 16:23 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git, Michal Nazarewicz
In-Reply-To: <cover.1360599057.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
Add a credential() function which is an interface to the git
credential command. The code is heavily based on credential_*
functions in <contrib/mw-to-git/git-remote-mediawiki>.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 147 insertions(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 9dded54..0e6fcf9 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,7 +59,8 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
- temp_acquire temp_release temp_reset temp_path);
+ temp_acquire temp_release temp_reset temp_path
+ credential credential_read credential_write);
=head1 DESCRIPTION
@@ -1013,6 +1014,151 @@ sub _close_cat_blob {
}
+=item credential_read( FILE_HANDLE )
+
+Reads credential key-value pairs from C<FILE_HANDLE>. Reading stops at EOF or
+when an empty line is encountered. Each line must be of the form C<key=value>
+with a non-empty key. Function returns a hash with all read values. Any
+white space (other then new-line character) is preserved.
+
+=cut
+
+sub credential_read {
+ my ($self, $reader) = _maybe_self(@_);
+ my %credential;
+ while (<$reader>) {
+ chomp;
+ if ($_ eq '') {
+ last;
+ } elsif (!/^([^=]+)=(.*)$/) {
+ throw Error::Simple("unable to parse git credential data:\n$_");
+ }
+ $credential{$1} = $2;
+ }
+ return %credential;
+}
+
+=item credential_read( FILE_HANDLE, CREDENTIAL_HASH )
+
+Writes credential key-value pairs from hash referenced by C<CREDENTIAL_HASH>
+to C<FILE_HANDLE>. Keys and values cannot contain new-line or NUL byte
+characters, and key cannot contain equal sign nor be empty (if they do
+Error::Simple is thrown). Any white space is preserved. If value for a key
+is C<undef>, it will be skipped.
+
+If C<'url'> key exists it will be written first. (All the other key-value
+pairs are written in sorted order but you should not depend on that). Once
+all lines are written, an empty line is printed.
+
+=cut
+
+sub credential_write {
+ my ($self, $writer, $credential) = _maybe_self(@_);
+ my ($key, $value);
+
+ # Check if $credential is valid prior to writing anything
+ while (($key, $value) = each %$credential) {
+ if (!defined $key || !length $key) {
+ throw Error::Simple("credential key empty or undefined");
+ } elsif ($key =~ /[=\n\0]/) {
+ throw Error::Simple("credential key contains invalid characters: $key");
+ } elsif (defined $value && $value =~ /[\n\0]/) {
+ throw Error::Simple("credential value for key=$key contains invalid characters: $value");
+ }
+ }
+
+ for $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential->{$key}) {
+ print $writer $key, '=', $credential->{$key}, "\n";
+ }
+ }
+ print $writer "\n";
+}
+
+sub _credential_run {
+ my ($self, $credential, $op) = _maybe_self(@_);
+
+ my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', $op);
+
+ credential_write $writer, $credential;
+ close $writer;
+
+ if ($op eq "fill") {
+ %$credential = credential_read $reader;
+ } elsif (<$reader>) {
+ throw Error::Simple("unexpected output from git credential $op response:\n$_\n");
+ }
+
+ command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASH [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASH, CODE )
+
+Executes C<git credential> for a given set of credentials and
+specified operation. In both form C<CREDENTIAL_HASH> needs to be
+a reference to a hash which stores credentials. Under certain
+conditions the hash can change.
+
+In the first form, C<OPERATION> can be C<'fill'> (or omitted),
+C<'approve'> or C<'reject'>, and function will execute corresponding
+C<git credential> sub-command. In case of C<'fill'> the values stored
+in C<CREDENTIAL_HASH> will be changed to the ones returned by the
+C<git credential> command. The usual usage would look something like:
+
+ my %cred = (
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ );
+ Git::credential \%cred;
+ if (try_to_authenticate($cred{'username'}, $cred{'password'})) {
+ Git::credential \%cred, 'approve';
+ ... do more stuff ...
+ } else {
+ Git::credential \%cred, 'reject';
+ }
+
+In the second form, C<CODE> needs to be a reference to a subroutine.
+The function will execute C<git credential fill> to fill provided
+credential hash, than call C<CODE> with C<CREDENTIAL> as the sole
+argument, and finally depending on C<CODE>'s return value execute
+C<git credential approve> (if return value yields true) or C<git
+credential reject> (otherwise). The return value is the same as what
+C<CODE> returned. With this form, the usage might look as follows:
+
+ if (Git::credential {
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ }, sub {
+ my $cred = shift;
+ return try_to_authenticate($cred->{'username'}, $cred->{'password'});
+ }) {
+ ... do more stuff ...
+ }
+
+=cut
+
+sub credential {
+ my ($self, $credential, $op_or_code) = (_maybe_self(@_), 'fill');
+
+ if ('CODE' eq ref $op_or_code) {
+ _credential_run $credential, 'fill';
+ my $ret = $op_or_code->($credential);
+ _credential_run $credential, $ret ? 'approve' : 'reject';
+ return $ret;
+ } else {
+ _credential_run $credential, $op_or_code;
+ }
+}
+
{ # %TEMP_* Lexical Context
my (%TEMP_FILEMAP, %TEMP_FILES);
--
1.8.1.3.571.g3f8bed7.dirty
^ permalink raw reply related
* Re: Pushing a git repository to a new server
From: Jeff King @ 2013-02-11 16:27 UTC (permalink / raw)
To: Ethan Reesor; +Cc: Konstantin Khomoutov, git
In-Reply-To: <CAE_TNin6-weutRDToZ7-BBGJTCcf0dwJn0ChUbFcACRU=SbjzA@mail.gmail.com>
On Mon, Feb 11, 2013 at 02:57:51AM -0500, Ethan Reesor wrote:
> On Mon, Feb 11, 2013 at 2:50 AM, Konstantin Khomoutov
> <kostix+git@007spb.ru> wrote:
> > What's wrong with
> > $ ssh myuser@remotehost 'mkdir /path/to/MyRepo.git; cd $_; git init --bare'
> > $ git push --all git@remotehost:MyOtherRepo.git
> > ?
>
> Nothing, I just wanted to make myself a command to do that for me.
We talked about this a long time ago. One problem is that it's
inherently unportable, as the procedure to make a repo is potentially
different on every server (and certainly that is the case between a
regular user running stock git and something like GitHub or Google Code;
I imagine even gitolite has some special procedures for creating repos,
too).
One proposal made in the previous discussion was to define a microformat
for repository administration commands. So that you could connect and
say "git admin-create-repo /path/to/MyRepo.git", and the server-provided
admin-create-repo command would take care of the details. Then stock git
could forward it to "git init --bare", GitHub could do the same and
create the necessary database records, etc.
And once that standardized method was in place, it would be easy to add
a "--create" option to "git push" to request an "admin-create-repo"
before pushing.
I still think that's a reasonable way forward, but nobody was interested
enough to start writing code for it.
-Peff
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Torsten Bögershausen @ 2013-02-11 16:28 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git
In-Reply-To: <1360589687-9233-1-git-send-email-kusmabite@gmail.com>
On 11.02.13 14:34, Erik Faye-Lund wrote:
> Even though parse-options doesn't support UTF-8 switches (which
> makes sense; non-ascii switches would be difficult to enter on
> some keyboard layouts), it can be useful to report incorrectly
> entered UTF-8 switches to make the output somewhat less ugly
> for those of us with keyboard layouts with UTF-8 characters on
> it.
>
> Make the reporting code grok UTF-8 in the option sequence, and
> write a variable-width output sequence.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
> As being both clumsy and Norwegian, I some times to enter the
> Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the
> correct ones when entering command-line options.
>
> However, since git only looks at one byte at the time for
> short-options, it ends up reporting a partial UTF-8 sequence
> in such cases, leading to corruption of the output.
>
> The "real fix" would probably be to add proper multi-byte
> support to the short-option parser, but this serves little
> purpose in Git; we don't internationalize the command-line
> switches.
>
> So perhaps this is a suitable band-aid instead?
>
> parse-options.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 67e98a6..20dc742 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -3,6 +3,7 @@
> #include "cache.h"
> #include "commit.h"
> #include "color.h"
> +#include "utf8.h"
>
> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> const char * const *usagestr,
> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
> if (ctx.argv[0][1] == '-') {
> error("unknown option `%s'", ctx.argv[0] + 2);
> } else {
> - error("unknown switch `%c'", *ctx.opt);
> + const char *next = ctx.opt;
> + utf8_width(&next, NULL);
> + error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
> }
> usage_with_options(usagestr, options);
> }
>
Would the following do the trick?
diff --git a/parse-options.c b/parse-options.c
index c1c66bd..f800552 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -471,7 +471,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
if (ctx.argv[0][1] == '-') {
error("unknown option `%s'", ctx.argv[0] + 2);
} else {
- error("unknown switch `%c'", *ctx.opt);
+ error("unknown switch `%s'", ctx.opt);
}
^ permalink raw reply related
* Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command
From: Junio C Hamano @ 2013-02-11 16:28 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, Sitaram Chamarty, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211075245.GO15329@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> +To disable interactive logins, displaying a greeting instead:
>>> ++
>>> +----------------
>>> +$ chsh -s /usr/bin/git-shell
>>> +$ mkdir $HOME/git-shell-commands
>>> +$ cat >$HOME/git-shell-commands/help <<\EOF
>>> +#!/bin/sh
>>> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
>>
>> Where in the sshd to git-shell exec chain is $USER variable set for
>> the user? Just being curious if this is the simplest but one of the
>> more robust ways to get the user's name.
>
> That's a good question. environment= in an authorized_keys file is
> obsolete, so USER generally represents the actual logged in user.
>
> That means the main way to base behavior on private key (letting one
> system user represent multiple people) is a gitolite-style command=
> wrapper that checks SSH_ORIGINAL_COMMAND. In that setup, there is no
> reason to forward simple no-args "are you there?" requests to the
> git-shell, so we can forget about it here.
>
> So by the time we get to git-shell, most likely either
>
> A) this is a generic system user, with a username like "git", and the
> above example would insult the client with "Hi git!" or "Hi
> project-x-git!"
>
> or
>
> B) each person has a separate account on the system, perhaps to help
> the admin to set filesystem permissions based on users and groups,
> and the above would address the user by her normal name.
What return value getuid(2) would give us was not something I was
worried about. Use of git-shell would be pointless if that does not
work to offer isolation between users.
I was wondering who would set the $USER variable based on the uid
assigned to the process during the remote login process and it is a
behaviour we can rely on across platforms. It appears that when
coming over ssh, it is the ssh daemon that sets USER (and LOGNAME,
HOME, etc.) before running the login shell (session.c::do_child()
that is called from do_exec_pty() or do_exec_no_pty() in openssh).
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Erik Faye-Lund @ 2013-02-11 16:36 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <51191C33.8070402@web.de>
On Mon, Feb 11, 2013 at 5:28 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 11.02.13 14:34, Erik Faye-Lund wrote:
>> Even though parse-options doesn't support UTF-8 switches (which
>> makes sense; non-ascii switches would be difficult to enter on
>> some keyboard layouts), it can be useful to report incorrectly
>> entered UTF-8 switches to make the output somewhat less ugly
>> for those of us with keyboard layouts with UTF-8 characters on
>> it.
>>
>> Make the reporting code grok UTF-8 in the option sequence, and
>> write a variable-width output sequence.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
>> As being both clumsy and Norwegian, I some times to enter the
>> Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the
>> correct ones when entering command-line options.
>>
>> However, since git only looks at one byte at the time for
>> short-options, it ends up reporting a partial UTF-8 sequence
>> in such cases, leading to corruption of the output.
>>
>> The "real fix" would probably be to add proper multi-byte
>> support to the short-option parser, but this serves little
>> purpose in Git; we don't internationalize the command-line
>> switches.
>>
>> So perhaps this is a suitable band-aid instead?
>>
>> parse-options.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/parse-options.c b/parse-options.c
>> index 67e98a6..20dc742 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -3,6 +3,7 @@
>> #include "cache.h"
>> #include "commit.h"
>> #include "color.h"
>> +#include "utf8.h"
>>
>> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
>> const char * const *usagestr,
>> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
>> if (ctx.argv[0][1] == '-') {
>> error("unknown option `%s'", ctx.argv[0] + 2);
>> } else {
>> - error("unknown switch `%c'", *ctx.opt);
>> + const char *next = ctx.opt;
>> + utf8_width(&next, NULL);
>> + error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
>> }
>> usage_with_options(usagestr, options);
>> }
>>
> Would the following do the trick?
>
> diff --git a/parse-options.c b/parse-options.c
> index c1c66bd..f800552 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -471,7 +471,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
> if (ctx.argv[0][1] == '-') {
> error("unknown option `%s'", ctx.argv[0] + 2);
> } else {
> - error("unknown switch `%c'", *ctx.opt);
> + error("unknown switch `%s'", ctx.opt);
> }
>
>
Nope; that would print the rest of the option-string, in cases of "git
<command> -abcd".
^ permalink raw reply
* Re: [PATCH] fixup! graph: output padding for merge subsequent parents
From: Junio C Hamano @ 2013-02-11 16:42 UTC (permalink / raw)
To: John Keeping; +Cc: git, Matthieu Moy, Michał Kiedrowicz
In-Reply-To: <20130211105433.GA3245@farnsworth.metanate.com>
John Keeping <john@keeping.me.uk> writes:
> Perhaps it's best to leave the patch as it originally was to guarantee
> that we can't get stuck in graph_show_commit(), even when it's called at
> an unexpected time, but I see you've already squashed this change in.
>
> Would you prefer me to resend the original patch or send an update with
> this change and the above reasoning in the commit message?
Yes, please. Let's have the original (I think I have it in my
reflog so no need to resend it) and this update on top as a separate
patch with an updated log message.
^ permalink raw reply
* Re: [PATCHv3 0/5] Add git-credential support to git-send-email
From: Jeff King @ 2013-02-11 16:51 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <cover.1360599057.git.mina86@mina86.com>
On Mon, Feb 11, 2013 at 05:23:34PM +0100, Michal Nazarewicz wrote:
> From: Michal Nazarewicz <mina86@mina86.com>
>
> The third version of the patch with changes suggested by Jeff in the
> 4/5 patch. Also credential_read and credential_write are now public
> functions in case someone wants to write a helper in perl.
Thanks, the changes you made look good. And I think it's a good idea to
make the read/write functions public.
I have two minor comments, which I'll reply inline with. But even with
those comments, I think this would be OK to merge.
-Peff
^ permalink raw reply
* Re: [PATCHv3 4/5] Git.pm: add interface for git credential command
From: Jeff King @ 2013-02-11 16:53 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Matthieu Moy, git, Michal Nazarewicz
In-Reply-To: <2ec5dd694878055e9ce9d650889ee85369073568.1360599712.git.mina86@mina86.com>
On Mon, Feb 11, 2013 at 05:23:38PM +0100, Michal Nazarewicz wrote:
> +=item credential_read( FILE_HANDLE )
> +
> +Reads credential key-value pairs from C<FILE_HANDLE>. Reading stops at EOF or
> +when an empty line is encountered. Each line must be of the form C<key=value>
> +with a non-empty key. Function returns a hash with all read values. Any
> +white space (other then new-line character) is preserved.
> +
> +=cut
> +
> +sub credential_read {
> + my ($self, $reader) = _maybe_self(@_);
> + my %credential;
> + while (<$reader>) {
> + chomp;
> + if ($_ eq '') {
> + last;
> + } elsif (!/^([^=]+)=(.*)$/) {
> + throw Error::Simple("unable to parse git credential data:\n$_");
> + }
> + $credential{$1} = $2;
> + }
> + return %credential;
> +}
Should this return a hash reference? It seems like that is how we end up
using and passing it elsewhere (since we have to anyway when passing it
as a parameter).
I don't have a strong preference, and it's somewhat a matter of taste.
And maybe returning the actual hash matches the rest of the module
better. I admit I don't really use Git.pm much.
-Peff
^ permalink raw reply
* Re: [PATCHv3 5/5] git-send-email: use git credential to obtain password
From: Jeff King @ 2013-02-11 17:01 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Matthieu Moy, git, Michal Nazarewicz
In-Reply-To: <fd7997960cad569d57f5330f2416f702db414169.1360599712.git.mina86@mina86.com>
On Mon, Feb 11, 2013 at 05:23:39PM +0100, Michal Nazarewicz wrote:
> + # TODO: Authentication may fail not because credentials were
> + # invalid but due to other reasons, in which we should not
> + # reject credentials.
> + $auth = Git::credential({
> + 'protocol' => 'smtp',
> + 'host' => join(':', $smtp_server, $smtp_server_port),
> + 'username' => $smtp_authuser,
> + # if there's no password, "git credential fill" will
> + # give us one, otherwise it'll just pass this one.
> + 'password' => $smtp_authpass
> + }, sub {
> + my $cred = shift;
> + return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
> + });
What do we want to do about this TODO?
I am happy to put it off until it becomes a problem, but I wonder if the
Git::credential() interface is sufficient to express what we would want.
It only allows two return values: true for approve, false for reject.
But we would want a tri-state: approve, reject, indeterminate.
Reading the Net::SMTP code, it doesn't look like the information is even
available to us (it really just passes out success or failure), so I
don't think we can even make it work now. But it may be better to
prepare the public Git::credential interface for it now, so we do not
have to deal with breaking compatibility later.
-Peff
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Torsten Bögershausen @ 2013-02-11 17:04 UTC (permalink / raw)
To: kusmabite; +Cc: Torsten Bögershausen, git
In-Reply-To: <CABPQNSYXk5_VdGP9QQttZKpcBnmohmNb+AOH=bMkOrXjbq1ZmQ@mail.gmail.com>
On 11.02.13 17:36, Erik Faye-Lund wrote:
> On Mon, Feb 11, 2013 at 5:28 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 11.02.13 14:34, Erik Faye-Lund wrote:
>>> Even though parse-options doesn't support UTF-8 switches (which
>>> makes sense; non-ascii switches would be difficult to enter on
>>> some keyboard layouts), it can be useful to report incorrectly
>>> entered UTF-8 switches to make the output somewhat less ugly
>>> for those of us with keyboard layouts with UTF-8 characters on
>>> it.
>>>
>>> Make the reporting code grok UTF-8 in the option sequence, and
>>> write a variable-width output sequence.
>>>
>>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>>> ---
>>> As being both clumsy and Norwegian, I some times to enter the
>>> Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the
>>> correct ones when entering command-line options.
>>>
>>> However, since git only looks at one byte at the time for
>>> short-options, it ends up reporting a partial UTF-8 sequence
>>> in such cases, leading to corruption of the output.
>>>
>>> The "real fix" would probably be to add proper multi-byte
>>> support to the short-option parser, but this serves little
>>> purpose in Git; we don't internationalize the command-line
>>> switches.
>>>
>>> So perhaps this is a suitable band-aid instead?
>>>
>>> parse-options.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/parse-options.c b/parse-options.c
>>> index 67e98a6..20dc742 100644
>>> --- a/parse-options.c
>>> +++ b/parse-options.c
>>> @@ -3,6 +3,7 @@
>>> #include "cache.h"
>>> #include "commit.h"
>>> #include "color.h"
>>> +#include "utf8.h"
>>>
>>> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
>>> const char * const *usagestr,
>>> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
>>> if (ctx.argv[0][1] == '-') {
>>> error("unknown option `%s'", ctx.argv[0] + 2);
>>> } else {
>>> - error("unknown switch `%c'", *ctx.opt);
>>> + const char *next = ctx.opt;
>>> + utf8_width(&next, NULL);
>>> + error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
>>> }
>>> usage_with_options(usagestr, options);
>>> }
>>>
>> Would the following do the trick?
>>
>> diff --git a/parse-options.c b/parse-options.c
>> index c1c66bd..f800552 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -471,7 +471,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
>> if (ctx.argv[0][1] == '-') {
>> error("unknown option `%s'", ctx.argv[0] + 2);
>> } else {
>> - error("unknown switch `%c'", *ctx.opt);
>> + error("unknown switch `%s'", ctx.opt);
>> }
>>
>>
> Nope; that would print the rest of the option-string, in cases of "git
> <command> -abcd".
Ok, may be pick_one_utf8_char() is a better choice than simply assuming ASCII.
We can make a guess, if it is utf-8, we use it. If not, assume ASCII.
Just thinking loud (the "if" could be written shorter using the "?" operator)
} else {
const char *start = ctx.opt;
unsigned c = pick_one_utf8_char(&start, NULL);
if (!c)
c = *ctx.opt;
error("unknown switch `%c'", c);
}
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Junio C Hamano @ 2013-02-11 17:07 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git
In-Reply-To: <1360589687-9233-1-git-send-email-kusmabite@gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> However, since git only looks at one byte at the time for
> short-options, it ends up reporting a partial UTF-8 sequence
> in such cases, leading to corruption of the output.
Isn't it a workable, easier and more robust alternative to punt and
use the entire ctx.argv[0] as unrecognized?
>
> The "real fix" would probably be to add proper multi-byte
> support to the short-option parser, but this serves little
> purpose in Git; we don't internationalize the command-line
> switches.
>
> So perhaps this is a suitable band-aid instead?
>
> parse-options.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 67e98a6..20dc742 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -3,6 +3,7 @@
> #include "cache.h"
> #include "commit.h"
> #include "color.h"
> +#include "utf8.h"
>
> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> const char * const *usagestr,
> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
> if (ctx.argv[0][1] == '-') {
> error("unknown option `%s'", ctx.argv[0] + 2);
> } else {
> - error("unknown switch `%c'", *ctx.opt);
> + const char *next = ctx.opt;
> + utf8_width(&next, NULL);
> + error("unknown switch `%.*s'", (int)(next - ctx.opt), ctx.opt);
> }
> usage_with_options(usagestr, options);
> }
^ permalink raw reply
* Re: [Request] Git export with hardlinks
From: Jeff King @ 2013-02-11 17:13 UTC (permalink / raw)
To: Thomas Koch; +Cc: git
In-Reply-To: <201302101133.28746.thomas@koch.ro>
On Sun, Feb 10, 2013 at 11:33:26AM +0100, Thomas Koch wrote:
> thank you very much for your idea! It's good and simple. It just breaks down
> for the case when a large folder got renamed.
Yes, it would never find renames, which a true sha1->path map could.
> But I already hacked the basic layout of the algorithm and it's not
> complicated at all, I believe:
>
> https://github.com/thkoch2001/git_export_hardlinks/blob/master/git_export_hardlinks.py
It looks like you create the sha1->path mapping by asking the user to
provide <tree_sha1>,<path> pairs, and then assuming that the exported
tree at <path> exactly matches <tree_sha1>. Which it would in the
workflow you've proposed, but it is also easy for that not to be the
case (e.g., somebody munges a file in <path> after it has been
exported).
So it's a bit dangerous as a general purpose tool, IMHO. It's also a
slight pain in that you have to keep track of the tree sha1 for each
exported path somehow.
A safer and more convenient (but slightly less efficient) solution would
be to keep a git index file for each exported tree. Then we can just
refresh that index, which would check that our sha1 for each path is up
to date (and in the common case of nothing changed, would only be as
expensive as stat()-ing each entry). And then we use that index as the
sha1->path map.
The simplest way to have an index for each export would be to actually
give each one its own git repo (which does not have to use much space,
if you use "-s" to share the objects with the master repo).
That's more complex, and uses more disk than what your script does, but
I do think the added safety would be worth it for a general-purpose
tool.
> I had to interrupt work on this and could not yet finish and test it. But I
> thought you might be interested. Maybe something like this might one day be
> rewritten in C and become part of git core?
I think if we had a `git export` command (and we do not, but there has
been discussion in a nearby thread about whether such a thing might be a
good idea), having a `--hard-link-from` option to link with other
checkouts would make sense. It could also potentially be an option to
git-checkout-index, and you could script around it at that low level.
-Peff
^ permalink raw reply
* Re: [PATCHv3 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-11 17:14 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <20130211165331.GD16402@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2347 bytes --]
On Mon, Feb 11 2013, Jeff King wrote:
> On Mon, Feb 11, 2013 at 05:23:38PM +0100, Michal Nazarewicz wrote:
>
>> +=item credential_read( FILE_HANDLE )
>> +
>> +Reads credential key-value pairs from C<FILE_HANDLE>. Reading stops at EOF or
>> +when an empty line is encountered. Each line must be of the form C<key=value>
>> +with a non-empty key. Function returns a hash with all read values. Any
>> +white space (other then new-line character) is preserved.
>> +
>> +=cut
>> +
>> +sub credential_read {
>> + my ($self, $reader) = _maybe_self(@_);
>> + my %credential;
>> + while (<$reader>) {
>> + chomp;
>> + if ($_ eq '') {
>> + last;
>> + } elsif (!/^([^=]+)=(.*)$/) {
>> + throw Error::Simple("unable to parse git credential data:\n$_");
>> + }
>> + $credential{$1} = $2;
>> + }
>> + return %credential;
>> +}
>
> Should this return a hash reference? It seems like that is how we end up
> using and passing it elsewhere (since we have to anyway when passing it
> as a parameter).
Admittedly I mostly just copied what git-remote-mediawiki did here and
don't really have any preference either way, even though with this
function returning a reference the call site would have to become:
%$credential = %{ credential_read $reader };
Another alternative would be for it to take a reference as an argument,
possibly an optional one:
+sub credential_read {
+ my ($self, $reader, $ret) = (_maybe_self(@_), {});
+ my %credential;
+ while (<$reader>) {
+ # ...
+ }
+ %$ret = %credential;
+ $ret;
+}
I'd avoid modifying the hash while reading though since I think it's
best if it's left intact in case of an error.
And of course, if we want to get even more crazy, credential_write could
accept either reference or a hash, like so:
+sub credential_write {
+ my ($self, $writer, @rest) = _maybe_self(@_);
+ my $credential = @rest == 1 ? $rest[0] : { @rest };
+ my ($key, $value);
+ # ...
+}
Bottom line is, anything can be coded, but a question is whether it
makes sense to do so. ;)
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* Re: [PATH/RFC] parse-options: report invalid UTF-8 switches
From: Erik Faye-Lund @ 2013-02-11 17:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7gmeok2e.fsf@alter.siamese.dyndns.org>
On Mon, Feb 11, 2013 at 6:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> However, since git only looks at one byte at the time for
>> short-options, it ends up reporting a partial UTF-8 sequence
>> in such cases, leading to corruption of the output.
>
> Isn't it a workable, easier and more robust alternative to punt and
> use the entire ctx.argv[0] as unrecognized?
>
Perhaps. It doesn't match the output of the usual GNU tools like we
currently do, but even the GNU tools only report a single byte.
However, I'm unsure if that totals to an improvement in the common
case. We stop telling the user exactly what option was problematic,
making it slightly more annoying to read through the options.
So, we'd end up making the common-case worse, by making a special case
(that only sometimes affects some users) more robust. Isn't that
making the user interface worse?
^ permalink raw reply
* Re: [PATCHv3 5/5] git-send-email: use git credential to obtain password
From: Michal Nazarewicz @ 2013-02-11 17:17 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <20130211170134.GE16402@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]
> On Mon, Feb 11, 2013 at 05:23:39PM +0100, Michal Nazarewicz wrote:
>> + # TODO: Authentication may fail not because credentials were
>> + # invalid but due to other reasons, in which we should not
>> + # reject credentials.
>> + $auth = Git::credential({
>> + 'protocol' => 'smtp',
>> + 'host' => join(':', $smtp_server, $smtp_server_port),
>> + 'username' => $smtp_authuser,
>> + # if there's no password, "git credential fill" will
>> + # give us one, otherwise it'll just pass this one.
>> + 'password' => $smtp_authpass
>> + }, sub {
>> + my $cred = shift;
>> + return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
>> + });
On Mon, Feb 11 2013, Jeff King wrote:
> What do we want to do about this TODO?
>
> I am happy to put it off until it becomes a problem, but I wonder if the
> Git::credential() interface is sufficient to express what we would want.
> It only allows two return values: true for approve, false for reject.
> But we would want a tri-state: approve, reject, indeterminate.
Being it tri-state is not a problem. The last can be easily represented
by undef.
> Reading the Net::SMTP code, it doesn't look like the information is even
> available to us (it really just passes out success or failure), so I
> don't think we can even make it work now. But it may be better to
> prepare the public Git::credential interface for it now, so we do not
> have to deal with breaking compatibility later.
I guess. I left it as is since git-send-email won't make use of the
indeterminate values, but I can add it in this patchset as well.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* Re: [PATCHv3 0/5] Add git-credential support to git-send-email
From: Michal Nazarewicz @ 2013-02-11 17:18 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <20130211165136.GC16402@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 478 bytes --]
On Mon, Feb 11 2013, Jeff King wrote:
> I have two minor comments, which I'll reply inline with. But even with
> those comments, I think this would be OK to merge.
I'll send a new patchset tomorrow with.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 17:18 UTC (permalink / raw)
To: Jeff King
Cc: Jonathan Nieder, Sitaram Chamarty, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211160057.GA16402@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Feb 10, 2013 at 11:17:24PM -0800, Junio C Hamano wrote:
>
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>
>> > Isn't that a criticism of the git-shell-commands facility in general?
>> > If it is common to have a lot of users with distinct home directories
>> > but all with git-shell as their login shell, then the
>> > git-shell-commands should not go in their home directory to begin
>> > with, no?
>>
>> You can give one set of commands to some users while restricting
>> others, no?
>
> But that seems to me to argue against /etc/git/shell-disabled or
> similar, which would apply to every user. Or are you proposing that the
> check be:
>
> if -d ~/git-shell-commands; then
> : ok, interactive
> elif -x /etc/git/shell-disabled; then
> exec /etc/git/shell-disabled
> else
> echo >&2 'go away'
> exit 1
> fi
That "shell-disabled" thing was to allow customizing the existing
die() that triggers here:
} else if (argc == 1) {
/* Allow the user to run an interactive shell */
cd_to_homedir();
if (access(COMMAND_DIR, R_OK | X_OK) == -1) {
die("Interactive git shell is not enabled.\n"
"hint: ~/" COMMAND_DIR " should exist "
"and have read and execute access.");
}
run_shell();
exit(0);
so it is more like
if ! test -d $HOME/git-shell-commands
then
if test -x /etc/git/shell-disabled
then
exec /etc/git/shell-disabled
else
die Interactive is not enabled
fi
fi
... do whatever in run_shell() ...
> That at least means you can apply _whether_ to disable the shell
> selectively for each user (by providing or not a git-shell-commands
> directory), but you cannot individually select the script that runs for
> that user. But it's probably still flexible enough;...
Such a flexibility is not a goal of /etc/git/shell-disabled. The
sole goal is to make the life easier for those site owners that do
not want any interactive shell access to give more friendly and
customized error message.
Those who want further flexibility can exit with non-zero from the
"help" (which is still a misnomer for a hook to disable interactive
for the user).
My primary objection is that implementing only that "more flexible
but requires more configuration work" solution without giving
simpler solution (i.e. just one thing to configure) to the majory of
site owners who only have simpler problem to solve (i.e. just want
to customize "no interactive here"), and saying that the latter can
be done on top. It is backwards mentality.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox