[PATCH 0/7] correctly calculate patches to rebase

[PATCH 0/7] correctly calculate patches to rebase

[Martin von Zweigbergk]

These seven patches replace the broken one I sent in
http://thread.gmane.org/gmane.comp.version-control.git/200644/focus=200648. I
hope I got the handling of empty commits right this time.

Martin von Zweigbergk (7):
  git-rebase--am.sh: avoid special-casing --keep-empty
  git-rebase--interactive.sh: extract function for adding "pick" line
  git-rebase--interactive: group all $preserve_merges code
  git-rebase--interactive.sh: look up subject in add_pick_line
  rebase -p: use --cherry-mark for todo file
  rebase -p: don't request --left-right only to ignore left side
  rebase (without -p): correctly calculate patches to rebase

 git-am.sh                  | 10 +++++-
 git-rebase--am.sh          | 20 +++--------
 git-rebase--interactive.sh | 87 ++++++++++++++++++++--------------------------
 git-rebase--merge.sh       |  2 +-
 git-rebase.sh              | 11 +++---
 t/t3401-rebase-partial.sh  | 17 +++++++++
 t/t3406-rebase-message.sh  | 14 ++++----
 7 files changed, 81 insertions(+), 80 deletions(-)

-- 
1.7.11.1.104.ge7b44f1

[PATCH 1/7] git-rebase--am.sh: avoid special-casing --keep-empty

[Martin von Zweigbergk]

Since 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26), the
patch body to apply when running 'git am --rebasing' is not taken from
the mbox, but directly from the commit. If such a commit is "empty",
'git am --rebasing' still happily applies it and commits. However,
since the input to 'git am --rebasing' only ever comes from 'git
format-patch', which completely leaves the commit out from its output
if it's empty, no empty commits are ever created by 'git am
--rebasing'. By teaching 'git am --rebasing' a --keep-empty option and
letting the caller decide whether or not to keep empty commits, we can
unify the two different mechanisms that git-rebase--am.sh uses for
rebasing.
---
 git-am.sh         | 10 +++++++++-
 git-rebase--am.sh | 20 ++++++--------------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index b6a5300..37641b7 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -37,7 +37,8 @@ abort           restore the original branch and abort the patching operation.
 committer-date-is-author-date    lie about committer date
 ignore-date     use current timestamp for author date
 rerere-autoupdate update the index with reused conflict resolution if possible
-rebasing*       (internal use for git-rebase)"
+rebasing*       (internal use for git-rebase)
+keep-empty*     (internal use for git-rebase)"
 
 . git-sh-setup
 . git-sh-i18n
@@ -375,6 +376,7 @@ git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
 allow_rerere_autoupdate=
+keep_empty=
 
 if test "$(git config --bool --get am.keepcr)" = true
 then
@@ -414,6 +416,8 @@ do
 		abort=t ;;
 	--rebasing)
 		rebasing=t threeway=t ;;
+	--keep-empty)
+		keep_empty=t ;;
 	-d|--dotest)
 		die "$(gettext "-d option is no longer supported.  Do not use.")"
 		;;
@@ -669,6 +673,10 @@ do
 			echo "$commit" >"$dotest/original-commit"
 			get_author_ident_from_commit "$commit" >"$dotest/author-script"
 			git diff-tree --root --binary "$commit" >"$dotest/patch"
+			test -s "$dotest/patch" || test -n "$keep_empty" || {
+				go_next
+				continue
+			}
 		else
 			git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
 				<"$dotest/$msgnum" >"$dotest/info" ||
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 392ebc9..37c1b23 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -17,20 +17,12 @@ skip)
 esac
 
 test -n "$rebase_root" && root_flag=--root
-
-if test -n "$keep_empty"
-then
-	# we have to do this the hard way.  git format-patch completely squashes
-	# empty commits and even if it didn't the format doesn't really lend
-	# itself well to recording empty patches.  fortunately, cherry-pick
-	# makes this easy
-	git cherry-pick --allow-empty "$revisions"
-else
-	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"
-fi && move_to_original_branch
+test -n "$keep_empty" && git_am_opt="$git_am_opt --keep-empty"
+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" && write_basic_state
-- 
1.7.11.1.104.ge7b44f1

[PATCH 2/7] git-rebase--interactive.sh: extract function for adding "pick" line

[Martin von Zweigbergk]

Extract the code that adds a possibly commented-out "pick" line to the
todo file. This lets us reuse it more easily later.
---
 git-rebase--interactive.sh | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..fa722b6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -828,23 +828,26 @@ else
 	revisions=$onto...$orig_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 -z "$keep_empty" && is_empty_commit $shortsha1
+add_pick_line () {
+	if test -z "$keep_empty" && is_empty_commit $1
 	then
 		comment_out="# "
 	else
 		comment_out=
 	fi
+	printf '%s\n' "${comment_out}pick $1 $2" >>"$todo"
+}
 
+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' "${comment_out}pick $shortsha1 $rest" >>"$todo"
+		add_pick_line $shortsha1 "$rest"
 	else
 		sha1=$(git rev-parse $shortsha1)
 		if test -z "$rebase_root"
@@ -863,7 +866,7 @@ do
 		if test f = "$preserve"
 		then
 			touch "$rewritten"/$sha1
-			printf '%s\n' "${comment_out}pick $shortsha1 $rest" >>"$todo"
+			add_pick_line $shortsha1 "$rest"
 		fi
 	fi
 done
-- 
1.7.11.1.104.ge7b44f1

[PATCH 3/7] git-rebase--interactive: group all $preserve_merges code

[Martin von Zweigbergk]

The code in git-rebase--interactive that creates the todo file
contains if-blocks that depend on whether $preserve_merges is
active. There is only a very small amount of code in between that is
shared with non-merge-preserving code path, so remove the repeated
conditions and duplicate the small amount of shared code instead.
---
 git-rebase--interactive.sh | 69 ++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fa722b6..4bb8e3f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -793,28 +793,6 @@ mkdir "$state_dir" || die "Could not create temporary $state_dir"
 
 : > "$state_dir"/interactive || die "Could not mark as interactive"
 write_basic_state
-if test t = "$preserve_merges"
-then
-	if test -z "$rebase_root"
-	then
-		mkdir "$rewritten" &&
-		for c in $(git merge-base --all $orig_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=
-else
-	merges_option="--no-merges --cherry-pick"
-fi
 
 shorthead=$(git rev-parse --short $orig_head)
 shortonto=$(git rev-parse --short $onto)
@@ -839,16 +817,30 @@ add_pick_line () {
 	printf '%s\n' "${comment_out}pick $1 $2" >>"$todo"
 }
 
-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"
+if test t = "$preserve_merges"
+then
+	if test -z "$rebase_root"
 	then
-		add_pick_line $shortsha1 "$rest"
+		mkdir "$rewritten" &&
+		for c in $(git merge-base --all $orig_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
+	git rev-list --pretty=oneline --abbrev-commit \
+		--abbrev=7 --reverse --left-right --topo-order \
+		$revisions |
+	sed -n "s/^>//p" |
+	while read -r shortsha1 rest
+	do
 		sha1=$(git rev-parse $shortsha1)
 		if test -z "$rebase_root"
 		then
@@ -868,12 +860,8 @@ do
 			touch "$rewritten"/$sha1
 			add_pick_line $shortsha1 "$rest"
 		fi
-	fi
-done
-
-# Watch for commits that been dropped by --cherry-pick
-if test t = "$preserve_merges"
-then
+	done
+	# Watch for commits that been dropped by --cherry-pick
 	mkdir "$dropped"
 	# Save all non-cherry-picked changes
 	git rev-list $revisions --left-right --cherry-pick | \
@@ -895,6 +883,15 @@ then
 			rm "$rewritten"/$rev
 		fi
 	done
+else
+	git rev-list --no-merges --cherry-pick --pretty=oneline --abbrev-commit \
+		--abbrev=7 --reverse --left-right --topo-order \
+		$revisions |
+	sed -n "s/^>//p" |
+	while read -r shortsha1 rest
+	do
+		add_pick_line $shortsha1 "$rest"
+	done
 fi
 
 test -s "$todo" || echo noop >> "$todo"
-- 
1.7.11.1.104.ge7b44f1

[PATCH 4/7] git-rebase--interactive.sh: look up subject in add_pick_line

[Martin von Zweigbergk]

The todo file is generated using (more-or-less) 'git rev-list
$revisions --pretty=oneline --abbrev-commit --abbrev=7', i.e. by
letting 'git rev-list' output both the abbreviated sha1 and the
subject line. To allow us to more easily generate the list of commits
to rebase by using commands that don't support outputting the subject
line, move this logic into add_pick_line.
---
 git-rebase--interactive.sh | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4bb8e3f..9715830 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -814,7 +814,8 @@ add_pick_line () {
 	else
 		comment_out=
 	fi
-	printf '%s\n' "${comment_out}pick $1 $2" >>"$todo"
+	line=$(git rev-list -1 --pretty=oneline --abbrev-commit --abbrev=7 $1)
+	printf '%s\n' "${comment_out}pick $line" >>"$todo"
 }
 
 if test t = "$preserve_merges"
@@ -835,13 +836,10 @@ then
 	# No cherry-pick because our first pass is to determine
 	# parents to rewrite and skipping dropped commits would
 	# prematurely end our probe
-	git rev-list --pretty=oneline --abbrev-commit \
-		--abbrev=7 --reverse --left-right --topo-order \
-		$revisions |
+	git rev-list $revisions --reverse --left-right --topo-order |
 	sed -n "s/^>//p" |
-	while read -r shortsha1 rest
+	while read -r sha1
 	do
-		sha1=$(git rev-parse $shortsha1)
 		if test -z "$rebase_root"
 		then
 			preserve=t
@@ -858,7 +856,7 @@ then
 		if test f = "$preserve"
 		then
 			touch "$rewritten"/$sha1
-			add_pick_line $shortsha1 "$rest"
+			add_pick_line $sha1
 		fi
 	done
 	# Watch for commits that been dropped by --cherry-pick
@@ -884,13 +882,12 @@ then
 		fi
 	done
 else
-	git rev-list --no-merges --cherry-pick --pretty=oneline --abbrev-commit \
-		--abbrev=7 --reverse --left-right --topo-order \
-		$revisions |
+	git rev-list $revisions --reverse --left-right --topo-order \
+		--no-merges --cherry-pick |
 	sed -n "s/^>//p" |
-	while read -r shortsha1 rest
+	while read -r sha1
 	do
-		add_pick_line $shortsha1 "$rest"
+		add_pick_line $sha1
 	done
 fi
 
-- 
1.7.11.1.104.ge7b44f1

[PATCH 5/7] rebase -p: use --cherry-mark for todo file

[Martin von Zweigbergk]

While building the todo file, 'rebase -p' needs to find the
cherry-picked commits in the branch that is about to be rebased. For
this, it calculates the set difference between the full set of commits
and the non-cherry-picked ones (as reported by 'git rev-list
--left-right --cherry-pick'). Now that have the 'git rev-list
--cherry-mark' option (since adbbb31 (revision.c: introduce
--cherry-mark, 2011-03-07)), we can instead use that option to get the
set of cherry-picked commits.
---
 git-rebase--interactive.sh | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9715830..47beb58 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -859,17 +859,12 @@ then
 			add_pick_line $sha1
 		fi
 	done
-	# Watch for commits that been dropped by --cherry-pick
+	# Now drop cherry-picked commits
 	mkdir "$dropped"
-	# Save all non-cherry-picked changes
-	git rev-list $revisions --left-right --cherry-pick | \
-		sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
-	# Now all commits and note which ones are missing in
-	# not-cherry-picks and hence being dropped
-	git rev-list $revisions |
+	git rev-list $revisions --cherry-mark --right-only | sed -ne "s/^=//p" |
 	while read rev
 	do
-		if test -f "$rewritten"/$rev -a "$(sane_grep "$rev" "$state_dir"/not-cherry-picks)" = ""
+		if test -f "$rewritten"/$rev
 		then
 			# Use -f2 because if rev-list is telling us this commit is
 			# not worthwhile, we don't want to track its multiple heads,
-- 
1.7.11.1.104.ge7b44f1

[PATCH 6/7] rebase -p: don't request --left-right only to ignore left side

[Martin von Zweigbergk]

While generating the todo file, rebase -p calls 'git rev-list
--left-right a...b' (with 'a' equal to $upstream or $onto and 'b'
equal to $orig_head) and its output is piped through 'sed -n
"s/^>//p"', making it equivalent to 'git rev-list --right-only
a...b'. Change the invocation to exactly that.

(One could alternatively change it to 'git rev-list a..b', which would
be even simpler, if it wasn't for the fact that we already have the
revision range expression in a variable.)
---
 git-rebase--interactive.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 47beb58..cd5a2cc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -836,8 +836,7 @@ then
 	# No cherry-pick because our first pass is to determine
 	# parents to rewrite and skipping dropped commits would
 	# prematurely end our probe
-	git rev-list $revisions --reverse --left-right --topo-order |
-	sed -n "s/^>//p" |
+	git rev-list $revisions --reverse --right-only --topo-order |
 	while read -r sha1
 	do
 		if test -z "$rebase_root"
-- 
1.7.11.1.104.ge7b44f1

[PATCH 7/7] rebase (without -p): correctly calculate patches to rebase

[Martin von Zweigbergk]

The different types of rebase use different ways of calculating the
patches to rebase.

'git rebase' (without -m/-i/-p) uses

  git format-patch --ignore-if-in-upstream $upstream..$orig_head

'git rebase -m' uses

  git rev-list $upstream..$orig_head

'git rebase -i' (without -p) uses

  git rev-list $upstream...$orig_head --left-right --no-merges \
          --cherry-pick | sed -n "s/^>//p"

, which could also have been written

  git rev-list $upstream...$orig_head --right-only --no-merges \
          --cherry-pick

'git rebase -p' uses

  git rev-list $upstream...$orig_head --right-only

followed by cherry-picked commits found by

  git rev-list $upstream...$orig_head --cherry-mark --right-only |
  | sed -ne "s/^=//p"

As Knut Franke reported in [1], the fact that there is no
--ignore-if-in-upstream or equivalent when using merge-based rebase
means that unnecessary conflicts can arise due to commits
cherry-picked between $orig_head and $upstream.

With all the other types, there is a different problem with the method
of calculating the commits to rebase. Copying the example history from
[1]:

      .-c
     /
a---b---d---e---f
         \
          .-g---E

Commit E is here a cherry-pick of e. If we now run 'git rebase [-i|-p]
--onto c f E', the commits to rebase will be those on E that are not
equivalent to any of those in f, which in this case would be only
'g'. Commit 'E' would thus be lost.

To solve both of the above problems, we want to find the commits in
$upstream..$orig_head that are not cherry-picked in
$orig_head..$onto. There is unfortunately no direct way of finding
these commits using 'git rev-list', so we will have to resort to using
'git cherry' and filter for lines starting with '+'. This works for
all but 'rebase -p', since 'git cherry' ignores merges.

As a side-effect, we also avoid the cost of formatting patches.

Test case updates for 'rebase -m' by Knut, the rest by Martin.

 [1] http://thread.gmane.org/gmane.comp.version-control.git/161917

Helped-by: Knut Franke <Knut.Franke@gmx.de>
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 git-rebase--am.sh          |  6 ++----
 git-rebase--interactive.sh |  8 +++-----
 git-rebase--merge.sh       |  2 +-
 git-rebase.sh              | 11 ++++-------
 t/t3401-rebase-partial.sh  | 17 +++++++++++++++++
 t/t3406-rebase-message.sh  | 14 +++++++-------
 6 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 37c1b23..fe3fdd1 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -16,11 +16,9 @@ skip)
 	;;
 esac
 
-test -n "$rebase_root" && root_flag=--root
 test -n "$keep_empty" && git_am_opt="$git_am_opt --keep-empty"
-git format-patch -k --stdout --full-index --ignore-if-in-upstream \
-	--src-prefix=a/ --dst-prefix=b/ \
-	--no-renames $root_flag "$revisions" |
+generate_revisions |
+sed -e 's/\([0-9a-f]\{40\}\)/From \1 Mon Sep 17 00:00:00 2001/' |
 git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" &&
 move_to_original_branch
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index cd5a2cc..da32ca7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -800,10 +800,8 @@ if test -z "$rebase_root"
 	# this is now equivalent to ! -z "$upstream"
 then
 	shortupstream=$(git rev-parse --short $upstream)
-	revisions=$upstream...$orig_head
 	shortrevisions=$shortupstream..$shorthead
 else
-	revisions=$onto...$orig_head
 	shortrevisions=$shorthead
 fi
 
@@ -822,6 +820,7 @@ if test t = "$preserve_merges"
 then
 	if test -z "$rebase_root"
 	then
+		revisions=$upstream...$orig_head
 		mkdir "$rewritten" &&
 		for c in $(git merge-base --all $orig_head $upstream)
 		do
@@ -829,6 +828,7 @@ then
 				die "Could not init rewritten commits"
 		done
 	else
+		revisions=$onto...$orig_head
 		mkdir "$rewritten" &&
 		echo $onto > "$rewritten"/root ||
 			die "Could not init rewritten commits"
@@ -876,9 +876,7 @@ then
 		fi
 	done
 else
-	git rev-list $revisions --reverse --left-right --topo-order \
-		--no-merges --cherry-pick |
-	sed -n "s/^>//p" |
+	generate_revisions |
 	while read -r sha1
 	do
 		add_pick_line $sha1
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index b10f2cf..bf4ec4b 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -131,7 +131,7 @@ echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
 
 msgnum=0
-for cmt in `git rev-list --reverse --no-merges "$revisions"`
+for cmt in $(generate_revisions)
 do
 	msgnum=$(($msgnum + 1))
 	echo "$cmt" > "$state_dir/cmt.$msgnum"
diff --git a/git-rebase.sh b/git-rebase.sh
index 1cd0633..0fdff87 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -530,6 +530,10 @@ then
 	GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
+generate_revisions () {
+	git cherry $onto $orig_head $upstream | sed -ne 's/^+ //p'
+}
+
 test "$type" = interactive && run_specific_rebase
 
 # Detach HEAD and reset the tree
@@ -546,11 +550,4 @@ then
 	exit 0
 fi
 
-if test -n "$rebase_root"
-then
-	revisions="$onto..$orig_head"
-else
-	revisions="$upstream..$orig_head"
-fi
-
 run_specific_rebase
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 7f8693b..9c5d815 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -50,4 +50,21 @@ test_expect_success 'rebase ignores empty commit' '
 	test $(git log --format=%s C..) = "D"
 '
 
+test_expect_success 'rebase --onto does not re-apply patches in $onto' '
+	git checkout C &&
+	test_commit C2 C.t &&
+	git checkout -B my-topic-branch master &&
+	test_commit E &&
+	git rebase --onto C2 A2 &&
+	test "$(git log --format=%s C2..)" = E
+'
+
+test_expect_success 'rebase --onto does not lose patches in $upstream' '
+	git rebase --onto A2 E &&
+	test "$(git log --format=%s A2..)" = "E
+C2
+C
+B"
+'
+
 test_done
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 6898377..3eecc66 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -5,8 +5,10 @@ test_description='messages from rebase operation'
 . ./test-lib.sh
 
 quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
+	fileno=$2
+	test -z "$fileno" && fileno=$1
+	echo "$1" >"file$fileno" &&
+	git add "file$fileno" &&
 	test_tick &&
 	git commit -m "$1"
 }
@@ -16,21 +18,19 @@ test_expect_success setup '
 	git branch topic &&
 	quick_one X &&
 	quick_one A &&
-	quick_one B &&
+	quick_one B A &&
 	quick_one Y &&
 
 	git checkout topic &&
 	quick_one A &&
-	quick_one B &&
+	quick_one B A &&
 	quick_one Z &&
 	git tag start
 
 '
 
 cat >expect <<\EOF
-Already applied: 0001 A
-Already applied: 0002 B
-Committed: 0003 Z
+Committed: 0001 Z
 EOF
 
 test_expect_success 'rebase -m' '
-- 
1.7.11.1.104.ge7b44f1

Re: [PATCH 0/7] correctly calculate patches to rebase

[Martin von Zweigbergk]

Argh! Sorry about the sendemail.chainreplyto=true. I must have read
that warning message incorrectly :-(.

Re: [PATCH 2/7] git-rebase--interactive.sh: extract function for adding "pick" line

[Neil Horman]

On Wed, Jul 18, 2012 at 12:27:30AM -0700, Martin von Zweigbergk wrote:
> Extract the code that adds a possibly commented-out "pick" line to the
> todo file. This lets us reuse it more easily later.
> ---
>  git-rebase--interactive.sh | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index bef7bc0..fa722b6 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -828,23 +828,26 @@ else
>  	revisions=$onto...$orig_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 -z "$keep_empty" && is_empty_commit $shortsha1
> +add_pick_line () {
> +	if test -z "$keep_empty" && is_empty_commit $1
>  	then
>  		comment_out="# "
>  	else
>  		comment_out=
>  	fi
> +	printf '%s\n' "${comment_out}pick $1 $2" >>"$todo"
> +}
>  
> +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' "${comment_out}pick $shortsha1 $rest" >>"$todo"
> +		add_pick_line $shortsha1 "$rest"
>  	else
>  		sha1=$(git rev-parse $shortsha1)
>  		if test -z "$rebase_root"
> @@ -863,7 +866,7 @@ do
>  		if test f = "$preserve"
>  		then
>  			touch "$rewritten"/$sha1
> -			printf '%s\n' "${comment_out}pick $shortsha1 $rest" >>"$todo"
> +			add_pick_line $shortsha1 "$rest"
>  		fi
>  	fi
>  done
> -- 
> 1.7.11.1.104.ge7b44f1
> 
> 

Thanks!
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH 4/7] git-rebase--interactive.sh: look up subject in add_pick_line

[Johannes Sixt]

Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
> @@ -814,7 +814,8 @@ add_pick_line () {
>  	else
>  		comment_out=
>  	fi
> -	printf '%s\n' "${comment_out}pick $1 $2" >>"$todo"
> +	line=$(git rev-list -1 --pretty=oneline --abbrev-commit --abbrev=7 $1)
> +	printf '%s\n' "${comment_out}pick $line" >>"$todo"

I don't like this. On Windows, rebase -i is already slow, and these extra
processes will make it even slower.

> +	git rev-list $revisions --reverse --left-right --topo-order |
>  	sed -n "s/^>//p" |
> -	while read -r shortsha1 rest
> +	while read -r sha1
>  	do
> -		sha1=$(git rev-parse $shortsha1)
>  		if test -z "$rebase_root"
...
> -			add_pick_line $shortsha1 "$rest"
> +			add_pick_line $sha1
>  		fi

This is 'rebase -p' case, and you trade the new processes for some old ones.

> +	git rev-list $revisions --reverse --left-right --topo-order \
> +		--no-merges --cherry-pick |
>  	sed -n "s/^>//p" |
> -	while read -r shortsha1 rest
> +	while read -r sha1
>  	do
> -		add_pick_line $shortsha1 "$rest"
> +		add_pick_line $sha1
>  	done

But in the regulare case, you don't; the processes are really new.

Anything that can be done about this? Perhaps the rev-list call can
generate all of the full SHA1, the short SHA1, and the subject with a
--pretty format?

-- Hannes

Re: [PATCH 7/7] rebase (without -p): correctly calculate patches to rebase

[Johannes Sixt]

Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 37c1b23..fe3fdd1 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -16,11 +16,9 @@ skip)
>  	;;
>  esac
>  
> -test -n "$rebase_root" && root_flag=--root
>  test -n "$keep_empty" && git_am_opt="$git_am_opt --keep-empty"
> -git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> -	--src-prefix=a/ --dst-prefix=b/ \
> -	--no-renames $root_flag "$revisions" |
> +generate_revisions |
> +sed -e 's/\([0-9a-f]\{40\}\)/From \1 Mon Sep 17 00:00:00 2001/' |
>  git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" &&
>  move_to_original_branch

Just curious (as all tests pass): What does this do? It looks like
format-patch is not called anymore and git-am sees only SHA1s. Does it
force git-am to cherry-pick the patches?

-- Hannes

Re: [PATCH 4/7] git-rebase--interactive.sh: look up subject in add_pick_line

[Martin von Zweigbergk]

Thanks for reviewing.

On Fri, Jul 20, 2012 at 1:14 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
>> @@ -814,7 +814,8 @@ add_pick_line () {
>>       else
>>               comment_out=
>>       fi
>> -     printf '%s\n' "${comment_out}pick $1 $2" >>"$todo"
>> +     line=$(git rev-list -1 --pretty=oneline --abbrev-commit --abbrev=7 $1)
>> +     printf '%s\n' "${comment_out}pick $line" >>"$todo"
>
> I don't like this. On Windows, rebase -i is already slow, and these extra
> processes will make it even slower.

I don't like it either :-(.

> Anything that can be done about this? Perhaps the rev-list call can
> generate all of the full SHA1, the short SHA1, and the subject with a
> --pretty format?

After patch 7/7, cherry is used instead of rev-list. Ideally, I would
have liked to teach "git rev-list --cherry-pick" to somehow use a
<limit> just like cherry does, but I couldn't think of a generic way
of doing that (in this case, we want to say something like "range
a..b, but drop commits that are equivalent to any in b..c"). I
actually don't remember if I gave up because I couldn't think of a
sensible way of specifying ranges like that, or if I just ran out of
time (not familiar with the revision-walking code). Now it seems to me
that something like "git rev-list a..b --not-cherry-picks b..c" makes
sense, but maybe it's just too specific and we should just support the
limited (no pun intended) case we need to emulate "git cherry", i.e.
something like "git rev-list --cherry-with-limit=a c...b". Feedback
appreciated.

Martin

Re: [PATCH 7/7] rebase (without -p): correctly calculate patches to rebase

[Martin von Zweigbergk]

On Fri, Jul 20, 2012 at 1:18 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
>> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
>> index 37c1b23..fe3fdd1 100644
>> --- a/git-rebase--am.sh
>> +++ b/git-rebase--am.sh
>> @@ -16,11 +16,9 @@ skip)
>>       ;;
>>  esac
>>
>> -test -n "$rebase_root" && root_flag=--root
>>  test -n "$keep_empty" && git_am_opt="$git_am_opt --keep-empty"
>> -git format-patch -k --stdout --full-index --ignore-if-in-upstream \
>> -     --src-prefix=a/ --dst-prefix=b/ \
>> -     --no-renames $root_flag "$revisions" |
>> +generate_revisions |
>> +sed -e 's/\([0-9a-f]\{40\}\)/From \1 Mon Sep 17 00:00:00 2001/' |
>>  git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" &&
>>  move_to_original_branch
>
> Just curious (as all tests pass): What does this do? It looks like
> format-patch is not called anymore and git-am sees only SHA1s. Does it
> force git-am to cherry-pick the patches?

That probably deserves to be mentioned in the commit message. Or maybe
in as a comment in the code. Either way, since 0fbb95d (am: don't call
mailinfo if $rebasing, 2012-06-26), 'git am --rebasing' never looks at
anything but the sha1, so most of the output from 'git format-patch'
is currently ignored. It doesn't do cherry-pick, though, but runs 'git
diff-tree' and other commands and then feeds the result to 'git
apply', just like a regular 'git am' invocation would.

Martin

Re: [PATCH 4/7] git-rebase--interactive.sh: look up subject in add_pick_line

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> After patch 7/7, cherry is used instead of rev-list. Ideally, I would
> have liked to teach "git rev-list --cherry-pick" to somehow use a
> <limit> just like cherry does, but I couldn't think of a generic way
> of doing that (in this case, we want to say something like "range
> a..b, but drop commits that are equivalent to any in b..c"). I
> actually don't remember if I gave up because I couldn't think of a
> sensible way of specifying ranges like that, or if I just ran out of
> time (not familiar with the revision-walking code).

Why not use patch-id output instead, then?  Grab patch-id for
commits b..c to make a mapping from patch-id to commits, do the same
for a..b and use the mapping to filter?