Re: [PATCH 2/2] Move sequencer to builtin

From: Duy Nguyen <pclouds@gmail.com>
Date: Jun 8, 2013, 11:42 UTC
Message-ID: <CACsJy8A-qc0tHcsp5=syxv_7FjixahU7fGcZuUV=cGn_-qyWwg@mail.gmail.com>
In-Reply-To: <CAMP44s29GiGJq3wyXAzJNo0FJY+Vbgd18bpBJMYQ47h-3M6sWA@mail.gmail.com>

On Sat, Jun 8, 2013 at 5:14 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 11 quoted lines

> On Fri, Jun 7, 2013 at 9:35 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sat, Jun 8, 2013 at 5:16 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> This code is only useful for cherry-pick and revert built-ins, nothing
>>> else, so let's make it a builtin object, but make sure 'git-sequencer'
>>> is not generated.
>>
>> As you can see, the convention is builtin/foo.c corresponds to git-foo
>> (and maybe more). Why make an exception for sequencer?
>
> Why not?

And while we are at "why not", why don't you fork git?

Show 5 quoted lines

>> I ask because I moved
>> fetch-pack from builtin out because of linking issues and I don't want
>> the same happen to sequencer.c.
>
> I'm sure those linking issues can be solved.

Yeah, I scratched my head for hours and finally gave in. Maybe you are better at the toolchain than me.

Show 11 quoted lines

> I don't see why libgit.a couldn't eventually be the same as libgit2.
> We need better organization tough (e.g. builtins/lib.a).
>
> If you are arguing favor of a more messy setup, then we should link
> all the builtin/*.o to libgit.a, because the current situation just
> doesn't cut it.
>
> For example, init_copy_notes_for_rewrite() cannot be accessed by
> sequencer.c, and while it's possible to move that function (and
> others) to libgit.a, it doesn't make sense, because it can only be
> used by builtins.

libgit.a is just a way of grouping a bunch of objects together, not a real library and not meant to be. If you aim something more organized, please show at least a roadmap what to move where. -- Duy