[PATCH 0/2] Fix t1512 (disambiguation of object names)

[PATCH 0/2] Fix t1512 (disambiguation of object names)

[Junio C Hamano]

Here is a small two-patch series to break a test that has been
passing by mistake.

Junio C Hamano (2):
  t1512: correct leftover constants from earlier edition
  get_short_sha1(): correctly disambiguate type-limited abbreviation

 sha1_name.c                         |  2 +-
 t/t1512-rev-parse-disambiguation.sh | 32 ++++++++++++++++++++++----------
 2 files changed, 23 insertions(+), 11 deletions(-)

-- 
1.8.3.2-795-g615e8f0

[PATCH 1/2] t1512: correct leftover constants from earlier edition

[Junio C Hamano]

The earliest iteration of this test script used a magic string
110282 as the common prefix for ambiguous object names, but the
final edition switched the common prefix to 0000000000 (10 "0"s).

Unfortunately, instances of the original prefix were left in the
comments and a few tests.  Replace them with the correct constants.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t1512-rev-parse-disambiguation.sh | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..5b58e4f 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -77,6 +77,7 @@ test_expect_success 'disambiguate blob' '
 
 test_expect_success 'disambiguate tree' '
 	commit=$(echo "d7xm" | git commit-tree 000000000) &&
+	# this commit is fffff2e and not ambiguous with the 00000* objects
 	test $(git rev-parse $commit^{tree}) = $(git rev-parse 0000000000cdc)
 '
 
@@ -99,10 +100,14 @@ test_expect_success 'disambiguate commit-ish' '
 
 test_expect_success 'disambiguate commit' '
 	commit=$(echo "hoaxj" | git commit-tree 0000000000cdc -p 000000000) &&
+	# this commit is ffffffd8 and not ambiguous with the 00000* objects
 	test $(git rev-parse $commit^) = $(git rev-parse 0000000000e4f)
 '
 
 test_expect_success 'log name1..name2 takes only commit-ishes on both ends' '
+	# These are underspecified from the prefix-length point of view
+	# to disambiguate the commit with other objects, but there is only
+	# one commit that has 00000* prefix at this point.
 	git log 000000000..000000000 &&
 	git log ..000000000 &&
 	git log 000000000.. &&
@@ -112,16 +117,19 @@ test_expect_success 'log name1..name2 takes only commit-ishes on both ends' '
 '
 
 test_expect_success 'rev-parse name1..name2 takes only commit-ishes on both ends' '
+	# Likewise.
 	git rev-parse 000000000..000000000 &&
 	git rev-parse ..000000000 &&
 	git rev-parse 000000000..
 '
 
 test_expect_success 'git log takes only commit-ish' '
+	# Likewise.
 	git log 000000000
 '
 
 test_expect_success 'git reset takes only commit-ish' '
+	# Likewise.
 	git reset 000000000
 '
 
@@ -131,26 +139,30 @@ test_expect_success 'first tag' '
 '
 
 test_expect_failure 'two semi-ambiguous commit-ish' '
+	# At this point, we have a tag 0000000000f8f that points
+	# at a commit 0000000000e4f, and a tree and a blob that
+	# share 0000000000 prefix with these tag and commit.
+	#
 	# Once the parser becomes ultra-smart, it could notice that
-	# 110282 before ^{commit} name many different objects, but
+	# 0000000000 before ^{commit} name many different objects, but
 	# that only two (HEAD and v1.0.0 tag) can be peeled to commit,
 	# and that peeling them down to commit yield the same commit
 	# without ambiguity.
-	git rev-parse --verify 110282^{commit} &&
+	git rev-parse --verify 0000000000^{commit} &&
 
 	# likewise
-	git log 000000000..000000000 &&
-	git log ..000000000 &&
-	git log 000000000.. &&
-	git log 000000000...000000000 &&
-	git log ...000000000 &&
-	git log 000000000...
+	git log 0000000000..0000000000 &&
+	git log ..0000000000 &&
+	git log 0000000000.. &&
+	git log 0000000000...0000000000 &&
+	git log ...0000000000 &&
+	git log 0000000000...
 '
 
 test_expect_failure 'three semi-ambiguous tree-ish' '
 	# Likewise for tree-ish.  HEAD, v1.0.0 and HEAD^{tree} share
 	# the prefix but peeling them to tree yields the same thing
-	git rev-parse --verify 000000000^{tree}
+	git rev-parse --verify 0000000000^{tree}
 '
 
 test_expect_success 'parse describe name' '
@@ -241,7 +253,7 @@ test_expect_success 'ambiguous commit-ish' '
 	# Now there are many commits that begin with the
 	# common prefix, none of these should pick one at
 	# random.  They all should result in ambiguity errors.
-	test_must_fail git rev-parse --verify 110282^{commit} &&
+	test_must_fail git rev-parse --verify 00000000^{commit} &&
 
 	# likewise
 	test_must_fail git log 000000000..000000000 &&
-- 
1.8.3.2-795-g615e8f0

[PATCH 2/2] get_short_sha1(): correctly disambiguate type-limited abbreviation

[Junio C Hamano]

One test in t1512 that expects a failure incorrectly passed.  The
test prepares a commit whose object name begins with ten "0"s, and
also prepares a tag that points at the commit.  The object name of
the tag also begins with ten "0"s.  There is no other commit-ish
object in the repository whose name begins with such a prefix.

Ideally, in such a repository:

    $ git rev-parse --verify 0000000000^{commit}

should yield that commit.  If 0000000000 is taken as the commit
0000000000e4f, peeling it to a commmit yields that commit itself,
and if 0000000000 is taken as the tag 0000000000f8f, peeling it to a
commit also yields the same commit, so in that twisted sense, the
extended SHA-1 expression 0000000000^{commit} is unambigous.  The
test that expects a failure is to check the above command.

The reason the test expects a failure is that we did not implement
such a "unification" of two candidate objects.  What we did (or at
least, meant to) implement was to recognise that a commit-ish is
required to expand 0000000000, and notice that there are two succh
commit-ish, and diagnose the request as ambiguous.

However, there was a bug in the logic to check the candidate
objects.  When the code saw 0000000000f8f (a tag) that shared the
shortened prefix (ten "0"s), it tried to make sure that the tag is a
commit-ish by looking at the tag object.  Because it incorrectly
used lookup_object() when the tag has not been parsed, however, we
incorrectly declared that the tag is _not_ a commit-ish, leaving the
sole commit in the repository, 0000000000e4f, that has the required
prefix as "unique match", causing the test to pass when it shouldn't.

This fixes the logic to inspect the type of the object a tag refers
to, to make the test that is expected to fail correctly fail.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_name.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index 8bc20c5..13c3d75 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -241,7 +241,7 @@ static int disambiguate_committish_only(const unsigned char *sha1, void *cb_data
 		return 0;
 
 	/* We need to do this the hard way... */
-	obj = deref_tag(lookup_object(sha1), NULL, 0);
+	obj = deref_tag(parse_object(sha1), NULL, 0);
 	if (obj && obj->type == OBJ_COMMIT)
 		return 1;
 	return 0;
-- 
1.8.3.2-795-g615e8f0