[RFC/PATCH] refs: tone down the dwimmery in refname_match() for {heads,tags,remotes}/*

From

Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Date

May 26, 2019, 22:54 UTC

Message-ID

<20190526225445.21618-1-avarab@gmail.com>

In-Reply-To

<CAHk-=wgzKzAwS=_ySikL1f=Gr62YXL_WXGh82wZKMOvzJ9+2VA@mail.gmail.com>

This resulted in a case[1] where someone on LKML did:

    git push kvm +HEAD:tags/for-linus

1. https://lore.kernel.org/lkml/2d55fd2a-afbf-1b7c-ca82-8bffaa18e0d0@redhat.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Sun, May 26 2019, Linus Torvalds wrote:

> On Sun, May 26, 2019 at 10:53 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> The interesting thing is that not only git will treat lightweight tags
>> like, well, tags:
>
> Yeah, that's very much by design - lightweight tags are very
> comvenient for local temporary stuff where you don't want signing etc
> (think automated test infrastructure, or just local reminders).
>
>> In addition, because I _locally_ had a tag object that
>> pointed to the same commit and had the same name, git-request-pull
>> included my local tag's message in its output!  I wonder if this could
>> be considered a bug.
>
> Yeah, I think git request-pull should at least *warn* about the tag
> not being the same object locally as in the remote you're asking me to
> pull.
>
> Are you sure you didn't get a warning, and just missed it? But adding
> Junio and the Git list just as a possible heads-up for this in case
> git request-pull really only compares the object the tag points to,
> rather than the SHA1 of the tag itself.

 refs.c                   |  8 +++++++-
 t/t5150-request-pull.sh  |  2 +-
 t/t5505-remote.sh        | 17 +++++++++++++++++
 t/t9101-git-svn-props.sh | 12 ++++++------
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 92d1f6dbdd..729b921328 100644
--- a/refs.c
+++ b/refs.c
@@ -514,9 +514,15 @@ int refname_match(const char *abbrev_name, const char *full_name)
 	const int abbrev_name_len = strlen(abbrev_name);
 	const int num_rules = NUM_REV_PARSE_RULES;
 
-	for (p = ref_rev_parse_rules; *p; p++)
+	for (p = ref_rev_parse_rules; *p; p++) {
+		if (!strcmp(*p, "refs/%.*s") &&
+		    (starts_with(abbrev_name, "tags/") ||
+		     starts_with(abbrev_name, "heads/") ||
+		     starts_with(abbrev_name, "remotes/")))
+			continue;
 		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name)))
 			return &ref_rev_parse_rules[num_rules] - p;
+	}
 
 	return 0;
 }
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index fca001eb9b..0265871cf4 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -212,7 +212,7 @@ test_expect_success 'pull request format' '
 		cd local &&
 		git checkout initial &&
 		git merge --ff-only master &&
-		git push origin tags/full &&
+		git push origin full:refs/tags/full &&
 		git request-pull initial "$downstream_url" tags/full >../request
 	) &&
 	<request sed -nf fuzz.sed >request.fuzzy &&
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 883b32efa0..52507b9e50 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1277,4 +1277,21 @@ test_expect_success 'refs/remotes/* <src> refspec and unqualified <dst> DWIM and
 	)
 '
 
+test_expect_success 'HEAD:tags/A and HEAD:tags/B should not be different one of refs/tags/[AB] exists' '
+	git clone "file://$PWD/two" tags-match &&
+	(
+		cd tags-match &&
+		test_commit A &&
+		git rev-parse HEAD >expected &&
+
+		git push origin HEAD:tags/my-not-a-tag &&
+		git -C ../two rev-parse refs/heads/tags/my-not-a-tag >actual &&
+		test_cmp expected actual &&
+
+		git push origin HEAD:tags/my-tag &&
+		git -C ../two rev-parse refs/heads/tags/my-tag >actual &&
+		test_cmp expected actual
+	)
+'
+
 test_done
diff --git a/t/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh
index c26c4b0927..f9e43f4e97 100755
--- a/t/t9101-git-svn-props.sh
+++ b/t/t9101-git-svn-props.sh
@@ -73,11 +73,11 @@ test_expect_success 'fetch revisions from svn' 'git svn fetch'
 
 name='test svn:keywords ignoring'
 test_expect_success "$name" \
-	'git checkout -b mybranch remotes/git-svn &&
+	'git checkout -b mybranch refs/remotes/git-svn &&
 	echo Hi again >> kw.c &&
 	git commit -a -m "test keywords ignoring" &&
-	git svn set-tree remotes/git-svn..mybranch &&
-	git pull . remotes/git-svn'
+	git svn set-tree refs/remotes/git-svn..mybranch &&
+	git pull . refs/remotes/git-svn'
 
 expect='/* $Id$ */'
 got="$(sed -ne 2p kw.c)"
@@ -95,7 +95,7 @@ test_expect_success "propset CR on crlf files" '
 
 test_expect_success 'fetch and pull latest from svn and checkout a new wc' \
 	'git svn fetch &&
-	 git pull . remotes/git-svn &&
+	 git pull . refs/remotes/git-svn &&
 	 svn_cmd co "$svnrepo" new_wc'
 
 for i in crlf ne_crlf lf ne_lf cr ne_cr empty_cr empty_lf empty empty_crlf
@@ -117,7 +117,7 @@ cd test_wc
 	 svn_cmd commit -m "propset CRLF on cr files"'
 cd ..
 test_expect_success 'fetch and pull latest from svn' \
-	'git svn fetch && git pull . remotes/git-svn'
+	'git svn fetch && git pull . refs/remotes/git-svn'
 
 b_cr="$(git hash-object cr)"
 b_ne_cr="$(git hash-object ne_cr)"
@@ -168,7 +168,7 @@ cat >create-ignore-index.expect <<\EOF
 EOF
 
 test_expect_success 'test create-ignore' "
-	git svn fetch && git pull . remotes/git-svn &&
+	git svn fetch && git pull . refs/remotes/git-svn &&
 	git svn create-ignore &&
 	cmp ./.gitignore create-ignore.expect &&
 	cmp ./deeply/.gitignore create-ignore.expect &&

-- 
2.21.0.1020.gf2820cf01a