Re: [PATCH 1/5] rebase: split the cherry-pick stuff

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> They do something completely different from 'git am', it belongs in a
> different file.

I would prefer to see it called --cherry-pick, not --cherry, as they
are different commands (the latter may be useful when deciding which
one to use the former).

>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  .gitignore            |  1 +
>  Makefile              |  1 +
>  git-rebase--am.sh     | 65 ++++++++++++++++++++++-----------------------------
>  git-rebase--cherry.sh | 34 +++++++++++++++++++++++++++
>  git-rebase.sh         |  4 ++++
>  5 files changed, 68 insertions(+), 37 deletions(-)
>  create mode 100644 git-rebase--cherry.sh
>
> diff --git a/.gitignore b/.gitignore
> index 6669bf0..284fc8f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -113,6 +113,7 @@
>  /git-read-tree
>  /git-rebase
>  /git-rebase--am
> +/git-rebase--cherry
>  /git-rebase--interactive
>  /git-rebase--merge
>  /git-receive-pack
> diff --git a/Makefile b/Makefile
> index 0f931a2..a3cd4bc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -469,6 +469,7 @@ SCRIPT_SH += git-web--browse.sh
>  SCRIPT_LIB += git-mergetool--lib
>  SCRIPT_LIB += git-parse-remote
>  SCRIPT_LIB += git-rebase--am
> +SCRIPT_LIB += git-rebase--cherry
>  SCRIPT_LIB += git-rebase--interactive
>  SCRIPT_LIB += git-rebase--merge
>  SCRIPT_LIB += git-sh-setup
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index f84854f..ee1b1b9 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -19,52 +19,43 @@ esac
>  test -n "$rebase_root" && root_flag=--root
>  
>  ret=0
> +rm -f "$GIT_DIR/rebased-patches"
>  
> -		cat >&2 <<-EOF
> +git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> +	--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
> +	$root_flag "$revisions" >"$GIT_DIR/rebased-patches"
> +ret=$?
>  
> +if test 0 != $ret
> +then
> +	rm -f "$GIT_DIR/rebased-patches"
> +	case "$head_name" in
> +	refs/heads/*)
> +		git checkout -q "$head_name"
> +		;;
> +	*)
> +		git checkout -q "$orig_head"
> +		;;
> +	esac
>  
> -		    $revisions
> +	cat >&2 <<-EOF
>  
> +	git encountered an error while preparing the patches to replay
> +	these revisions:
>  
> +	    $revisions
>  
> +	As a result, git cannot rebase them.
> +	EOF
> +	exit $?
>  fi
>  
> +git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches"
> +ret=$?
> +
> +rm -f "$GIT_DIR/rebased-patches"
> +
>  if test 0 != $ret
>  then
>  	test -d "$state_dir" && write_basic_state

OK, this part seems a straightforward reindentation of "else" side.

> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> new file mode 100644
> index 0000000..cbf80f9
> --- /dev/null
> +++ b/git-rebase--cherry.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Junio C Hamano.
> +#
> +
> +case "$action" in
> +continue)
> +	git am --resolved --resolvemsg="$resolvemsg" &&
> +	move_to_original_branch
> +	exit
> +	;;
> +skip)
> +	git am --skip --resolvemsg="$resolvemsg" &&
> +	move_to_original_branch
> +	exit
> +	;;
> +esac
> +
> +test -n "$rebase_root" && root_flag=--root

Hmm, do we want to duplicate this part between the two?  Perhaps it
will be unified in a later patch---I'll read on.

> +# we have to do this the hard way.  git format-patch completely squashes
> +# empty commits and even if it didn't the format doesn't really lend
> +# itself well to recording empty patches.  fortunately, cherry-pick
> +# makes this easy
> +git cherry-pick --allow-empty "$revisions"
> +ret=$?
> +
> +if test 0 != $ret
> +then
> +	test -d "$state_dir" && write_basic_state
> +	exit $ret
> +fi
> +
> +move_to_original_branch

OK.  This is the "then" side of the original, with clean-up phase
copied.

> diff --git a/git-rebase.sh b/git-rebase.sh
> index 2c692c3..2754985 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -379,6 +379,10 @@ elif test -n "$do_merge"
>  then
>  	type=merge
>  	state_dir="$merge_dir"
> +elif test -n "$keep_empty"
> +then
> +	type=cherry
> +	state_dir="$apply_dir"
>  else
>  	type=am
>  	state_dir="$apply_dir"