On Mon, Jun 10, 2013 at 8:53 PM, Junio C Hamano [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

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