Subject: Re: libgit2 - a true git library

From: Pierre Habouzit <[email-masked]>
Date: Fri, 31 Oct 2008 21:12:35 +0100

On Fri, Oct 31, 2008 at 07:57:11PM +0000, Shawn O. Pearce wrote:
> This "fixes" the "we leak everything" behavior and allows
> applications to have multiple pools on different threads if
> it needs to.  Thus the core git_revp_t isn't thread-safe and
> doesn't get weighed down by locking if the application wants
> multiple threads.

Note that on Linux (and also BSDs I think) many pthread locking
functions have stubs in the glibc and cost 0 until you use the
libpthread. So unless we need to use locking inside the tight-loop, it's
virtually free to write a thread-safe library (a function call is
basically 10 times cheaper than an xchg-based lock).

(Though for git it would suck as we get libpthread in our depends
through some of the other dependencies.

Another way is to use function pointers for the locking, and have a
function to make the object store thread safe by setting the pointers to
functions actually doing locking, and to let it point to functions doing
nothing else.

It looks unrealistic to me to let people deal with the locking,
especially if we mean this library to _also_ be used in language
bindings, hence used in sloppily written scripts.

But of course, if locking calls are used in the tight loop that would
rather suck :/

> Pierre Habouzit <[email-masked]> wrote:
> > 
> > Well, I propose the following: we set-up on GNU-ld + gcc enabled systems
> > all what is needed to use symbol visibility, which isn't that intrusive,
> > and also rather easy given your GIT_EXPORT macro definition.
> 
> Yes, agreed.  Only I don't know how to do it myself.  I know its
> possible, so if someone wants to contribute a patch for this ... :-)

I will, basically, you need to build everything with -fvisibilty=hidden
in your CFLAGS, and mark the prototypes of symbols you want to export
with __attribute__((visibility("default"))) (that you can set into your
EXPORT_GIT macro when you're building with __GNUC__).

You don't even need a linker script (unless we're going to do some
symbol versioning but I'm unsure whether it's that useful for now).

> > No, my worry was rather wrt git core itself, I really think we _must_
> > make it link against libgit2 if we want libgit2 to stay current, but git
> > core will _very likely_ need the private stuff, and it _will_ be a
> > problem. I mean we cannot seriously so-name a library and show its guts
> > at the same time, and I'm unsure how to fix that problem. _that_ was my
> > actual question.
> 
> Hmmph.  I agree git-core needs to link to libgit2.
> 
> I disagree it needs private bits.  If we do the library right
> git-core can use the public API.  And where it cannot its either
> not something that is "right" for libgit2

> (e.g. its parseopts and
> we aren't committed yet to including option parsing)

Sure, I didn't mean we have to put it in libgit2, it's highly UI related
and other tool may want to use something else, and it makes no sense for
many languages that have their library already (python, perl do e.g.).
 
> or its highly
> experimental and we shouldn't put it into the library (and thus
> also git-core) until its more frozen.
> 
> That said, I don't think its criminal to have git-core include and
> link to a static libgit2, especially if git-core's usage of the
> library is ahead of what the library itself is able to expose at
> the present time.

Okay, we'll see how that turns out to work then :)

> Off the top of my head some really important ones:
> 
> 	diff-delta.c
> 	object.c
> 	patch-delta.c
> 	refs.c
> 	revision.c
> 	sha1_file.c
> 	sha1_name.c
> 
> They form a pretty large part of the guts of what most people want
> from a git library.
> 
> Slightly less important, but still fairly core:
> 
> 	builtin-fetch-pack.c
> 	builtin-send-pack.c
> 	connect.c
> 	remote.c
> 	transport.c
> 
> Is most of the client side of the git:// transport, something people want.

Okay I'll let people mention what they would like to see too, and I'll
work from that then.
-- 
·O·  Pierre Habouzit
··O                                                [email-masked]
OOO                                                http://www.madism.org