git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH/RFC 00/20] Refactor rebase
@ 2010-11-25 19:57 Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 01/20] rebase: clearer names for directory variables Martin von Zweigbergk
                   ` (20 more replies)
  0 siblings, 21 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

This is a first draft of my attempt to refactor the rebase code. I
have tried to refactor it as Hannes suggested, namely "to write a
command line processor, git-rebase.sh, that sets shell variables from
options that it collects from various sources, then dispatches to one
of git-rebase--interactive.sh, git-rebase--merge.sh, or
git-rebase--am.sh (the latter two would be stripped-down copies of the
current git-rebase.sh)."


Patches 01-04 try to make git-rebase.sh more readable and extensible.

Patches 05-16 factor out common code between git-rebase.sh and
git-rebase--interactive.sh.

Patches 17-20 finally achieve, I hope, what Hannes suggested.


I have aligned a lot of the error checking and error messages, but I
still have barely gotten to align any of the command line options
supported by 'git rebase' and 'git rebase -i'. How do you think I
should continue? Some specific questions:

1. What should -v do? Interactive rebase currently prints most
commands it is about to execute, while non-interactive rebase only
prints a header to the diffstat. Is there any reason they should be
different? If not, what should they print?

2. Interactive rebase currently saves most command line options when
the rebase is initiated and then reads then back on '--continue'
etc. Non-interactive rebase does not store any options and allows them
to be passed on the command line when the rebase is continued
instead. Any reason for the difference? What do we want?


All feedback would be greatly appreciated! I'm new to the Git code,
new to development on Linux and even quite new to bash, so please
review very carefully. Thanks!


Martin von Zweigbergk (20):
  rebase: clearer names for directory variables
  rebase: refactor reading of state
  rebase: read state outside loop
  rebase: remove unused rebase state 'prev_head'
  rebase: act on command line outside parsing loop
  rebase: collect check for existing rebase
  rebase: stricter check on arguments
  rebase: align variable names
  rebase: align variable content
  rebase: factor out command line option processing
  rebase -i: remove now unnecessary directory checks
  rebase: reorder validation steps
  rebase: factor out reference parsing
  rebase: factor out clean work tree check
  rebase: factor out call to pre-rebase hook
  rebase -i: support --stat
  rebase: improve detection of rebase in progress
  rebase -m: extract code to new source file
  rebase: extract am code to new source file
  rebase: show consistent conflict resolution hint

 .gitignore                 |    2 +
 Makefile                   |    2 +
 git-rebase--am.sh          |   34 +++
 git-rebase--interactive.sh |  554 ++++++++++++++++----------------------------
 git-rebase--merge.sh       |  154 ++++++++++++
 git-rebase.sh              |  434 ++++++++++++-----------------------
 6 files changed, 538 insertions(+), 642 deletions(-)
 create mode 100644 git-rebase--am.sh
 create mode 100644 git-rebase--merge.sh

-- 
1.7.3.2.864.gbbb96

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH/RFC 01/20] rebase: clearer names for directory variables
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 02/20] rebase: refactor reading of state Martin von Zweigbergk
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Instead of using the old variable name 'dotest' for
"$GIT_DIR"/rebase-merge and no variable for "$GIT_DIR"/rebase-apply,
introduce two variables 'merge_dir' and 'apply_dir' for these paths.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |  141 +++++++++++++++++++++++++++++----------------------------
 1 files changed, 71 insertions(+), 70 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 933ca49..a529bab 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -46,7 +46,8 @@ unset newbase
 strategy=recursive
 strategy_opts=
 do_merge=
-dotest="$GIT_DIR"/rebase-merge
+merge_dir="$GIT_DIR"/rebase-merge
+apply_dir="$GIT_DIR"/rebase-apply
 prec=4
 verbose=
 diffstat=$(git config --bool rebase.stat)
@@ -57,7 +58,7 @@ allow_rerere_autoupdate=
 
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
-	test -d "$dotest" || die "$dotest directory does not exist"
+	test -d "$merge_dir" || die "$merge_dir directory does not exist"
 
 	unmerged=$(git ls-files -u)
 	if test -n "$unmerged"
@@ -67,7 +68,7 @@ continue_merge () {
 		die "$RESOLVEMSG"
 	fi
 
-	cmt=`cat "$dotest/current"`
+	cmt=`cat "$merge_dir/current"`
 	if ! git diff-index --quiet --ignore-submodules HEAD --
 	then
 		if ! git commit --no-verify -C "$cmt"
@@ -80,7 +81,7 @@ continue_merge () {
 		then
 			printf "Committed: %0${prec}d " $msgnum
 		fi
-		echo "$cmt $(git rev-parse HEAD^0)" >> "$dotest/rewritten"
+		echo "$cmt $(git rev-parse HEAD^0)" >> "$merge_dir/rewritten"
 	else
 		if test -z "$GIT_QUIET"
 		then
@@ -92,22 +93,22 @@ continue_merge () {
 
 	prev_head=`git rev-parse HEAD^0`
 	# save the resulting commit so we can read-tree on it later
-	echo "$prev_head" > "$dotest/prev_head"
+	echo "$prev_head" > "$merge_dir/prev_head"
 
 	# onto the next patch:
 	msgnum=$(($msgnum + 1))
-	echo "$msgnum" >"$dotest/msgnum"
+	echo "$msgnum" >"$merge_dir/msgnum"
 }
 
 call_merge () {
-	cmt="$(cat "$dotest/cmt.$1")"
-	echo "$cmt" > "$dotest/current"
+	cmt="$(cat "$merge_dir/cmt.$1")"
+	echo "$cmt" > "$merge_dir/current"
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
-	msgnum=$(cat "$dotest/msgnum")
-	end=$(cat "$dotest/end")
+	msgnum=$(cat "$merge_dir/msgnum")
+	end=$(cat "$merge_dir/end")
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
-	eval GITHEAD_$hd='$(cat "$dotest/onto_name")'
+	eval GITHEAD_$hd='$(cat "$merge_dir/onto_name")'
 	export GITHEAD_$cmt GITHEAD_$hd
 	if test -n "$GIT_QUIET"
 	then
@@ -137,9 +138,9 @@ call_merge () {
 
 move_to_original_branch () {
 	test -z "$head_name" &&
-		head_name="$(cat "$dotest"/head-name)" &&
-		onto="$(cat "$dotest"/onto)" &&
-		orig_head="$(cat "$dotest"/orig-head)"
+		head_name="$(cat "$merge_dir"/head-name)" &&
+		onto="$(cat "$merge_dir"/onto)" &&
+		orig_head="$(cat "$merge_dir"/orig-head)"
 	case "$head_name" in
 	refs/*)
 		message="rebase finished: $head_name onto $onto"
@@ -153,12 +154,12 @@ move_to_original_branch () {
 
 finish_rb_merge () {
 	move_to_original_branch
-	git notes copy --for-rewrite=rebase < "$dotest"/rewritten
+	git notes copy --for-rewrite=rebase < "$merge_dir"/rewritten
 	if test -x "$GIT_DIR"/hooks/post-rewrite &&
-		test -s "$dotest"/rewritten; then
-		"$GIT_DIR"/hooks/post-rewrite rebase < "$dotest"/rewritten
+		test -s "$merge_dir"/rewritten; then
+		"$GIT_DIR"/hooks/post-rewrite rebase < "$merge_dir"/rewritten
 	fi
-	rm -r "$dotest"
+	rm -r "$merge_dir"
 	say All done.
 }
 
@@ -182,7 +183,7 @@ is_interactive () {
 		export GIT_EDITOR
 	fi
 
-	test -n "$interactive_rebase" || test -f "$dotest"/interactive
+	test -n "$interactive_rebase" || test -f "$merge_dir"/interactive
 }
 
 run_pre_rebase_hook () {
@@ -194,7 +195,7 @@ run_pre_rebase_hook () {
 	fi
 }
 
-test -f "$GIT_DIR"/rebase-apply/applying &&
+test -f "$apply_dir"/applying &&
 	die 'It looks like git-am is in progress. Cannot rebase.'
 
 is_interactive "$@" && exec git-rebase--interactive "$@"
@@ -209,7 +210,7 @@ do
 		OK_TO_SKIP_PRE_REBASE=
 		;;
 	--continue)
-		test -d "$dotest" -o -d "$GIT_DIR"/rebase-apply ||
+		test -d "$merge_dir" -o -d "$apply_dir" ||
 			die "No rebase in progress?"
 
 		git update-index --ignore-submodules --refresh &&
@@ -218,13 +219,13 @@ do
 			echo "mark them as resolved using git add"
 			exit 1
 		}
-		if test -d "$dotest"
+		if test -d "$merge_dir"
 		then
-			prev_head=$(cat "$dotest/prev_head")
-			end=$(cat "$dotest/end")
-			msgnum=$(cat "$dotest/msgnum")
-			onto=$(cat "$dotest/onto")
-			GIT_QUIET=$(cat "$dotest/quiet")
+			prev_head=$(cat "$merge_dir/prev_head")
+			end=$(cat "$merge_dir/end")
+			msgnum=$(cat "$merge_dir/msgnum")
+			onto=$(cat "$merge_dir/onto")
+			GIT_QUIET=$(cat "$merge_dir/quiet")
 			continue_merge
 			while test "$msgnum" -le "$end"
 			do
@@ -234,28 +235,28 @@ do
 			finish_rb_merge
 			exit
 		fi
-		head_name=$(cat "$GIT_DIR"/rebase-apply/head-name) &&
-		onto=$(cat "$GIT_DIR"/rebase-apply/onto) &&
-		orig_head=$(cat "$GIT_DIR"/rebase-apply/orig-head) &&
-		GIT_QUIET=$(cat "$GIT_DIR"/rebase-apply/quiet)
+		head_name=$(cat "$apply_dir"/head-name) &&
+		onto=$(cat "$apply_dir"/onto) &&
+		orig_head=$(cat "$apply_dir"/orig-head) &&
+		GIT_QUIET=$(cat "$apply_dir"/quiet)
 		git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
 		move_to_original_branch
 		exit
 		;;
 	--skip)
-		test -d "$dotest" -o -d "$GIT_DIR"/rebase-apply ||
+		test -d "$merge_dir" -o -d "$apply_dir" ||
 			die "No rebase in progress?"
 
 		git reset --hard HEAD || exit $?
-		if test -d "$dotest"
+		if test -d "$merge_dir"
 		then
 			git rerere clear
-			prev_head=$(cat "$dotest/prev_head")
-			end=$(cat "$dotest/end")
-			msgnum=$(cat "$dotest/msgnum")
+			prev_head=$(cat "$merge_dir/prev_head")
+			end=$(cat "$merge_dir/end")
+			msgnum=$(cat "$merge_dir/msgnum")
 			msgnum=$(($msgnum + 1))
-			onto=$(cat "$dotest/onto")
-			GIT_QUIET=$(cat "$dotest/quiet")
+			onto=$(cat "$merge_dir/onto")
+			GIT_QUIET=$(cat "$merge_dir/quiet")
 			while test "$msgnum" -le "$end"
 			do
 				call_merge "$msgnum"
@@ -264,31 +265,31 @@ do
 			finish_rb_merge
 			exit
 		fi
-		head_name=$(cat "$GIT_DIR"/rebase-apply/head-name) &&
-		onto=$(cat "$GIT_DIR"/rebase-apply/onto) &&
-		orig_head=$(cat "$GIT_DIR"/rebase-apply/orig-head) &&
-		GIT_QUIET=$(cat "$GIT_DIR"/rebase-apply/quiet)
+		head_name=$(cat "$apply_dir"/head-name) &&
+		onto=$(cat "$apply_dir"/onto) &&
+		orig_head=$(cat "$apply_dir"/orig-head) &&
+		GIT_QUIET=$(cat "$apply_dir"/quiet)
 		git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
 		move_to_original_branch
 		exit
 		;;
 	--abort)
-		test -d "$dotest" -o -d "$GIT_DIR"/rebase-apply ||
+		test -d "$merge_dir" -o -d "$apply_dir" ||
 			die "No rebase in progress?"
 
 		git rerere clear
 
-		test -d "$dotest" || dotest="$GIT_DIR"/rebase-apply
+		test -d "$merge_dir" || merge_dir="$apply_dir"
 
-		head_name="$(cat "$dotest"/head-name)" &&
+		head_name="$(cat "$merge_dir"/head-name)" &&
 		case "$head_name" in
 		refs/*)
 			git symbolic-ref HEAD $head_name ||
 			die "Could not move back to $head_name"
 			;;
 		esac
-		git reset --hard $(cat "$dotest/orig-head")
-		rm -r "$dotest"
+		git reset --hard $(cat "$merge_dir/orig-head")
+		rm -r "$merge_dir"
 		exit
 		;;
 	--onto)
@@ -387,31 +388,31 @@ test $# -gt 2 && usage
 
 if test $# -eq 0 && test -z "$rebase_root"
 then
-	test -d "$dotest" -o -d "$GIT_DIR"/rebase-apply || usage
-	test -d "$dotest" -o -f "$GIT_DIR"/rebase-apply/rebasing &&
+	test -d "$merge_dir" -o -d "$apply_dir" || usage
+	test -d "$merge_dir" -o -f "$apply_dir"/rebasing &&
 		die 'A rebase is in progress, try --continue, --skip or --abort.'
 fi
 
-# Make sure we do not have $GIT_DIR/rebase-apply
+# Make sure we do not have $apply_dir or $merge_dir
 if test -z "$do_merge"
 then
-	if mkdir "$GIT_DIR"/rebase-apply 2>/dev/null
+	if mkdir "$apply_dir" 2>/dev/null
 	then
-		rmdir "$GIT_DIR"/rebase-apply
+		rmdir "$apply_dir"
 	else
 		echo >&2 '
 It seems that I cannot create a rebase-apply directory, and
 I wonder if you are in the middle of patch application or another
 rebase.  If that is not the case, please
-	rm -fr '"$GIT_DIR"'/rebase-apply
+	rm -fr '"$apply_dir"'
 and run me again.  I am stopping in case you still have something
 valuable there.'
 		exit 1
 	fi
 else
-	if test -d "$dotest"
+	if test -d "$merge_dir"
 	then
-		die "previous rebase directory $dotest still exists." \
+		die "previous rebase directory $merge_dir still exists." \
 			'Try git rebase (--continue | --abort | --skip)'
 	fi
 fi
@@ -559,35 +560,35 @@ then
 	git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
 	move_to_original_branch
 	ret=$?
-	test 0 != $ret -a -d "$GIT_DIR"/rebase-apply &&
-		echo $head_name > "$GIT_DIR"/rebase-apply/head-name &&
-		echo $onto > "$GIT_DIR"/rebase-apply/onto &&
-		echo $orig_head > "$GIT_DIR"/rebase-apply/orig-head &&
-		echo "$GIT_QUIET" > "$GIT_DIR"/rebase-apply/quiet
+	test 0 != $ret -a -d "$apply_dir" &&
+		echo $head_name > "$apply_dir/head-name" &&
+		echo $onto > "$apply_dir/onto" &&
+		echo $orig_head > "$apply_dir/orig-head" &&
+		echo "$GIT_QUIET" > "$apply_dir/quiet"
 	exit $ret
 fi
 
 # start doing a rebase with git-merge
 # this is rename-aware if the recursive (default) strategy is used
 
-mkdir -p "$dotest"
-echo "$onto" > "$dotest/onto"
-echo "$onto_name" > "$dotest/onto_name"
+mkdir -p "$merge_dir"
+echo "$onto" > "$merge_dir/onto"
+echo "$onto_name" > "$merge_dir/onto_name"
 prev_head=$orig_head
-echo "$prev_head" > "$dotest/prev_head"
-echo "$orig_head" > "$dotest/orig-head"
-echo "$head_name" > "$dotest/head-name"
-echo "$GIT_QUIET" > "$dotest/quiet"
+echo "$prev_head" > "$merge_dir/prev_head"
+echo "$orig_head" > "$merge_dir/orig-head"
+echo "$head_name" > "$merge_dir/head-name"
+echo "$GIT_QUIET" > "$merge_dir/quiet"
 
 msgnum=0
 for cmt in `git rev-list --reverse --no-merges "$revisions"`
 do
 	msgnum=$(($msgnum + 1))
-	echo "$cmt" > "$dotest/cmt.$msgnum"
+	echo "$cmt" > "$merge_dir/cmt.$msgnum"
 done
 
-echo 1 >"$dotest/msgnum"
-echo $msgnum >"$dotest/end"
+echo 1 >"$merge_dir/msgnum"
+echo $msgnum >"$merge_dir/end"
 
 end=$msgnum
 msgnum=1
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 02/20] rebase: refactor reading of state
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 01/20] rebase: clearer names for directory variables Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 03/20] rebase: read state outside loop Martin von Zweigbergk
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

The code reading the state saved in $merge_dir or $rebase_dir is
currently spread out in many places, making it harder to read and to
introduce additional state. Extract this code into one method that reads
the state.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |   53 +++++++++++++++++++++++------------------------------
 1 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index a529bab..a325dd9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -56,6 +56,22 @@ rebase_root=
 force_rebase=
 allow_rerere_autoupdate=
 
+read_state () {
+	if test -d "$merge_dir"
+	then
+		state_dir="$merge_dir"
+		prev_head=$(cat "$merge_dir"/prev_head) &&
+		end=$(cat "$merge_dir"/end) &&
+		msgnum=$(cat "$merge_dir"/msgnum)
+	else
+		state_dir="$apply_dir"
+	fi &&
+	head_name=$(cat "$state_dir"/head-name) &&
+	onto=$(cat "$state_dir"/onto) &&
+	orig_head=$(cat "$state_dir"/orig-head) &&
+	GIT_QUIET=$(cat "$state_dir"/quiet)
+}
+
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
 	test -d "$merge_dir" || die "$merge_dir directory does not exist"
@@ -137,10 +153,6 @@ call_merge () {
 }
 
 move_to_original_branch () {
-	test -z "$head_name" &&
-		head_name="$(cat "$merge_dir"/head-name)" &&
-		onto="$(cat "$merge_dir"/onto)" &&
-		orig_head="$(cat "$merge_dir"/orig-head)"
 	case "$head_name" in
 	refs/*)
 		message="rebase finished: $head_name onto $onto"
@@ -219,13 +231,9 @@ do
 			echo "mark them as resolved using git add"
 			exit 1
 		}
+		read_state
 		if test -d "$merge_dir"
 		then
-			prev_head=$(cat "$merge_dir/prev_head")
-			end=$(cat "$merge_dir/end")
-			msgnum=$(cat "$merge_dir/msgnum")
-			onto=$(cat "$merge_dir/onto")
-			GIT_QUIET=$(cat "$merge_dir/quiet")
 			continue_merge
 			while test "$msgnum" -le "$end"
 			do
@@ -235,10 +243,6 @@ do
 			finish_rb_merge
 			exit
 		fi
-		head_name=$(cat "$apply_dir"/head-name) &&
-		onto=$(cat "$apply_dir"/onto) &&
-		orig_head=$(cat "$apply_dir"/orig-head) &&
-		GIT_QUIET=$(cat "$apply_dir"/quiet)
 		git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
 		move_to_original_branch
 		exit
@@ -248,15 +252,11 @@ do
 			die "No rebase in progress?"
 
 		git reset --hard HEAD || exit $?
+		read_state
 		if test -d "$merge_dir"
 		then
 			git rerere clear
-			prev_head=$(cat "$merge_dir/prev_head")
-			end=$(cat "$merge_dir/end")
-			msgnum=$(cat "$merge_dir/msgnum")
 			msgnum=$(($msgnum + 1))
-			onto=$(cat "$merge_dir/onto")
-			GIT_QUIET=$(cat "$merge_dir/quiet")
 			while test "$msgnum" -le "$end"
 			do
 				call_merge "$msgnum"
@@ -265,10 +265,6 @@ do
 			finish_rb_merge
 			exit
 		fi
-		head_name=$(cat "$apply_dir"/head-name) &&
-		onto=$(cat "$apply_dir"/onto) &&
-		orig_head=$(cat "$apply_dir"/orig-head) &&
-		GIT_QUIET=$(cat "$apply_dir"/quiet)
 		git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
 		move_to_original_branch
 		exit
@@ -278,18 +274,15 @@ do
 			die "No rebase in progress?"
 
 		git rerere clear
-
-		test -d "$merge_dir" || merge_dir="$apply_dir"
-
-		head_name="$(cat "$merge_dir"/head-name)" &&
+		read_state
 		case "$head_name" in
 		refs/*)
 			git symbolic-ref HEAD $head_name ||
 			die "Could not move back to $head_name"
 			;;
 		esac
-		git reset --hard $(cat "$merge_dir/orig-head")
-		rm -r "$merge_dir"
+		git reset --hard $orig_head
+		rm -r "$state_dir"
 		exit
 		;;
 	--onto)
@@ -572,12 +565,12 @@ fi
 # this is rename-aware if the recursive (default) strategy is used
 
 mkdir -p "$merge_dir"
-echo "$onto" > "$merge_dir/onto"
 echo "$onto_name" > "$merge_dir/onto_name"
 prev_head=$orig_head
 echo "$prev_head" > "$merge_dir/prev_head"
-echo "$orig_head" > "$merge_dir/orig-head"
 echo "$head_name" > "$merge_dir/head-name"
+echo "$onto" > "$merge_dir/onto"
+echo "$orig_head" > "$merge_dir/orig-head"
 echo "$GIT_QUIET" > "$merge_dir/quiet"
 
 msgnum=0
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 03/20] rebase: read state outside loop
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 01/20] rebase: clearer names for directory variables Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 02/20] rebase: refactor reading of state Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head' Martin von Zweigbergk
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

The 'onto_name' state is used in 'git rebase --merge' is currently read
once for each commit that need to be applied. It doesn't change between
each iteration, however, so it should be moved out of the loop. This also
makes the code more readable. Also remove the unused variable 'end'.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index a325dd9..b1fadb6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -61,6 +61,7 @@ read_state () {
 	then
 		state_dir="$merge_dir"
 		prev_head=$(cat "$merge_dir"/prev_head) &&
+		onto_name=$(cat "$merge_dir"/onto_name) &&
 		end=$(cat "$merge_dir"/end) &&
 		msgnum=$(cat "$merge_dir"/msgnum)
 	else
@@ -122,9 +123,8 @@ call_merge () {
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
 	msgnum=$(cat "$merge_dir/msgnum")
-	end=$(cat "$merge_dir/end")
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
-	eval GITHEAD_$hd='$(cat "$merge_dir/onto_name")'
+	eval GITHEAD_$hd='$onto_name'
 	export GITHEAD_$cmt GITHEAD_$hd
 	if test -n "$GIT_QUIET"
 	then
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head'
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (2 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 03/20] rebase: read state outside loop Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-26  7:54   ` Michael J Gruber
  2010-11-25 19:57 ` [PATCH/RFC 05/20] rebase: act on command line outside parsing loop Martin von Zweigbergk
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

The rebase state 'prev_head' is not used. Remove it.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index b1fadb6..aaee06b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -60,7 +60,6 @@ read_state () {
 	if test -d "$merge_dir"
 	then
 		state_dir="$merge_dir"
-		prev_head=$(cat "$merge_dir"/prev_head) &&
 		onto_name=$(cat "$merge_dir"/onto_name) &&
 		end=$(cat "$merge_dir"/end) &&
 		msgnum=$(cat "$merge_dir"/msgnum)
@@ -74,7 +73,6 @@ read_state () {
 }
 
 continue_merge () {
-	test -n "$prev_head" || die "prev_head must be defined"
 	test -d "$merge_dir" || die "$merge_dir directory does not exist"
 
 	unmerged=$(git ls-files -u)
@@ -108,10 +106,6 @@ continue_merge () {
 	test -z "$GIT_QUIET" &&
 	GIT_PAGER='' git log --format=%s -1 "$cmt"
 
-	prev_head=`git rev-parse HEAD^0`
-	# save the resulting commit so we can read-tree on it later
-	echo "$prev_head" > "$merge_dir/prev_head"
-
 	# onto the next patch:
 	msgnum=$(($msgnum + 1))
 	echo "$msgnum" >"$merge_dir/msgnum"
@@ -566,8 +560,6 @@ fi
 
 mkdir -p "$merge_dir"
 echo "$onto_name" > "$merge_dir/onto_name"
-prev_head=$orig_head
-echo "$prev_head" > "$merge_dir/prev_head"
 echo "$head_name" > "$merge_dir/head-name"
 echo "$onto" > "$merge_dir/onto"
 echo "$orig_head" > "$merge_dir/orig-head"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 05/20] rebase: act on command line outside parsing loop
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (3 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head' Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 06/20] rebase: collect check for existing rebase Martin von Zweigbergk
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

To later be able to use the command line processing in git-rebase.sh for
both interactive and non-interactive rebases, move anything that is
specific to non-interactive rebase outside of the parsing loop. Keep only
parsing and validation of command line options in the loop.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
You may want to try '--ignore-all-space' on this patch.

 git-rebase--interactive.sh |  300 ++++++++++++++++++++++----------------------
 git-rebase.sh              |  131 ++++++++++---------
 2 files changed, 219 insertions(+), 212 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a5ffd9a..8cbdd3f 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -866,152 +866,158 @@ first and then run 'git rebase --continue' again."
 		;;
 	--)
 		shift
-		test -z "$REBASE_ROOT" -a $# -ge 1 -a $# -le 2 ||
-		test ! -z "$REBASE_ROOT" -a $# -le 1 || usage
-		test -d "$DOTEST" &&
-			die "Interactive rebase already started"
-
-		git var GIT_COMMITTER_IDENT >/dev/null ||
-			die "You need to set your committer info first"
-
-		if test -z "$REBASE_ROOT"
-		then
-			UPSTREAM_ARG="$1"
-			UPSTREAM=$(git rev-parse --verify "$1") || die "Invalid base"
-			test -z "$ONTO" && ONTO=$UPSTREAM
-			shift
-		else
-			UPSTREAM=
-			UPSTREAM_ARG=--root
-			test -z "$ONTO" &&
-				die "You must specify --onto when using --root"
-		fi
-		run_pre_rebase_hook "$UPSTREAM_ARG" "$@"
-
-		comment_for_reflog start
-
-		require_clean_work_tree "rebase" "Please commit or stash them."
-
-		if test ! -z "$1"
-		then
-			output git checkout "$1" ||
-				die "Could not checkout $1"
-		fi
+		break
+		;;
+	esac
+	shift
+done
 
-		HEAD=$(git rev-parse --verify HEAD) || die "No HEAD?"
-		mkdir "$DOTEST" || die "Could not create temporary $DOTEST"
+test -z "$REBASE_ROOT" -a $# -ge 1 -a $# -le 2 ||
+test ! -z "$REBASE_ROOT" -a $# -le 1 || usage
+test -d "$DOTEST" &&
+	die "Interactive rebase already started"
 
-		: > "$DOTEST"/interactive || die "Could not mark as interactive"
-		git symbolic-ref HEAD > "$DOTEST"/head-name 2> /dev/null ||
-			echo "detached HEAD" > "$DOTEST"/head-name
+git var GIT_COMMITTER_IDENT >/dev/null ||
+	die "You need to set your committer info first"
 
-		echo $HEAD > "$DOTEST"/head
-		case "$REBASE_ROOT" in
-		'')
-			rm -f "$DOTEST"/rebase-root ;;
-		*)
-			: >"$DOTEST"/rebase-root ;;
-		esac
-		echo $ONTO > "$DOTEST"/onto
-		test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
-		test t = "$VERBOSE" && : > "$DOTEST"/verbose
-		if test t = "$PRESERVE_MERGES"
-		then
-			if test -z "$REBASE_ROOT"
-			then
-				mkdir "$REWRITTEN" &&
-				for c in $(git merge-base --all $HEAD $UPSTREAM)
-				do
-					echo $ONTO > "$REWRITTEN"/$c ||
-						die "Could not init rewritten commits"
-				done
-			else
-				mkdir "$REWRITTEN" &&
-				echo $ONTO > "$REWRITTEN"/root ||
-					die "Could not init rewritten commits"
-			fi
-			# No cherry-pick because our first pass is to determine
-			# parents to rewrite and skipping dropped commits would
-			# prematurely end our probe
-			MERGES_OPTION=
-			first_after_upstream="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
-		else
-			MERGES_OPTION="--no-merges --cherry-pick"
-		fi
-
-		SHORTHEAD=$(git rev-parse --short $HEAD)
-		SHORTONTO=$(git rev-parse --short $ONTO)
-		if test -z "$REBASE_ROOT"
-			# this is now equivalent to ! -z "$UPSTREAM"
-		then
-			SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
-			REVISIONS=$UPSTREAM...$HEAD
-			SHORTREVISIONS=$SHORTUPSTREAM..$SHORTHEAD
-		else
-			REVISIONS=$ONTO...$HEAD
-			SHORTREVISIONS=$SHORTHEAD
-		fi
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --topo-order \
-			$REVISIONS | \
-			sed -n "s/^>//p" |
-		while read -r shortsha1 rest
+if test -z "$REBASE_ROOT"
+then
+	UPSTREAM_ARG="$1"
+	UPSTREAM=$(git rev-parse --verify "$1") || die "Invalid base"
+	test -z "$ONTO" && ONTO=$UPSTREAM
+	shift
+else
+	UPSTREAM=
+	UPSTREAM_ARG=--root
+	test -z "$ONTO" &&
+	die "You must specify --onto when using --root"
+fi
+run_pre_rebase_hook "$UPSTREAM_ARG" "$@"
+
+comment_for_reflog start
+
+require_clean_work_tree "rebase" "Please commit or stash them."
+
+if test ! -z "$1"
+then
+	output git checkout "$1" ||
+		die "Could not checkout $1"
+fi
+
+HEAD=$(git rev-parse --verify HEAD) || die "No HEAD?"
+mkdir "$DOTEST" || die "Could not create temporary $DOTEST"
+
+: > "$DOTEST"/interactive || die "Could not mark as interactive"
+git symbolic-ref HEAD > "$DOTEST"/head-name 2> /dev/null ||
+	echo "detached HEAD" > "$DOTEST"/head-name
+
+echo $HEAD > "$DOTEST"/head
+case "$REBASE_ROOT" in
+'')
+	rm -f "$DOTEST"/rebase-root ;;
+*)
+	: >"$DOTEST"/rebase-root ;;
+esac
+echo $ONTO > "$DOTEST"/onto
+test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
+test t = "$VERBOSE" && : > "$DOTEST"/verbose
+if test t = "$PRESERVE_MERGES"
+then
+	if test -z "$REBASE_ROOT"
+	then
+		mkdir "$REWRITTEN" &&
+		for c in $(git merge-base --all $HEAD $UPSTREAM)
 		do
-			if test t != "$PRESERVE_MERGES"
-			then
-				printf '%s\n' "pick $shortsha1 $rest" >> "$TODO"
-			else
-				sha1=$(git rev-parse $shortsha1)
-				if test -z "$REBASE_ROOT"
-				then
-					preserve=t
-					for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
-					do
-						if test -f "$REWRITTEN"/$p -a \( $p != $ONTO -o $sha1 = $first_after_upstream \)
-						then
-							preserve=f
-						fi
-					done
-				else
-					preserve=f
-				fi
-				if test f = "$preserve"
-				then
-					touch "$REWRITTEN"/$sha1
-					printf '%s\n' "pick $shortsha1 $rest" >> "$TODO"
-				fi
-			fi
+			echo $ONTO > "$REWRITTEN"/$c ||
+				die "Could not init rewritten commits"
 		done
-
-		# Watch for commits that been dropped by --cherry-pick
-		if test t = "$PRESERVE_MERGES"
+	else
+		mkdir "$REWRITTEN" &&
+		echo $ONTO > "$REWRITTEN"/root ||
+			die "Could not init rewritten commits"
+	fi
+	# No cherry-pick because our first pass is to determine
+	# parents to rewrite and skipping dropped commits would
+	# prematurely end our probe
+	MERGES_OPTION=
+	first_after_upstream="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
+else
+	MERGES_OPTION="--no-merges --cherry-pick"
+fi
+
+SHORTHEAD=$(git rev-parse --short $HEAD)
+SHORTONTO=$(git rev-parse --short $ONTO)
+if test -z "$REBASE_ROOT"
+	# this is now equivalent to ! -z "$UPSTREAM"
+then
+	SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
+	REVISIONS=$UPSTREAM...$HEAD
+	SHORTREVISIONS=$SHORTUPSTREAM..$SHORTHEAD
+else
+	REVISIONS=$ONTO...$HEAD
+	SHORTREVISIONS=$SHORTHEAD
+fi
+git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
+	--abbrev=7 --reverse --left-right --topo-order \
+	$REVISIONS | \
+	sed -n "s/^>//p" |
+while read -r shortsha1 rest
+do
+	if test t != "$PRESERVE_MERGES"
+	then
+		printf '%s\n' "pick $shortsha1 $rest" >> "$TODO"
+	else
+		sha1=$(git rev-parse $shortsha1)
+		if test -z "$REBASE_ROOT"
 		then
-			mkdir "$DROPPED"
-			# Save all non-cherry-picked changes
-			git rev-list $REVISIONS --left-right --cherry-pick | \
-				sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks
-			# Now all commits and note which ones are missing in
-			# not-cherry-picks and hence being dropped
-			git rev-list $REVISIONS |
-			while read rev
+			preserve=t
+			for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
 			do
-				if test -f "$REWRITTEN"/$rev -a "$(sane_grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
+				if test -f "$REWRITTEN"/$p -a \( $p != $ONTO -o $sha1 = $first_after_upstream \)
 				then
-					# Use -f2 because if rev-list is telling us this commit is
-					# not worthwhile, we don't want to track its multiple heads,
-					# just the history of its first-parent for others that will
-					# be rebasing on top of it
-					git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$DROPPED"/$rev
-					short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
-					sane_grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
-					rm "$REWRITTEN"/$rev
+					preserve=f
 				fi
 			done
+		else
+			preserve=f
+		fi
+		if test f = "$preserve"
+		then
+			touch "$REWRITTEN"/$sha1
+			printf '%s\n' "pick $shortsha1 $rest" >> "$TODO"
 		fi
+	fi
+done
 
-		test -s "$TODO" || echo noop >> "$TODO"
-		test -n "$AUTOSQUASH" && rearrange_squash "$TODO"
-		cat >> "$TODO" << EOF
+# Watch for commits that been dropped by --cherry-pick
+if test t = "$PRESERVE_MERGES"
+then
+	mkdir "$DROPPED"
+	# Save all non-cherry-picked changes
+	git rev-list $REVISIONS --left-right --cherry-pick | \
+		sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks
+	# Now all commits and note which ones are missing in
+	# not-cherry-picks and hence being dropped
+	git rev-list $REVISIONS |
+	while read rev
+	do
+		if test -f "$REWRITTEN"/$rev -a "$(sane_grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
+		then
+			# Use -f2 because if rev-list is telling us this commit is
+			# not worthwhile, we don't want to track its multiple heads,
+			# just the history of its first-parent for others that will
+			# be rebasing on top of it
+			git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$DROPPED"/$rev
+			short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
+			sane_grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
+			rm "$REWRITTEN"/$rev
+		fi
+	done
+fi
+
+test -s "$TODO" || echo noop >> "$TODO"
+test -n "$AUTOSQUASH" && rearrange_squash "$TODO"
+cat >> "$TODO" << EOF
 
 # Rebase $SHORTREVISIONS onto $SHORTONTO
 #
@@ -1028,22 +1034,18 @@ first and then run 'git rebase --continue' again."
 #
 EOF
 
-		has_action "$TODO" ||
-			die_abort "Nothing to do"
+has_action "$TODO" ||
+	die_abort "Nothing to do"
 
-		cp "$TODO" "$TODO".backup
-		git_editor "$TODO" ||
-			die_abort "Could not execute editor"
+cp "$TODO" "$TODO".backup
+git_editor "$TODO" ||
+	die_abort "Could not execute editor"
 
-		has_action "$TODO" ||
-			die_abort "Nothing to do"
+has_action "$TODO" ||
+	die_abort "Nothing to do"
 
-		test -d "$REWRITTEN" || test -n "$NEVER_FF" || skip_unnecessary_picks
+test -d "$REWRITTEN" || test -n "$NEVER_FF" || skip_unnecessary_picks
 
-		output git checkout $ONTO || die_abort "could not detach HEAD"
-		git update-ref ORIG_HEAD $HEAD
-		do_rest
-		;;
-	esac
-	shift
-done
+output git checkout $ONTO || die_abort "could not detach HEAD"
+git update-ref ORIG_HEAD $HEAD
+do_rest
diff --git a/git-rebase.sh b/git-rebase.sh
index aaee06b..52e1c6e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -55,6 +55,7 @@ git_am_opt=
 rebase_root=
 force_rebase=
 allow_rerere_autoupdate=
+action=
 
 read_state () {
 	if test -d "$merge_dir"
@@ -215,69 +216,10 @@ do
 	--verify)
 		OK_TO_SKIP_PRE_REBASE=
 		;;
-	--continue)
-		test -d "$merge_dir" -o -d "$apply_dir" ||
-			die "No rebase in progress?"
-
-		git update-index --ignore-submodules --refresh &&
-		git diff-files --quiet --ignore-submodules || {
-			echo "You must edit all merge conflicts and then"
-			echo "mark them as resolved using git add"
-			exit 1
-		}
-		read_state
-		if test -d "$merge_dir"
-		then
-			continue_merge
-			while test "$msgnum" -le "$end"
-			do
-				call_merge "$msgnum"
-				continue_merge
-			done
-			finish_rb_merge
-			exit
-		fi
-		git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
-		move_to_original_branch
-		exit
-		;;
-	--skip)
-		test -d "$merge_dir" -o -d "$apply_dir" ||
-			die "No rebase in progress?"
-
-		git reset --hard HEAD || exit $?
-		read_state
-		if test -d "$merge_dir"
-		then
-			git rerere clear
-			msgnum=$(($msgnum + 1))
-			while test "$msgnum" -le "$end"
-			do
-				call_merge "$msgnum"
-				continue_merge
-			done
-			finish_rb_merge
-			exit
-		fi
-		git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
-		move_to_original_branch
-		exit
-		;;
-	--abort)
-		test -d "$merge_dir" -o -d "$apply_dir" ||
-			die "No rebase in progress?"
-
-		git rerere clear
-		read_state
-		case "$head_name" in
-		refs/*)
-			git symbolic-ref HEAD $head_name ||
-			die "Could not move back to $head_name"
-			;;
-		esac
-		git reset --hard $orig_head
-		rm -r "$state_dir"
-		exit
+	--continue|--skip|--abort)
+		action=${1##--}
+		shift
+		break
 		;;
 	--onto)
 		test 2 -le "$#" || usage
@@ -373,6 +315,69 @@ do
 done
 test $# -gt 2 && usage
 
+if test -n "$action"
+then
+	test -d "$merge_dir" -o -d "$apply_dir" || die "No rebase in progress?"
+fi
+
+case "$action" in
+continue)
+	git update-index --ignore-submodules --refresh &&
+	git diff-files --quiet --ignore-submodules || {
+		echo "You must edit all merge conflicts and then"
+		echo "mark them as resolved using git add"
+		exit 1
+	}
+	read_state
+	if test -d "$merge_dir"
+	then
+		continue_merge
+		while test "$msgnum" -le "$end"
+		do
+			call_merge "$msgnum"
+			continue_merge
+		done
+		finish_rb_merge
+		exit
+	fi
+	git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
+	move_to_original_branch
+	exit
+	;;
+skip)
+	git reset --hard HEAD || exit $?
+	read_state
+	if test -d "$merge_dir"
+	then
+		git rerere clear
+		msgnum=$(($msgnum + 1))
+		while test "$msgnum" -le "$end"
+		do
+			call_merge "$msgnum"
+			continue_merge
+		done
+		finish_rb_merge
+		exit
+	fi
+	git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
+	move_to_original_branch
+	exit
+	;;
+abort)
+	git rerere clear
+	read_state
+	case "$head_name" in
+	refs/*)
+		git symbolic-ref HEAD $head_name ||
+		die "Could not move back to $head_name"
+		;;
+	esac
+	git reset --hard $orig_head
+	rm -r "$state_dir"
+	exit
+	;;
+esac
+
 if test $# -eq 0 && test -z "$rebase_root"
 then
 	test -d "$merge_dir" -o -d "$apply_dir" || usage
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 06/20] rebase: collect check for existing rebase
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (4 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 05/20] rebase: act on command line outside parsing loop Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 07/20] rebase: stricter check on arguments Martin von Zweigbergk
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Put the code that checks for rebase in progress in one place.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |   56 +++++++++++++++++++++++++-------------------------------
 1 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 52e1c6e..2358e2f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -318,6 +318,31 @@ test $# -gt 2 && usage
 if test -n "$action"
 then
 	test -d "$merge_dir" -o -d "$apply_dir" || die "No rebase in progress?"
+else
+	# Make sure we do not have $apply_dir or $merge_dir
+	if test -z "$do_merge"
+	then
+		if mkdir "$apply_dir" 2>/dev/null
+		then
+			rmdir "$apply_dir"
+		else
+			echo >&2 '
+It seems that I cannot create a rebase-apply directory, and
+I wonder if you are in the middle of patch application or another
+rebase.  If that is not the case, please
+	rm -fr '"$apply_dir"'
+and run me again.  I am stopping in case you still have something
+valuable there.'
+		exit 1
+		fi
+	else
+		if test -d "$merge_dir"
+		then
+			die "previous rebase directory $merge_dir still exists." \
+				'Try git rebase (--continue | --abort | --skip)'
+		fi
+	fi
+	test $# -eq 0 && test -z "$rebase_root" && usage
 fi
 
 case "$action" in
@@ -378,37 +403,6 @@ abort)
 	;;
 esac
 
-if test $# -eq 0 && test -z "$rebase_root"
-then
-	test -d "$merge_dir" -o -d "$apply_dir" || usage
-	test -d "$merge_dir" -o -f "$apply_dir"/rebasing &&
-		die 'A rebase is in progress, try --continue, --skip or --abort.'
-fi
-
-# Make sure we do not have $apply_dir or $merge_dir
-if test -z "$do_merge"
-then
-	if mkdir "$apply_dir" 2>/dev/null
-	then
-		rmdir "$apply_dir"
-	else
-		echo >&2 '
-It seems that I cannot create a rebase-apply directory, and
-I wonder if you are in the middle of patch application or another
-rebase.  If that is not the case, please
-	rm -fr '"$apply_dir"'
-and run me again.  I am stopping in case you still have something
-valuable there.'
-		exit 1
-	fi
-else
-	if test -d "$merge_dir"
-	then
-		die "previous rebase directory $merge_dir still exists." \
-			'Try git rebase (--continue | --abort | --skip)'
-	fi
-fi
-
 require_clean_work_tree "rebase" "Please commit or stash them."
 
 if test -z "$rebase_root"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 07/20] rebase: stricter check on arguments
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (5 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 06/20] rebase: collect check for existing rebase Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 08/20] rebase: align variable names Martin von Zweigbergk
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

To align git-rebase.sh better with git-rebase--interactive.sh, be
stricter about checking that, when used, '--continue', '--skip' and
'--abort' is the only argument.

Since $strategy and $verbose are currently saved only by interactive
rebase, checking that they are not passed on the command line would
break any existing uses of 'git rebase --strategy=<x> --continue'.
Instead, relax the check and let the options be ignored by interactive
rebase once refactored (the relaxation will not be effective until
further refactoring, however).

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 2358e2f..feda4be 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -317,6 +317,10 @@ test $# -gt 2 && usage
 
 if test -n "$action"
 then
+	test $# -eq 0 &&
+	test -z "$onto" &&
+	test -z "$preserve_merges" ||
+	usage
 	test -d "$merge_dir" -o -d "$apply_dir" || die "No rebase in progress?"
 else
 	# Make sure we do not have $apply_dir or $merge_dir
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 08/20] rebase: align variable names
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (6 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 07/20] rebase: stricter check on arguments Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 09/20] rebase: align variable content Martin von Zweigbergk
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Use the same names for variables that git-rebase--interactive.sh will
later inherit from git-rebase.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |  116 ++++++++++++++++++++++----------------------
 git-rebase.sh              |    8 ++--
 2 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8cbdd3f..41a9eb1 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -105,15 +105,15 @@ AMEND="$DOTEST"/amend
 REWRITTEN_LIST="$DOTEST"/rewritten-list
 REWRITTEN_PENDING="$DOTEST"/rewritten-pending
 
-PRESERVE_MERGES=
-STRATEGY=
-ONTO=
-VERBOSE=
+preserve_merges=
+strategy=
+onto=
+verbose=
 OK_TO_SKIP_PRE_REBASE=
-REBASE_ROOT=
-AUTOSQUASH=
-test "$(git config --bool rebase.autosquash)" = "true" && AUTOSQUASH=t
-NEVER_FF=
+rebase_root=
+autosquash=
+test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
+force_rebase=
 
 GIT_CHERRY_PICK_HELP="\
 hint: after resolving the conflicts, mark the corrected paths
@@ -125,7 +125,7 @@ warn () {
 }
 
 output () {
-	case "$VERBOSE" in
+	case "$verbose" in
 	'')
 		output=$("$@" 2>&1 )
 		status=$?
@@ -177,7 +177,7 @@ mark_action_done () {
 	then
 		last_count=$count
 		printf "Rebasing (%d/%d)\r" $count $total
-		test -z "$VERBOSE" || echo
+		test -z "$verbose" || echo
 	fi
 }
 
@@ -228,11 +228,11 @@ do_with_author () {
 pick_one () {
 	ff=--ff
 	case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
-	case "$NEVER_FF" in '') ;; ?*) ff= ;; esac
+	case "$force_rebase" in '') ;; ?*) ff= ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
 		pick_one_preserving_merges "$@" && return
-	if test -n "$REBASE_ROOT"
+	if test -n "$rebase_root"
 	then
 		output git cherry-pick "$@"
 		return
@@ -339,7 +339,7 @@ pick_one_preserving_merges () {
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
 			if ! do_with_author output \
-				git merge $STRATEGY -m "$msg" $new_parents
+				git merge $strategy -m "$msg" $new_parents
 			then
 				printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
 				die_with_patch $sha1 "Error redoing merge $sha1"
@@ -618,11 +618,11 @@ skip_unnecessary_picks () {
 		# fd=3 means we skip the command
 		case "$fd,$command" in
 		3,pick|3,p)
-			# pick a commit whose parent is current $ONTO -> skip
+			# pick a commit whose parent is current $onto -> skip
 			sha1=${rest%% *}
 			case "$(git rev-parse --verify --quiet "$sha1"^)" in
-			"$ONTO"*)
-				ONTO=$sha1
+			"$onto"*)
+				onto=$sha1
 				;;
 			*)
 				fd=1
@@ -641,7 +641,7 @@ skip_unnecessary_picks () {
 	mv -f "$TODO".new "$TODO" &&
 	case "$(peek_next_command)" in
 	squash|s|fixup|f)
-		record_in_rewritten "$ONTO"
+		record_in_rewritten "$onto"
 		;;
 	esac ||
 	die "Could not skip unnecessary pick commands"
@@ -650,17 +650,17 @@ skip_unnecessary_picks () {
 # check if no other options are set
 is_standalone () {
 	test $# -eq 2 -a "$2" = '--' &&
-	test -z "$ONTO" &&
-	test -z "$PRESERVE_MERGES" &&
-	test -z "$STRATEGY" &&
-	test -z "$VERBOSE"
+	test -z "$onto" &&
+	test -z "$preserve_merges" &&
+	test -z "$strategy" &&
+	test -z "$verbose"
 }
 
 get_saved_options () {
-	test -d "$REWRITTEN" && PRESERVE_MERGES=t
-	test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
-	test -f "$DOTEST"/verbose && VERBOSE=t
-	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
+	test -d "$REWRITTEN" && preserve_merges=t
+	test -f "$DOTEST"/strategy && strategy="$(cat "$DOTEST"/strategy)"
+	test -f "$DOTEST"/verbose && verbose=t
+	test -f "$DOTEST"/rebase-root && rebase_root=t
 }
 
 # Rearrange the todo list that has both "pick sha1 msg" and
@@ -827,11 +827,11 @@ first and then run 'git rebase --continue' again."
 	-s)
 		case "$#,$1" in
 		*,*=*)
-			STRATEGY="-s "$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
+			strategy="-s "$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
 		1,*)
 			usage ;;
 		*)
-			STRATEGY="-s $2"
+			strategy="-s $2"
 			shift ;;
 		esac
 		;;
@@ -839,29 +839,29 @@ first and then run 'git rebase --continue' again."
 		# we use merge anyway
 		;;
 	-v)
-		VERBOSE=t
+		verbose=t
 		;;
 	-p)
-		PRESERVE_MERGES=t
+		preserve_merges=t
 		;;
 	-i)
 		# yeah, we know
 		;;
 	--no-ff)
-		NEVER_FF=t
+		force_rebase=t
 		;;
 	--root)
-		REBASE_ROOT=t
+		rebase_root=t
 		;;
 	--autosquash)
-		AUTOSQUASH=t
+		autosquash=t
 		;;
 	--no-autosquash)
-		AUTOSQUASH=
+		autosquash=
 		;;
 	--onto)
 		shift
-		ONTO=$(parse_onto "$1") ||
+		onto=$(parse_onto "$1") ||
 			die "Does not point to a valid commit: $1"
 		;;
 	--)
@@ -872,25 +872,25 @@ first and then run 'git rebase --continue' again."
 	shift
 done
 
-test -z "$REBASE_ROOT" -a $# -ge 1 -a $# -le 2 ||
-test ! -z "$REBASE_ROOT" -a $# -le 1 || usage
+test -z "$rebase_root" -a $# -ge 1 -a $# -le 2 ||
+test ! -z "$rebase_root" -a $# -le 1 || usage
 test -d "$DOTEST" &&
 	die "Interactive rebase already started"
 
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
 
-if test -z "$REBASE_ROOT"
+if test -z "$rebase_root"
 then
 	UPSTREAM_ARG="$1"
 	UPSTREAM=$(git rev-parse --verify "$1") || die "Invalid base"
-	test -z "$ONTO" && ONTO=$UPSTREAM
+	test -z "$onto" && onto=$UPSTREAM
 	shift
 else
 	UPSTREAM=
 	UPSTREAM_ARG=--root
-	test -z "$ONTO" &&
-	die "You must specify --onto when using --root"
+	test -z "$onto" &&
+		die "You must specify --onto when using --root"
 fi
 run_pre_rebase_hook "$UPSTREAM_ARG" "$@"
 
@@ -912,28 +912,28 @@ git symbolic-ref HEAD > "$DOTEST"/head-name 2> /dev/null ||
 	echo "detached HEAD" > "$DOTEST"/head-name
 
 echo $HEAD > "$DOTEST"/head
-case "$REBASE_ROOT" in
+case "$rebase_root" in
 '')
 	rm -f "$DOTEST"/rebase-root ;;
 *)
 	: >"$DOTEST"/rebase-root ;;
 esac
-echo $ONTO > "$DOTEST"/onto
-test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
-test t = "$VERBOSE" && : > "$DOTEST"/verbose
-if test t = "$PRESERVE_MERGES"
+echo $onto > "$DOTEST"/onto
+test -z "$strategy" || echo "$strategy" > "$DOTEST"/strategy
+test t = "$verbose" && : > "$DOTEST"/verbose
+if test t = "$preserve_merges"
 then
-	if test -z "$REBASE_ROOT"
+	if test -z "$rebase_root"
 	then
 		mkdir "$REWRITTEN" &&
 		for c in $(git merge-base --all $HEAD $UPSTREAM)
 		do
-			echo $ONTO > "$REWRITTEN"/$c ||
+			echo $onto > "$REWRITTEN"/$c ||
 				die "Could not init rewritten commits"
 		done
 	else
 		mkdir "$REWRITTEN" &&
-		echo $ONTO > "$REWRITTEN"/root ||
+		echo $onto > "$REWRITTEN"/root ||
 			die "Could not init rewritten commits"
 	fi
 	# No cherry-pick because our first pass is to determine
@@ -946,15 +946,15 @@ else
 fi
 
 SHORTHEAD=$(git rev-parse --short $HEAD)
-SHORTONTO=$(git rev-parse --short $ONTO)
-if test -z "$REBASE_ROOT"
+SHORTONTO=$(git rev-parse --short $onto)
+if test -z "$rebase_root"
 	# this is now equivalent to ! -z "$UPSTREAM"
 then
 	SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
 	REVISIONS=$UPSTREAM...$HEAD
 	SHORTREVISIONS=$SHORTUPSTREAM..$SHORTHEAD
 else
-	REVISIONS=$ONTO...$HEAD
+	REVISIONS=$onto...$HEAD
 	SHORTREVISIONS=$SHORTHEAD
 fi
 git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
@@ -963,17 +963,17 @@ git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 	sed -n "s/^>//p" |
 while read -r shortsha1 rest
 do
-	if test t != "$PRESERVE_MERGES"
+	if test t != "$preserve_merges"
 	then
 		printf '%s\n' "pick $shortsha1 $rest" >> "$TODO"
 	else
 		sha1=$(git rev-parse $shortsha1)
-		if test -z "$REBASE_ROOT"
+		if test -z "$rebase_root"
 		then
 			preserve=t
 			for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
 			do
-				if test -f "$REWRITTEN"/$p -a \( $p != $ONTO -o $sha1 = $first_after_upstream \)
+				if test -f "$REWRITTEN"/$p -a \( $p != $onto -o $sha1 = $first_after_upstream \)
 				then
 					preserve=f
 				fi
@@ -990,7 +990,7 @@ do
 done
 
 # Watch for commits that been dropped by --cherry-pick
-if test t = "$PRESERVE_MERGES"
+if test t = "$preserve_merges"
 then
 	mkdir "$DROPPED"
 	# Save all non-cherry-picked changes
@@ -1016,7 +1016,7 @@ then
 fi
 
 test -s "$TODO" || echo noop >> "$TODO"
-test -n "$AUTOSQUASH" && rearrange_squash "$TODO"
+test -n "$autosquash" && rearrange_squash "$TODO"
 cat >> "$TODO" << EOF
 
 # Rebase $SHORTREVISIONS onto $SHORTONTO
@@ -1044,8 +1044,8 @@ git_editor "$TODO" ||
 has_action "$TODO" ||
 	die_abort "Nothing to do"
 
-test -d "$REWRITTEN" || test -n "$NEVER_FF" || skip_unnecessary_picks
+test -d "$REWRITTEN" || test -n "$force_rebase" || skip_unnecessary_picks
 
-output git checkout $ONTO || die_abort "could not detach HEAD"
+output git checkout $onto || die_abort "could not detach HEAD"
 git update-ref ORIG_HEAD $HEAD
 do_rest
diff --git a/git-rebase.sh b/git-rebase.sh
index feda4be..a5ede42 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -42,7 +42,7 @@ When you have resolved this problem run \"git rebase --continue\".
 If you would prefer to skip this patch, instead run \"git rebase --skip\".
 To restore the original branch and stop rebasing run \"git rebase --abort\".
 "
-unset newbase
+unset onto
 strategy=recursive
 strategy_opts=
 do_merge=
@@ -223,7 +223,7 @@ do
 		;;
 	--onto)
 		test 2 -le "$#" || usage
-		newbase="$2"
+		onto="$2"
 		shift
 		;;
 	-M|-m|--m|--me|--mer|--merg|--merge)
@@ -419,7 +419,7 @@ then
 	unset root_flag
 	upstream_arg="$upstream_name"
 else
-	test -z "$newbase" && die "--root must be used with --onto"
+	test -z "$onto" && die "--root must be used with --onto"
 	unset upstream_name
 	unset upstream
 	root_flag="--root"
@@ -427,7 +427,7 @@ else
 fi
 
 # Make sure the branch to rebase onto is valid.
-onto_name=${newbase-"$upstream_name"}
+onto_name=${onto-"$upstream_name"}
 case "$onto_name" in
 *...*)
 	if	left=${onto_name%...*} right=${onto_name#*...} &&
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 09/20] rebase: align variable content
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (7 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 08/20] rebase: align variable names Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 10/20] rebase: factor out command line option processing Martin von Zweigbergk
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Make sure to interpret variables with the same name in the same way in
git-rebase.sh and git-rebase--interactive.sh

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |   14 ++++++++++----
 git-rebase.sh              |    4 +++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 41a9eb1..886f80b 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -827,11 +827,11 @@ first and then run 'git rebase --continue' again."
 	-s)
 		case "$#,$1" in
 		*,*=*)
-			strategy="-s "$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
+			strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
 		1,*)
 			usage ;;
 		*)
-			strategy="-s $2"
+			strategy="$2"
 			shift ;;
 		esac
 		;;
@@ -860,9 +860,9 @@ first and then run 'git rebase --continue' again."
 		autosquash=
 		;;
 	--onto)
+		test 2 -le "$#" || usage
+		onto="$2"
 		shift
-		onto=$(parse_onto "$1") ||
-			die "Does not point to a valid commit: $1"
 		;;
 	--)
 		shift
@@ -872,6 +872,12 @@ first and then run 'git rebase --continue' again."
 	shift
 done
 
+if test -n "$onto"
+then
+	onto=$(parse_onto "$onto") || die "Does not point to a valid commit: $1"
+fi
+test -n "$strategy" && strategy="-s $strategy"
+
 test -z "$rebase_root" -a $# -ge 1 -a $# -le 2 ||
 test ! -z "$rebase_root" -a $# -le 1 || usage
 test -d "$DOTEST" &&
diff --git a/git-rebase.sh b/git-rebase.sh
index a5ede42..ed1d4f5 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,7 +43,7 @@ If you would prefer to skip this patch, instead run \"git rebase --skip\".
 To restore the original branch and stop rebasing run \"git rebase --abort\".
 "
 unset onto
-strategy=recursive
+strategy=
 strategy_opts=
 do_merge=
 merge_dir="$GIT_DIR"/rebase-merge
@@ -125,6 +125,7 @@ call_merge () {
 	then
 		GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
 	fi
+	test -z "$strategy" && strategy=recursive
 	eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
 	rv=$?
 	case "$rv" in
@@ -245,6 +246,7 @@ do
 		esac
 		strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--$newopt")"
 		do_merge=t
+		test -z "$strategy" && strategy=recursive
 		;;
 	-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
 		--strateg=*|--strategy=*|\
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 10/20] rebase: factor out command line option processing
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (8 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 09/20] rebase: align variable content Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 11/20] rebase -i: remove now unnecessary directory checks Martin von Zweigbergk
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Factor out the command line processing in git-rebase--interactive.sh
to git-rebase.sh. Store the options in variables in git-rebase.sh and
export them before calling git-rebase--interactive.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |  223 ++++++++++++-------------------------------
 git-rebase.sh              |   45 +++++----
 2 files changed, 88 insertions(+), 180 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 886f80b..6389a9f 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -10,29 +10,6 @@
 # The original idea comes from Eric W. Biederman, in
 # http://article.gmane.org/gmane.comp.version-control.git/22407
 
-OPTIONS_KEEPDASHDASH=
-OPTIONS_SPEC="\
-git-rebase [-i] [options] [--] <upstream> [<branch>]
-git-rebase [-i] (--continue | --abort | --skip)
---
- Available options are
-v,verbose          display a diffstat of what changed upstream
-onto=              rebase onto given branch instead of upstream
-p,preserve-merges  try to recreate merges instead of ignoring them
-s,strategy=        use the given merge strategy
-no-ff              cherry-pick all commits, even if unchanged
-m,merge            always used (no-op)
-i,interactive      always used (no-op)
- Actions:
-continue           continue rebasing process
-abort              abort rebasing process and restore original branch
-skip               skip current patch and continue rebasing process
-no-verify          override pre-rebase hook from stopping the operation
-verify             allow pre-rebase hook to run
-root               rebase all reachable commmits up to the root(s)
-autosquash         move commits that begin with squash!/fixup! under -i
-"
-
 . git-sh-setup
 require_work_tree
 
@@ -105,16 +82,6 @@ AMEND="$DOTEST"/amend
 REWRITTEN_LIST="$DOTEST"/rewritten-list
 REWRITTEN_PENDING="$DOTEST"/rewritten-pending
 
-preserve_merges=
-strategy=
-onto=
-verbose=
-OK_TO_SKIP_PRE_REBASE=
-rebase_root=
-autosquash=
-test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
-force_rebase=
-
 GIT_CHERRY_PICK_HELP="\
 hint: after resolving the conflicts, mark the corrected paths
 hint: with 'git add <paths>' and run 'git rebase --continue'"
@@ -647,15 +614,6 @@ skip_unnecessary_picks () {
 	die "Could not skip unnecessary pick commands"
 }
 
-# check if no other options are set
-is_standalone () {
-	test $# -eq 2 -a "$2" = '--' &&
-	test -z "$onto" &&
-	test -z "$preserve_merges" &&
-	test -z "$strategy" &&
-	test -z "$verbose"
-}
-
 get_saved_options () {
 	test -d "$REWRITTEN" && preserve_merges=t
 	test -f "$DOTEST"/strategy && strategy="$(cat "$DOTEST"/strategy)"
@@ -743,134 +701,77 @@ parse_onto () {
 	git rev-parse --verify "$1^0"
 }
 
-while test $# != 0
-do
-	case "$1" in
-	--no-verify)
-		OK_TO_SKIP_PRE_REBASE=yes
-		;;
-	--verify)
-		OK_TO_SKIP_PRE_REBASE=
-		;;
-	--continue)
-		is_standalone "$@" || usage
-		get_saved_options
-		comment_for_reflog continue
-
-		test -d "$DOTEST" || die "No interactive rebase running"
-
-		# Sanity check
-		git rev-parse --verify HEAD >/dev/null ||
-			die "Cannot read HEAD"
-		git update-index --ignore-submodules --refresh &&
-			git diff-files --quiet --ignore-submodules ||
-			die "Working tree is dirty"
-
-		# do we have anything to commit?
-		if git diff-index --cached --quiet --ignore-submodules HEAD --
+case "$action" in
+continue)
+	get_saved_options
+	comment_for_reflog continue
+
+	test -d "$DOTEST" || die "No interactive rebase running"
+
+	# Sanity check
+	git rev-parse --verify HEAD >/dev/null ||
+		die "Cannot read HEAD"
+	git update-index --ignore-submodules --refresh &&
+		git diff-files --quiet --ignore-submodules ||
+		die "Working tree is dirty"
+
+	# do we have anything to commit?
+	if git diff-index --cached --quiet --ignore-submodules HEAD --
+	then
+		: Nothing to commit -- skip this
+	else
+		. "$AUTHOR_SCRIPT" ||
+			die "Cannot find the author identity"
+		amend=
+		if test -f "$AMEND"
 		then
-			: Nothing to commit -- skip this
-		else
-			. "$AUTHOR_SCRIPT" ||
-				die "Cannot find the author identity"
-			amend=
-			if test -f "$AMEND"
-			then
-				amend=$(git rev-parse --verify HEAD)
-				test "$amend" = $(cat "$AMEND") ||
-				die "\
+			amend=$(git rev-parse --verify HEAD)
+			test "$amend" = $(cat "$AMEND") ||
+			die "\
 You have uncommitted changes in your working tree. Please, commit them
 first and then run 'git rebase --continue' again."
-				git reset --soft HEAD^ ||
-				die "Cannot rewind the HEAD"
-			fi
-			do_with_author git commit --no-verify -F "$MSG" -e || {
-				test -n "$amend" && git reset --soft $amend
-				die "Could not commit staged changes."
-			}
+			git reset --soft HEAD^ ||
+			die "Cannot rewind the HEAD"
 		fi
+		do_with_author git commit --no-verify -F "$MSG" -e || {
+			test -n "$amend" && git reset --soft $amend
+			die "Could not commit staged changes."
+		}
+	fi
 
-		record_in_rewritten "$(cat "$DOTEST"/stopped-sha)"
+	record_in_rewritten "$(cat "$DOTEST"/stopped-sha)"
 
-		require_clean_work_tree "rebase"
-		do_rest
-		;;
-	--abort)
-		is_standalone "$@" || usage
-		get_saved_options
-		comment_for_reflog abort
-
-		git rerere clear
-		test -d "$DOTEST" || die "No interactive rebase running"
-
-		HEADNAME=$(cat "$DOTEST"/head-name)
-		HEAD=$(cat "$DOTEST"/head)
-		case $HEADNAME in
-		refs/*)
-			git symbolic-ref HEAD $HEADNAME
-			;;
-		esac &&
-		output git reset --hard $HEAD &&
-		rm -rf "$DOTEST"
-		exit
-		;;
-	--skip)
-		is_standalone "$@" || usage
-		get_saved_options
-		comment_for_reflog skip
+	require_clean_work_tree "rebase"
+	do_rest
+	;;
+abort)
+	get_saved_options
+	comment_for_reflog abort
 
-		git rerere clear
-		test -d "$DOTEST" || die "No interactive rebase running"
+	git rerere clear
+	test -d "$DOTEST" || die "No interactive rebase running"
 
-		output git reset --hard && do_rest
-		;;
-	-s)
-		case "$#,$1" in
-		*,*=*)
-			strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
-		1,*)
-			usage ;;
-		*)
-			strategy="$2"
-			shift ;;
-		esac
-		;;
-	-m)
-		# we use merge anyway
-		;;
-	-v)
-		verbose=t
-		;;
-	-p)
-		preserve_merges=t
-		;;
-	-i)
-		# yeah, we know
-		;;
-	--no-ff)
-		force_rebase=t
-		;;
-	--root)
-		rebase_root=t
-		;;
-	--autosquash)
-		autosquash=t
-		;;
-	--no-autosquash)
-		autosquash=
-		;;
-	--onto)
-		test 2 -le "$#" || usage
-		onto="$2"
-		shift
-		;;
-	--)
-		shift
-		break
+	HEADNAME=$(cat "$DOTEST"/head-name)
+	HEAD=$(cat "$DOTEST"/head)
+	case $HEADNAME in
+	refs/*)
+		git symbolic-ref HEAD $HEADNAME
 		;;
-	esac
-	shift
-done
+	esac &&
+	output git reset --hard $HEAD &&
+	rm -rf "$DOTEST"
+	exit
+	;;
+skip)
+	get_saved_options
+	comment_for_reflog skip
+
+	git rerere clear
+	test -d "$DOTEST" || die "No interactive rebase running"
+
+	output git reset --hard && do_rest
+	;;
+esac
 
 if test -n "$onto"
 then
diff --git a/git-rebase.sh b/git-rebase.sh
index ed1d4f5..df4e184 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -56,6 +56,9 @@ rebase_root=
 force_rebase=
 allow_rerere_autoupdate=
 action=
+preserve_merges=
+autosquash=
+test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
 
 read_state () {
 	if test -d "$merge_dir"
@@ -171,27 +174,14 @@ finish_rb_merge () {
 	say All done.
 }
 
-is_interactive () {
-	while test $# != 0
-	do
-		case "$1" in
-			-i|--interactive)
-				interactive_rebase=explicit
-				break
-			;;
-			-p|--preserve-merges)
-				interactive_rebase=implied
-			;;
-		esac
-		shift
-	done
-
+run_interactive_rebase () {
 	if [ "$interactive_rebase" = implied ]; then
 		GIT_EDITOR=:
 		export GIT_EDITOR
 	fi
-
-	test -n "$interactive_rebase" || test -f "$merge_dir"/interactive
+	export onto autosquash strategy strategy_opts verbose rebase_root \
+	force_rebase action preserve_merges OK_TO_SKIP_PRE_REBASE
+	exec git-rebase--interactive "$@"
 }
 
 run_pre_rebase_hook () {
@@ -206,8 +196,6 @@ run_pre_rebase_hook () {
 test -f "$apply_dir"/applying &&
 	die 'It looks like git-am is in progress. Cannot rebase.'
 
-is_interactive "$@" && exec git-rebase--interactive "$@"
-
 while test $# != 0
 do
 	case "$1" in
@@ -227,6 +215,19 @@ do
 		onto="$2"
 		shift
 		;;
+	-i|--interactive)
+		interactive_rebase=explicit
+		;;
+	-p|--preserve-merges)
+		preserve_merges=t
+		test -z "$interactive_rebase" && interactive_rebase=implied
+		;;
+	--autosquash)
+		autosquash=t
+		;;
+	--no-autosquash)
+		autosquash=
+		;;
 	-M|-m|--m|--me|--mer|--merg|--merge)
 		do_merge=t
 		;;
@@ -351,6 +352,10 @@ valuable there.'
 	test $# -eq 0 && test -z "$rebase_root" && usage
 fi
 
+test -f "$merge_dir"/interactive && interactive_rebase=explicit
+
+test -n "$action" && test -n "$interactive_rebase" && run_interactive_rebase
+
 case "$action" in
 continue)
 	git update-index --ignore-submodules --refresh &&
@@ -409,6 +414,8 @@ abort)
 	;;
 esac
 
+test -n "$interactive_rebase" && run_interactive_rebase "$@"
+
 require_clean_work_tree "rebase" "Please commit or stash them."
 
 if test -z "$rebase_root"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 11/20] rebase -i: remove now unnecessary directory checks
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (9 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 10/20] rebase: factor out command line option processing Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 12/20] rebase: reorder validation steps Martin von Zweigbergk
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Remove directory checks from git-rebase--interactive.sh that are done in
git-rebase.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |    6 ------
 git-rebase.sh              |    3 ++-
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6389a9f..07c10f3 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -706,8 +706,6 @@ continue)
 	get_saved_options
 	comment_for_reflog continue
 
-	test -d "$DOTEST" || die "No interactive rebase running"
-
 	# Sanity check
 	git rev-parse --verify HEAD >/dev/null ||
 		die "Cannot read HEAD"
@@ -749,7 +747,6 @@ abort)
 	comment_for_reflog abort
 
 	git rerere clear
-	test -d "$DOTEST" || die "No interactive rebase running"
 
 	HEADNAME=$(cat "$DOTEST"/head-name)
 	HEAD=$(cat "$DOTEST"/head)
@@ -767,7 +764,6 @@ skip)
 	comment_for_reflog skip
 
 	git rerere clear
-	test -d "$DOTEST" || die "No interactive rebase running"
 
 	output git reset --hard && do_rest
 	;;
@@ -781,8 +777,6 @@ test -n "$strategy" && strategy="-s $strategy"
 
 test -z "$rebase_root" -a $# -ge 1 -a $# -le 2 ||
 test ! -z "$rebase_root" -a $# -le 1 || usage
-test -d "$DOTEST" &&
-	die "Interactive rebase already started"
 
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
diff --git a/git-rebase.sh b/git-rebase.sh
index df4e184..2b19d2f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -324,7 +324,8 @@ then
 	test -z "$onto" &&
 	test -z "$preserve_merges" ||
 	usage
-	test -d "$merge_dir" -o -d "$apply_dir" || die "No rebase in progress?"
+	test -d "$apply_dir" -a -z "$interactive_rebase" ||
+	test -d "$merge_dir" || die "No rebase in progress?"
 else
 	# Make sure we do not have $apply_dir or $merge_dir
 	if test -z "$do_merge"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 12/20] rebase: reorder validation steps
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (10 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 11/20] rebase -i: remove now unnecessary directory checks Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 13/20] rebase: factor out reference parsing Martin von Zweigbergk
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Reorder validation steps in preparation for the validation to be factored
out from git-rebase--interactive.sh into git-rebase.sh.

The main functional difference is that the pre-rebase hook will no longer
be run if the work tree is dirty.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |    4 ++--
 git-rebase.sh              |   10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 07c10f3..f07472a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -793,12 +793,12 @@ else
 	test -z "$onto" &&
 		die "You must specify --onto when using --root"
 fi
+require_clean_work_tree "rebase" "Please commit or stash them."
+
 run_pre_rebase_hook "$UPSTREAM_ARG" "$@"
 
 comment_for_reflog start
 
-require_clean_work_tree "rebase" "Please commit or stash them."
-
 if test ! -z "$1"
 then
 	output git checkout "$1" ||
diff --git a/git-rebase.sh b/git-rebase.sh
index 2b19d2f..5b0b73a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -417,8 +417,6 @@ esac
 
 test -n "$interactive_rebase" && run_interactive_rebase "$@"
 
-require_clean_work_tree "rebase" "Please commit or stash them."
-
 if test -z "$rebase_root"
 then
 	# The upstream head must be given.  Make sure it is valid.
@@ -460,9 +458,6 @@ case "$onto_name" in
 	;;
 esac
 
-# If a hook exists, give it a chance to interrupt
-run_pre_rebase_hook "$upstream_arg" "$@"
-
 # If the branch to rebase is given, that is the branch we will rebase
 # $branch_name -- branch being rebased, or HEAD (already detached)
 # $orig_head -- commit object name of tip of the branch before rebasing
@@ -500,6 +495,8 @@ case "$#" in
 esac
 orig_head=$branch
 
+require_clean_work_tree "rebase" "Please commit or stash them."
+
 # Now we are rebasing commits $upstream..$branch (or with --root,
 # everything leading up to $branch) on top of $onto
 
@@ -521,6 +518,9 @@ then
 	fi
 fi
 
+# If a hook exists, give it a chance to interrupt
+run_pre_rebase_hook "$upstream_arg" "$@"
+
 # Detach HEAD and reset the tree
 say "First, rewinding head to replay your work on top of it..."
 git checkout -q "$onto^0" || die "could not detach HEAD"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 13/20] rebase: factor out reference parsing
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (11 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 12/20] rebase: reorder validation steps Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 14/20] rebase: factor out clean work tree check Martin von Zweigbergk
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Remove the parsing and validation of references (onto, upstream, branch)
from git-rebase--interactive.sh and rely on the information exported from
git-rebase.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |   59 +++++++------------------------------------
 git-rebase.sh              |   12 +++++---
 2 files changed, 17 insertions(+), 54 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f07472a..fe56d98 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -682,25 +682,6 @@ rearrange_squash () {
 	rm -f "$1.sq" "$1.rearranged"
 }
 
-LF='
-'
-parse_onto () {
-	case "$1" in
-	*...*)
-		if	left=${1%...*} right=${1#*...} &&
-			onto=$(git merge-base --all ${left:-HEAD} ${right:-HEAD})
-		then
-			case "$onto" in
-			?*"$LF"?* | '')
-				exit 1 ;;
-			esac
-			echo "$onto"
-			exit 0
-		fi
-	esac
-	git rev-parse --verify "$1^0"
-}
-
 case "$action" in
 continue)
 	get_saved_options
@@ -769,48 +750,28 @@ skip)
 	;;
 esac
 
-if test -n "$onto"
-then
-	onto=$(parse_onto "$onto") || die "Does not point to a valid commit: $1"
-fi
 test -n "$strategy" && strategy="-s $strategy"
 
-test -z "$rebase_root" -a $# -ge 1 -a $# -le 2 ||
-test ! -z "$rebase_root" -a $# -le 1 || usage
-
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
 
-if test -z "$rebase_root"
-then
-	UPSTREAM_ARG="$1"
-	UPSTREAM=$(git rev-parse --verify "$1") || die "Invalid base"
-	test -z "$onto" && onto=$UPSTREAM
-	shift
-else
-	UPSTREAM=
-	UPSTREAM_ARG=--root
-	test -z "$onto" &&
-		die "You must specify --onto when using --root"
-fi
 require_clean_work_tree "rebase" "Please commit or stash them."
 
-run_pre_rebase_hook "$UPSTREAM_ARG" "$@"
+run_pre_rebase_hook "$upstream_arg" "$@"
 
 comment_for_reflog start
 
-if test ! -z "$1"
+if test ! -z "$switch_to"
 then
-	output git checkout "$1" ||
-		die "Could not checkout $1"
+	output git checkout "$switch_to" ||
+		die "Could not checkout $switch_to"
 fi
 
 HEAD=$(git rev-parse --verify HEAD) || die "No HEAD?"
 mkdir "$DOTEST" || die "Could not create temporary $DOTEST"
 
 : > "$DOTEST"/interactive || die "Could not mark as interactive"
-git symbolic-ref HEAD > "$DOTEST"/head-name 2> /dev/null ||
-	echo "detached HEAD" > "$DOTEST"/head-name
+echo "$head_name" > "$DOTEST"/head-name
 
 echo $HEAD > "$DOTEST"/head
 case "$rebase_root" in
@@ -827,7 +788,7 @@ then
 	if test -z "$rebase_root"
 	then
 		mkdir "$REWRITTEN" &&
-		for c in $(git merge-base --all $HEAD $UPSTREAM)
+		for c in $(git merge-base --all $HEAD $upstream)
 		do
 			echo $onto > "$REWRITTEN"/$c ||
 				die "Could not init rewritten commits"
@@ -841,7 +802,7 @@ then
 	# parents to rewrite and skipping dropped commits would
 	# prematurely end our probe
 	MERGES_OPTION=
-	first_after_upstream="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
+	first_after_upstream="$(git rev-list --reverse --first-parent $upstream..$HEAD | head -n 1)"
 else
 	MERGES_OPTION="--no-merges --cherry-pick"
 fi
@@ -849,10 +810,10 @@ fi
 SHORTHEAD=$(git rev-parse --short $HEAD)
 SHORTONTO=$(git rev-parse --short $onto)
 if test -z "$rebase_root"
-	# this is now equivalent to ! -z "$UPSTREAM"
+	# this is now equivalent to ! -z "$upstream"
 then
-	SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
-	REVISIONS=$UPSTREAM...$HEAD
+	SHORTUPSTREAM=$(git rev-parse --short $upstream)
+	REVISIONS=$upstream...$HEAD
 	SHORTREVISIONS=$SHORTUPSTREAM..$SHORTHEAD
 else
 	REVISIONS=$onto...$HEAD
diff --git a/git-rebase.sh b/git-rebase.sh
index 5b0b73a..17b5042 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -180,7 +180,8 @@ run_interactive_rebase () {
 		export GIT_EDITOR
 	fi
 	export onto autosquash strategy strategy_opts verbose rebase_root \
-	force_rebase action preserve_merges OK_TO_SKIP_PRE_REBASE
+	force_rebase action preserve_merges OK_TO_SKIP_PRE_REBASE upstream \
+	upstream_arg switch_to head_name
 	exec git-rebase--interactive "$@"
 }
 
@@ -415,8 +416,6 @@ abort)
 	;;
 esac
 
-test -n "$interactive_rebase" && run_interactive_rebase "$@"
-
 if test -z "$rebase_root"
 then
 	# The upstream head must be given.  Make sure it is valid.
@@ -427,7 +426,7 @@ then
 	unset root_flag
 	upstream_arg="$upstream_name"
 else
-	test -z "$onto" && die "--root must be used with --onto"
+	test -z "$onto" && die "You must specify --onto when using --root"
 	unset upstream_name
 	unset upstream
 	root_flag="--root"
@@ -454,7 +453,8 @@ case "$onto_name" in
 	fi
 	;;
 *)
-	onto=$(git rev-parse --verify "${onto_name}^0") || exit
+	onto=$(git rev-parse --verify "${onto_name}^0") ||
+	die "Does not point to a valid commit: $1"
 	;;
 esac
 
@@ -495,6 +495,8 @@ case "$#" in
 esac
 orig_head=$branch
 
+test -n "$interactive_rebase" && run_interactive_rebase "$@"
+
 require_clean_work_tree "rebase" "Please commit or stash them."
 
 # Now we are rebasing commits $upstream..$branch (or with --root,
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 14/20] rebase: factor out clean work tree check
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (12 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 13/20] rebase: factor out reference parsing Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 15/20] rebase: factor out call to pre-rebase hook Martin von Zweigbergk
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Remove the check for clean work tree from git-rebase--interactive.sh and
rely on the check in git-rebase.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |    2 --
 git-rebase.sh              |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fe56d98..36bf259 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -755,8 +755,6 @@ test -n "$strategy" && strategy="-s $strategy"
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
 
-require_clean_work_tree "rebase" "Please commit or stash them."
-
 run_pre_rebase_hook "$upstream_arg" "$@"
 
 comment_for_reflog start
diff --git a/git-rebase.sh b/git-rebase.sh
index 17b5042..9d398eb 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -495,10 +495,10 @@ case "$#" in
 esac
 orig_head=$branch
 
-test -n "$interactive_rebase" && run_interactive_rebase "$@"
-
 require_clean_work_tree "rebase" "Please commit or stash them."
 
+test -n "$interactive_rebase" && run_interactive_rebase "$@"
+
 # Now we are rebasing commits $upstream..$branch (or with --root,
 # everything leading up to $branch) on top of $onto
 
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 15/20] rebase: factor out call to pre-rebase hook
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (13 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 14/20] rebase: factor out clean work tree check Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:57 ` [PATCH/RFC 16/20] rebase -i: support --stat Martin von Zweigbergk
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Remove the call to the pre-rebase hook from
git-rebase--interactive.sh and rely on the call in
git-rebase.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |   14 --------------
 git-rebase.sh              |   15 ++++++++-------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 36bf259..d60977d 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -110,18 +110,6 @@ commit_message () {
 	git cat-file commit "$1" | sed "1,/^$/d"
 }
 
-run_pre_rebase_hook () {
-	if test -z "$OK_TO_SKIP_PRE_REBASE" &&
-	   test -x "$GIT_DIR/hooks/pre-rebase"
-	then
-		"$GIT_DIR/hooks/pre-rebase" ${1+"$@"} || {
-			echo >&2 "The pre-rebase hook refused to rebase."
-			exit 1
-		}
-	fi
-}
-
-
 ORIG_REFLOG_ACTION="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -755,8 +743,6 @@ test -n "$strategy" && strategy="-s $strategy"
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
 
-run_pre_rebase_hook "$upstream_arg" "$@"
-
 comment_for_reflog start
 
 if test ! -z "$switch_to"
diff --git a/git-rebase.sh b/git-rebase.sh
index 9d398eb..11cea3c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -180,9 +180,8 @@ run_interactive_rebase () {
 		export GIT_EDITOR
 	fi
 	export onto autosquash strategy strategy_opts verbose rebase_root \
-	force_rebase action preserve_merges OK_TO_SKIP_PRE_REBASE upstream \
-	upstream_arg switch_to head_name
-	exec git-rebase--interactive "$@"
+	force_rebase action preserve_merges upstream switch_to head_name
+	exec git-rebase--interactive
 }
 
 run_pre_rebase_hook () {
@@ -497,15 +496,15 @@ orig_head=$branch
 
 require_clean_work_tree "rebase" "Please commit or stash them."
 
-test -n "$interactive_rebase" && run_interactive_rebase "$@"
-
 # Now we are rebasing commits $upstream..$branch (or with --root,
 # everything leading up to $branch) on top of $onto
 
 # Check if we are already based on $onto with linear history,
-# but this should be done only when upstream and onto are the same.
+# but this should be done only when upstream and onto are the same
+# and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$branch")
-if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
+	test "$mb" = "$onto" &&
 	# linear history?
 	! (git rev-list --parents "$onto".."$branch" | sane_grep " .* ") > /dev/null
 then
@@ -523,6 +522,8 @@ fi
 # If a hook exists, give it a chance to interrupt
 run_pre_rebase_hook "$upstream_arg" "$@"
 
+test -n "$interactive_rebase" && run_interactive_rebase
+
 # Detach HEAD and reset the tree
 say "First, rewinding head to replay your work on top of it..."
 git checkout -q "$onto^0" || die "could not detach HEAD"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 16/20] rebase -i: support --stat
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (14 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 15/20] rebase: factor out call to pre-rebase hook Martin von Zweigbergk
@ 2010-11-25 19:57 ` Martin von Zweigbergk
  2010-11-25 19:58 ` [PATCH/RFC 17/20] rebase: improve detection of rebase in progress Martin von Zweigbergk
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:57 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Move up the code that displays the diffstat if '--stat' is passed, so
that it will be executed before calling git-rebase--interactive.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 11cea3c..43cab41 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -522,13 +522,6 @@ fi
 # If a hook exists, give it a chance to interrupt
 run_pre_rebase_hook "$upstream_arg" "$@"
 
-test -n "$interactive_rebase" && run_interactive_rebase
-
-# Detach HEAD and reset the tree
-say "First, rewinding head to replay your work on top of it..."
-git checkout -q "$onto^0" || die "could not detach HEAD"
-git update-ref ORIG_HEAD $branch
-
 if test -n "$diffstat"
 then
 	if test -n "$verbose"
@@ -539,6 +532,13 @@ then
 	GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
+test -n "$interactive_rebase" && run_interactive_rebase
+
+# Detach HEAD and reset the tree
+say "First, rewinding head to replay your work on top of it..."
+git checkout -q "$onto^0" || die "could not detach HEAD"
+git update-ref ORIG_HEAD $branch
+
 # If the $onto is a proper descendant of the tip of the branch, then
 # we just fast-forwarded.
 if test "$mb" = "$branch"
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 17/20] rebase: improve detection of rebase in progress
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (15 preceding siblings ...)
  2010-11-25 19:57 ` [PATCH/RFC 16/20] rebase -i: support --stat Martin von Zweigbergk
@ 2010-11-25 19:58 ` Martin von Zweigbergk
  2010-11-25 19:58 ` [PATCH/RFC 18/20] rebase -m: extract code to new source file Martin von Zweigbergk
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:58 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Detect early on if a rebase is in progress and what kind of rebase it
is. This prepares for further refactoring where plain (am) rebase will
be dispatched to git-rebase--am.sh and merge rebase will be dispatched
to git-rebase--merge.sh.

The idea is to use the same variables whether the type of rebase was
detected from rebase-apply/ or rebase-merge/ directories or from on
the command line options.

Also show a consistent error message independent of the kind of rebase
that was in progress. Also remove the obsolete wording about an being
in the middle of a 'patch application', since that (an existing
"$GIT_DIR"/rebase-apply/applying) aborts 'git rebase' at an earlier
stage.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase.sh |   74 ++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 43cab41..d233da6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -59,16 +59,19 @@ action=
 preserve_merges=
 autosquash=
 test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
+# Non-empty if a rebase was in progress when 'git rebase' was invoked
+in_progress=
+# One of {am, merge, interactive}
+type=
+# One of {"$GIT_DIR"/rebase-apply, "$GIT_DIR"/rebase-merge}
+state_dir=
 
 read_state () {
-	if test -d "$merge_dir"
+	if test "$type" = merge
 	then
-		state_dir="$merge_dir"
 		onto_name=$(cat "$merge_dir"/onto_name) &&
 		end=$(cat "$merge_dir"/end) &&
 		msgnum=$(cat "$merge_dir"/msgnum)
-	else
-		state_dir="$apply_dir"
 	fi &&
 	head_name=$(cat "$state_dir"/head-name) &&
 	onto=$(cat "$state_dir"/onto) &&
@@ -196,6 +199,23 @@ run_pre_rebase_hook () {
 test -f "$apply_dir"/applying &&
 	die 'It looks like git-am is in progress. Cannot rebase.'
 
+if test -d "$apply_dir"
+then
+	type=am
+	state_dir="$apply_dir"
+elif test -d "$merge_dir"
+then
+	if test -f "$merge_dir"/interactive
+	then
+		type=interactive
+		interactive_rebase=explicit
+	else
+		type=merge
+	fi
+	state_dir="$merge_dir"
+fi
+test -n "$type" && in_progress=t
+
 while test $# != 0
 do
 	case "$1" in
@@ -324,37 +344,24 @@ then
 	test -z "$onto" &&
 	test -z "$preserve_merges" ||
 	usage
-	test -d "$apply_dir" -a -z "$interactive_rebase" ||
-	test -d "$merge_dir" || die "No rebase in progress?"
+	test -z "$in_progress" && die "No rebase in progress?"
 else
-	# Make sure we do not have $apply_dir or $merge_dir
-	if test -z "$do_merge"
+	# Make sure we do not have a rebase in progress
+	if test -n "$in_progress"
 	then
-		if mkdir "$apply_dir" 2>/dev/null
-		then
-			rmdir "$apply_dir"
-		else
-			echo >&2 '
-It seems that I cannot create a rebase-apply directory, and
-I wonder if you are in the middle of patch application or another
-rebase.  If that is not the case, please
-	rm -fr '"$apply_dir"'
+		die '
+It seems that there is already a "${state_dir##*/}" directory, and
+I wonder if you are in the middle of another rebase.  If that is the
+case, please try
+	git rebase (--continue | --abort | --skip)
+If that is not the case, please
+	rm -fr '"$state_dir"'
 and run me again.  I am stopping in case you still have something
 valuable there.'
-		exit 1
-		fi
-	else
-		if test -d "$merge_dir"
-		then
-			die "previous rebase directory $merge_dir still exists." \
-				'Try git rebase (--continue | --abort | --skip)'
-		fi
 	fi
 	test $# -eq 0 && test -z "$rebase_root" && usage
 fi
 
-test -f "$merge_dir"/interactive && interactive_rebase=explicit
-
 test -n "$action" && test -n "$interactive_rebase" && run_interactive_rebase
 
 case "$action" in
@@ -415,6 +422,19 @@ abort)
 	;;
 esac
 
+if test -n "$interactive_rebase"
+then
+	type=interactive
+	state_dir="$merge_dir"
+elif test -n "$do_merge"
+then
+	type=merge
+	state_dir="$merge_dir"
+else
+	type=am
+	state_dir="$apply_dir"
+fi
+
 if test -z "$rebase_root"
 then
 	# The upstream head must be given.  Make sure it is valid.
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 18/20] rebase -m: extract code to new source file
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (16 preceding siblings ...)
  2010-11-25 19:58 ` [PATCH/RFC 17/20] rebase: improve detection of rebase in progress Martin von Zweigbergk
@ 2010-11-25 19:58 ` Martin von Zweigbergk
  2010-11-25 19:58 ` [PATCH/RFC 19/20] rebase: extract am " Martin von Zweigbergk
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:58 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Extract the code for merge-based rebase to git-rebase--merge.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 .gitignore           |    1 +
 Makefile             |    1 +
 git-rebase--merge.sh |  154 +++++++++++++++++++++++++++++++++++++++++++++
 git-rebase.sh        |  168 +++++---------------------------------------------
 4 files changed, 172 insertions(+), 152 deletions(-)
 create mode 100644 git-rebase--merge.sh

diff --git a/.gitignore b/.gitignore
index b279de3..8f68f8a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -103,6 +103,7 @@
 /git-read-tree
 /git-rebase
 /git-rebase--interactive
+/git-rebase--merge
 /git-receive-pack
 /git-reflog
 /git-relink
diff --git a/Makefile b/Makefile
index 8357106..213ceaf 100644
--- a/Makefile
+++ b/Makefile
@@ -397,6 +397,7 @@ SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase--interactive.sh
+SCRIPT_SH += git-rebase--merge.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-repack.sh
 SCRIPT_SH += git-request-pull.sh
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
new file mode 100644
index 0000000..8cfdcf1
--- /dev/null
+++ b/git-rebase--merge.sh
@@ -0,0 +1,154 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Junio C Hamano.
+#
+
+. git-sh-setup
+
+prec=4
+
+read_state () {
+	onto_name=$(cat "$state_dir"/onto_name) &&
+	end=$(cat "$state_dir"/end) &&
+	msgnum=$(cat "$state_dir"/msgnum)
+}
+
+continue_merge () {
+	test -d "$state_dir" || die "$state_dir directory does not exist"
+
+	unmerged=$(git ls-files -u)
+	if test -n "$unmerged"
+	then
+		echo "You still have unmerged paths in your index"
+		echo "did you forget to use git add?"
+		die "$RESOLVEMSG"
+	fi
+
+	cmt=`cat "$state_dir/current"`
+	if ! git diff-index --quiet --ignore-submodules HEAD --
+	then
+		if ! git commit --no-verify -C "$cmt"
+		then
+			echo "Commit failed, please do not call \"git commit\""
+			echo "directly, but instead do one of the following: "
+			die "$RESOLVEMSG"
+		fi
+		if test -z "$GIT_QUIET"
+		then
+			printf "Committed: %0${prec}d " $msgnum
+		fi
+		echo "$cmt $(git rev-parse HEAD^0)" >> "$state_dir/rewritten"
+	else
+		if test -z "$GIT_QUIET"
+		then
+			printf "Already applied: %0${prec}d " $msgnum
+		fi
+	fi
+	test -z "$GIT_QUIET" &&
+	GIT_PAGER='' git log --format=%s -1 "$cmt"
+
+	# onto the next patch:
+	msgnum=$(($msgnum + 1))
+	echo "$msgnum" >"$state_dir/msgnum"
+}
+
+call_merge () {
+	cmt="$(cat "$state_dir/cmt.$1")"
+	echo "$cmt" > "$state_dir/current"
+	hd=$(git rev-parse --verify HEAD)
+	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
+	msgnum=$(cat "$state_dir/msgnum")
+	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
+	eval GITHEAD_$hd='$onto_name'
+	export GITHEAD_$cmt GITHEAD_$hd
+	if test -n "$GIT_QUIET"
+	then
+		GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
+	fi
+	test -z "$strategy" && strategy=recursive
+	eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
+	rv=$?
+	case "$rv" in
+	0)
+		unset GITHEAD_$cmt GITHEAD_$hd
+		return
+		;;
+	1)
+		git rerere $allow_rerere_autoupdate
+		die "$RESOLVEMSG"
+		;;
+	2)
+		echo "Strategy: $rv $strategy failed, try another" 1>&2
+		die "$RESOLVEMSG"
+		;;
+	*)
+		die "Unknown exit code ($rv) from command:" \
+			"git-merge-$strategy $cmt^ -- HEAD $cmt"
+		;;
+	esac
+}
+
+finish_rb_merge () {
+	move_to_original_branch
+	git notes copy --for-rewrite=rebase < "$state_dir"/rewritten
+	if test -x "$GIT_DIR"/hooks/post-rewrite &&
+		test -s "$state_dir"/rewritten; then
+		"$GIT_DIR"/hooks/post-rewrite rebase < "$state_dir"/rewritten
+	fi
+	rm -r "$state_dir"
+	say All done.
+}
+
+case "$action" in
+continue)
+	read_state
+	continue_merge
+	while test "$msgnum" -le "$end"
+	do
+		call_merge "$msgnum"
+		continue_merge
+	done
+	finish_rb_merge
+	exit
+	;;
+skip)
+	read_state
+	git rerere clear
+	msgnum=$(($msgnum + 1))
+	while test "$msgnum" -le "$end"
+	do
+		call_merge "$msgnum"
+		continue_merge
+	done
+	finish_rb_merge
+	exit
+	;;
+esac
+
+mkdir -p "$state_dir"
+echo "$onto_name" > "$state_dir/onto_name"
+echo "$head_name" > "$state_dir/head-name"
+echo "$onto" > "$state_dir/onto"
+echo "$orig_head" > "$state_dir/orig-head"
+echo "$GIT_QUIET" > "$state_dir/quiet"
+
+msgnum=0
+for cmt in `git rev-list --reverse --no-merges "$revisions"`
+do
+	msgnum=$(($msgnum + 1))
+	echo "$cmt" > "$state_dir/cmt.$msgnum"
+done
+
+echo 1 >"$state_dir/msgnum"
+echo $msgnum >"$state_dir/end"
+
+end=$msgnum
+msgnum=1
+
+while test "$msgnum" -le "$end"
+do
+	call_merge "$msgnum"
+	continue_merge
+done
+
+finish_rb_merge
diff --git a/git-rebase.sh b/git-rebase.sh
index d233da6..0773968 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -48,7 +48,6 @@ strategy_opts=
 do_merge=
 merge_dir="$GIT_DIR"/rebase-merge
 apply_dir="$GIT_DIR"/rebase-apply
-prec=4
 verbose=
 diffstat=$(git config --bool rebase.stat)
 git_am_opt=
@@ -66,94 +65,13 @@ type=
 # One of {"$GIT_DIR"/rebase-apply, "$GIT_DIR"/rebase-merge}
 state_dir=
 
-read_state () {
-	if test "$type" = merge
-	then
-		onto_name=$(cat "$merge_dir"/onto_name) &&
-		end=$(cat "$merge_dir"/end) &&
-		msgnum=$(cat "$merge_dir"/msgnum)
-	fi &&
+read_basic_state () {
 	head_name=$(cat "$state_dir"/head-name) &&
 	onto=$(cat "$state_dir"/onto) &&
 	orig_head=$(cat "$state_dir"/orig-head) &&
 	GIT_QUIET=$(cat "$state_dir"/quiet)
 }
 
-continue_merge () {
-	test -d "$merge_dir" || die "$merge_dir directory does not exist"
-
-	unmerged=$(git ls-files -u)
-	if test -n "$unmerged"
-	then
-		echo "You still have unmerged paths in your index"
-		echo "did you forget to use git add?"
-		die "$RESOLVEMSG"
-	fi
-
-	cmt=`cat "$merge_dir/current"`
-	if ! git diff-index --quiet --ignore-submodules HEAD --
-	then
-		if ! git commit --no-verify -C "$cmt"
-		then
-			echo "Commit failed, please do not call \"git commit\""
-			echo "directly, but instead do one of the following: "
-			die "$RESOLVEMSG"
-		fi
-		if test -z "$GIT_QUIET"
-		then
-			printf "Committed: %0${prec}d " $msgnum
-		fi
-		echo "$cmt $(git rev-parse HEAD^0)" >> "$merge_dir/rewritten"
-	else
-		if test -z "$GIT_QUIET"
-		then
-			printf "Already applied: %0${prec}d " $msgnum
-		fi
-	fi
-	test -z "$GIT_QUIET" &&
-	GIT_PAGER='' git log --format=%s -1 "$cmt"
-
-	# onto the next patch:
-	msgnum=$(($msgnum + 1))
-	echo "$msgnum" >"$merge_dir/msgnum"
-}
-
-call_merge () {
-	cmt="$(cat "$merge_dir/cmt.$1")"
-	echo "$cmt" > "$merge_dir/current"
-	hd=$(git rev-parse --verify HEAD)
-	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
-	msgnum=$(cat "$merge_dir/msgnum")
-	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
-	eval GITHEAD_$hd='$onto_name'
-	export GITHEAD_$cmt GITHEAD_$hd
-	if test -n "$GIT_QUIET"
-	then
-		GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
-	fi
-	test -z "$strategy" && strategy=recursive
-	eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
-	rv=$?
-	case "$rv" in
-	0)
-		unset GITHEAD_$cmt GITHEAD_$hd
-		return
-		;;
-	1)
-		git rerere $allow_rerere_autoupdate
-		die "$RESOLVEMSG"
-		;;
-	2)
-		echo "Strategy: $rv $strategy failed, try another" 1>&2
-		die "$RESOLVEMSG"
-		;;
-	*)
-		die "Unknown exit code ($rv) from command:" \
-			"git-merge-$strategy $cmt^ -- HEAD $cmt"
-		;;
-	esac
-}
-
 move_to_original_branch () {
 	case "$head_name" in
 	refs/*)
@@ -166,25 +84,17 @@ move_to_original_branch () {
 	esac
 }
 
-finish_rb_merge () {
-	move_to_original_branch
-	git notes copy --for-rewrite=rebase < "$merge_dir"/rewritten
-	if test -x "$GIT_DIR"/hooks/post-rewrite &&
-		test -s "$merge_dir"/rewritten; then
-		"$GIT_DIR"/hooks/post-rewrite rebase < "$merge_dir"/rewritten
-	fi
-	rm -r "$merge_dir"
-	say All done.
-}
-
-run_interactive_rebase () {
+run_specific_rebase () {
 	if [ "$interactive_rebase" = implied ]; then
 		GIT_EDITOR=:
 		export GIT_EDITOR
 	fi
 	export onto autosquash strategy strategy_opts verbose rebase_root \
-	force_rebase action preserve_merges upstream switch_to head_name
-	exec git-rebase--interactive
+	force_rebase action preserve_merges upstream switch_to head_name \
+	in_progress state_dir head_name orig_head GIT_QUIET revisions \
+	RESOLVEMSG
+	export -f move_to_original_branch
+	test "$type" != am && exec git-rebase--$type
 }
 
 run_pre_rebase_hook () {
@@ -362,7 +272,7 @@ valuable there.'
 	test $# -eq 0 && test -z "$rebase_root" && usage
 fi
 
-test -n "$action" && test -n "$interactive_rebase" && run_interactive_rebase
+test -n "$action" && test -n "$interactive_rebase" && run_specific_rebase
 
 case "$action" in
 continue)
@@ -372,44 +282,23 @@ continue)
 		echo "mark them as resolved using git add"
 		exit 1
 	}
-	read_state
-	if test -d "$merge_dir"
-	then
-		continue_merge
-		while test "$msgnum" -le "$end"
-		do
-			call_merge "$msgnum"
-			continue_merge
-		done
-		finish_rb_merge
-		exit
-	fi
+	read_basic_state
+	run_specific_rebase
 	git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
 	move_to_original_branch
 	exit
 	;;
 skip)
 	git reset --hard HEAD || exit $?
-	read_state
-	if test -d "$merge_dir"
-	then
-		git rerere clear
-		msgnum=$(($msgnum + 1))
-		while test "$msgnum" -le "$end"
-		do
-			call_merge "$msgnum"
-			continue_merge
-		done
-		finish_rb_merge
-		exit
-	fi
+	read_basic_state
+	run_specific_rebase
 	git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
 	move_to_original_branch
 	exit
 	;;
 abort)
 	git rerere clear
-	read_state
+	read_basic_state
 	case "$head_name" in
 	refs/*)
 		git symbolic-ref HEAD $head_name ||
@@ -552,7 +441,7 @@ then
 	GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test -n "$interactive_rebase" && run_interactive_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "First, rewinding head to replay your work on top of it..."
@@ -591,33 +480,8 @@ then
 	exit $ret
 fi
 
+
 # start doing a rebase with git-merge
 # this is rename-aware if the recursive (default) strategy is used
 
-mkdir -p "$merge_dir"
-echo "$onto_name" > "$merge_dir/onto_name"
-echo "$head_name" > "$merge_dir/head-name"
-echo "$onto" > "$merge_dir/onto"
-echo "$orig_head" > "$merge_dir/orig-head"
-echo "$GIT_QUIET" > "$merge_dir/quiet"
-
-msgnum=0
-for cmt in `git rev-list --reverse --no-merges "$revisions"`
-do
-	msgnum=$(($msgnum + 1))
-	echo "$cmt" > "$merge_dir/cmt.$msgnum"
-done
-
-echo 1 >"$merge_dir/msgnum"
-echo $msgnum >"$merge_dir/end"
-
-end=$msgnum
-msgnum=1
-
-while test "$msgnum" -le "$end"
-do
-	call_merge "$msgnum"
-	continue_merge
-done
-
-finish_rb_merge
+run_specific_rebase
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 19/20] rebase: extract am code to new source file
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (17 preceding siblings ...)
  2010-11-25 19:58 ` [PATCH/RFC 18/20] rebase -m: extract code to new source file Martin von Zweigbergk
@ 2010-11-25 19:58 ` Martin von Zweigbergk
  2010-11-25 19:58 ` [PATCH/RFC 20/20] rebase: show consistent conflict resolution hint Martin von Zweigbergk
  2010-11-25 20:23 ` [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:58 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

Extract the code for am-based rebase to git-rebase--am.sh.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 .gitignore        |    1 +
 Makefile          |    1 +
 git-rebase--am.sh |   34 ++++++++++++++++++++++++++++++++++
 git-rebase.sh     |   34 +++-------------------------------
 4 files changed, 39 insertions(+), 31 deletions(-)
 create mode 100644 git-rebase--am.sh

diff --git a/.gitignore b/.gitignore
index 8f68f8a..22b7907 100644
--- a/.gitignore
+++ b/.gitignore
@@ -102,6 +102,7 @@
 /git-quiltimport
 /git-read-tree
 /git-rebase
+/git-rebase--am
 /git-rebase--interactive
 /git-rebase--merge
 /git-receive-pack
diff --git a/Makefile b/Makefile
index 213ceaf..62e9e1e 100644
--- a/Makefile
+++ b/Makefile
@@ -396,6 +396,7 @@ SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
+SCRIPT_SH += git-rebase--am.sh
 SCRIPT_SH += git-rebase--interactive.sh
 SCRIPT_SH += git-rebase--merge.sh
 SCRIPT_SH += git-rebase.sh
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
new file mode 100644
index 0000000..162dd6c
--- /dev/null
+++ b/git-rebase--am.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Junio C Hamano.
+#
+
+. git-sh-setup
+
+case "$action" in
+continue)
+	git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
+	move_to_original_branch
+	exit
+	;;
+skip)
+	git am --skip --3way --resolvemsg="$RESOLVEMSG" &&
+	move_to_original_branch
+	exit
+	;;
+esac
+
+test -n "$rebase_root" && root_flag=--root
+
+git format-patch -k --stdout --full-index --ignore-if-in-upstream \
+	--src-prefix=a/ --dst-prefix=b/ \
+	--no-renames $root_flag "$revisions" |
+git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
+move_to_original_branch
+ret=$?
+test 0 != $ret -a -d "$state_dir" &&
+	echo $head_name > "$state_dir/head-name" &&
+	echo $onto > "$state_dir/onto" &&
+	echo $orig_head > "$state_dir/orig-head" &&
+	echo "$GIT_QUIET" > "$state_dir/quiet"
+exit $ret
diff --git a/git-rebase.sh b/git-rebase.sh
index 0773968..e2e97af 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -92,9 +92,9 @@ run_specific_rebase () {
 	export onto autosquash strategy strategy_opts verbose rebase_root \
 	force_rebase action preserve_merges upstream switch_to head_name \
 	in_progress state_dir head_name orig_head GIT_QUIET revisions \
-	RESOLVEMSG
+	RESOLVEMSG git_am_opt
 	export -f move_to_original_branch
-	test "$type" != am && exec git-rebase--$type
+	exec git-rebase--$type
 }
 
 run_pre_rebase_hook () {
@@ -284,17 +284,11 @@ continue)
 	}
 	read_basic_state
 	run_specific_rebase
-	git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
-	move_to_original_branch
-	exit
 	;;
 skip)
 	git reset --hard HEAD || exit $?
 	read_basic_state
 	run_specific_rebase
-	git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
-	move_to_original_branch
-	exit
 	;;
 abort)
 	git rerere clear
@@ -331,14 +325,12 @@ then
 	shift
 	upstream=`git rev-parse --verify "${upstream_name}^0"` ||
 	die "invalid upstream $upstream_name"
-	unset root_flag
 	upstream_arg="$upstream_name"
 else
 	test -z "$onto" && die "You must specify --onto when using --root"
 	unset upstream_name
 	unset upstream
-	root_flag="--root"
-	upstream_arg="$root_flag"
+	upstream_arg=--root
 fi
 
 # Make sure the branch to rebase onto is valid.
@@ -464,24 +456,4 @@ else
 	revisions="$upstream..$orig_head"
 fi
 
-if test -z "$do_merge"
-then
-	git format-patch -k --stdout --full-index --ignore-if-in-upstream \
-		--src-prefix=a/ --dst-prefix=b/ \
-		--no-renames $root_flag "$revisions" |
-	git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
-	move_to_original_branch
-	ret=$?
-	test 0 != $ret -a -d "$apply_dir" &&
-		echo $head_name > "$apply_dir/head-name" &&
-		echo $onto > "$apply_dir/onto" &&
-		echo $orig_head > "$apply_dir/orig-head" &&
-		echo "$GIT_QUIET" > "$apply_dir/quiet"
-	exit $ret
-fi
-
-
-# start doing a rebase with git-merge
-# this is rename-aware if the recursive (default) strategy is used
-
 run_specific_rebase
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH/RFC 20/20] rebase: show consistent conflict resolution hint
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (18 preceding siblings ...)
  2010-11-25 19:58 ` [PATCH/RFC 19/20] rebase: extract am " Martin von Zweigbergk
@ 2010-11-25 19:58 ` Martin von Zweigbergk
  2010-11-25 20:23 ` [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
  20 siblings, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 19:58 UTC (permalink / raw
  To: git
  Cc: Johannes Schindelin, Johannes Sixt, Christian Couder,
	Martin von Zweigbergk

When rebase stops due to conflict, interactive rebase currently
displays a different hint to the user than non-interactive rebase
does. Use the same message latter message for both types of rebase.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--interactive.sh |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d60977d..b810197 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -82,9 +82,7 @@ AMEND="$DOTEST"/amend
 REWRITTEN_LIST="$DOTEST"/rewritten-list
 REWRITTEN_PENDING="$DOTEST"/rewritten-pending
 
-GIT_CHERRY_PICK_HELP="\
-hint: after resolving the conflicts, mark the corrected paths
-hint: with 'git add <paths>' and run 'git rebase --continue'"
+GIT_CHERRY_PICK_HELP="$RESOLVEMSG"
 export GIT_CHERRY_PICK_HELP
 
 warn () {
-- 
1.7.3.2.864.gbbb96

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 00/20] Refactor rebase
  2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
                   ` (19 preceding siblings ...)
  2010-11-25 19:58 ` [PATCH/RFC 20/20] rebase: show consistent conflict resolution hint Martin von Zweigbergk
@ 2010-11-25 20:23 ` Martin von Zweigbergk
  2010-11-26 14:10   ` Sverre Rabbelier
  20 siblings, 1 reply; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-25 20:23 UTC (permalink / raw
  To: git; +Cc: Johannes Schindelin, Johannes Sixt, Christian Couder


I forgot to say that the patches should be applied on pu.

/Martin

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head'
  2010-11-25 19:57 ` [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head' Martin von Zweigbergk
@ 2010-11-26  7:54   ` Michael J Gruber
  2010-11-26 18:45     ` Martin von Zweigbergk
  2010-11-29 21:06     ` Junio C Hamano
  0 siblings, 2 replies; 29+ messages in thread
From: Michael J Gruber @ 2010-11-26  7:54 UTC (permalink / raw
  To: Martin von Zweigbergk
  Cc: git, Johannes Schindelin, Johannes Sixt, Christian Couder

Martin von Zweigbergk venit, vidit, dixit 25.11.2010 20:57:
> The rebase state 'prev_head' is not used. Remove it.
> 
> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
> ---

The actual value of prev_head was never used. But the check for its
non-nullness made sure that git-rev-parse HEAD^0 succeeded when
$merge_dir was created. Have you made sure that we don't need that check?

Michael

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 00/20] Refactor rebase
  2010-11-25 20:23 ` [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
@ 2010-11-26 14:10   ` Sverre Rabbelier
  2010-11-26 19:16     ` Martin von Zweigbergk
  0 siblings, 1 reply; 29+ messages in thread
From: Sverre Rabbelier @ 2010-11-26 14:10 UTC (permalink / raw
  To: Martin von Zweigbergk
  Cc: git, Johannes Schindelin, Johannes Sixt, Christian Couder

Heya,

On Thu, Nov 25, 2010 at 21:23, Martin von Zweigbergk
<martin.von.zweigbergk@gmail.com> wrote:
> I forgot to say that the patches should be applied on pu.

This is undesirable. In general you should base your patches on
master, unless they depend on a multiple topics in next, in which case
it's acceptable to base them on next (or directly on that particular
topic, if there's only one).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head'
  2010-11-26  7:54   ` Michael J Gruber
@ 2010-11-26 18:45     ` Martin von Zweigbergk
  2010-11-29 21:06     ` Junio C Hamano
  1 sibling, 0 replies; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-26 18:45 UTC (permalink / raw
  To: Michael J Gruber
  Cc: Martin von Zweigbergk, git, Johannes Schindelin, Johannes Sixt,
	Christian Couder


On Fri, 26 Nov 2010, Michael J Gruber wrote:

> Martin von Zweigbergk venit, vidit, dixit 25.11.2010 20:57:
> > The rebase state 'prev_head' is not used. Remove it.
> > 
> > Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
> > ---
> 
> The actual value of prev_head was never used. But the check for its
> non-nullness made sure that git-rev-parse HEAD^0 succeeded when
> $merge_dir was created. Have you made sure that we don't need that check?

In continue_merge, the value written to prev_head is the output from
`git rev-parse HEAD^0`. The value written to prev_head when the rebase
is initiated seems to be `git rev-parse -q --verify` for some
reference. In the latter case, the rebase is aborted if the exit code
from rev-parse is non-zero. The prev_head file is only deleted when
the whole $merge_dir is deleted.

Unless I'm mistaken, the above taken together seems to mean that
$prev_head can only be empty if the output of rev-parse is empty. I'm
still way to unfamiliar with the Git C code to be able to figure out
if and when that may happen, but I'm sure you guys can help. The most
likely candidate I could think of was an orphan branch (same thing as
an unborn branch?), but even in that case was the output non-empty
(`git rev-parse HEAD^0` simply resulted in 'HEAD^0').

Do you see any case where we do need the check?


/Martin

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 00/20] Refactor rebase
  2010-11-26 14:10   ` Sverre Rabbelier
@ 2010-11-26 19:16     ` Martin von Zweigbergk
  2010-11-27  1:24       ` Sverre Rabbelier
  0 siblings, 1 reply; 29+ messages in thread
From: Martin von Zweigbergk @ 2010-11-26 19:16 UTC (permalink / raw
  To: Sverre Rabbelier
  Cc: Martin von Zweigbergk, git, Johannes Schindelin, Johannes Sixt,
	Christian Couder

On Fri, 26 Nov 2010, Sverre Rabbelier wrote:

> Heya,
> 
> On Thu, Nov 25, 2010 at 21:23, Martin von Zweigbergk
> <martin.von.zweigbergk@gmail.com> wrote:
> > I forgot to say that the patches should be applied on pu.
> 
> This is undesirable. In general you should base your patches on
> master, unless they depend on a multiple topics in next, in which case
> it's acceptable to base them on next (or directly on that particular
> topic, if there's only one).

They touch the same code as 729ec9e (rebase --abort: do not update
branch ref), 7baf9c4 (rebase: support --verify) and 92c62a3 (Porcelain
scripts: Rewrite cryptic "needs update" error message). However, it is
only 7baf9c4 that would have to be more or less redone if I did not
base the patch set on it. Do I understand correctly that I should
therefore have based them directly off of 7baf9c4?

I suppose it is not worth resending this time. Tell me if you think
otherwise.

/Martin

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 00/20] Refactor rebase
  2010-11-26 19:16     ` Martin von Zweigbergk
@ 2010-11-27  1:24       ` Sverre Rabbelier
  0 siblings, 0 replies; 29+ messages in thread
From: Sverre Rabbelier @ 2010-11-27  1:24 UTC (permalink / raw
  To: Martin von Zweigbergk, Junio C Hamano
  Cc: git, Johannes Schindelin, Johannes Sixt, Christian Couder

Heya,

On Fri, Nov 26, 2010 at 20:16, Martin von Zweigbergk
<martin.von.zweigbergk@gmail.com> wrote:
> They touch the same code as 729ec9e (rebase --abort: do not update
> branch ref), 7baf9c4 (rebase: support --verify) and 92c62a3 (Porcelain
> scripts: Rewrite cryptic "needs update" error message). However, it is
> only 7baf9c4 that would have to be more or less redone if I did not
> base the patch set on it. Do I understand correctly that I should
> therefore have based them directly off of 7baf9c4?

Yes, I think that'd be preferable, unless Junio disagrees?

> I suppose it is not worth resending this time. Tell me if you think
> otherwise.

Yeah, you can just wait to rebase with the next iteration.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head'
  2010-11-26  7:54   ` Michael J Gruber
  2010-11-26 18:45     ` Martin von Zweigbergk
@ 2010-11-29 21:06     ` Junio C Hamano
  2010-11-30  7:29       ` Michael J Gruber
  1 sibling, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2010-11-29 21:06 UTC (permalink / raw
  To: Michael J Gruber
  Cc: Martin von Zweigbergk, git, Johannes Schindelin, Johannes Sixt,
	Christian Couder

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Martin von Zweigbergk venit, vidit, dixit 25.11.2010 20:57:
>> The rebase state 'prev_head' is not used. Remove it.
>> 
>> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
>> ---
>
> The actual value of prev_head was never used. But the check for its
> non-nullness made sure that git-rev-parse HEAD^0 succeeded when
> $merge_dir was created. Have you made sure that we don't need that check?

I think we are Ok.

It was a bit sloppy of 9e4bc7d (rebase: cleanup rebasing with --merge,
2006-06-24) to use the output from `git-rev-parse HEAD^0`, which would
give you "HEAD^0" as string when HEAD does not exist.  Even though the
command itself will exit with non-zero status, the exit status was never
checked.  After 9e4bc7d (rebase: cleanup rebasing with --merge,
2006-06-24), we didn't seem to have any reason to use $prev_head, iow,
this change was probably way overdue.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head'
  2010-11-29 21:06     ` Junio C Hamano
@ 2010-11-30  7:29       ` Michael J Gruber
  0 siblings, 0 replies; 29+ messages in thread
From: Michael J Gruber @ 2010-11-30  7:29 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Martin von Zweigbergk, git, Johannes Schindelin, Johannes Sixt,
	Christian Couder

Junio C Hamano venit, vidit, dixit 29.11.2010 22:06:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Martin von Zweigbergk venit, vidit, dixit 25.11.2010 20:57:
>>> The rebase state 'prev_head' is not used. Remove it.
>>>
>>> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
>>> ---
>>
>> The actual value of prev_head was never used. But the check for its
>> non-nullness made sure that git-rev-parse HEAD^0 succeeded when
>> $merge_dir was created. Have you made sure that we don't need that check?
> 
> I think we are Ok.
> 
> It was a bit sloppy of 9e4bc7d (rebase: cleanup rebasing with --merge,
> 2006-06-24) to use the output from `git-rev-parse HEAD^0`, which would
> give you "HEAD^0" as string when HEAD does not exist.  Even though the
> command itself will exit with non-zero status, the exit status was never
> checked.  After 9e4bc7d (rebase: cleanup rebasing with --merge,
> 2006-06-24), we didn't seem to have any reason to use $prev_head, iow,
> this change was probably way overdue.

Good! I just wanted to make sure that it wasn't used in some opaque way.

Michael

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2010-11-30  7:31 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-25 19:57 [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 01/20] rebase: clearer names for directory variables Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 02/20] rebase: refactor reading of state Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 03/20] rebase: read state outside loop Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 04/20] rebase: remove unused rebase state 'prev_head' Martin von Zweigbergk
2010-11-26  7:54   ` Michael J Gruber
2010-11-26 18:45     ` Martin von Zweigbergk
2010-11-29 21:06     ` Junio C Hamano
2010-11-30  7:29       ` Michael J Gruber
2010-11-25 19:57 ` [PATCH/RFC 05/20] rebase: act on command line outside parsing loop Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 06/20] rebase: collect check for existing rebase Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 07/20] rebase: stricter check on arguments Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 08/20] rebase: align variable names Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 09/20] rebase: align variable content Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 10/20] rebase: factor out command line option processing Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 11/20] rebase -i: remove now unnecessary directory checks Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 12/20] rebase: reorder validation steps Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 13/20] rebase: factor out reference parsing Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 14/20] rebase: factor out clean work tree check Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 15/20] rebase: factor out call to pre-rebase hook Martin von Zweigbergk
2010-11-25 19:57 ` [PATCH/RFC 16/20] rebase -i: support --stat Martin von Zweigbergk
2010-11-25 19:58 ` [PATCH/RFC 17/20] rebase: improve detection of rebase in progress Martin von Zweigbergk
2010-11-25 19:58 ` [PATCH/RFC 18/20] rebase -m: extract code to new source file Martin von Zweigbergk
2010-11-25 19:58 ` [PATCH/RFC 19/20] rebase: extract am " Martin von Zweigbergk
2010-11-25 19:58 ` [PATCH/RFC 20/20] rebase: show consistent conflict resolution hint Martin von Zweigbergk
2010-11-25 20:23 ` [PATCH/RFC 00/20] Refactor rebase Martin von Zweigbergk
2010-11-26 14:10   ` Sverre Rabbelier
2010-11-26 19:16     ` Martin von Zweigbergk
2010-11-27  1:24       ` Sverre Rabbelier

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).