git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Junio C HamanoNext: Paul Tan
Message 41 of 43 in “Make git-pull a builtin”
  1. 00/14 Make git-pull a builtinPaul Tan, May 18, 2015
  2. 01/14 pull: implement fetch + mergePaul Tan, May 18, 2015
  3. Johannes SchindelinMay 18, 2015
  4. 02/14 pull: pass verbosity, --progress flags to fetch and mergePaul Tan, May 18, 2015
  5. Johannes SchindelinMay 18, 2015
  6. Paul TanMay 21, 2015
  7. Johannes SchindelinMay 21, 2015
  8. Paul TanMay 22, 2015
  9. 03/14 pull: pass git-merge's options to git-mergePaul Tan, May 18, 2015
  10. 04/14 pull: pass git-fetch's options to git-fetchPaul Tan, May 18, 2015
  11. 05/14 pull: error on no merge candidatesPaul Tan, May 18, 2015
  12. Johannes SchindelinMay 18, 2015
  13. 06/14 pull: support pull.ff configPaul Tan, May 18, 2015
  14. Johannes SchindelinMay 18, 2015
  15. Paul TanMay 21, 2015
  16. 07/14 pull: check if in unresolved merge statePaul Tan, May 18, 2015
  17. Johannes SchindelinMay 18, 2015
  18. 08/14 pull: fast-forward working tree if head is updatedPaul Tan, May 18, 2015
  19. Johannes SchindelinMay 18, 2015
  20. 09/14 pull: implement pulling into an unborn branchPaul Tan, May 18, 2015
  21. 10/14 pull: set reflog messagePaul Tan, May 18, 2015
  22. Johannes SchindelinMay 18, 2015
  23. Junio C HamanoMay 18, 2015
  24. Paul TanMay 21, 2015
  25. 11/14 pull: teach git pull about --rebasePaul Tan, May 18, 2015
  26. Stefan BellerMay 18, 2015
  27. Johannes SchindelinMay 19, 2015
  28. Paul TanMay 31, 2015
  29. Paul TanJun 2, 2015
  30. 12/14 pull: configure --rebase via branch.<name>.rebase or pull.rebasePaul Tan, May 18, 2015
  31. Stefan BellerMay 18, 2015
  32. 13/14 pull --rebase: exit early when the working directory is dirtyPaul Tan, May 18, 2015
  33. 14/14 pull --rebase: error on no merge candidate casesPaul Tan, May 18, 2015
  34. Stefan BellerMay 19, 2015
  35. Johannes SchindelinMay 19, 2015
  36. Junio C HamanoMay 19, 2015
  37. Paul TanMay 22, 2015
  38. Johannes SchindelinMay 22, 2015
  39. Stefan BellerMay 22, 2015
  40. Junio C HamanoMay 18, 2015
  41. Paul TanMay 30, 2015
  42. Paul TanMay 30, 2015
  43. Johannes SchindelinMay 18, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.