All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Paul Tan <pyokagan@gmail.com>
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Stephen Robin <stephen.robin@gmail.com>
Subject: Re: [PATCH 00/14] Make git-pull a builtin
Date: Mon, 18 May 2015 12:21:37 -0700	[thread overview]
Message-ID: <xmqqh9r9bsji.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1431961571-20370-1-git-send-email-pyokagan@gmail.com> (Paul Tan's message of "Mon, 18 May 2015 23:05:57 +0800")

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.

  parent reply	other threads:[~2015-05-18 19:21 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-18 15:05 [PATCH 00/14] Make git-pull a builtin Paul Tan
2015-05-18 15:05 ` [PATCH 01/14] pull: implement fetch + merge Paul Tan
2015-05-18 15:55   ` Johannes Schindelin
2015-05-18 15:05 ` [PATCH 02/14] pull: pass verbosity, --progress flags to fetch and merge Paul Tan
2015-05-18 17:41   ` Johannes Schindelin
2015-05-21  9:48     ` Paul Tan
2015-05-21 15:59       ` Johannes Schindelin
2015-05-22 13:38         ` Paul Tan
2015-05-18 15:06 ` [PATCH 03/14] pull: pass git-merge's options to git-merge Paul Tan
2015-05-18 15:06 ` [PATCH 04/14] pull: pass git-fetch's options to git-fetch Paul Tan
2015-05-18 15:06 ` [PATCH 05/14] pull: error on no merge candidates Paul Tan
2015-05-18 18:56   ` Johannes Schindelin
2015-05-18 15:06 ` [PATCH 06/14] pull: support pull.ff config Paul Tan
2015-05-18 19:02   ` Johannes Schindelin
2015-05-21  9:53     ` Paul Tan
2015-05-18 15:06 ` [PATCH 07/14] pull: check if in unresolved merge state Paul Tan
2015-05-18 19:06   ` Johannes Schindelin
2015-05-18 15:06 ` [PATCH 08/14] pull: fast-forward working tree if head is updated Paul Tan
2015-05-18 19:18   ` Johannes Schindelin
2015-05-18 15:06 ` [PATCH 09/14] pull: implement pulling into an unborn branch Paul Tan
2015-05-18 15:06 ` [PATCH 10/14] pull: set reflog message Paul Tan
2015-05-18 19:27   ` Johannes Schindelin
2015-05-18 21:53     ` Junio C Hamano
2015-05-21 10:08       ` Paul Tan
2015-05-18 15:06 ` [PATCH 11/14] pull: teach git pull about --rebase Paul Tan
2015-05-18 23:36   ` Stefan Beller
2015-05-19 13:04   ` Johannes Schindelin
2015-05-31  8:18     ` Paul Tan
2015-06-02 11:26       ` Paul Tan
2015-05-18 15:06 ` [PATCH 12/14] pull: configure --rebase via branch.<name>.rebase or pull.rebase Paul Tan
2015-05-18 23:58   ` Stefan Beller
2015-05-18 15:06 ` [PATCH 13/14] pull --rebase: exit early when the working directory is dirty Paul Tan
2015-05-18 15:06 ` [PATCH 14/14] pull --rebase: error on no merge candidate cases Paul Tan
2015-05-19  0:12   ` Stefan Beller
2015-05-19 13:10     ` Johannes Schindelin
2015-05-19 16:27       ` Junio C Hamano
2015-05-22 13:48         ` Paul Tan
2015-05-22 14:14           ` Johannes Schindelin
2015-05-22 17:12             ` Stefan Beller
2015-05-18 19:21 ` Junio C Hamano [this message]
2015-05-30  7:29   ` [PATCH 00/14] Make git-pull a builtin Paul Tan
2015-05-30  8:00     ` Paul Tan
2015-05-18 19:41 ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh9r9bsji.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=pyokagan@gmail.com \
    --cc=sbeller@google.com \
    --cc=stephen.robin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.