Subject: Re: libgit2 - a true git library

From: Andreas Ericsson <[email-masked]>
Date: Sat, 01 Nov 2008 20:18:05 +0100

Shawn O. Pearce wrote:
> During the GitTogether we were kicking around the idea of a ground-up
> implementation of a Git library.  This may be easier than trying
> to grind down git.git into a library, as we aren't tied to any
> of the current global state baggage or the current die() based
> error handling.
> 
> I've started an _extremely_ rough draft.  The code compiles into a
> libgit.a but it doesn't even implement what it describes in the API,
> let alone a working Git implementation.  Really what I'm trying to
> incite here is some discussion on what the API looks like.
> 
> API Docs:
> http://www.spearce.org/projects/scm/libgit2/apidocs/html/modules.html
> 
> Source Code Clone URL:
> http://www.spearce.org/projects/scm/libgit2/libgit2.git
> 

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.
  Instead it would be better to have GIT_PRIVATE(), which could
  set visibility to "internal" or "hidden", meaning the symbol it's
  attached to can be used for lookups when creating a shared library
  but won't be usable from programs linking to that shared library
  (visibility-attributes have zero effect on static libraries). At
  least on all archs anyone really cares about.
* Prefixing the files themselves with git_ is useless and only leads
  to developer frustration. I imagine we'd be installing any header
  files in a git/ directory anyway, so we're gaining absolutely
  nothing with the git_ prefix on source-files.

Apart from that, it seems you've been designing a lot rather than
trying to use the API to actually do something. It would, imo, be
a lot better to start development with adding functionality shared
between all programs and then expand further on that, such as
incorporating all functions needed for manipulating tags into the
library and then modify existing code to use the library to get
tag-ish things done. That would also mean that the library would
quickly get used by core git, as once a certain part of it is
complete patches can be fitted to the library rather than to the
current non-libish dying() functions.

I also think it's quite alright to not strive *too* hard to make
all functions thread-safe, as very few of them will actually need
that. It's unlikely that a user program will spawn one thread to
write a lot of tags while another is trying to parse them, for
example.

By adding an init routine that determines the workdir and the
gitdir, one could start using the library straight away.

int git_init(const char *db, const char *worktree)
{
    if (git_set_db_dir(db))
        return -1;
    git_set_worktree((worktree))
        return -1;

    return 0;
}

and already you have a some few small helpers that are nifty to
to have around:
int git_is_gitdir(const char *path);  /* returns 1 on success */
int git_has_gitdir(const char *path); /* returns 1 on success */
const char *git_mkpath(const char *fmt, ...)

This way one will notice rather quickly what's needed (making it
easy to keep a more-or-less public TODO available, with small stuff
on it for the most part), and one can then go look for it in the
existing git code and, if possible, convert stuff or, best case
scenario, steal it straight off so that more apps can benefit from
tried and tested code.

-- 
Andreas Ericsson                   [email-masked]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231