threads / patch / 25635

patch, 2 partsrebase: better rearranging of fixup!/squash! lines with --autosquash

Subject: [PATCH 1/2] rebase: better rearranging of fixup!/squash! lines with --autosquash

## tl;dr

13 messages between Nov 4, 2010 and Nov 8, 2010. Diffs are folded; open one to read it.

replies: 12people: 5as markdown or json

Kevin Ballard· Nov 4, 2010, 02:41 UTC · lore

The current behvaior of --autosquash can duplicate fixup!/squash! lines if they match multiple commits, and it can also apply them to commits that come after them in the todo list. Even more oddly, a commit that looks like "fixup! fixup!" will match itself and be duplicated in the todo list.

Change the todo list rearranging to mark all commits as used as soon as they are emitted, and to avoid emitting a fixup/squash commit if the commit has already been marked as used.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
 git-rebase--interactive.sh   |    4 +++
 t/t3415-rebase-autosquash.sh |   43 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)
Show changes to 2 files +47 −0

git-rebase--interactive.sh, t/t3415-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a27952d..379bbac 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -687,8 +687,12 @@ rearrange_squash () {
 		*" $sha1 "*) continue ;;
 		esac
 		printf '%s\n' "$pick $sha1 $message"
+		used="$used$sha1 "
 		while read -r squash action msg
 		do
+			case " $used" in
+			*" $squash "*) continue ;;
+			esac
 			case "$message" in
 			"$msg"*)
 				printf '%s\n' "$action $squash $action! $msg"
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index fd2184c..712bbe8 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -94,4 +94,47 @@ test_expect_success 'misspelled auto squash' '
 	test 0 = $(git rev-list final-missquash...HEAD | wc -l)
 '
 
+test_expect_success 'auto squash that matches 2 commits' '
+	git reset --hard base &&
+	echo 4 >file4 &&
+	git add file4 &&
+	test_tick &&
+	git commit -m "first new commit" &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! first" &&
+	git tag final-multisquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD~4 &&
+	git log --oneline >actual &&
+	test 4 = $(wc -l <actual) &&
+	git diff --exit-code final-multisquash &&
+	test 1 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&
+	test 1 = $(git cat-file commit HEAD | grep first | wc -l)
+'
+
+test_expect_success 'auto squash that matches a commit after the squash' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! third" &&
+	echo 4 >file4 &&
+	git add file4 &&
+	test_tick &&
+	git commit -m "third commit" &&
+	git tag final-presquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD~4 &&
+	git log --oneline >actual &&
+	test 5 = $(wc -l <actual) &&
+	git diff --exit-code final-presquash &&
+	test 0 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
+	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
+'
+
 test_done

-- 
1.7.3.2.201.g24941.dirty
Kevin Ballard· Nov 4, 2010, 02:41 UTC · re: Kevin Ballard · lore

[PATCH 2/2] rebase: teach --autosquash to match on sha1 in addition to message

Support lines of the form "fixup! 7a235b" that specify an exact commit in addition to the normal "squash! Old commit message" form.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
I chose 4 characters as the restriction because that's the minimum number
that `git rev-parse --short=<n>` will emit.
 git-rebase--interactive.sh   |   13 +++++++++----
 t/t3415-rebase-autosquash.sh |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)
Show changes to 2 files +42 −4

git-rebase--interactive.sh, t/t3415-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 379bbac..9121bb6 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -693,12 +693,17 @@ rearrange_squash () {
 			case " $used" in
 			*" $squash "*) continue ;;
 			esac
-			case "$message" in
-			"$msg"*)
+			emit=0
+			case "$message" in "$msg"*) emit=1;; esac
+			if test $emit != 1; then
+				case "$sha1" in "$msg"*) emit=1;; esac
+				# ensure the message is at least 4 characters long
+				case "$msg" in ????*);; *) emit=0;; esac
+			fi
+			if test $emit = 1; then
 				printf '%s\n' "$action $squash $action! $msg"
 				used="$used$squash "
-				;;
-			esac
+			fi
 		done <"$1.sq"
 	done >"$1.rearranged" <"$1"
 	cat "$1.rearranged" >"$1"
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 712bbe8..14cdbeb 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -136,5 +136,38 @@ test_expect_success 'auto squash that matches a commit after the squash' '
 	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
 	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
 '
+test_expect_success 'auto squash that matches a sha1' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! $(git rev-parse --short HEAD^)" &&
+	git tag final-shasquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-shasquash &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+'
+
+# this test just ensures that < 4 characters can't match a sha1
+test_expect_success 'auto squash that accidentally matches a sha1' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! $(git rev-parse HEAD^ | cut -c 1-3)" &&
+	git tag final-badshasquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 4 = $(wc -l <actual) &&
+	git diff --exit-code final-badshasquash &&
+	test 0 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 0 = $(git cat-file commit HEAD^^ | grep squash | wc -l) &&
+	test 1 = $(git cat-file commit HEAD | grep squash | wc -l)
+'
 
 test_done

-- 
1.7.3.2.201.g24941.dirty
Kevin Ballard· Nov 4, 2010, 04:49 UTC · re: Kevin Ballard · lore

Re: [PATCH 2/2] rebase: teach --autosquash to match on sha1 in addition to message

On Nov 3, 2010, at 7:41 PM, Kevin Ballard wrote:
> Support lines of the form "fixup! 7a235b" that specify an exact commit
> in addition to the normal "squash! Old commit message" form.

I just realized that this only works for sha1's of up to 7 characters. If you provide more it won't match, as it's comparing against the sha1 given in the todo list. I wonder if it's worth resolving all sha1s to their full length if the provided string is longer than 7 characters?

-Kevin Ballard
Sverre Rabbelier· Nov 4, 2010, 10:44 UTC · re: Kevin Ballard · lore

Re: [PATCH 2/2] rebase: teach --autosquash to match on sha1 in addition to message

Heya,
On Thu, Nov 4, 2010 at 05:49, Kevin Ballard <kevin@sb.org> wrote:
> I just realized that this only works for sha1's of up to 7 characters.
> If you provide more it won't match, as it's comparing against the sha1
> given in the todo list. I wonder if it's worth resolving all sha1s to
> their full length if the provided string is longer than 7 characters?

Well, not if you're resolving them based on the 7-character string from the rebase todo list. If you run in to ambiguity with those 7-length hash we should instead increase the length of the hashes in the todo list.

So I'd say, solve this by doing a prefix match?
-- 
Cheers,

Sverre Rabbelier
Kevin Ballard· Nov 4, 2010, 21:35 UTC · re: Sverre Rabbelier · lore

Re: [PATCH 2/2] rebase: teach --autosquash to match on sha1 in addition to message

On Nov 4, 2010, at 3:44 AM, Sverre Rabbelier wrote:
Show 12 quoted lines
> On Thu, Nov 4, 2010 at 05:49, Kevin Ballard <kevin@sb.org> wrote:
>> I just realized that this only works for sha1's of up to 7 characters.
>> If you provide more it won't match, as it's comparing against the sha1
>> given in the todo list. I wonder if it's worth resolving all sha1s to
>> their full length if the provided string is longer than 7 characters?
> 
> Well, not if you're resolving them based on the 7-character string
> from the rebase todo list. If you run in to ambiguity with those
> 7-length hash we should instead increase the length of the hashes in
> the todo list.
> 
> So I'd say, solve this by doing a prefix match?

Prefix match of what against what? If the 7-character string used in the TODO list is already ambiguous, then the rebase itself would be expected to fail. I'm inclined to just try to resolve the message in the fixup! line itself to a full SHA1, and if that works, then try to match the 7-character sha1 with a prefix comparison.

-Kevin Ballard
Junio C Hamano· Nov 4, 2010, 19:09 UTC · re: Kevin Ballard · lore

Re: [PATCH 2/2] rebase: teach --autosquash to match on sha1 in addition to message

Kevin Ballard <kevin@sb.org> writes:
Show 9 quoted lines
> On Nov 3, 2010, at 7:41 PM, Kevin Ballard wrote:
>
>> Support lines of the form "fixup! 7a235b" that specify an exact commit
>> in addition to the normal "squash! Old commit message" form.
>
> I just realized that this only works for sha1's of up to 7 characters.
> If you provide more it won't match, as it's comparing against the sha1
> given in the todo list. I wonder if it's worth resolving all sha1s to
> their full length if the provided string is longer than 7 characters?
Yeah, this is a nice feature I sometimes missed ;-)
Kevin Ballard· Nov 4, 2010, 22:36 UTC · re: Junio C Hamano · lore

[PATCHv2 1/2] rebase: better rearranging of fixup!/squash! lines with --autosquash

The current behvaior of --autosquash can duplicate fixup!/squash! lines if they match multiple commits, and it can also apply them to commits that come after them in the todo list. Even more oddly, a commit that looks like "fixup! fixup!" will match itself and be duplicated in the todo list.

Change the todo list rearranging to mark all commits as used as soon as they are emitted, and to avoid emitting a fixup/squash commit if the commit has already been marked as used.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
This patch is unchanged from the previous version.
 git-rebase--interactive.sh   |    4 +++
 t/t3415-rebase-autosquash.sh |   43 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)
Show changes to 2 files +47 −0

git-rebase--interactive.sh, t/t3415-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a27952d..379bbac 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -687,8 +687,12 @@ rearrange_squash () {
 		*" $sha1 "*) continue ;;
 		esac
 		printf '%s\n' "$pick $sha1 $message"
+		used="$used$sha1 "
 		while read -r squash action msg
 		do
+			case " $used" in
+			*" $squash "*) continue ;;
+			esac
 			case "$message" in
 			"$msg"*)
 				printf '%s\n' "$action $squash $action! $msg"
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index fd2184c..712bbe8 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -94,4 +94,47 @@ test_expect_success 'misspelled auto squash' '
 	test 0 = $(git rev-list final-missquash...HEAD | wc -l)
 '
 
+test_expect_success 'auto squash that matches 2 commits' '
+	git reset --hard base &&
+	echo 4 >file4 &&
+	git add file4 &&
+	test_tick &&
+	git commit -m "first new commit" &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! first" &&
+	git tag final-multisquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD~4 &&
+	git log --oneline >actual &&
+	test 4 = $(wc -l <actual) &&
+	git diff --exit-code final-multisquash &&
+	test 1 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&
+	test 1 = $(git cat-file commit HEAD | grep first | wc -l)
+'
+
+test_expect_success 'auto squash that matches a commit after the squash' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! third" &&
+	echo 4 >file4 &&
+	git add file4 &&
+	test_tick &&
+	git commit -m "third commit" &&
+	git tag final-presquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD~4 &&
+	git log --oneline >actual &&
+	test 5 = $(wc -l <actual) &&
+	git diff --exit-code final-presquash &&
+	test 0 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
+	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
+'
+
 test_done

-- 
1.7.3.2.202.g3b863.dirty
Kevin Ballard· Nov 4, 2010, 22:36 UTC · re: Junio C Hamano · lore

[PATCHv2 2/2] rebase: teach --autosquash to match on sha1 in addition to message

Support lines of the form "fixup! 7a235b" that specify an exact commit in addition to the normal "squash! Old commit message" form.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
This version attempts to resolve all single-word fixup!/squash! messages to
full sha1s. It only tries this once per line and records the results in the
$TODO.sq file. The reasoning for the duplication of such lines is to allow
a line like
  fixup! 8a23b5f 7a5436

to still prefix-match "7a5436" against commit messages in addition to matching the fullly-resolved sha1 against the other sha1's.

 git-rebase--interactive.sh   |   40 +++++++++++++++++++++++++++++++++-------
 t/t3415-rebase-autosquash.sh |   31 +++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 7 deletions(-)
Show changes to 2 files +64 −7

git-rebase--interactive.sh, t/t3415-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 379bbac..c2383bf 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -675,9 +675,27 @@ get_saved_options () {
 # comes immediately after the former, and change "pick" to
 # "fixup"/"squash".
 rearrange_squash () {
-	sed -n -e 's/^pick \([0-9a-f]*\) \(squash\)! /\1 \2 /p' \
-		-e 's/^pick \([0-9a-f]*\) \(fixup\)! /\1 \2 /p' \
-		"$1" >"$1.sq"
+	# extract fixup!/squash! lines and resolve any referenced sha1's
+	while read -r pick sha1 message
+	do
+		case "$message" in
+		"squash! "*|"fixup! "*)
+			action="${message%%!*}"
+			rest="${message#*! }"
+			echo "$sha1 $action $rest"
+			# if it's a single word, try to resolve to a full sha1 and
+			# emit a second copy. This allows us to match on both message
+			# and on sha1 prefix
+			if test "${rest#* }" = "$rest"; then
+				fullsha="$(git rev-parse -q --verify "$rest" 2>/dev/null)"
+				if test -n "$fullsha"; then
+					# prefix the action to uniquely identify this line as
+					# intended for full sha1 match
+					echo "$sha1 +$action $fullsha"
+				fi
+			fi
+		esac
+	done >"$1.sq" <"$1"
 	test -s "$1.sq" || return
 
 	used=
@@ -693,12 +711,20 @@ rearrange_squash () {
 			case " $used" in
 			*" $squash "*) continue ;;
 			esac
-			case "$message" in
-			"$msg"*)
+			emit=0
+			case "$action" in
+			+*)
+				action="${action#+}"
+				# full sha1 prefix test
+				case "$msg" in "$sha1"*) emit=1;; esac ;;
+			*)
+				# message prefix test
+				case "$message" in "$msg"*) emit=1;; esac ;;
+			esac
+			if test $emit = 1; then
 				printf '%s\n' "$action $squash $action! $msg"
 				used="$used$squash "
-				;;
-			esac
+			fi
 		done <"$1.sq"
 	done >"$1.rearranged" <"$1"
 	cat "$1.rearranged" >"$1"
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 712bbe8..ca16b70 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -136,5 +136,36 @@ test_expect_success 'auto squash that matches a commit after the squash' '
 	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
 	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
 '
+test_expect_success 'auto squash that matches a sha1' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! $(git rev-parse --short HEAD^)" &&
+	git tag final-shasquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-shasquash &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+'
+
+test_expect_success 'auto squash that matches longer sha1' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! $(git rev-parse --short=11 HEAD^)" &&
+	git tag final-longshasquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-longshasquash &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+'
 
 test_done

-- 
1.7.3.2.202.g3b863.dirty
Peter Krefting· Nov 5, 2010, 13:04 UTC · re: Kevin Ballard · lore

Re: [PATCH 2/2] rebase: teach --autosquash to match on sha1 in addition to message

Kevin Ballard:
> Support lines of the form "fixup! 7a235b" that specify an exact commit
> in addition to the normal "squash! Old commit message" form.

The patch I posted for this in June (Message-Id: <20100617125746.E86B42FC00@perkele>) also updated the manual page for git-rebase. You may want to re-use parts of that.

http://www.spinics.net/lists/git/msg133571.html has an archived version of it.
-- 
\\// Peter - http://www.softwolves.pp.se/
Kevin Ballard· Nov 8, 2010, 10:48 UTC · re: Peter Krefting · lore

[PATCHv3 1/2] rebase: better rearranging of fixup!/squash! lines with --autosquash

The current behvaior of --autosquash can duplicate fixup!/squash! lines if they match multiple commits, and it can also apply them to commits that come after them in the todo list. Even more oddly, a commit that looks like "fixup! fixup!" will match itself and be duplicated in the todo list.

Change the todo list rearranging to mark all commits as used as soon as they are emitted, and to avoid emitting a fixup/squash commit if the commit has already been marked as used.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
This patch is unchanged from v2.
 git-rebase--interactive.sh   |    4 +++
 t/t3415-rebase-autosquash.sh |   43 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)
Show changes to 2 files +47 −0

git-rebase--interactive.sh, t/t3415-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e29fd91..56cfdb5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -707,8 +707,12 @@ rearrange_squash () {
 		*" $sha1 "*) continue ;;
 		esac
 		printf '%s\n' "$pick $sha1 $message"
+		used="$used$sha1 "
 		while read -r squash action msg
 		do
+			case " $used" in
+			*" $squash "*) continue ;;
+			esac
 			case "$message" in
 			"$msg"*)
 				printf '%s\n' "$action $squash $action! $msg"
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index fd2184c..712bbe8 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -94,4 +94,47 @@ test_expect_success 'misspelled auto squash' '
 	test 0 = $(git rev-list final-missquash...HEAD | wc -l)
 '
 
+test_expect_success 'auto squash that matches 2 commits' '
+	git reset --hard base &&
+	echo 4 >file4 &&
+	git add file4 &&
+	test_tick &&
+	git commit -m "first new commit" &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! first" &&
+	git tag final-multisquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD~4 &&
+	git log --oneline >actual &&
+	test 4 = $(wc -l <actual) &&
+	git diff --exit-code final-multisquash &&
+	test 1 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&
+	test 1 = $(git cat-file commit HEAD | grep first | wc -l)
+'
+
+test_expect_success 'auto squash that matches a commit after the squash' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! third" &&
+	echo 4 >file4 &&
+	git add file4 &&
+	test_tick &&
+	git commit -m "third commit" &&
+	git tag final-presquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD~4 &&
+	git log --oneline >actual &&
+	test 5 = $(wc -l <actual) &&
+	git diff --exit-code final-presquash &&
+	test 0 = "$(git cat-file blob HEAD^^:file1)" &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
+	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
+'
+
 test_done

-- 
1.7.3.2.195.gc69dde
Yann Dirson· Nov 8, 2010, 10:52 UTC · re: Kevin Ballard · lore

Re: [PATCHv3 1/2] rebase: better rearranging of fixup!/squash! lines with --autosquash

Kevin wrote:
>The current behvaior of --autosquash can duplicate fixup!/squash! lines
>if they match multiple commits, and it can also apply them to commits
>that come after them in the todo list.

That last part of the sentence does not match my experience - it looks like you describe as a bug a nonexistent feature I miss :)

See http://marc.info/?l=git&m=128784082701920 for a testcase that exhibits what I think is a problem.

-- 
Yann Dirson - Bertin Technologies
Kevin Ballard· Nov 8, 2010, 11:10 UTC · re: Yann Dirson · lore

Re: [PATCHv3 1/2] rebase: better rearranging of fixup!/squash! lines with --autosquash

On Nov 8, 2010, at 2:52 AM, Yann Dirson wrote:
Show 10 quoted lines
> Kevin wrote:
>> The current behvaior of --autosquash can duplicate fixup!/squash! lines
>> if they match multiple commits, and it can also apply them to commits
>> that come after them in the todo list.
> 
> That last part of the sentence does not match my experience - it looks
> like you describe as a bug a nonexistent feature I miss :)
> 
> See http://marc.info/?l=git&m=128784082701920 for a testcase that
> exhibits what I think is a problem.

The bug is definitely present. It seems what you're describing is that you want it to continue to place that fixup! there, but you want it to skip the pick line that occurs earlier in the TODO list. My feeling is that this is purely a bug, and should be fixed to not place the fixup! line after the later commit.

If you do want the behavior where it skips the pick and places the fixup! line after the later commit, I would encourage you to file a separate patch for that.

-Kevin Ballard
Kevin Ballard· Nov 8, 2010, 10:48 UTC · re: Peter Krefting · lore

[PATCHv3 2/2] rebase: teach --autosquash to match on sha1 in addition to message

Support lines of the form "fixup! 7a235b" that specify an exact commit in addition to the normal "squash! Old commit message" form.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---

The only change from v2 is I've added the manpage change from Peter Krefting's previous patch on this subject (http://www.spinics.net/lists/git/msg133571.html), as referenced in message id <alpine.DEB.2.00.1011051401090.7611@ds9.cixit.se>.

 Documentation/git-rebase.txt |    8 ++++----
 git-rebase--interactive.sh   |   40 +++++++++++++++++++++++++++++++++-------
 t/t3415-rebase-autosquash.sh |   31 +++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 11 deletions(-)
Show changes to 3 files +68 −11

Documentation/git-rebase.txt, git-rebase--interactive.sh, t/t3415-rebase-autosquash.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 30e5c0e..17f50cb 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -339,10 +339,10 @@ idea unless you know what you are doing (see BUGS below).
 --no-autosquash::
 	When the commit log message begins with "squash! ..." (or
 	"fixup! ..."), and there is a commit whose title begins with
-	the same ..., automatically modify the todo list of rebase -i
-	so that the commit marked for squashing comes right after the
-	commit to be modified, and change the action of the moved
-	commit from `pick` to `squash` (or `fixup`).
+	the same ..., or whose hash is ..., automatically modify the
+	todo list of rebase -i so that the commit marked for squashing
+	comes right after the commit to be modified, and change the
+	action of the moved commit from `pick` to `squash` (or `fixup`).
 +
 This option is only valid when the '--interactive' option is used.
 +
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 56cfdb5..31abbff 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -695,9 +695,27 @@ get_saved_options () {
 # comes immediately after the former, and change "pick" to
 # "fixup"/"squash".
 rearrange_squash () {
-	sed -n -e 's/^pick \([0-9a-f]*\) \(squash\)! /\1 \2 /p' \
-		-e 's/^pick \([0-9a-f]*\) \(fixup\)! /\1 \2 /p' \
-		"$1" >"$1.sq"
+	# extract fixup!/squash! lines and resolve any referenced sha1's
+	while read -r pick sha1 message
+	do
+		case "$message" in
+		"squash! "*|"fixup! "*)
+			action="${message%%!*}"
+			rest="${message#*! }"
+			echo "$sha1 $action $rest"
+			# if it's a single word, try to resolve to a full sha1 and
+			# emit a second copy. This allows us to match on both message
+			# and on sha1 prefix
+			if test "${rest#* }" = "$rest"; then
+				fullsha="$(git rev-parse -q --verify "$rest" 2>/dev/null)"
+				if test -n "$fullsha"; then
+					# prefix the action to uniquely identify this line as
+					# intended for full sha1 match
+					echo "$sha1 +$action $fullsha"
+				fi
+			fi
+		esac
+	done >"$1.sq" <"$1"
 	test -s "$1.sq" || return
 
 	used=
@@ -713,12 +731,20 @@ rearrange_squash () {
 			case " $used" in
 			*" $squash "*) continue ;;
 			esac
-			case "$message" in
-			"$msg"*)
+			emit=0
+			case "$action" in
+			+*)
+				action="${action#+}"
+				# full sha1 prefix test
+				case "$msg" in "$sha1"*) emit=1;; esac ;;
+			*)
+				# message prefix test
+				case "$message" in "$msg"*) emit=1;; esac ;;
+			esac
+			if test $emit = 1; then
 				printf '%s\n' "$action $squash $action! $msg"
 				used="$used$squash "
-				;;
-			esac
+			fi
 		done <"$1.sq"
 	done >"$1.rearranged" <"$1"
 	cat "$1.rearranged" >"$1"
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 712bbe8..ca16b70 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -136,5 +136,36 @@ test_expect_success 'auto squash that matches a commit after the squash' '
 	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
 	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
 '
+test_expect_success 'auto squash that matches a sha1' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! $(git rev-parse --short HEAD^)" &&
+	git tag final-shasquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-shasquash &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+'
+
+test_expect_success 'auto squash that matches longer sha1' '
+	git reset --hard base &&
+	echo 1 >file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "squash! $(git rev-parse --short=11 HEAD^)" &&
+	git tag final-longshasquash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-longshasquash &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+'
 
 test_done

-- 
1.7.3.2.195.gc69dde

← back to recent threads