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

From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Felipe Contreras <felipe.contreras@gmail.com>,
	Jeff King <peff@peff.net>, Vincent van Ravesteijn <vfr@lyx.org>,
	John Keeping <john@keeping.me.uk>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	Git Mailing List <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: Tue, 11 Jun 2013 12:59:48 -0700	[thread overview]
Message-ID: <7vehc8a05n.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CA+55aFwYAFuz5p0=8QiAFDy4e66f1pF3v=D5nnL6+3um7Z3L2g@mail.gmail.com> (Linus Torvalds's message of "Tue, 11 Jun 2013 11:14:54 -0700")

Linus Torvalds <torvalds@linux-foundation.org> writes:

> This whole thread has been one long argument about totally pointless
> things that wouldn't improve anything one way or the other. It's
> bikeshedding of the worst kind. Just let it go.

The proposal to move sequencer.c to builtins/sequencer.c and then
adding a filter in Makefile to exclude so that "git-sequencer" is
not built is "it wouldn't improve anything one way or the other".
It is to throw in something into a set to which it does not belong,
and then working around that mistake with another kludge.

The problem that triggered the wrong solution actually is real,
however.

A function that sequencer.c (in libgit.a so that it could be used by
standalone) may want to use in the future currently lives in
builtin/notes.c.  If you add a call to that function to sequencer.c
without doing anything else, standalones like git-upload-pack will
stop linking correctly.  The git-upload-pack wants the revision
traversal machinery in revision.o, which in turn wants to be able to
see log-tree.o, which in turn wants to link with sequencer.o to see
one global variable (there may be other dependencies).  All of these
objects are currently in libgit.a so that both builtins and standalones
can use them.

Moving sequencer.c to builtin/ is not even a solution.  Linking
git-upload-pack will still pull in builtin/notes.o along with
cmd_notes(), which is not called from main(); as you remember,
cmd_foo() in all builtin/*.o are designed to be called from
git.c::main().

There is only one right solution.  If a useful function is buried in
builtin/*.o as a historical accident (i.e. it started its life as a
helper for that particular command, and nobody else used it from
outside so far) and that makes it impossible to use the function
from outside builtin/*.o, refactor the function and its callers and
move it to libgit.a.

So I do not think this is not even a bikeshedding.  Just one side
being right, and the other side continuing to repeat nonsense
without listening.