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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 8, 2013, 18:22 UTC
Message-ID: <CAMP44s0cozMsTo7KQAjnqkqmvMwMw9D3SZrVxg48MOXkH9UQJQ@mail.gmail.com>
In-Reply-To: <CALkWK0mA7MXQv1k5bFpZLARDOHxU5kzKFXzcyUfb6NLZZY-=FA@mail.gmail.com>

On Sat, Jun 8, 2013 at 1:02 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 6 quoted lines

> Felipe Contreras wrote:
>> There's no libgit, and there will never be, every object file in Git is
>> the same, and there's wish to organize them in any way; they are *all*
>> for the 'git' binary and its builtin commands.
>
> Nice joke patch to illustrate your point ;)

It's not a joke. This is seriously the direction the others say is the correct one.

One direction or the other, the problem that top-level objects can't access code from builtin objects must be fixed. And if the others don't want to fix it by properly splitting code between library-like objects, and builtin objects, there's only one other way to fix it; this way.

> On a more serious note, please be a little more patient while everyone
> copes with what you're attempting.

I don't think patience will help. What do you suggest? Wait until the problem fixes itself? (I'll be waiting until the end times). Wait until somebody changed their opinion by themselves? (I don't see that happening).

> I've already made it clear that
> I'm in favor of moving forward with your plan to lib'ify git.

Unfortunately you are the only one.

Show 6 quoted lines

> The
> problem is that you're sending your changes in fragmented comments and
> diffs, and nobody is able to piece together what the big picture is.
>
> Please write one cogent email (preferably with code included)
> explaining your plan.

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.

But to be honest, I don't really care, all I want is the problem of the bogus split to be solved. One way to solve it is going the proper library way, but the other is to stash everything together into git.a. Both ways solve the problem.

Give this a try:

--- a/sequencer.c
+++ b/sequencer.c
@@ -14,6 +14,7 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "builtin.h"

 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"

@@ -102,6 +103,17 @@ static int has_conforming_footer(struct strbuf
*sb, struct strbuf *sob,
        return 1;
 }

+static void copy_notes(const char *name)
+{
+       struct notes_rewrite_cfg *cfg;
+
+       cfg = init_copy_notes_for_rewrite(name);
+       if (!cfg)
+               return;
+
+       finish_copy_notes_for_rewrite(cfg);
+}
+
 static void remove_sequencer_state(void)
 {
        struct strbuf seq_dir = STRBUF_INIT;
@@ -997,6 +1009,8 @@ static int pick_commits(struct commit_list
*todo_list, struct replay_opts *opts)
                        return res;
        }

+       copy_notes("cherry-pick");
+
        /*
         * Sequence of picks finished successfully; cleanup by
         * removing the .git/sequencer directory

What happens?

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'

It is not the first time, nor the last that top-level code needs
builtin code, and the solution is easy; organize the code. Alas, this
simple solution reject on the basis that we shouldn't organize the
code, because the code is not meant to be organized.

Cheers.

-- 
Felipe Contreras