[PATCH 0/3] Get rebase to work with :/foomery committish

Since the early preview, I realized that peel_committish() is required
in exactly two places: the <onto> and <upstream> parsers facing
end-user data.  Updated appropriately.

Thanks.

Ramkumar Ramachandra (3):
  t/rebase: add failing tests for a peculiar revision
  sh-setup: add new peel_committish() helper
  rebase: use peel_committish() where appropriate

 git-rebase.sh                 |  4 ++--
 git-sh-setup.sh               | 12 ++++++++++++
 t/t3400-rebase.sh             | 11 +++++++++++
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 4 files changed, 36 insertions(+), 2 deletions(-)

-- 
1.8.3.1.381.g12ca056.dirty

The following commands fail, even if :/quuxery and :/foomery resolve to
perfectly valid commits:

  $ git rebase [-i] --onto :/quuxery :/foomery

This is because rebase [-i] attempts to rev-parse ${REV}^0 to verify
that the given revision resolves to a commit.  Add tests to document
these failures.

Signed-off-by: Ramkumar Ramachandra <redacted>
---
 t/t3400-rebase.sh             | 11 +++++++++++
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..81ec517 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh

@@ -88,6 +88,17 @@ test_expect_success 'rebase fast-forward to master' '
 	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
 '
 
+test_expect_failure 'rebase, with <onto> and <upstream> specified as :/quuxery' '
+	test_when_finished "git branch -D torebase" &&
+	git checkout -b torebase my-topic-branch^ &&
+	upstream=$(git rev-parse ":/Add B") &&
+	onto=$(git rev-parse ":/Add A") &&
+	git rebase --onto $onto $upstream &&
+	git reset --hard my-topic-branch^ &&
+	git rebase --onto ":/Add A" ":/Add B" &&
+	git checkout my-topic-branch
+'
+
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 79e8d3c..eb241f5 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh

@@ -947,4 +947,15 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
+test_expect_failure 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
+	test_when_finished "git branch -D torebase" &&
+	git checkout -b torebase branch1 &&
+	upstream=$(git rev-parse ":/J") &&
+	onto=$(git rev-parse ":/A") &&
+	git rebase --onto $onto $upstream &&
+	git reset --hard branch1 &&
+	git rebase --onto ":/A" ":/J" &&
+	git checkout branch1
+'
+
 test_done

-- 
1.8.3.1.381.g12ca056.dirty

The normal way to check whether a certain revision resolves to a valid
commit is:

  $ git rev-parse --verify $REV^0

Unfortunately, this does not work when $REV is of the type :/quuxery.
Write a helper to work around this limitation.

Suggested-by: Junio C Hamano <redacted>
Signed-off-by: Ramkumar Ramachandra <redacted>
---
 git-sh-setup.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 2f78359..7a964ad 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh

@@ -313,3 +313,15 @@ then
 	}
 	: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}
 fi
+
+peel_committish () {
+	case "$1" in
+	:/*)
+		peeltmp=$(git rev-parse --verify "$1") &&
+		git rev-parse --verify "${peeltmp}^0"
+		;;
+	*)
+		git rev-parse --verify "${1}^0"
+		;;
+	esac
+}

-- 
1.8.3.1.381.g12ca056.dirty

The revisions specified on the command-line as <onto> and <upstream>
arguments could be of the form :/quuxery; so, use peel_committish() to
resolve them.  The failing tests in t/rebase and t/rebase-interactive
now pass.

Signed-off-by: Ramkumar Ramachandra <redacted>
---
 git-rebase.sh                 | 4 ++--
 t/t3400-rebase.sh             | 2 +-
 t/t3404-rebase-interactive.sh | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index d0c11a9..6987b9b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh

@@ -434,7 +434,7 @@ then
 		shift
 		;;
 	esac
-	upstream=`git rev-parse --verify "${upstream_name}^0"` ||
+	upstream=$(peel_committish "${upstream_name}") ||
 	die "$(eval_gettext "invalid upstream \$upstream_name")"
 	upstream_arg="$upstream_name"
 else

@@ -470,7 +470,7 @@ case "$onto_name" in
 	fi
 	;;
 *)
-	onto=$(git rev-parse --verify "${onto_name}^0") ||
+	onto=$(peel_committish "$onto_name") ||
 	die "$(eval_gettext "Does not point to a valid commit: \$onto_name")"
 	;;
 esac

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 81ec517..cbca71e 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh

@@ -88,7 +88,7 @@ test_expect_success 'rebase fast-forward to master' '
 	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
 '
 
-test_expect_failure 'rebase, with <onto> and <upstream> specified as :/quuxery' '
+test_expect_success 'rebase, with <onto> and <upstream> specified as :/quuxery' '
 	test_when_finished "git branch -D torebase" &&
 	git checkout -b torebase my-topic-branch^ &&
 	upstream=$(git rev-parse ":/Add B") &&

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index eb241f5..86917d1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh

@@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
-test_expect_failure 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
+test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
 	test_when_finished "git branch -D torebase" &&
 	git checkout -b torebase branch1 &&
 	upstream=$(git rev-parse ":/J") &&

-- 
1.8.3.1.381.g12ca056.dirty

From: "Ramkumar Ramachandra" <redacted>
Sent: Friday, June 14, 2013 2:17 PM

The normal way to check whether a certain revision resolves to a valid
commit is:

 $ git rev-parse --verify $REV^0

Unfortunately, this does not work when $REV is of the type :/quuxery.

Is there a proper name for this style of revision specification? I've 
been letting this 'style' wash over me in the hope that I'd understand 
eventually, but it hasn't.

Loking at git-rev-parse I now see that it might be the 'Commit Message 
Regex' rev specifier.

If re-rolled, can this elucidation be included in the commit message?

Philip 

Philip Oakley wrote:

Is there a proper name for this style of revision specification? I've been
letting this 'style' wash over me in the hope that I'd understand
eventually, but it hasn't.

See gitrevisions(7).  None of them have any names.

Loking at git-rev-parse I now see that it might be the 'Commit Message
Regex' rev specifier.

Did you just invent that term?  I couldn't find any mentions of it.

From: "Ramkumar Ramachandra" <redacted>
Sent: Friday, June 14, 2013 5:18 PM

Philip Oakley wrote:

quoted

Is there a proper name for this style of revision specification? I've 
been
letting this 'style' wash over me in the hope that I'd understand
eventually, but it hasn't.

See gitrevisions(7).  None of them have any names.

... which is a shame...

quoted

Loking at git-rev-parse I now see that it might be the 'Commit 
Message
Regex' rev specifier.

Did you just invent that term?  I couldn't find any mentions of it.

Yep, hence the 'it might be'.   Maybe I missed a word and should have 
written 'it might be called' ;-)

It would be useful to have some descriptive names for these unusual 
(uncommon) methods.
There's already the "pathspec :(glob) syntax".

Philip