On Mon, Jun 10, 2013 at 8:53 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> And I do not see the reason why builtin/*.o should not depend on >> each other. It is not messed up at all. They are meant to be >> linked into a single binary---that is what being "built-in" is. >> >> A good way forward, the way it *SHOULD* be, is to slim the builtin/*.o >> by moving parts that do not have to be in the single "git" binary >> but are also usable in standalone binaries out of them. > > Actually, as long as these pieces are currently used by builtins, > moving them (e.g. init_copy_notes_for_rewrite()) out of builtin/*.o > will not make these parts not to be in the single "git" binary at > all, so the above is grossly misstated. > > - There may be pieces of usefully reusable code buried in > builtin/*.o; > > - By definition, any code (piece of data or function definition) in > builtin/*.o cannot be used in standalone binaries, because all of > builtin/*.o expect to link with git.o and expect their cmd_foo() > getting called from main in it; > > - By moving the useful reusable pieces ont of builtin/*.o and > adding them to libgit.a, these pieces become usable from > standalone binaries as well. What if these reusable pieces should not be used by standalone binaries? > And that is the reason why slimming builtin/*.o is the way it > *SHOULD* be. > > Another thing to think about is looking at pieces of data and > functions defined in each *.o files and moving things around within > them. For example, looking at the dependency chain I quoted earlier > for sequencer.o to build upload-pack, which is about responding to > "git fetch" on the sending side: > > upload-pack.c wants handle_revision_opt etc. > revision.c provides handle_revision_opt > wants name_decoration etc. > log-tree.c provides name_decoration > wants append_signoff > sequencer.c provides append_signoff > > It is already crazy. There is no reason for the pack sender to be > linking with the sequencer interpreter machinery. If the function > definition (and possibly other ones) are split into separate source > files (still in libgit.a), git-upload-pack binary does not have to > pull in the whole sequencer.c at all. Agreed, which is precisely why my patches move that code out of sequencer.c. Maybe log-tree.c is not the right destination, but it is a step in the right direction. > Coming back to the categorization Peff earlier made in the thread, I > think I am OK with adding new two subdirectories to the root level, > i.e. > > builtin/ - the ones that define cmd_foo() As is the case right now. > commands/ - the ones that has main() for standalone commands Good. > libsrc/ - the ones that go in libgit.a lib/ is probably descriptive enough. But this doesn't answer the question; what about code that is shared between builtins, but cannot be used by standalone programs? I'd wager it belongs to builtin/ and should be linked to builtin/lib.a. Maybe you would like to have a separate builtin/lib/ directory, but I think that's overkill. > We may also want to add another subdirectory to hold scripted > Porcelains, but the primary topic of this thread is what to do about > the C library, so it is orthogonal in that sense, but if we were to > go in the "group things in subdirectories to slim the root level" > direction, it may be worth considering doing so at the same time. Agreed. Plus there's completions, shell prompt, and other script-like tools that shouldn't really belong in contrib/, and probably installed by default. -- 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