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: Mon, 03 Nov 2008 14:08:16 +0100	[thread overview]
Message-ID: <490EF7C0.3000909@op5.se> (raw)
In-Reply-To: <20081101204259.GC15463@spearce.org>

Shawn O. Pearce wrote:
> 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*
> 

Thanks for the detailed explanation.

>> * 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.
> 
> Yes, I realized that this morning.  I plan on changing that mess
> around so we have "include/git/oid.h" and library and application
> code can use "#include <git/oid.h>".  Library modules should just
> be "src/oid.c" then.
> 

I noticed when I fetched the latest head today that it's already
done. I fail to understand why headers need to be in a separate
path so that oid.c can't just '#include "oid.h"'.

With the risk of nitpicking you to death, put public headers in a
separate dir (I'd suggest public/%.h in Make-speak, but I have no
strong preference) and keep private headers next to %.c. Always
#include the public header file from the private one (that should
probably be in CONVENTIONS).

>> Apart from that, it seems you've been designing a lot rather than
>> trying to use the API to actually do something.
> 
> I wanted to get a solid idea of what our API conventions should be,
> before we started writing a lot of code around them.  Part of the
> problem with the git.git code is we don't have conventions that are
> really suited for use in a shared library (assuming we even have
> conventions in there) so we can't use that code as a library today.
> 

Right. I guess I'm too firm a believer in system evolution by constant
refactoring (with fluctuating api's, yes) rather than thinking initial
design can ever be done exactly right.

>> 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.
> 
> Tags are mostly pointless.  Its a tiny part of the code that isn't
> that interesting to most people.  And it requires object database
> access anyway if you want to talk about parsing or reading a tag.
> There's almost no point in a git library that can't read the on
> disk object database, or write to it.
> 

True, but designing top-down means you'll need to write one more
API to get the first stuff working, so you'll always be using the
new code you write immediately and for something real. IMO, that
makes it much more fun and productive to write the lib itself.

>> 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.
> 
> Oh really?
> 
> Maybe true for tags, just because they are such an unimportant part
> of the git suite compared to everything else.
> 
> But right now I'm running a production system using a threaded server
> process that is operating on Git repositories.  Fortunately threads
> suck less on Java than they do on POSIX, and we have a 100% pure
> Java library available for Git.
> 
> It would be nice if a library created in the late part of 2008
> recognized that threads exist, aren't going to disappear tomorrow,
> and that consumers of libraries actually may need to run the library
> within a threaded process.
> 
> Or are you one of those developers who think threads only exist
> in the giant monolithic kernel land, and all user space should
> be isolated process?  I often wonder who such people can justify
> the kernel address space being multi-threaded but userland being
> stuck to single threaded applications.  Oh, right, the kernel has
> to go fast...
> 

No, I'm one of those developers who think that if implementing a
function as thread-safe means it'll take 50 times longer than just
writing something that works, the right decision is to go with the
faster way to get the job done and then expand on it later when the
need arises. Reading my original post, I realize I should have made
that more clear. Sorry for making your gall rise unnecessarily.

-- 
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-03 13:09 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
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 [this message]
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=490EF7C0.3000909@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).