Re: [PATCH] credential-osxkeychain: load Security framework dynamically

From: Jay Soffian <jaysoffian@gmail.com>
Date: Sep 14, 2011, 23:56 UTC
Message-ID: <CAG+J_Dxenu7GDsTzUCCQiYg38Vto8CtD8ODX1J9mA8o1n2_YJA@mail.gmail.com>
In-Reply-To: <20110914230816.GA5754@sigill.intra.peff.net>

On Wed, Sep 14, 2011 at 7:08 PM, Jeff King <peff@peff.net> wrote:

Show 6 quoted lines

> On Wed, Sep 14, 2011 at 06:55:26PM -0400, Jay Soffian wrote:
>
> To be honest, I was surprised to see you linking against git. I had
> always envisioned OS-specific helpers as living outside of the git.git
> repo.  That's why I provided git-credential-getpass; it should be the
> only part of git that helpers really want to reuse.

Okay, I think I was led astray by the fact that credential-{cache,store}.c (at least the latter of which is meant as nothing more than an example helper right?) links with git.

> What are you getting from git that is useful? From my skim of your
> patch, it looks like xmalloc/die, parse_options, and credential_getpass.

Correct.

Show 15 quoted lines

> The first can be replaced with a few trivial lines of code. The second
> is overkill, I think. The helper code will always hand you the
> "--option=value" form, and I always intended it to stay that way
> (whether that is well documented, I'm not sure). But a simple loop with
> strncmps would be fine.
>
> The hardest part is credential_getpass. You can call "git
> credential-getpass", but you'll have to read the output yourself (though
> it's quite simple to parse; see read_credential_response).
>
> I'm not a fan of cutting and pasting code, and this will add a number of
> lines to your C program. But I feel like keeping the build completely
> separate from core git is probably a good boundary (especially because
> this will not be getting built or tested all the time, as most of the
> core code is).

Okay.

j.