Re: libgit2 - a true git library

From: Pierre Habouzit <madcoder@debian.org>
Date: Nov 2, 2008, 09:19 UTC
Message-ID: <20081102091902.GB4066@artemis>
In-Reply-To: <20081101204259.GC15463@spearce.org>

On Sat, Nov 01, 2008 at 08:42:59PM +0000, Shawn O. Pearce wrote:

Show 26 quoted lines

> Andreas Ericsson <ae@op5.se> wrote:
> > Shawn O. Pearce wrote:
> >> During the GitTogether we were kicking around the idea of a ground-up
> >> implementation of a Git library.
> >
> > Having looked briefly at the code, I've got a couple of comments:
> > * GIT_EXTERN() does nothing. Ever. It's noise and should be removed.
> 
> I feel the same way.
> 
> But I was also under the impression that the brilliant engineers
> who work for Microsoft decided that on their platform special
> annotations have to be inserted on functions that a DLL wants to
> export to applications.
> 
> Hence any cross-platform library that I have seen annotates their
> exported functions this way, with the macro being empty on POSIX
> and expanding to some magic keyword on Microsoft's OS.  I think it
> goes between the return type and the function name too...
> 
> >  Instead it would be better to have GIT_PRIVATE(),
> 
> I can see why you said this; needing GIT_PRIVATE() is a lot more
> rare than needing GIT_EXTERN().  Only a handful of cross-module,
> but private, functions are likely to exist, so it makes sense to
> mark the smaller subset.  But see above.  *sigh*

Not only there is the windows thing, but the *best* way to design a library is to make _everything_ by default and only show what you mean to.

GIT_EXTERN allow us to do just that with __attribute__((visibility("default"))) on GNU-ld/GCC.

Of course we can achieve the same using nothing on public symbols and __attribute__((visibility("hidden"))) on the private ones, but it's way more cumbersome and there's always a chance to forget one. It's bad design IMHO.

FWIW GIT_EXTERN(...) prototype(arguments); isn't unredable to me, everyone does that, it doesn't break tags, it's _good_. And you can ask doxygen to substitute this GIT_EXTERN(x) with x so that it doesn't shows up in the documentation at all (it can include these kind of partially preprocessed headers in the documentation) for the people who are really annoyed with them.

But it's definitely a necessary evil.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org