Re: [JGIT PATCH 1/2] Issue 23: Resolve tag^0 as tag^{commit}

[Robin Rosenberg <robin.rosenberg.lists@dewire.com>]

fredagen den 12 september 2008 02.00.07 skrev Jonas Fonseca:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote Fri, Sep 12, 2008:
> > torsdagen den 11 september 2008 23.39.27 skrev Jonas Fonseca:
> > > 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
tag^2 means you have a merge commit. I think rev^0 is mentioned here
to mean that it is actually defined (and how).

> 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!

> > 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.
> 
> > 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.
Yes, please resubmit after updating the rev-parsing code.

> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java
> index 2a1a4ad..1fc73b4 100644
> --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java
> +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0008_testparserev.java
> @@ -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