Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 12, 2013, 19:18 UTC
Message-ID: <51b8c9816155a_501d1297e8483820@nysa.mail>
In-Reply-To: <CALKQrgfPktWOcUKnWecQcE-wMVwTqMES112nHcqnCrZzLLqOeg@mail.gmail.com>

Johan Herland wrote:

Show 32 quoted lines

> On Wed, Jun 12, 2013 at 8:28 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland <johan@herland.net> wrote:
> >> On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> >>> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland <johan@herland.net> wrote:
> >>>> This is a pure code movement of the machinery for copying notes to
> >>>> rewritten objects. This code was located in builtin/notes.c for
> >>>> historical reasons. In order to make it available to builtin/commit.c
> >>>> it was declared in builtin.h. This was more of an accident of history
> >>>> than a concious design, and we now want to make this machinery more
> >>>> widely available.
> >>>>
> >>>> Hence, this patch moves the code into the new notes-utils.[hc] files
> >>>> which are included into libgit.a. Except for adjusting #includes
> >>>> accordingly, this patch merely moves the relevant functions verbatim
> >>>> into the new files.
> >>>>
> >>>> Cc: Thomas Rast <trast@inf.ethz.ch>
> >>>> Signed-off-by: Johan Herland <johan@herland.net>
> >>>
> >>> I wonder where you got that idea from. Did you come up with that out thin air?
> >>
> >> Obviously not. I should add
> >>
> >> Suggested-by: Junio C Hamano <gitster@pobox.com>
> >
> > You are still not explaining where the idea came from. And you are
> > doing that with the express purpose of annoying.
> 
> Truly, I am not trying to annoy anyone. I have not followed the
> preceding discussion closely, and I wrote the patch based solely on
> one paragraph from Junio's email[1].

Here is another pagraph:

> 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().

Which clearly refers to: http://article.gmane.org/gmane.comp.version-control.git/226752

Show 6 quoted lines

> > Where did the idea come from?
> 
> I got it from Junio. I do not know if I might have accidentally
> plagiarized something you already submitted to the mailing list,
> although I would be surprised if that was the case, since - as far as
> I understand - you are opposed to this solution.

You are aware I opposed this *solution*, yet were not aware that I sent the first patch in this thread, which clearly states the *problem*?

> This way there will not be linking issues when top-level objects try to
> access functions of builtin objects.

http://article.gmane.org/gmane.comp.version-control.git/226845

> Originally-envisioned-by: Felipe Contreras <felipe.contreras@gmail.com>?

Do I have to do it for you? Your commit message is all wrong, because nowhere are you pointing out *why* you are making the change.

--- Move copy_note_for_rewrite + friends to notes-utils.c

In order to make these functionas available to top-level objects (e.g. sequencer.o), we need to move them out of the builtin/ subdirectory.

Reported-by: Felipe Contreras <felipe.contreras@gmail.com>
---

-- 
Felipe Contreras