Re: [PATCH 00/14] Make git-pull a builtin
- From
Paul Tan <pyokagan@gmail.com>
- Date
- May 30, 2015, 07:29 UTC
- Message-ID
- <CACRoPnThv97e+f3ymuRCFB_u7_dbcOUcCtxUh6BmwDcUZMPjmA@mail.gmail.com>
- In-Reply-To
- <xmqqh9r9bsji.fsf@gitster.dls.corp.google.com>
Hi,
On Tue, May 19, 2015 at 3:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 38 quoted lines
> Paul Tan <pyokagan@gmail.com> writes:
>
>> This series rewrites git-pull.sh into a C builtin, thus improving its
>> performance and portability. It is part of my GSoC project to rewrite git-pull
>> and git-am into builtins[2].
>
> Earlier you were worried about 'git pull' being used in many tests
> for the purpose of testing other parts of the system and not testing
> 'pull' itself. For a program as complex as 'git pull', you may want
> to take the 'competing implementations' approach.
>
> (1) write an empty cmd_pull() that relaunches "git pull" scripted
> porcelain from the $GIT_EXEC_PATH with given parameters, and
> wire all the necessary bits to git.c.
>
> (2) enhance cmd_pull() a bit by bit, but keep something like this
>
> if (getenv(GIT_USE_BUILTIN_PULL)) {
> /* original run_command("git-pull.sh") code */
> exit $?
> }
>
> ... your "C" version ...
>
> (3) add "GIT_USE_BUILTIN_PULL=Yes; export GIT_USE_BUILTIN_PULL" at
> the beginning of "t55??" test scripts (but not others that rely
> on working pull and that are not interested in catching bugs in
> pull).
>
> (4) once cmd_pull() becomes fully operational, drop (3) and also the
> conditional one you added in (2), and retire the environment
> variable. Retire the git-pull.sh script to contrib/examples/
> boneyard.
>
> That way, you will always have a reference you can use throughout
> the development.
>
> Just a suggestion, not a requirement.Okay, I'm trying this out in the next re-roll. I do agree that this patch series should not touch anything in t/ at all.
One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2 targets to ./git-pull (they will clobber each other). For GNU Make, the last defined target will win, so in this case it just happens that git-pull.sh will win because the build targets for the shell scripts are defined after the build targets for the builtins, so this works in our favor I guess.
Regards, Paul