Re: [PATCH 01/14] pull: implement fetch + merge

Hi Paul,

On 2015-05-18 17:05, Paul Tan wrote:

Start off the rewrite of git-pull.sh to C by implementing a simple
git-pull builtin that:

1. Parses the command line arguments into [<repo> [<refspecs>...]]

2. Runs git-fetch, passing the repo and refspecs to it.

3. Runs git-merge FETCH_HEAD.

Due to missing features, several tests are disabled:

* Passing flags to git fetch, rebase, merge:

  * t5521: verbosity flags -v, -s

* Passing flags to git-merge, git-rebase:

  * t5150, t5572: Pass --ff, --no-ff, --ff-only to git-merge

  * t5520, t5521, t7406: Support --rebase, branch.*.rebase and
    pull.rebase config

  * t6029, t4013: -s/--strategy

  * t6037: -X/--strategy-option

  * t5524: --log

* Passing flags to git-fetch:

  * t5521: --dry-run

  * t5500: --depth

  * t5521: --force

  * t5521: --all

  * t7403: --no-recurse-submodules

* t7601: pull.ff config sets --ff, --no-ff, --ff-only

* t5520: branch.*.rebase or pull.rebase sets --rebase, --rebase=preserve

* t5520: Pulling into void

* t5520: appropriate user-friendly error messages on no merge candidates
  case

* t5520: appropriate user-friendly error messages if index has
  unresolved entries.

* t5520: fast-forward working tree if branch head is updated by
  git-fetch

* t5520: set reflog action to "pull args..."

These features will be re-implemented in succeeding patches.

Signed-off-by: Paul Tan <redacted>
---
 Makefile                     |   2 +-
 builtin.h                    |   1 +
 builtin/pull.c               |  83 +++++++++++
 contrib/examples/git-pull.sh | 339 +++++++++++++++++++++++++++++++++++++++++++
 git-pull.sh                  | 339 -------------------------------------------
 git.c                        |   1 +
 t/t4013-diff-various.sh      |   3 +
 t/t5150-request-pull.sh      |   2 +-
 t/t5500-fetch-pack.sh        |  10 +-
 t/t5520-pull.sh              |  68 ++++-----
 t/t5521-pull-options.sh      |  20 +--
 t/t5524-pull-msg.sh          |   2 +-
 t/t5572-pull-submodule.sh    |   4 +
 t/t6029-merge-subtree.sh     |   6 +-
 t/t6037-merge-ours-theirs.sh |   2 +-
 t/t7403-submodule-sync.sh    |   8 +-
 t/t7406-submodule-update.sh  |   2 +
 t/t7601-merge-pull-config.sh |   4 +-

This is an excellent way to develop the patch series. For the final patch series, I would like to propose that `git-pull.sh` is replaced by the builtin with the very last patch, though. That way, the builtin can be implemented in the way you did, but without touching the tests. In the final form, the builtin git-pull will be a drop-in replacement for the shell script, and the final patch will just make that switch.

In other words, patch 1/N would leave SCRIPT_SH in the Makefile untouched, and leave out the "pull" line from `git.c`.

Oh, by the way, if you pass the `-M` option to `git format-patch`, the diffstat will report the move rather than an added and a deleted git-pull.sh.

Ciao,
Dscho