threads / discuss / 41012

(unknown)

Subject: (unknown)

## tl;dr

5 messages between Dec 16, 2015 and Dec 18, 2015.

replies: 4people: 3as markdown or json

David Greene· Dec 16, 2015, 03:02 UTC · lore

This patch isn't ready for prime-time yet but I wanted to get it out for some discussion.

While cleaning up and enhancing git-subtree, I've come across the need to have rebase behave nicely in the case of empty and redundant commits. There's a case in pick_one_preserving_merges where git-cherry pick is used to process the rebase and if cherry-pick fails, the rebase aborts. This change does two things:

- If --keep-empty is specified, invoke cherry-pick with --allow-empty.
- If new option --keep-redundant is specified, invoke cherry-pick with
  --keep-redundant-commits.

This allows the rebase to go forward without intrruption in the included tests.

I will also need a third option that has cherry-pick ignore redundant commits and remove them from the history. Unfortunately, I can't make out exactly how to do that in commit.c, which is where I gather the cherry-pick stuff happens. I'll need some help with that if there's general agreement that this is a useful enhancement.

During the course of developing this, I've encountered some strange rebase behavior. I'll send another message about that.

I'd appreciate feedback on this direction and any help with the cherry-pick stuff. Thanks!

                         -David
David Greene· Dec 16, 2015, 03:02 UTC · re: David Greene · lore

[PATCH] Support rebase --keep-empty and --keep-redundant

From: "David A. Greene" <greened@obbligato.org>
Teach rebase how to invoke cherry-pick to keep empty commits.

Add a new option --keep-redundant equivalent to cherry-pick's --keep-redundant-commits. With this option, rebase will preserve empty commits generated as a result of the merging process.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 git-rebase--interactive.sh |  11 +++-
 git-rebase.sh              |   5 ++
 t/t3427-rebase-empty.sh    | 127 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100755 t/t3427-rebase-empty.sh
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b938a6d..8466cb9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -393,7 +393,16 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output eval git cherry-pick \
+			cherry_keep_empty=
+			if test -n "$keep_empty"; then
+				cherry_keep_empty="--allow-empty"
+			fi
+			cherry_keep_redundant=
+			if test -n "$keep_redundant"; then
+				cherry_keep_redundant="--keep-redundant-commits"
+			fi
+			output eval git cherry-pick "$cherry_keep_empty" \
+				"$cherry_keep_redundant" \
 				${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \
 				"$strategy_args" "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
diff --git a/git-rebase.sh b/git-rebase.sh
index af7ba5f..1eae688 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -24,6 +24,7 @@ m,merge!           use merging strategies to rebase
 i,interactive!     let the user edit the list of commits to rebase
 x,exec=!           add exec lines after each commit of the editable list
 k,keep-empty	   preserve empty commits during rebase
+keep-redundant     preserve redundant commits during rebase
 f,force-rebase!    force rebase even if branch is up to date
 X,strategy-option=! pass the argument through to the merge strategy
 stat!              display a diffstat of what changed upstream
@@ -86,6 +87,7 @@ action=
 preserve_merges=
 autosquash=
 keep_empty=
+keep_redundant=
 test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
 gpg_sign_opt=
 
@@ -255,6 +257,9 @@ do
 	--keep-empty)
 		keep_empty=yes
 		;;
+	--keep-redundant)
+		keep_redundant=yes
+		;;
 	--preserve-merges)
 		preserve_merges=t
 		test -z "$interactive_rebase" && interactive_rebase=implied
diff --git a/t/t3427-rebase-empty.sh b/t/t3427-rebase-empty.sh
new file mode 100755
index 0000000..9e67e00
--- /dev/null
+++ b/t/t3427-rebase-empty.sh
@@ -0,0 +1,127 @@
+#!/bin/sh
+
+test_description='git rebase tests for empty commits
+
+This test runs git rebase and tests handling of empty commits.
+'
+. ./test-lib.sh
+
+addfile() {
+    name=$1
+    echo $(basename ${name}) > ${name}
+    ${git} add ${name}
+    ${git} commit -m "Add $(basename ${name})"
+}
+
+check_equal()
+{
+	test_debug 'echo'
+	test_debug "echo \"check a:\" \"{$1}\""
+	test_debug "echo \"      b:\" \"{$2}\""
+	if [ "$1" = "$2" ]; then
+		return 0
+	else
+		return 1
+	fi
+}
+
+last_commit_message()
+{
+	git log --pretty=format:%s -1
+}
+
+test_expect_success 'setup' '
+	test_commit README &&
+	mkdir files &&
+	cd files &&
+	git init &&
+	test_commit master1 &&
+	test_commit master2 &&
+	test_commit master3 &&
+	cd .. &&
+	test_debug "echo Add project master to master" &&
+	git fetch files master &&
+	git branch files-master FETCH_HEAD &&
+	test_debug "echo Add subtree master to master via subtree" &&
+	git read-tree --prefix=files_subtree files-master &&
+	git checkout -- files_subtree &&
+	tree=$(git write-tree) &&
+	head=$(git rev-parse HEAD) &&
+	rev=$(git rev-parse --verify files-master^0) &&
+	commit=$(git commit-tree -p ${head} -p ${rev} -m "Add subproject master" ${tree}) &&
+	git reset ${commit} &&
+	cd files_subtree &&
+	test_commit master4 &&
+	cd .. &&
+	test_commit files_subtree/master5
+'
+
+# Does not preserve master4 and master5.
+#test_expect_success 'Rebase default' '
+#	git checkout -b rebase-default master &&
+#	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+#	git commit -m "Empty commit" --allow-empty &&
+#	git rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&
+#	check_equal "$(last_commit_message)" "files_subtree/master5"
+#'
+
+test_expect_success 'Rebase --root' '
+	git checkout -b rebase-default-root master &&
+	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git commit -m "Empty commit" --allow-empty &&
+	test_must_fail git rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master --root &&
+	git rebase --abort
+'
+
+# Does not preserve master4, master5 and empty.
+#test_expect_success 'Rebase --keep-empty' '
+#	git checkout -b rebase-keep-empty master &&
+#	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+#	git commit -m "Empty commit" --allow-empty &&
+#	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
+#	check_equal "$(last_commit_message)" "Empty commit"
+#'
+
+test_expect_success 'Rebase --keep-empty --root' '
+	git checkout -b rebase-keep-empty-root master &&
+	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git commit -m "Empty commit" --allow-empty &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master --root &&
+	git rebase --abort
+'
+
+# Does not preserve master4 and master5.
+#test_expect_success 'Rebase --keep-redundant' '
+#	git checkout -b rebase-keep-redundant master &&
+#	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+#	git commit -m "Empty commit" --allow-empty &&
+#	git rebase -Xsubtree=files_subtree --keep-redundant --preserve-merges --onto files-master master &&
+#	check_equal "$(last_commit_message)" "files_subtree/master5"
+#'
+
+test_expect_success 'Rebase --keep-redundant --root' '
+	git checkout -b rebase-keep-redundant-root master &&
+	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git commit -m "Empty commit" --allow-empty &&
+	git rebase -Xsubtree=files_subtree --keep-redundant --preserve-merges --onto files-master --root &&
+	check_equal "$(last_commit_message)" "files_subtree/master5"
+'
+
+# Does not preserve master4, master5 and empty.
+#test_expect_success 'Rebase --keep-empty --keep-redundant' '
+#	git checkout -b rebase-keep-empty-keep-redundant master &&
+#	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+#	git commit -m "Empty commit" --allow-empty &&
+#	git rebase -Xsubtree=files_subtree --keep-empty --keep-redundant --preserve-merges --onto files-master master &&
+#	check_equal "$(last_commit_message)" "Empty commit"
+#'
+
+test_expect_success 'Rebase --keep-empty --keep-redundant --root' '
+	git checkout -b rebase-keep-empty-keep-redundant-root master &&
+	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git commit -m "Empty commit" --allow-empty &&
+	git rebase -Xsubtree=files_subtree --keep-empty --keep-redundant --preserve-merges --onto files-master --root &&
+	check_equal "$(last_commit_message)" "Empty commit"
+'
+
+test_done
-- 
2.6.1
Junio C Hamano· Dec 16, 2015, 05:57 UTC · re: David Greene · lore

Re: (unknown)

David Greene <greened@obbligato.org> writes:
> - If new option --keep-redundant is specified, invoke cherry-pick with
>   --keep-redundant-commits.

This came up in the past several weeks, I think; you would need to disable patch-equivalence based commit filtering if you really want to do a --keep-redundant that is reproducible and/or reliable.

Patrick Steinhardt· Dec 16, 2015, 08:44 UTC · re: Junio C Hamano · lore

Re: (unknown)

On Tue, Dec 15, 2015 at 09:57:50PM -0800, Junio C Hamano wrote:
Show 8 quoted lines
> David Greene <greened@obbligato.org> writes:
> 
> > - If new option --keep-redundant is specified, invoke cherry-pick with
> >   --keep-redundant-commits.
> 
> This came up in the past several weeks, I think; you would need to
> disable patch-equivalence based commit filtering if you really want
> to do a --keep-redundant that is reproducible and/or reliable.

Here are the links to the previous proposal [1] and following discussion [2] (see 'ps/rebase-keep-empty') if you are interested.

Patrick

[1]: http://thread.gmane.org/gmane.comp.version-control.git/281515 [2]: http://thread.gmane.org/gmane.comp.version-control.git/281917

David Greene· Dec 18, 2015, 17:35 UTC · re: Patrick Steinhardt · lore

Re: (unknown)

Patrick Steinhardt <ps@pks.im> writes:
Show 17 quoted lines
> On Tue, Dec 15, 2015 at 09:57:50PM -0800, Junio C Hamano wrote:
>> David Greene <greened@obbligato.org> writes:
>> 
>> > - If new option --keep-redundant is specified, invoke cherry-pick with
>> >   --keep-redundant-commits.
>> 
>> This came up in the past several weeks, I think; you would need to
>> disable patch-equivalence based commit filtering if you really want
>> to do a --keep-redundant that is reproducible and/or reliable.
>
> Here are the links to the previous proposal [1] and following
> discussion [2] (see 'ps/rebase-keep-empty') if you are
> interested.
>
> Patrick
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/281515[2]: http://thread.gmane.org/gmane.comp.version-control.git/281917
Thanks.  That makes total sense.

I actually would prefer a behavior where cherry-pick would just drop redundant commits rather than stopping and asking the user to reset. The problem is that rebase --preserve-merges seems to force the drop to use cherry-pick and cherry-pick doesn't behave well (from a scripting perspective) in the presence of redundant commits.

As it is, it's difficult to rebase as part of a scripted operation due to this issue.

Any ideas on how to teach cherry-pick to automatically drop such commits?

                           -David

← back to recent threads