threads / patch / 43387

patchRe: [PATCH] Oops, get the commit first, then the tree.

Subject: Re: [PATCH] Oops, get the commit first, then the tree.

## tl;dr

2 messages between Dec 3, 2006 and Dec 3, 2006. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Robin Rosenberg· Dec 3, 2006, 00:45 UTC · lore

[PATCH] Oops, get the commit first, then the tree.

This made the eclipse plugin unable to connect to Git.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/lib/Repository.java       |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Show changes to org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +2 −1

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java 
b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index bfdcd3e..2344817 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -168,7 +168,8 @@ public class Repository {
     }
 
     public Tree mapTree(final String revstr) throws IOException {
-	final ObjectId id = resolve(revstr);
+	Commit commit = mapCommit(revstr);
+	final ObjectId id = commit.getTreeId();
 	return id != null ? mapTree(id) : null;
     }

-- 
1.4.4.gf05d

Shawn Pearce· Dec 3, 2006, 02:34 UTC · re: Robin Rosenberg · lore

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:

> This made the eclipse plugin unable to connect to Git.

...

Show 10 quoted lines

> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -168,7 +168,8 @@ public class Repository {
>      }
>  
>      public Tree mapTree(final String revstr) throws IOException {
> -	final ObjectId id = resolve(revstr);
> +	Commit commit = mapCommit(revstr);
> +	final ObjectId id = commit.getTreeId();
>  	return id != null ? mapTree(id) : null;

I don't want to apply the fix above because mapTree(String) should be able to take an arbitrary SHA1 expression and return the Tree object corresponding to that. Your change only permits commits to be resolved; but if the revstr resolved to a tag we'd need to peel back the tag to get to the tree. Ideally that should be done in mapTree(ObjectId), based on the type of thing ObjectId points to.

I think the bug was caused by a recent breakage in Repository.mapTree (42691339). I fixed it in 020e0ee but neglected to push it out. Actually I'm surprised mapTree(ObjectId) even works before 020e0ee; I just don't see how its right...