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

From: Jeff King <peff@peff.net>
Date: Sep 14, 2011, 23:08 UTC
Message-ID: <20110914230816.GA5754@sigill.intra.peff.net>
In-Reply-To: <1316040926-89429-1-git-send-email-jaysoffian@gmail.com>

On Wed, Sep 14, 2011 at 06:55:26PM -0400, Jay Soffian wrote:

> Something like this. I'm going to pause here for feedback. Is the (not yet
> existant) followup commit referenced above allowing git-credential-osxkeychain
> to be a hard link to git a worthwhile endeavor? Or would a better approach be
> to make git-credential-osxkeychain.c not use any git code?

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.

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.

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).

-Peff