From: Jeff King <peff@peff•net>
To: Junio C Hamano <gitster@pobox•com>
Cc: Koji Nakamaru via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org, Koji Nakamaru <koji.nakamaru@gree•net>
Subject: Re: [PATCH] osxkeychain: avoid incorrectly skipping store operation
Date: Tue, 18 Nov 2025 04:57:14 -0500 [thread overview]
Message-ID: <20251118095714.GD530545@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqo6p5llsw.fsf@gitster.g>
On Thu, Nov 13, 2025 at 12:28:15PM -0800, Junio C Hamano wrote:
> "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
next prev parent reply other threads:[~2025-11-18 9:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-13 15:26 [PATCH] osxkeychain: avoid incorrectly skipping store operation Koji Nakamaru via GitGitGadget
2025-11-13 20:28 ` Junio C Hamano
2025-11-13 20:35 ` Junio C Hamano
2025-11-14 3:23 ` Koji Nakamaru
2025-11-18 9:57 ` Jeff King [this message]
2025-11-14 6:04 ` [PATCH v2] " Koji Nakamaru via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251118095714.GD530545@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=gitster@pobox$(echo .)com \
--cc=koji.nakamaru@gree$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox