threads / patch / 31048

patch, 7 partsgit-rebase--interactive: group all $preserve_merges code

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

## tl;dr

15 messages between Jul 18, 2012 and Jul 22, 2012. Diffs are folded; open one to read it.

replies: 14people: 4as markdown or json

Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · lore

[PATCH 0/7] correctly calculate patches to rebase

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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to 2 files +15 −15

git-am.sh, git-rebase--am.sh

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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to git-rebase--interactive.sh +12 −9
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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore
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(-)
Show changes to git-rebase--interactive.sh +33 −36
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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to git-rebase--interactive.sh +9 −12
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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to git-rebase--interactive.sh +3 −8
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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to git-rebase--interactive.sh +1 −2
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
Martin von Zweigbergk· Jul 18, 2012, 07:27 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to 6 files +34 −24

git-rebase--am.sh, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, t/t3401-rebase-partial.sh, t/t3406-rebase-message.sh

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
Johannes Sixt· Jul 20, 2012, 08:18 UTC · re: Martin von Zweigbergk · lore

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

Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
Show 17 quoted lines
> 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
Martin von Zweigbergk· Jul 20, 2012, 15:58 UTC · re: Johannes Sixt · lore

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

On Fri, Jul 20, 2012 at 1:18 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 22 quoted lines
> 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
Johannes Sixt· Jul 20, 2012, 08:14 UTC · re: Martin von Zweigbergk · lore

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

Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
Show 7 quoted lines
> @@ -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.

Show 7 quoted lines
> +	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.
Show 9 quoted lines
> +	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
Martin von Zweigbergk· Jul 20, 2012, 15:47 UTC · re: Johannes Sixt · lore

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

Thanks for reviewing.
On Fri, Jul 20, 2012 at 1:14 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 11 quoted lines
> 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
Junio C Hamano· Jul 22, 2012, 20:51 UTC · re: Martin von Zweigbergk · lore

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

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
Show 8 quoted lines
> 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?

Neil Horman· Jul 18, 2012, 12:48 UTC · re: Martin von Zweigbergk · lore

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

On Wed, Jul 18, 2012 at 12:27:30AM -0700, Martin von Zweigbergk wrote:
Show 58 quoted lines
> 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>

← back to recent threads