Ted Zlatanov <tzz@xxxxxxxxxxxx> writes:
> Signed-off-by: Ted Zlatanov <tzz@xxxxxxxxxxxx>
The space above your S-o-b: could be utilized a bit better ;-)
> @@ -0,0 +1,236 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;
> +
> +use Getopt::Long;
> +use File::Basename;
> +
> +my $VERSION = "0.1";
> +
> +my %options = (
> + help => 0,
> + debug => 0,
> +
> + # identical token maps, e.g. host -> host, will be inserted later
> + tmap => {
> + port => 'protocol',
> + machine => 'host',
> + path => 'path',
> + login => 'username',
> + user => 'username',
> + password => 'password',
> + }
> + );
> +
> +# map each credential protocol token to itself on the netrc side
> +$options{tmap}->{$_} = $_ foreach values %{$options{tmap}};
> +
> +foreach my $suffix ('.gpg', '') {
> + foreach my $base (qw/authinfo netrc/) {
> + my $file = glob("~/.$base$suffix");
> + next unless (defined $file && -f $file);
> + $options{file} = $file ;
> + }
> +}
This checks .gpg first and then unencrypted, and checks authinfo
first and netrc second, both of which makes sense. It is good to
encourage use of encrypted files, and it is good to use newer
authinfo files over netrc files.
However, it is strange that you let the ones that are discovered
later in the loop to override the ones that are discovered earlier.
Perhaps you meant
next unless (... exists there ...);
$options{"file"} = $file;
last;
instead?
> +Getopt::Long::Configure("bundling");
Hmm, OK.
> +# TODO: maybe allow the token map $options{tmap} to be configurable.
> +GetOptions(\%options,
> + "help|h",
> + "debug|d",
> + "file|f=s",
> + );
> +
> +if ($options{help}) {
> + my $shortname = basename($0);
> + $shortname =~ s/git-credential-//;
> +
> + print <<EOHIPPUS;
> +
> +$0 [-f AUTHFILE] [-d] get
> +
> +Version $VERSION by tzz\@lifelogs.com. License: BSD.
> +
> +Options:
> + -f AUTHFILE: specify a netrc-style file
> + -d: turn on debugging
> +
> +To enable (note that Git will prepend "git-credential-" to the helper
> +name and look for it in the path):
> +
> + git config credential.helper '$shortname -f AUTHFILE'
> +
> +And if you want lots of debugging info:
> +
> + git config credential.helper '$shortname -f AUTHFILE -d'
> +
> +Only "get" mode is supported by this credential helper. It opens
> +AUTHFILE and looks for entries that match the requested search
> +criteria:
> +
> + 'port|protocol':
> + The protocol that will be used (e.g., https). (protocol=X)
> +
> + 'machine|host':
> + The remote hostname for a network credential. (host=X)
> +
> + 'path':
> + The path with which the credential will be used. (path=X)
> +
> + 'login|user|username':
> + The credential’s username, if we already have one. (username=X)
> +
> +Thus, when we get this query on STDIN:
> +
> +protocol=https
> +username=tzz
> +
> +this credential helper will look for lines in AUTHFILE that match
> +
> +port https login tzz
> +
> +OR
> +
> +protocol https login tzz
> +
> +OR... etc. acceptable tokens as listed above. Any unknown tokens are
> +simply ignored.
I recall that netrc/authinfo files are _not_ line oriented. Earlier
you said "looks for entries that match" which is a lot more correct,
but then we see "look for lines in authfile".
> +Then, the helper will print out whatever tokens it got from the line,
> +including "password" tokens, mapping e.g. "port" back to "protocol".
Again "line" is mentioned twice, above and below.
> +The first matching line is used. Tokens can be quoted as 'STRING' or
> +"STRING".
> +
> +No caching is performed by this credential helper.
> +
> +EOHIPPUS
> +
> + exit;
> +}
> +my $mode = shift @ARGV;
> +
> +# credentials may get 'get', 'store', or 'erase' as parameters but
> +# only acknowledge 'get'
> +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
> +
> +# only support 'get' mode
> +exit unless $mode eq 'get';
The above looks strange. Why does the invoker get the error message
only when it runs this without arguments? Did you mean to say more
like this?
unless (defined $mode && $mode eq 'get') {
die "...";
}
By the way, I think statement modifiers tend to get overused and
make the resulting program harder to read. die "..." at the
beginning of line makes the reader go "Whoa, it already is done and
existing on error", and then forces the eyes to scan the error
message to find "unless" and the condition.
It may be a cute syntax and some may find it even cool, but cuteness
or coolness is less valuable compared with the readability.
> +my $debug = $options{debug};
> +my $file = $options{file};
> +
> +unless (defined $file) {
> + print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
> + exit 0;
> +}
> +
> +unless (-f $file) {
> + print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
> + exit 0;
> +}
Perhaps "-r $file", if you say "is not accessible"?
Is it sensible to squelch the error message by default and force
user to specify --debug? You could argue that the option is to
debug the user's configuration, but the name of the option sounds
more like it is for debugging this script itself.
I saw Peff already pointed out error conditions, but I am not sure
why all of these exit with 0. If the user has configured
git config credential.helper 'netrc -f $HOME/.netcr'
shouldn't it be diagnosed as an error? It is understandable to let
this go silently
git config credential.helper 'netrc'
and let other credential helpers take over when no $HOME/.{netrc,authinfo}{,.gpg}
file exist, but in that case the user may still want to remove the
config item that is not doing anything useful and erroring out with
a message may be a way to help the user know about the situation.
> +my @data;
> +if ($file =~ m/\.gpg$/) {
> + @data = load('-|', qw(gpg --decrypt), $file)
> +}
> +else {
> + @data = load('<', $file);
> +}
> +
> +chomp @data;
> +
> +unless (scalar @data) {
Shouldn't this error check come logically before chomping?
> + print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
> + exit;
> +}
Is this really an error? The file perhaps was empty. Shouldn't
that case treated the same way as the case where no entry that
matches the criteria invoker gave you was found?
--
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