Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

> On Mon, Feb 04, 2013 at 02:56:06PM -0800, Junio C Hamano wrote:
>
>> > +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 "...";
>> 	}
>
> Not having a mode is an invocation error; the credential-helper
> documentation indicates that the helper will always be invoked with an
> action. The likely culprit for not having one is the user invoking it
> manually, and showing the usage there is a sensible action.
>
> Whereas invoking it with a mode other than "get" is not an error at all.
> Git will run it with the "store" and "erase" actions, too. Those happen
> to be no-ops for this helper, so it exits silently. The credential docs
> specify that any other actions should be ignored, too, to allow for
> future expansion.

> but here is how I
> would have written the outer logic of the script:
>
>   my $tokens = read_credential_data_from_stdin();
>   if ($options{file}) {
>           my @entries = load_netrc($options{file})
>                   or die "unable to open $options{file}: $!";
>           check_netrc($tokens, @entries);
>   }
>   else {
>           foreach my $ext ('.gpg', '') {
>                   foreach my $base (qw(authinfo netrc)) {
>                           my @entries = load_netrc("$base$ext")
>                                   or next;
>                           if (check_netrc($tokens, @entries)) {
>                                   last;
>                           }
>                   }
>           }
>   }
>
> I.e., to fail on "-f", but otherwise treat unreadable auto-selected
> files as a no-op, for whatever reason. I'd also consider checking all
> files if they are available, in case the user has multiple (e.g., they
> keep low-quality junk unencrypted but some high-security passwords in a
> .gpg file). Not that likely, but not any harder to implement.