git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>, Felipe Contreras <felipe.contreras@gmail.com>
Cc: Vincent van Ravesteijn <vfr@lyx.org>,
	John Keeping <john@keeping.me.uk>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
	Duy Nguyen <pclouds@gmail.com>
Subject: Re: [PATCH] build: get rid of the notion of a git library
Date: Mon, 10 Jun 2013 18:53:35 -0700	[thread overview]
Message-ID: <7vwqq1ct0g.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7v4nd5ecmy.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Mon, 10 Jun 2013 17:04:21 -0700")

Junio C Hamano <gitster@pobox.com> 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.

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.

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()
    commands/   - the ones that has main() for standalone commands
    libsrc/     - the ones that go in libgit.a

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.

  reply	other threads:[~2013-06-11  1:53 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-08 17:29 [PATCH] build: get rid of the notion of a git library Felipe Contreras
2013-06-08 18:02 ` Ramkumar Ramachandra
2013-06-08 18:22   ` Felipe Contreras
2013-06-09 14:56     ` Ramkumar Ramachandra
2013-06-09 15:12       ` John Keeping
2013-06-09 15:40         ` Felipe Contreras
2013-06-09 16:02           ` John Keeping
2013-06-09 16:22             ` Felipe Contreras
2013-06-09 16:42               ` John Keeping
2013-06-09 17:03                 ` Ramkumar Ramachandra
2013-06-09 17:12                   ` Ramkumar Ramachandra
2013-06-09 17:13                   ` Felipe Contreras
2013-06-09 17:32                     ` John Keeping
2013-06-09 17:45                       ` Felipe Contreras
2013-06-09 16:36             ` Ramkumar Ramachandra
2013-06-09 17:30           ` Vincent van Ravesteijn
2013-06-09 17:35             ` Felipe Contreras
2013-06-10 21:45             ` Jeff King
2013-06-10 21:52               ` Felipe Contreras
2013-06-10 22:06                 ` Jeff King
2013-06-10 22:22                   ` Felipe Contreras
2013-06-10 23:05                   ` Junio C Hamano
2013-06-10 23:41                     ` Junio C Hamano
2013-06-10 23:51                       ` Felipe Contreras
2013-06-11  0:04                         ` Junio C Hamano
2013-06-11  1:53                           ` Junio C Hamano [this message]
2013-06-11  4:15                             ` Felipe Contreras
2013-06-11 17:33                               ` Junio C Hamano
2013-06-11 17:41                                 ` Felipe Contreras
2013-06-11 17:58                                   ` Junio C Hamano
2013-06-11 18:06                                     ` Felipe Contreras
2013-06-11 18:14                                       ` Linus Torvalds
2013-06-11 19:15                                         ` Felipe Contreras
2013-06-11 19:59                                         ` Junio C Hamano
2013-06-11 20:12                                           ` Felipe Contreras
2013-06-12  0:12                                           ` [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch] Johan Herland
2013-06-12  0:12                                             ` [PATCH 1/3] finish_copy_notes_for_rewrite(): Let caller provide commit message Johan Herland
2013-06-12 17:27                                               ` Junio C Hamano
2013-06-12  0:13                                             ` [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c Johan Herland
2013-06-12  0:32                                               ` Felipe Contreras
2013-06-12  7:10                                                 ` Johan Herland
2013-06-12 18:28                                                   ` Felipe Contreras
2013-06-12 19:14                                                     ` Johan Herland
2013-06-12 19:18                                                       ` Felipe Contreras
2013-06-13  6:45                                                     ` Andreas Krey
2013-06-13 13:13                                                       ` Felipe Contreras
2013-06-12 20:02                                               ` Junio C Hamano
2013-06-12  0:13                                             ` [PATCH 3/3] Move create_notes_commit() from notes-merge.c into notes-utils.c Johan Herland
2013-06-12 20:02                                             ` [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch] Junio C Hamano
2013-06-12 20:11                                               ` Felipe Contreras
2013-06-13 17:24                                                 ` Junio C Hamano
2013-06-13 18:16                                                   ` Felipe Contreras
2013-06-13 18:50                                                     ` Felipe Contreras
2013-06-11 18:17                                       ` [PATCH] build: get rid of the notion of a git library Junio C Hamano
2013-06-11 19:01                                         ` Felipe Contreras
2013-06-11 19:24                                           ` Junio C Hamano
2013-06-11 19:49                                             ` Felipe Contreras
2013-06-11  4:04                           ` Felipe Contreras
2013-06-09 15:41       ` Felipe Contreras

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=7vwqq1ct0g.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=artagnon@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=john@keeping.me.uk \
    --cc=jrnieder@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=vfr@lyx.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).