Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <>
To: Calvin Wan <>
Cc: Git Mailing List <>,
	 "brian m. carlson" <>,,,
	 Kyle Lippincott <>,
	 Josh Steadmon <>,
	 Emily Shaffer <>,
	Enrico Mrass <>
Subject: Re: [RFD] Libification proposal: separate internal and external interfaces
Date: Mon, 22 Apr 2024 13:28:51 -0700	[thread overview]
Message-ID: <xmqqpluhuoos.fsf@gitster.g> (raw)
In-Reply-To: <> (Calvin Wan's message of "Mon, 22 Apr 2024 16:26:17 +0000")

Calvin Wan <> writes:

Thanks for writing this down.

> The first idea involves turning `strbuf_grow()` into a wrapper function
> that invokes its equivalent library function, eg.
> `libgit_strbuf_grow()`:
> int libgit_strbuf_grow(struct strbuf *sb, size_t extra)
> {
> 	int new_buf = !sb->alloc;
> 	if (unsigned_add_overflows(extra, 1) ||
> 	    unsigned_add_overflows(sb->len, extra + 1))
> 		return -1;
> 	if (new_buf)
> 		sb->buf = NULL;
> 	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
> 	if (new_buf)
> 		sb->buf[0] = '\0';
>         return 0;
> }
> void strbuf_grow(struct strbuf *sb, size_t extra)
> {
>         if (libgit_strbuf_grow(sb, extra))
>                 die("you want to use way too much memory");
> }
> (Note a context object could also be added as a parameter to
> `libgit_strbuf_grow()` for error messages and possibly global variables.)
> In this scenario, we would be exposing `libgit_strbuf_grow()` to
> external consumers of the library, while not having to refactor internal
> uses of `strbuf_grow()`.

Yes, this is how I envision the things will go.  And it is a good
place to STOP for "git" the command line program everybody knows for
the past 20 years.  It is good that you mentioned the "context"
here, too.  As to the naming, I am OK with "libgit_".  Just wanted
to say that "libgit2" does not seem to use it as their prefix (and
they do not use "libgit2_" as their prefix, either) so if somebody
wants to bikeshed, they do not have to worry about them.

> This method would reduce initial churn within
> the codebase, however, we would want to eventually get rid of
> `strbuf_grow()` and use `libgit_strbuf_grow()` internally as well. 

The "however" and everything that follows wants justification.

I think after we pass the "Traditional API git offered are thin
wrappers around git-std-lib" point, the returns (the "clean-up"
value) will quickly diminish.  Even with diminished returns, there
may still be "clean-up" value left, but it would be simpler to
consider that as outside the scope of "libification" discussion.
Once the "Traditional API are thin wrappers" state is achieved, the
"libification" is done.

> The second idea removes the need for two different functions by removing
> the wrapper function and instead refactoring all callers of
> `strbuf_grow()` (and subsequently callers of other library functions).
> ...
> One shortcoming of this approach is the need to refactor all callers of
> library functions, but that can be handled better and the churn made

There is one huge downside you did not mention.

The appraoch makes "git the command line program" to commit to the
"errors must percolate all the way up to the top", and the
addditional effort to take us there after we have achieved the
libification goals has dubious trade-off.

I do not see why it bothers you, the libification person when he
wears git-std-lib hat, that "git the command line program" that is a
mere one of customers of git-std-lib happens to have a function
whose name is strbuf_grow() and uses libgit_strbuf_grow().  It
shouldn't bother you more than "git the command line program" having
a program called cmd_show() that uses many functions from libgit_
suite.  After we reach the "our implementation of traditional API
are all wrappers around git-std-lib API functions" [*] state, we may
decide to refactor further and may replace those "wrappers" with
direct calls, but that can happen far in the future, when the
libified result is rock solid.

If you try to do everything from the top to bottom at once, you
cannot easily keep the system (the combination of git-std-lib still
work-in-progress plus the client code that is "git the command line
program") as solid as the first approach.

  reply	other threads:[~2024-04-22 20:28 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-02 14:18 [RFD] Libification proposal: separate internal and external interfaces Calvin Wan
2024-04-07 21:33 ` brian m. carlson
2024-04-07 21:48   ` rsbecker
2024-04-08  1:09     ` brian m. carlson
2024-04-08 11:07       ` rsbecker
2024-04-08 21:29       ` Junio C Hamano
2024-04-09  0:35         ` brian m. carlson
2024-04-09 17:26           ` Calvin Wan
2024-04-09  9:40         ` Phillip Wood
2024-04-09 17:30           ` Calvin Wan
2024-04-22 16:26 ` Calvin Wan
2024-04-22 20:28   ` Junio C Hamano [this message]
2024-04-23  9:57   ` phillip.wood123
2024-05-09  1:00   ` Kyle Lippincott
2024-05-10  9:52     ` Phillip Wood
2024-05-10 21:35       ` Kyle Lippincott
2024-05-09 19:45   ` Kyle Lippincott
2024-05-09 20:14     ` Junio C Hamano

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqpluhuoos.fsf@gitster.g \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).