Re: [RFD] Libification proposal: separate internal and external interfaces
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 22, 2024, 20:28 UTC
- Message-ID
- <xmqqpluhuoos.fsf@gitster.g>
- In-Reply-To
- <20240422162617.308366-1-calvinwan@google.com>
Calvin Wan <calvinwan@google.com> writes:
Thanks for writing this down.
Show 30 quoted lines
> 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.
Show 6 quoted lines
> 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.