Re: [PATCH] osxkeychain: avoid incorrectly skipping store operation

From: Jeff King <peff@peff.net>
Date: Nov 18, 2025, 09:57 UTC
Message-ID: <20251118095714.GD530545@coredump.intra.peff.net>
In-Reply-To: <xmqqo6p5llsw.fsf@gitster.g>

On Thu, Nov 13, 2025 at 12:28:15PM -0800, Junio C Hamano wrote:

Show 15 quoted lines

> "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > +/*
> > + * NOTE: We could use functions in strbuf.h and/or wrapper.h, but those
> > + * introduce significant dependencies. Therefore, we define simplified
> > + * versions here to keep this code self-contained.
> > + */
> 
> Sorry, but I do not quite understand this comment.  The program is
> shipped as a part of Git, and using these functions and linking with
> libgit.a may pull strbuf.o and some other *.o files out of libgit.a
> to link with git-credential-osxkeychain.o to produce the executable,
> but how can that be "significant dependencies"?  For anybody who is
> building git-credential-osxkeychain, the necessary sources come for
> free.

Back when we added the contrib/credential helpers, I tried to avoid linking with Git for two reasons:

  1. The idea was that these _could_ be independent projects, and we
     would not be on the hook for writing or maintaining everyone's pet
     platform helper. So even though they are in our tree, the hope was
     that they'd be simple enough to be totally independent programs
     (and would not even have to be written in C). And avoiding any
     dependencies kept us honest there.

     It may be that the cost of not being able to re-use our usual code
     is too high for the philosophical benefit, though.

  2. If stuff in contrib/ depends on code in libgit.a, then changes in
     the latter can break them. And I don't think we have a great flow
     for detecting such breakage. Maybe one of the CI jobs builds
     osxkeychain now? I'm not even sure.

     The xmalloc and strbuf interfaces are pretty stable, so it may be
     that the right rule is "you can depend on libgit.a, but only
     lightly".

Mostly just offering my two cents (and a little backstory). I'm not terribly opposed to loosening the rule, but we may expect some breakage via (2) from time to time.

-Peff