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

[Ted Zlatanov <tzz@lifelogs.com>]

On Mon, 04 Feb 2013 16:15:47 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:

>> - do you want to support backslashed newlines?

JCH> What for?  netrc/authinfo is not a line oriented file format at all,
JCH> and

JCH> 	machine k.org
JCH> 	        	login me
JCH>                         password mysecret

JCH> is a single entry; you do not need backslash at the end of any line.

Hmm. The parser I implemented only does single-line parsing, and I
misunderstood the format to be single-line (partly because I have never
seen anyone using the multi-line format you show).  Looking at
Net::Netrc more carefully, it seems that the "machine" token is what
defines an entry, so a new entry starts with a new line that contains a
"machine" token.  Is that acceptable and does it match your
understanding of the format?  It matches Net::Netrc, at least.

I'll add this change to PATCHv4 with the assumption you agree.

>> - should all die() calls just print to STDERR and exit(0)?

JCH> Where "when unhandled, the helper should silently exit with 0" is
JCH> expected by the invoker, we shouldn't say anything to error stream,
JCH> and exit with zero.  Please leave a comment to make it easy to
JCH> understand to the readers that is what is going on there.

JCH> If on the other hand it diagnosed an error (not a bug in the
JCH> implementation but a misconfiguration on the user's side), I _think_
JCH> it should loudly die() so that the user can notice and take
JCH> corrective action.

OK, I'll review these for PATCHv4 (also see below).  Thanks.

>> - do you want to support multiple netrc files, as you and Peff suggested?

JCH> I didn't even suggest such thing IIRC---I expected it to iterate
JCH> from the most desirable (.authinfo.gpg) to the least (.netrc) and
JCH> stop at the first found one.  There may be use cases people use more
JCH> than one and expect an entry to be found in any file, but I suspect
JCH> that might be more confusing than it is worth.  But I do not care
JCH> very deeply myself either way.

After thinking about it, I agree with Peff multiple sources make sense
and will simplify the code flow (especially the default case, which
won't need to be handled separately).  And the functionality doesn't
have to be confusing with the right debugging messages.  So I'll add
them in PATCHv4.

The debugging messages will be fewer and simpler with this approach,
which makes it feel like the right track :)

Thanks
Ted