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

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

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

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.

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.

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