Skip to contentSkip to conversation

Issue 23: Resolve tag^0 as tag^{commit}

PATCH In review Jonas Fonseca 4 participants 10 messages Jun 15, 2016

lore
Share & tools
lore

Jonas Fonseca

author
#0 lore raw

PATCH 1/2 Issue 23: Resolve tag^0 as tag^{commit}

Repository.resolve("tag^0") failed with "not a commit". Fix it and add a test case for it.

Signed-off-by Jonas Fonseca <[email protected]>
---
 .../org/spearce/jgit/lib/T0008_testparserev.java   |    9 ++++++++-
 .../src/org/spearce/jgit/lib/Repository.java       |    5 +++++
 2 files changed, 13 insertions(+), 1 deletions(-)
org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +8 -1
index 8883b8b..3457574 100644
@@ -109,10 +109,17 @@ public void testDerefTag() throws IOException {
 		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("spearce-gpg-pub^{blob}").name());
 	}
 
+	public void testDerefTagIssue23() throws IOException {
+		assertEquals("17768080a2318cd89bba4c8b87834401e2095703",db.resolve("refs/tags/B").name());
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B^{commit}").name());
+		// The special tag^0 == tag^{commit} rule
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B^0").name());
+	}
+
 	public void testDerefBlob() throws IOException {
 		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("fd608fbe625a2b456d9f15c2b1dc41f252057dd7^{}").name());
 		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("fd608fbe625a2b456d9f15c2b1dc41f252057dd7^{blob}").name());
 	}
-	
+
 	// TODO: ^{tree} for a tag pointing to a tag
 }
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +5 -0
index b27c23d..260a39d 100644
@@ -562,6 +562,11 @@ public ObjectId resolve(final String revstr) throws IOException {
 					case '9':
 						int j;
 						ref = mapObject(refId, null);
+						if (ref instanceof Tag && rev[i + 1] == '0') {
+							Tag tag = (Tag)ref;
+							refId = tag.getObjId();
+							ref = mapObject(refId, null);
+						}
 						if (!(ref instanceof Commit))
 							throw new IncorrectObjectTypeException(refId, Constants.TYPE_COMMIT);
 						for (j=i+1; j<rev.length; ++j) {
Signature · 5 lines
1.6.0.336.ga07ba


-- 
Jonas Fonseca
#1 lore raw

torsdagen den 11 september 2008 23.39.27 skrev Jonas Fonseca:

Quoted 2 lines

Repository.resolve("tag^0") failed with "not a commit". Fix it and add a test case for it.

It seems this case is not *that* special. It is tag^N == tag^{commit}^N Same for tag~N == tag^{commit}~N-

My fault, i guess, but it would be nice of you fixed it while you are at it.

Second, the testcase in the second patch fails on my machine.

testDerefTaggedTagTree(org.spearce.jgit.lib.T0008_testparserev)
junit.framework.ComparisonFailure: expected:<[269e1253bad5c247c6bde37aa48ae1e03138206c]> but was:<[be83157b4ffe650d728ba4f98ad47b623b0d0c20]>
at junit.framework.Assert.assertEquals(Assert.java:81)
at junit.framework.Assert.assertEquals(Assert.java:87)
at org.spearce.jgit.lib.T0008_testparserev.testDerefTaggedTagTree(T0008_testparserev.java:123)

-- robin

author
#2 lore raw

Robin Rosenberg <[email protected]> wrote Fri, Sep 12, 2008:

Quoted 6 lines

torsdagen den 11 september 2008 23.39.27 skrev Jonas Fonseca:

Quoted 2 lines

Repository.resolve("tag^0") failed with "not a commit". Fix it and add a test case for it.

It seems this case is not *that* special. It is tag^N == tag^{commit}^N Same for tag~N == tag^{commit}~N-

git-rev-parse(1) says:

· A suffix ^ to a revision parameter means the first parent of that
commit object. ^<n> means the <n>th parent (i.e. rev^ is equivalent to rev^1). As a special rule, rev^0 means the commit itself and is used when rev is the object name of a tag object that refers to a commit object.

but does take tag^1, however not tag^2. It looks like tag~N is broken in JGit ("not a commit") so should also be fixed if we want to be compatible with git-rev-parse.

BTW, I just noticed that ^{} is not handled correctly either for tags.

· A suffix ^ followed by an empty brace pair (e.g. v0.99.8^{}) means
the object could be a tag, and dereference the tag recursively until a non-tag object is found.

Only one derefence is performed.

Quoted 1 line

My fault, i guess, but it would be nice of you fixed it while you are at it.

I will try to make fixes for the above cases tomorrow.

Quoted 7 lines

Second, the testcase in the second patch fails on my machine.

testDerefTaggedTagTree(org.spearce.jgit.lib.T0008_testparserev)
junit.framework.ComparisonFailure: expected:<[269e1253bad5c247c6bde37aa48ae1e03138206c]> but was:<[be83157b4ffe650d728ba4f98ad47b623b0d0c20]>
at junit.framework.Assert.assertEquals(Assert.java:81)
at junit.framework.Assert.assertEquals(Assert.java:87)
at org.spearce.jgit.lib.T0008_testparserev.testDerefTaggedTagTree(T0008_testparserev.java:123)

I reran all the test before formatting the patch, but after doing some last minut changes. When I run all the tests using maven (inside NetBeans or from the command line) it tells me:

	Tests run: 428, Failures: 0, Errors: 0, Skipped: 0

From the output it looks like the tests in the files named T000* are never run. However, where the breakage is (could be my setup) I don't know. Imran?

Anyway, for now I will just run the single test specifically. Perhaps I should just put this into the same patch. Anyway, the following should fix it.

org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +1 -1
index 2a1a4ad..1fc73b4 100644
@@ -117,7 +117,7 @@ public void testDerefTagIssue23() throws IOException {
 	}
 
 	public void testDerefTaggedTagTree() throws IOException {
-		assertEquals("269e1253bad5c247c6bde37aa48ae1e03138206c",db.resolve("refs/tags/C").name());
+		assertEquals("be83157b4ffe650d728ba4f98ad47b623b0d0c20",db.resolve("refs/tags/C").name());
 		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/C^{commit}").name());
 		assertEquals("856ec208ae6cadac25a6d74f19b12bb27a24fe24",db.resolve("refs/tags/C^{tree}").name());
 	}
Signature · 1 line
Jonas Fonseca
#3 lore raw

fredagen den 12 september 2008 02.00.07 skrev Jonas Fonseca:

Quoted 10 lines

Robin Rosenberg <[email protected]> wrote Fri, Sep 12, 2008:

Quoted 6 lines

torsdagen den 11 september 2008 23.39.27 skrev Jonas Fonseca:

Quoted 2 lines

Repository.resolve("tag^0") failed with "not a commit". Fix it and add a test case for it.

It seems this case is not *that* special. It is tag^N == tag^{commit}^N Same for tag~N == tag^{commit}~N-

git-rev-parse(1) says:

Quoted 7 lines

· A suffix ^ to a revision parameter means the first parent of that
commit object. ^<n> means the <n>th parent (i.e. rev^ is equivalent to rev^1). As a special rule, rev^0 means the commit itself and is used when rev is the object name of a tag object that refers to a commit object.

but does take tag^1, however not tag^2. It looks like tag~N is broken in

tag^2 means you have a merge commit. I think rev^0 is mentioned here to mean that it is actually defined (and how).

Quoted 9 lines

JGit ("not a commit") so should also be fixed if we want to be compatible with git-rev-parse.

BTW, I just noticed that ^{} is not handled correctly either for tags.
· A suffix ^ followed by an empty brace pair (e.g. v0.99.8^{}) means
the object could be a tag, and dereference the tag recursively until a non-tag object is found.

Only one derefence is performed.

Good catch!

Quoted 18 lines
Quoted 1 line

My fault, i guess, but it would be nice of you fixed it while you are at it.

I will try to make fixes for the above cases tomorrow.

Quoted 7 lines

Second, the testcase in the second patch fails on my machine.

testDerefTaggedTagTree(org.spearce.jgit.lib.T0008_testparserev)
junit.framework.ComparisonFailure: expected:<[269e1253bad5c247c6bde37aa48ae1e03138206c]> but was:<[be83157b4ffe650d728ba4f98ad47b623b0d0c20]>
at junit.framework.Assert.assertEquals(Assert.java:81)
at junit.framework.Assert.assertEquals(Assert.java:87)
at org.spearce.jgit.lib.T0008_testparserev.testDerefTaggedTagTree(T0008_testparserev.java:123)

I reran all the test before formatting the patch, but after doing some last minut changes. When I run all the tests using maven (inside NetBeans or from the command line) it tells me:

	Tests run: 428, Failures: 0, Errors: 0, Skipped: 0
Quoted 7 lines

From the output it looks like the tests in the files named T000* are never run. However, where the breakage is (could be my setup) I don't know. Imran?

Anyway, for now I will just run the single test specifically. Perhaps I should just put this into the same patch. Anyway, the following should fix it.

Yes, please resubmit after updating the rev-parsing code.

Quoted 6 lines
org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +0 -0
index 2a1a4ad..1fc73b4 100644

org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java public void testDerefTagIssue23() throws IOException { · line 123

Quoted 8 lines
org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +1 -1
@@ -117,7 +117,7 @@ public void testDerefTagIssue23() throws IOException {
 	}
 
 	public void testDerefTaggedTagTree() throws IOException {
-		assertEquals("269e1253bad5c247c6bde37aa48ae1e03138206c",db.resolve("refs/tags/C").name());
+		assertEquals("be83157b4ffe650d728ba4f98ad47b623b0d0c20",db.resolve("refs/tags/C").name());
 		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/C^{commit}").name());
 		assertEquals("856ec208ae6cadac25a6d74f19b12bb27a24fe24",db.resolve("refs/tags/C^{tree}").name());
 	}

My guess is you added to original test tag C manually so it got an "non-deterministic" time stamp.

-- robin

author
#4 lore raw

PATCH 1/3 Fix IncorrectObjectTypeException thrown for incorrect ^{blob}

Signed-off-by Jonas Fonseca <[email protected]>
---
 .../src/org/spearce/jgit/lib/Repository.java       |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +1 -1
index b27c23d..730a267 100644
@@ -628,7 +628,7 @@ else if (item.equals("blob")) {
 									ref = mapObject(refId, null);
 								}
 								if (!(ref instanceof byte[]))
-									throw new IncorrectObjectTypeException(refId,  Constants.TYPE_COMMIT);
+									throw new IncorrectObjectTypeException(refId,  Constants.TYPE_BLOB);
 							}
 							else if (item.equals("")) {
 								ref = mapObject(refId, null);
Signature · 4 lines
1.6.0.1.451.gc8d31

-- 
Jonas Fonseca
author
#5 lore raw

PATCH 2/3 Fix off by one distance during resolving of commit~N

In reply to Robin Rosenberg

To be compatible with git-rev-parse, commit~0 should resolve to commit, commit~1 to commit^, etc.

Signed-off-by Jonas Fonseca <[email protected]>
---
 .../org/spearce/jgit/lib/T0008_testparserev.java   |    9 +++++----
 .../src/org/spearce/jgit/lib/Repository.java       |    2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)
org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +5 -4
index 8883b8b..506f51f 100644
@@ -82,10 +82,11 @@ public void testRef_refname() throws IOException {
 	}
 
 	public void testDistance() throws IOException {
-		assertEquals("6e1475206e57110fcef4b92320436c1e9872a322",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~0").name());
-		assertEquals("1203b03dc816ccbb67773f28b3c19318654b0bc8",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~1").name());
-		assertEquals("bab66b48f836ed950c99134ef666436fb07a09a0",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~2").name());
-		assertEquals("bab66b48f836ed950c99134ef666436fb07a09a0",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~02").name());
+		assertEquals("49322bb17d3acc9146f98c97d078513228bbf3c0",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~0").name());
+		assertEquals("6e1475206e57110fcef4b92320436c1e9872a322",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~1").name());
+		assertEquals("1203b03dc816ccbb67773f28b3c19318654b0bc8",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~2").name());
+		assertEquals("bab66b48f836ed950c99134ef666436fb07a09a0",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~3").name());
+		assertEquals("bab66b48f836ed950c99134ef666436fb07a09a0",db.resolve("49322bb17d3acc9146f98c97d078513228bbf3c0~03").name());
 	}
 
 	public void testTree() throws IOException {
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +1 -1
index 730a267..894fe3b 100644
@@ -688,7 +688,7 @@ else if (item.equals("")) {
 					throw new RevisionSyntaxException(
 							"Invalid ancestry length", revstr);
 				}
-				while (dist >= 0) {
+				while (dist > 0) {
 					final ObjectId[] parents = ((Commit) ref).getParentIds();
 					if (parents.length == 0) {
 						refId = null;
Signature · 4 lines
1.6.0.1.451.gc8d31

-- 
Jonas Fonseca
author
#6 lore raw

PATCH 3/3 Tests and fixes for dereferencing tags in Repository.resolve()

In reply to Robin Rosenberg

Fix Repository.resolve("tag^0") failing with "not a commit" (issue 23). Add test for resolving "tag^{tree}" where "tag" points to a tag

Signed-off-by Jonas Fonseca <[email protected]>
---
 .../spearce/jgit/test/resources/create-second-pack |    8 +++++
 ...ck-546ff360fe3488adb20860ce3436a2d6373d2796.idx |  Bin 0 -> 1324 bytes
 ...k-546ff360fe3488adb20860ce3436a2d6373d2796.pack |  Bin 0 -> 1265 bytes
 .../org/spearce/jgit/test/resources/packed-refs    |   18 ++++++++++++
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |    1 +
 .../org/spearce/jgit/lib/T0008_testparserev.java   |   24 +++++++++++++---
 .../src/org/spearce/jgit/lib/Repository.java       |   29 ++++++++++++++++---
 7 files changed, 71 insertions(+), 9 deletions(-)
 create mode 100755 org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/pack-546ff360fe3488adb20860ce3436a2d6373d2796.idx
 create mode 100755 org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/pack-546ff360fe3488adb20860ce3436a2d6373d2796.pack
org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/create-second-pack +8 -0
index 03f83dc..052877d 100755
@@ -130,6 +130,14 @@ git tag -a -m "An annotated tag" B a^
 
 git repack -d
 
+Bnth=B
+for nth in 2nd 3rd 4th 5th 6th 7th 8th 9th 10th; do
+	git tag -a -m "An $nth level annotated tag" "B$nth" "$Bnth"
+	Bnth="B$nth"
+done
+
+git repack -d
+
 git pack-refs --all
org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/pack-546ff360fe3488adb20860ce3436a2d6373d2796.idx new file binary +0 -0
new file mode 100755
index 0000000000000000000000000000000000000000..60331335d9bfa0a1830b707a36648bef5c434de9
GIT binary patch
org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/pack-546ff360fe3488adb20860ce3436a2d6373d2796.pack new file binary +0 -0
new file mode 100755
index 0000000000000000000000000000000000000000..203c157657b7f48faaaf23b794ce68264f3fd6a8
GIT binary patch
org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs +18 -0
index f67a3ef..746bd6b 100644
@@ -9,5 +9,23 @@ d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864 refs/heads/pa
 6db9c2ebf75590eef973081736730a9ea169a0c4 refs/tags/A
 17768080a2318cd89bba4c8b87834401e2095703 refs/tags/B
 ^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+032c063ce34486359e3ee3d4f9e5c225b9e1a4c2 refs/tags/B10th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+214cae792433672d28b3aeb9f75c1ae84fd54628 refs/tags/B2nd
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+1170b77a48d3ea2d58b043648b1ec63d606e3efa refs/tags/B3rd
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+8dfd42699e7b10e568fa1eaebe249e33e98da81e refs/tags/B4th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+efee904c794b943a06931c76c576dd552212e8bc refs/tags/B5th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+d54e006ebbef94b7d3a5cd56d154f1e6f08efb94 refs/tags/B6th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+a773cd2d9dbca00d08793dac0d7002a49f0428c0 refs/tags/B7th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+bf5123bb77c7b5a379f7de9c1293558e3e24dfb8 refs/tags/B8th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
+dd144af286452bfd6a1ea02b0d3745bcdb555e9d refs/tags/B9th
+^d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864
 8bbde7aacf771a9afb6992434f1ae413e010c6d8 refs/tags/spearce-gpg-pub
 ^fd608fbe625a2b456d9f15c2b1dc41f252057dd7
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +1 -0
index 270b90a..9d7d133 100644
@@ -141,6 +141,7 @@ public void run() {
 				"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f",
 				"pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371",
 				"pack-9fb5b411fe6dfa89cc2e6b89d2bd8e5de02b5745",
+				"pack-546ff360fe3488adb20860ce3436a2d6373d2796",
 				"pack-e6d07037cbcf13376308a0a995d1fa48f8f76aaa"
 		};
 		final File packDir = new File(db.getObjectsDirectory(), "pack");
org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +20 -4
index 506f51f..47105cf 100644
@@ -106,14 +106,30 @@ public void testDerefCommit() throws IOException {
 	}
 
 	public void testDerefTag() throws IOException {
-		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("spearce-gpg-pub^{}").name());
-		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("spearce-gpg-pub^{blob}").name());
+		assertEquals("17768080a2318cd89bba4c8b87834401e2095703",db.resolve("refs/tags/B").name());
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B^{commit}").name());
+		assertEquals("032c063ce34486359e3ee3d4f9e5c225b9e1a4c2",db.resolve("refs/tags/B10th").name());
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B10th^{commit}").name());
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B10th^{}").name());
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B10th^0").name());
+		assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/B10th~0").name());
+		assertEquals("0966a434eb1a025db6b71485ab63a3bfbea520b6",db.resolve("refs/tags/B10th^").name());
+		assertEquals("0966a434eb1a025db6b71485ab63a3bfbea520b6",db.resolve("refs/tags/B10th^1").name());
+		assertEquals("0966a434eb1a025db6b71485ab63a3bfbea520b6",db.resolve("refs/tags/B10th~1").name());
+		assertEquals("2c349335b7f797072cf729c4f3bb0914ecb6dec9",db.resolve("refs/tags/B10th~2").name());
 	}
 
 	public void testDerefBlob() throws IOException {
+		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("spearce-gpg-pub^{}").name());
+		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("spearce-gpg-pub^{blob}").name());
 		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("fd608fbe625a2b456d9f15c2b1dc41f252057dd7^{}").name());
 		assertEquals("fd608fbe625a2b456d9f15c2b1dc41f252057dd7",db.resolve("fd608fbe625a2b456d9f15c2b1dc41f252057dd7^{blob}").name());
 	}
-	
-	// TODO: ^{tree} for a tag pointing to a tag
+
+	public void testDerefTree() throws IOException {
+		assertEquals("032c063ce34486359e3ee3d4f9e5c225b9e1a4c2",db.resolve("refs/tags/B10th").name());
+		assertEquals("856ec208ae6cadac25a6d74f19b12bb27a24fe24",db.resolve("032c063ce34486359e3ee3d4f9e5c225b9e1a4c2^{tree}").name());
+		assertEquals("856ec208ae6cadac25a6d74f19b12bb27a24fe24",db.resolve("refs/tags/B10th^{tree}").name());
+	}
+
 }
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +24 -5
index 894fe3b..dfce1b8 100644
@@ -562,6 +562,11 @@ public ObjectId resolve(final String revstr) throws IOException {
 					case '9':
 						int j;
 						ref = mapObject(refId, null);
+						while (ref instanceof Tag) {
+							Tag tag = (Tag)ref;
+							refId = tag.getObjId();
+							ref = mapObject(refId, null);
+						}
 						if (!(ref instanceof Commit))
 							throw new IncorrectObjectTypeException(refId, Constants.TYPE_COMMIT);
 						for (j=i+1; j<rev.length; ++j) {

@@ -632,10 +637,10 @@ else if (item.equals("blob")) {
 							}
 							else if (item.equals("")) {
 								ref = mapObject(refId, null);
-								if (ref instanceof Tag)
-									refId = ((Tag)ref).getObjId();
-								else {
-									// self
+								while (ref instanceof Tag) {
+									Tag t = (Tag)ref;
+									refId = t.getObjId();
+									ref = mapObject(refId, null);
 								}
 							}
 							else

@@ -658,6 +663,11 @@ else if (item.equals("")) {
 					}
 				} else {
 					ref = mapObject(refId, null);
+					while (ref instanceof Tag) {
+						Tag tag = (Tag)ref;
+						refId = tag.getObjId();
+						ref = mapObject(refId, null);
+					}
 					if (ref instanceof Commit) {
 						final ObjectId parents[] = ((Commit) ref)
 								.getParentIds();

@@ -673,8 +683,17 @@ else if (item.equals("")) {
 				if (ref == null) {
 					String refstr = new String(rev,0,i);
 					refId = resolveSimple(refstr);
-					ref = mapCommit(refId);
+					if (refId == null)
+						return null;
+					ref = mapObject(refId, null);
+				}
+				while (ref instanceof Tag) {
+					Tag tag = (Tag)ref;
+					refId = tag.getObjId();
+					ref = mapObject(refId, null);
 				}
+				if (!(ref instanceof Commit))
+					throw new IncorrectObjectTypeException(refId, Constants.TYPE_COMMIT);
 				int l;
 				for (l = i + 1; l < rev.length; ++l) {
 					if (!Character.isDigit(rev[l]))
Signature · 4 lines
1.6.0.1.451.gc8d31

-- 
Jonas Fonseca
#7 lore raw
In reply to Jonas Fonseca
Quoted 12 lines · Jonas Fonseca
Quoted 7 lines

Second, the testcase in the second patch fails on my machine.

testDerefTaggedTagTree(org.spearce.jgit.lib.T0008_testparserev)
junit.framework.ComparisonFailure: expected:<[269e1253bad5c247c6bde37aa48ae1e03138206c]> but was:<[be83157b4ffe650d728ba4f98ad47b623b0d0c20]>
at junit.framework.Assert.assertEquals(Assert.java:81)
at junit.framework.Assert.assertEquals(Assert.java:87)
at org.spearce.jgit.lib.T0008_testparserev.testDerefTaggedTagTree(T0008_testparserev.java:123)

I reran all the test before formatting the patch, but after doing some last minut changes. When I run all the tests using maven (inside NetBeans or from the command line) it tells me:

Quoted 6 lines
       Tests run: 428, Failures: 0, Errors: 0, Skipped: 0

From the output it looks like the tests in the files named T000* are never run. However, where the breakage is (could be my setup) I don't know. Imran?

Hmm, I will have to check it. Will come back with some feedback after checking. Usually if a class is a TestCase then it should have run.

Quoted 21 lines

Anyway, for now I will just run the single test specifically. Perhaps I should just put this into the same patch. Anyway, the following should fix it.

org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java +1 -2
index 2a1a4ad..1fc73b4 100644
@@ -117,7 +117,7 @@ public void testDerefTagIssue23() throws IOException {
       }
 
       public void testDerefTaggedTagTree() throws IOException {
-               assertEquals("269e1253bad5c247c6bde37aa48ae1e03138206c",db.resolve("refs/tags/C").name());
+               assertEquals("be83157b4ffe650d728ba4f98ad47b623b0d0c20",db.resolve("refs/tags/C").name());
               assertEquals("d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864",db.resolve("refs/tags/C^{commit}").name());
               assertEquals("856ec208ae6cadac25a6d74f19b12bb27a24fe24",db.resolve("refs/tags/C^{tree}").name());
       }
 
--

Jonas Fonseca

Signature · 7 lines
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: [email protected]
Blog: http://imyousuf-tech.blogs.smartitengineering.com/
Mobile: +880-1711402557
author
#8 lore raw

PATCH 1/1 Configure the maven surefire plugin to specifically include all tests

By default, the test cases named T000* were not included. With this patch maven reports that 508 tests have been run.

Signed-off-by Jonas Fonseca <[email protected]>
---
 jgit-maven/jgit/pom.xml |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

Imran M Yousuf <[email protected]> wrote Fri, Sep 12, 2008:

Quoted 6 lines · Jonas Fonseca
Quoted 5 lines

When I run all the tests using maven (inside
NetBeans or from the command line) it tells me:

       Tests run: 428, Failures: 0, Errors: 0, Skipped: 0
Quoted 7 lines
Quoted 4 lines

From the output it looks like the tests in the files named T000* are never run. However, where the breakage is (could be my setup) I don't know. Imran?

Hmm, I will have to check it. Will come back with some feedback after checking. Usually if a class is a TestCase then it should have run.

I started looking for possible bug reports at jira.codehaus.org and found an issue for extendeding the default path patterns used for including and excluding tests. Maybe something like this patch is clean enough, at least it increases "my coverage" to 508 tests.

jgit-maven/jgit/pom.xml +11 -0
index a64f53c..a123470 100644
@@ -158,6 +158,17 @@ ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
                     <encoding>UTF-8</encoding>
                 </configuration>
             </plugin>
+            <plugin>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>2.4.2</version>
+                <configuration>
+                    <includes>
+                        <include>**/*Test.java</include>
+                        <include>**/*TestCase.java</include>
+                        <include>**/T000*.java</include>
+                    </includes>
+                </configuration>
+           </plugin>
         </plugins>
     </build>
     <dependencies>
Signature · 4 lines
1.6.0.1.451.gc8d31

-- 
Jonas Fonseca
#9 lore raw

Re: [JGIT PATCH] Configure the maven surefire plugin to specifically include all tests

Quoted 2 lines · Jonas Fonseca

By default, the test cases named T000* were not included. With this patch maven reports that 508 tests have been run.

Thanks. 508 is the correct current number of tests.

Quoted 11 lines
jgit-maven/jgit/pom.xml +11 -0
+            <plugin>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>2.4.2</version>
+                <configuration>
+                    <includes>
+                        <include>**/*Test.java</include>
+                        <include>**/*TestCase.java</include>
+                        <include>**/T000*.java</include>
+                    </includes>
+                </configuration>
+           </plugin>
Signature · 1 line
Shawn.