Re: [RFC PATCH 7/8] git-std-lib: introduce git standard library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Calvin

On 28/06/2023 22:15, Calvin Wan wrote:
On 27/06/2023 20:52, Calvin Wan wrote:
The Git Standard Library intends to serve as the foundational library
and root dependency that other libraries in Git will be built off of.
That is to say, suppose we have libraries X and Y; a user that wants to
use X and Y would need to include X, Y, and this Git Standard Library.

I think having a library of commonly used functions and structures is a
good idea. While I appreciate that we don't want to include everything
I'm surprised to see it does not include things like "hashmap.c" and
"string-list.c" that will be required by the config library as well as
other code in "libgit.a". I don't think we want "libgitconfig.a" and
"libgit.a" to both contain a copy of "hashmap.o" and "string-list.o"

I chose not to include hashmap and string-list in git-std-lib.a in the
first pass since they can exist as libraries built on top of
git-std-lib.a. There is no harm starting off with more libraries than
fewer besides having something like the config library be dependent on
lib-hashmap.a, lib-string-list.a, and git-std-lib.a rather than only
git-std-lib.a. They can always be added into git-std-lib.a in the
future. That being said, I do find it extremely unlikely that someone
would want to swap out the implementation for hashmap or string-list
so it is also very reasonable to include them into git-std-lib.a

Finding the right boundary for git-std-lib is a bit of a judgement call. We certainly could have separate libraries for things like hashmap, string-list, strvec, strmap and wildmatch but there is some overhead adding each one to the Makefile. I think their use is common enough that it would be continent to have them in git-std-lib but we can always add them later.

diff --git a/Makefile b/Makefile
index e9ad9f9ef1..255bd10b82 100644
--- a/Makefile
+++ b/Makefile
@@ -2162,6 +2162,11 @@ ifdef FSMONITOR_OS_SETTINGS
       COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o
   endif

+ifdef GIT_STD_LIB
+     BASIC_CFLAGS += -DGIT_STD_LIB
+     BASIC_CFLAGS += -DNO_GETTEXT

I can see other projects may want to build git-std-lib without gettext
support but if we're going to use git-std-lib within git it needs to be
able to be built with that support. The same goes for the trace
functions that you are redefining in usage.h

Taking a closer look at gettext.[ch], I believe I can also include it
into git-std-lib.a with a couple of minor changes.

That's great

I'm currently
thinking about how the trace functions should interact with
git-std-lib.a since Victoria had similar comments on patch 1. I'll
reply to that thread when I come up with an answer.

One thought I had was to have a compile time flag so someone building git-std-lib for an external project could build it with

	make git-std-lib NO_TRACE2=YesPlease

and then we'd either compile against a stub version of trace2 that does nothing or use some #define magic to get rid of the calls if that is not too invasive.

Best Wishes

Phillip




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux