[PATCH] git-filter-branch could be confused by similar names

'git-filter-branch branch' could fail producing the error:
"Which ref do you want to rewrite?" if existed another branch
or tag, which name was 'branch-something' or 'something/branch'.

Signed-off-by: Dmitry Potapov <redacted>
---
 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index dbab1a9..b89a720 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh

@@ -219,7 +219,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep '^refs/[^/]\+/'"$ref"'$')"
 	esac
 
 	git check-ref-format "$ref" && echo "$ref"

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 5f60b22..c3e5207 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh

@@ -36,6 +36,16 @@ test_expect_success 'result is really identical' '
 	test $H = $(git rev-parse HEAD)
 '
 
+test_expect_success 'rewrite branch with similar names' '
+	git branch my &&
+	git tag my/orig &&
+	git tag my-orig &&
+	git tag orig/my &&
+	git tag orig-my &&
+	git-filter-branch my &&
+	test $H = $(git rev-parse HEAD)
+'
+
 test_expect_success 'rewrite, renaming a specific file' '
 	git-filter-branch -f --tree-filter "mv d doh || :" HEAD
 '

-- 
1.5.3.5

Hi,

On Tue, 25 Dec 2007, Dmitry Potapov wrote:

Hmm.  I wonder if this is a proper solution.  It still does not error out 
when you have a tag and a branch of the same name.

I kinda hoped that by 1.5.4, rewrite-commits would be finished, but it 
seems that nothing happened in that area after 1.5.3-rcX.

It would be so much easier to have checks like this -- returning the real 
refname for short but unique short refnames -- in C.

Ciao,
Dscho

Hi,

On Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:

On Tue, 25 Dec 2007, Dmitry Potapov wrote:

quoted

'git-filter-branch branch' could fail producing the error:
"Which ref do you want to rewrite?" if existed another branch
or tag, which name was 'branch-something' or 'something/branch'.

Signed-off-by: Dmitry Potapov <redacted>
---
 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index dbab1a9..b89a720 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh

@@ -219,7 +219,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep '^refs/[^/]\+/'"$ref"'$')"

Hmm.  I wonder if this is a proper solution.  It still does not error out 
when you have a tag and a branch of the same name.

Are you sure? I had created a tag and a branch with the same name, and
then tried git filter-branch on it, and it did error out:
===
warning: refname 'test1' is ambiguous.
Which ref do you want to rewrite?
===

Maybe, my fix is not a perfect solution, but it works correctly in all
known to me situations, while the original code is clearly broken in
most common cases, like when you have created a tag with a name that
consists of the name of a branch plus some arbitrary suffix. When you
run git-filter-branch on that branch, you only get: "Which ref do you
want to rewrite?", which is very confusing, because you have only one
reference with the given name.


Dmitry

Hi,

On Sun, 30 Dec 2007, Dmitry Potapov wrote:

On Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:

quoted

On Tue, 25 Dec 2007, Dmitry Potapov wrote:

quoted

'git-filter-branch branch' could fail producing the error: "Which 
ref do you want to rewrite?" if existed another branch or tag, which 
name was 'branch-something' or 'something/branch'.

Signed-off-by: Dmitry Potapov <redacted>
---
 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index dbab1a9..b89a720 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh

@@ -219,7 +219,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep '^refs/[^/]\+/'"$ref"'$')"

Hmm.  I wonder if this is a proper solution.  It still does not error 
out when you have a tag and a branch of the same name.

Are you sure? I had created a tag and a branch with the same name, and
then tried git filter-branch on it, and it did error out:
===
warning: refname 'test1' is ambiguous.
Which ref do you want to rewrite?
===

Okay, bad example.  But try "heads/master".  Or "origin" in a repository 
which has "refs/remotes/origin/HEAD".

Ciao,
Dscho

On Sun, Dec 30, 2007 at 11:46:59AM +0100, Johannes Schindelin wrote:

On Sun, 30 Dec 2007, Dmitry Potapov wrote:

quoted

On Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:

quoted

On Tue, 25 Dec 2007, Dmitry Potapov wrote:

quoted

'git-filter-branch branch' could fail producing the error: "Which 
ref do you want to rewrite?" if existed another branch or tag, which 
name was 'branch-something' or 'something/branch'.

Signed-off-by: Dmitry Potapov <redacted>
---
 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index dbab1a9..b89a720 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh

@@ -219,7 +219,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep '^refs/[^/]\+/'"$ref"'$')"

Hmm.  I wonder if this is a proper solution.  It still does not error 
out when you have a tag and a branch of the same name.

Are you sure? I had created a tag and a branch with the same name, and
then tried git filter-branch on it, and it did error out:
===
warning: refname 'test1' is ambiguous.
Which ref do you want to rewrite?
===

Okay, bad example.  But try "heads/master". 

You are right. Somehow, I forgot about this possibility. How about this:

+			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"

Or "origin" in a repository 
which has "refs/remotes/origin/HEAD".

Well, it does not work, but it would not work before either, because you
are very likely to have something else in origin. Actually, I doubt that
anyone will want to filter "origin", but if you insist, here is another
grep expression, which should accommodate that case too:

+			grep '^refs/\([^/]\+/\)\?'"$ref"'\(/HEAD\)\?$')"

In any case, I believe it would be better to have a more strict grep
expression than one that is used by git-filter-branch now, because now
you either have a very confusing error message, or accidentally you
could filter a wrong branch. And as you said before, the proper C
solution is not feasible for 1.5.4, so I believe a better grep
expression is the right thing to do for now.

If you have no other objection, I will resent the patch with the
corrected version of the grep expression.

Dmitry

Hi,

On Sun, 30 Dec 2007, Dmitry Potapov wrote:

How about this:

+			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"

Maybe.  I wonder whether just adding a "$" (which I obviously forgot) 
would not be enough...

Ciao,
Dscho

On Sun, Dec 30, 2007 at 05:03:32PM +0100, Johannes Schindelin wrote:

On Sun, 30 Dec 2007, Dmitry Potapov wrote:

quoted

How about this:

+			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"

Maybe.  I wonder whether just adding a "$" (which I obviously forgot) 
would not be enough...

Adding '$' will certainly make things much better, but you will still
have the same problem if you want to filter "master", but you have
"origin/master" in your repo.

Dmitry

'git-filter-branch branch' could fail producing the error:
"Which ref do you want to rewrite?" if existed another branch
or tag, which name was 'branch-something' or 'something/branch'.

Signed-off-by: Dmitry Potapov <redacted>
---

I have corrected my previous patch to allow "heads" or "tags"
in the name of a branch or tag, i.e. to write it like this:
   git filter-branch heads/master

 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index dbab1a9..5de8b12 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh

@@ -219,7 +219,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"
 	esac
 
 	git check-ref-format "$ref" && echo "$ref"

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 5f60b22..c3e5207 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh

@@ -36,6 +36,16 @@ test_expect_success 'result is really identical' '
 	test $H = $(git rev-parse HEAD)
 '
 
+test_expect_success 'rewrite branch with similar names' '
+	git branch my &&
+	git tag my/orig &&
+	git tag my-orig &&
+	git tag orig/my &&
+	git tag orig-my &&
+	git-filter-branch my &&
+	test $H = $(git rev-parse HEAD)
+'
+
 test_expect_success 'rewrite, renaming a specific file' '
 	git-filter-branch -f --tree-filter "mv d doh || :" HEAD
 '

-- 
1.5.3.5