Re: [PATCH v5 2/3] git-std-lib: introduce Git Standard Library

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 29, 2024, 17:23 UTC
Message-ID: <xmqq1q8vi3pz.fsf@gitster.g>
In-Reply-To: <db8b2398-c6d6-47a1-a4d4-7e4726d13312@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Calvin
> On 22/02/2024 17:50, Calvin Wan wrote:

Thanks for reviewing -- I held mine back as I expected this will see another reroll soonish, but you already have raised many points I also had trouble with, so I do not have to ;-)

Below, I'll liberally omit everything you wrote that I agree with.

Show 6 quoted lines

>> +libgit.a, which is then linked against by common-main.o with other
>> +external dependencies and turned into the Git executable.
>
> I found this description a bit confusing. As I understand it to build
> git we link git.o against common-main.o, libgit.a, xdiff/lib.a,
> reftable/libreftable.a and libpcre etc.

In addition, there is no single "the Git executable", simply because not everything is builtin command. The purpose of using libgit.a is because we are too lazy to list and maintain all the internal dependencies to link final executables like 'git' (which has all the built-in command implementations) and 'git-remote-curl' (which is a standalone program). Instead of feeding exact list of object files to "$(CC) -o git" command line, we throw everything into libgit.a and let the linker pick what is needed. To link "git", we may include all builtin/*.o, git.o, common-main.o, libgit.a and the external [*] library dependencies they have. To link "git-daemon", we may not link builtin/*.o and git.o and link daemon.o instead.

	Side note: here I am counting xdiff/lib.a as an external
	library as it is mostly a borrowed code.

In other words, libgit.a is not a true library in the sense that it was designed to be _used_ as a library. It was merely a detail of how we implemented lazy dependency management in our build process, which happend with 0a02ce72 (Clean up the Makefile a bit., 2005-04-18) whose commit log message uses air-quotes around the word "library".

Show 15 quoted lines

>> +From the above dependency graph, we can see that git-std-lib.a could be
>> +many smaller libraries rather than a singular library. So why choose a
>> +singular library when multiple libraries can be individually easier to
>> +swap and are more modular? A singular library requires less work to
>> +separate out circular dependencies within itself so it becomes a
>> +tradeoff question between work and reward. While there may be a point in
>> +the future where a file like usage.c would want its own library so that
>> +someone can have custom die() or error(), the work required to refactor
>> +out the circular dependencies in some files would be enormous due to
>> +their ubiquity so therefore I believe it is not worth the tradeoff
>
> I'm not sure if we want to use the first person in our technical
> documentation, unlike the cover letter to a patch series it is not
> immediately obvious to the reader who "I" is. This applies the
> passages in the first person below as well.

I found it highly annoying while reading it, too. If the document (not the commit that introduced the document) were signed and written to state the position of one author, as opposed to spell out the position the project will collectively take, it would have been OK, but this document is meant to set a course of the project (and discussion on it is the process to decide which course to take), the first person singular "I" did not sit well for me.

Another thing to consider that I do not think you covered is the name of the resulting .a archive. By starting it with "lib", a customer can find your libstdgit.a with -lstdgit on the command line, once libstdgit.a is installed at an appropriate location (or the build-time library path is configured to point the location you have libstdgit.a at). libgit.a that wasn't really designed to be used as such a library did not have to follow the naming convention, but if the thing being proposed is meant to be eventually used as a library by external entities, git-std-lib.a is a rather poor name for it.

PS.

I seem to have been hit by a power outage and am on UPS, so I'll probably be offline until the power comes back.