Re: [PATCH] build: get rid of the notion of a git library

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 9, 2013, 16:22 UTC
Message-ID: <CAMP44s0Zsejk4Ex6QfzOFOom3cyWv_hziWGkAK-LawSUkT9V3Q@mail.gmail.com>
In-Reply-To: <20130609160225.GB22905@serenity.lan>

On Sun, Jun 9, 2013 at 11:02 AM, John Keeping <john@keeping.me.uk> wrote:

Show 38 quoted lines

> On Sun, Jun 09, 2013 at 10:40:32AM -0500, Felipe Contreras wrote:
>> On Sun, Jun 9, 2013 at 10:12 AM, John Keeping <john@keeping.me.uk> wrote:
>> > On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:
>> >> Felipe Contreras wrote:
>> >> > The plan is simple; make libgit.a a proper library, starting by
>> >> > clarifying what goes into libgit.a, and what doesn't. If there's any
>> >> > hopes of ever having a public library, it's clear what code doesn't
>> >> > belong in libgit.a; code that is meant for builtins, that code belongs
>> >> > in builtins/lib.a, or similar.
>> >> >
>> >> > Give this a try:
>> >> >
>> >> > --- a/sequencer.c
>> >> > +++ b/sequencer.c
>> >> >
>> >> > libgit.a(sequencer.o): In function `copy_notes':
>> >> > /home/felipec/dev/git/sequencer.c:110: undefined reference to
>> >> > `init_copy_notes_for_rewrite'
>> >> > /home/felipec/dev/git/sequencer.c:114: undefined reference to
>> >> > `finish_copy_notes_for_rewrite'
>> >>
>> >> This is a good example: yes, I'm convinced that the code does need to
>> >> be reorganized.  Please resend your {sequencer.c ->
>> >> builtin/sequencer.c} patch with this example as the rationale, and
>> >> let's work towards improving libgit.a.
>> >
>> > Why should sequencer.c move into builtin/ to solve this?  Why not pull
>> > init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into
>> > notes.c?
>>
>> Because finish_copy_notes_for_rewrite is only useful for builtin
>> commands, so it belongs in builtin/. If there's any meaning to the
>> ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just
>> squash all objects into libgit.a and be done with it.
>
> How is it only useful for builtin commands?  By that logic everything
> belongs in builtin/ because it's all only used by builtin commands (I
> realise that is what you're arguing towards).

Which is precisely the point of this patch. If everything is for builtin commands, then we don't have a git library, and git.a should contain everything under builtin/*.o.

> But we make a distinction between things that are specific to one
> command (especially argument parsing and user interaction) and more
> generally useful features.

No, we don't. Everything under ./*.o goes to libgit.a, and everything under ./builtin/*.o goes to 'git'. So builtin/commit.o can access code from builtin/notes.o, but sequencer.o can't.

-- 
Felipe Contreras