git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Ericsson <ae@op5.se>
To: "Shawn O. Pearce" <spearce@spearce.org>
Cc: git@vger.kernel.org, Scott Chacon <schacon@gmail.com>
Subject: Re: libgit2 - a true git library
Date: Sat, 01 Nov 2008 20:18:05 +0100	[thread overview]
Message-ID: <490CAB6D.90209@op5.se> (raw)
In-Reply-To: <20081031170704.GU14786@spearce.org>

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                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

  parent reply	other threads:[~2008-11-01 19:19 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-31 17:07 libgit2 - a true git library Shawn O. Pearce
2008-10-31 17:28 ` Pieter de Bie
2008-10-31 17:29   ` Pieter de Bie
2008-10-31 17:47 ` Pierre Habouzit
2008-10-31 18:41   ` Shawn O. Pearce
2008-10-31 18:54     ` Pierre Habouzit
2008-10-31 19:57       ` Shawn O. Pearce
2008-10-31 20:12         ` Pierre Habouzit
2008-10-31 20:05     ` Junio C Hamano
2008-10-31 21:58       ` Shawn O. Pearce
2008-11-01 17:30     ` Pierre Habouzit
2008-11-01 18:44       ` Andreas Ericsson
2008-11-01 18:48         ` Pierre Habouzit
2008-11-01 20:29         ` Shawn O. Pearce
2008-11-01 21:58           ` Andreas Ericsson
2008-11-02  1:50             ` Shawn O. Pearce
2008-11-03 10:17               ` Andreas Ericsson
2008-11-02  1:56       ` Shawn O. Pearce
2008-11-02  9:25         ` Pierre Habouzit
2008-10-31 20:24   ` Nicolas Pitre
2008-10-31 20:29     ` david
2008-10-31 20:56       ` Nicolas Pitre
2008-10-31 21:43         ` Shawn O. Pearce
2008-10-31 21:50           ` Shawn O. Pearce
2008-10-31 21:51           ` Pierre Habouzit
2008-10-31 21:31     ` Pierre Habouzit
2008-10-31 22:10       ` Nicolas Pitre
2008-11-01 10:52         ` Andreas Ericsson
2008-10-31 23:24       ` Pieter de Bie
2008-10-31 23:28         ` Shawn O. Pearce
2008-10-31 23:49           ` Junio C Hamano
2008-11-01  0:02             ` Pierre Habouzit
2008-11-01  0:19               ` Shawn O. Pearce
2008-11-01  1:02                 ` Pierre Habouzit
2008-11-01  0:13             ` Shawn O. Pearce
2008-11-01  1:15               ` Nicolas Pitre
2008-11-01  1:19                 ` Shawn O. Pearce
2008-11-01  1:45                   ` Nicolas Pitre
2008-11-01  1:52                     ` Shawn O. Pearce
2008-11-01  2:26                       ` Johannes Schindelin
2008-11-01 11:01                         ` Pierre Habouzit
2008-11-01 13:50                           ` Nicolas Pitre
2008-11-01 17:01                             ` Pierre Habouzit
2008-11-01 20:26                             ` Johannes Schindelin
2008-10-31 23:14     ` Junio C Hamano
2008-10-31 23:33       ` Pierre Habouzit
2008-10-31 23:41       ` Shawn O. Pearce
2008-10-31 23:56         ` Jakub Narebski
2008-11-01  0:41         ` david
2008-11-01  1:00           ` Shawn O. Pearce
2008-11-01  1:04             ` david
2008-11-01  1:08               ` Pierre Habouzit
2008-11-01  1:33                 ` Nicolas Pitre
2008-11-01  1:38                   ` Pierre Habouzit
2008-11-01  1:49                     ` Nicolas Pitre
2008-11-01  1:43                   ` Shawn O. Pearce
2008-11-01  1:53                     ` Nicolas Pitre
2008-11-01 22:57                       ` Shawn O. Pearce
2008-11-02  0:26                         ` Scott Chacon
2008-11-02  1:07                           ` Scott Chacon
2008-11-02  1:36                             ` Shawn O. Pearce
2008-11-02  5:09                             ` David Brown
2008-11-03 16:20                               ` Shawn O. Pearce
2008-11-01  1:06           ` Pierre Habouzit
2008-11-01  1:36             ` david
2008-10-31 20:24   ` Brian Gernhardt
2008-10-31 21:59   ` Andreas Ericsson
2008-10-31 22:01     ` Shawn O. Pearce
2008-10-31 22:51       ` Junio C Hamano
2008-11-01 11:17       ` Andreas Ericsson
2008-10-31 23:22     ` Johannes Schindelin
2008-10-31 23:18 ` Bruno Santos
2008-10-31 23:25   ` Shawn O. Pearce
2008-11-01 19:18 ` Andreas Ericsson [this message]
2008-11-01 20:42   ` Shawn O. Pearce
2008-11-02  2:30     ` Johannes Schindelin
2008-11-02  9:19     ` Pierre Habouzit
2008-11-03 13:08     ` Andreas Ericsson
2008-11-08 13:26 ` Steve Frécinaux
2008-11-08 14:35   ` Andreas Ericsson
2008-11-08 17:27     ` Pierre Habouzit
2008-11-09 10:17       ` Andreas Ericsson
2008-11-09 21:02         ` Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=490CAB6D.90209@op5.se \
    --to=ae@op5.se \
    --cc=git@vger.kernel.org \
    --cc=schacon@gmail.com \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).