On Sun, Jun 9, 2013 at 11:02 AM, John Keeping <john@xxxxxxxxxxxxx> wrote: > 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@xxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html