(unknown),

(unknown),

[David Greene]

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

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

[David Greene]

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

Re: (unknown)

[Junio C Hamano]

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.

Re: (unknown)

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: (unknown)

[David Greene]

Patrick Steinhardt <ps@pks.im> writes:

> 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