From: Shawn O. Pearce <spearce@spearce.org>
Date: Mon, 06 Oct 2008 07:43:03 GMT
Subject: Re: [EGIT PATCH 2/6] Peel annotated tags when getting all refs
Message-ID: <20081006074303.GB27516@spearce.org>
In-Reply-To: <1223249802-9959-3-git-send-email-robin.rosenberg@dewire.com>

FWIW I'm still going through the series.  But this jumped out at me.

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> For packed refs we got this automatically from packed-refs,
> but for loose tags we have to follow the tags and get the leaf
> object in order to comply with the documentation.
...
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
> index 5a1b85f..1ee70d9 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
> @@ -271,7 +271,16 @@ private void readOneLooseRef(final Map<String, Ref> avail,
>  					return;
>  				}
>  
> -				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id);
> +				Object tt = db.mapObject(id, refName);
> +				if (tt != null && tt instanceof Tag) {
> +					Tag t = (Tag)tt;

Owwww.

I don't want to be doing peeling of loose annotated tags anytime
we loop through the loose objects.  That is just painful and very
expensive.  I'd rather the peeling be caused on demand by callers
who need it, but if its done through a method on Repository then
we can push the peeled ObjectId back into the RefDatabase cache.

I'm thinking more like:

	Repository db = ...;
	...
	ObjectId p = ref.getPeeledObjectId()
	if (p == null)
		p = db.peel(ref)

-- 
Shawn.