[J/E-GIT PATCH 0/7] Tag decoration v2

[J/E-GIT PATCH 0/7] Tag decoration v2

[Robin Rosenberg]

This is a cleaned up version of the tag decoration patch set. It
also draws the labels slightly nicer, especially when the background
has alternating colors for each row.

-- robin

Robin Rosenberg (7):
  Test the origName part of the key vs the ref itself
  Add constant for "refs/"
  Keep original ref name when reading refs
  Handle peeling of loose refs.
  Add a method to get refs by object Id
  Add tags to the graphical history display.
  Add decorate option to log program

 .../egit/ui/internal/history/SWTCommit.java        |    5 +-
 .../egit/ui/internal/history/SWTPlotRenderer.java  |   71 ++++++++++++++++++-
 .../spearce/egit/ui/internal/history/SWTWalk.java  |    2 +-
 .../src/org/spearce/jgit/pgm/Log.java              |   33 +++++++++
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |   10 +++
 .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |   46 ++++++++++++
 .../src/org/spearce/jgit/lib/Constants.java        |    3 +
 org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java |   77 ++++++++++++++++++--
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   71 ++++++++++++++----
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   14 ++---
 .../src/org/spearce/jgit/lib/Repository.java       |   47 ++++++++++++
 .../spearce/jgit/revplot/AbstractPlotRenderer.java |   23 ++++++-
 .../src/org/spearce/jgit/revplot/PlotCommit.java   |    8 ++-
 .../src/org/spearce/jgit/revplot/PlotWalk.java     |   60 +++++++++++++++-
 14 files changed, 432 insertions(+), 38 deletions(-)

[EGIT PATCH 1/7] Test the origName part of the key vs the ref itself

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 6e2cfa8..12f9ada 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -39,6 +39,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import org.spearce.jgit.lib.RefUpdate.Result;
 
@@ -127,4 +129,12 @@ public void testDeleteEmptyDirs() throws IOException {
 	private void assertExists(final boolean expected, final String name) {
 		assertEquals(expected, new File(db.getDirectory(), name).exists());
 	}
+
+	public void testRefKeySameAsOrigName() {
+		Map<String, Ref> allRefs = db.getAllRefs();
+		for (Entry<String, Ref> e : allRefs.entrySet()) {
+			assertEquals(e.getKey(), e.getValue().getOrigName());
+
+		}
+	}
 }
-- 
1.6.0.3.578.g6a50

[EGIT PATCH 2/7] Add constant for "refs/"

[Robin Rosenberg]

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
index f316881..9613d07 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
@@ -216,6 +216,9 @@
 	/** Prefix for tag refs */
 	public static final String R_TAGS = "refs/tags/";
 
+	/** Prefix for any ref */
+	public static final String R_REFS = "refs/";
+
 	/** Logs folder name */
 	public static final String LOGS = "logs";
 
-- 
1.6.0.3.578.g6a50

[EGIT PATCH 3/7] Keep original ref name when reading refs

[Robin Rosenberg]

We want to know the original name of refs, not just the target name.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java |   63 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   45 +++++++++-----
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   14 ++---
 3 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
index db94875..2f102af 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
@@ -43,6 +43,11 @@
  * A ref in Git is (more or less) a variable that holds a single object
  * identifier. The object identifier can be any valid Git object (blob, tree,
  * commit, annotated tag, ...).
+ * <p>
+ * The ref name has the attributes of the ref that was asked for as well as
+ * the ref it was resolved to for symbolic refs plus the object id it points
+ * to and (for tags) the peeled target object id, i.e. the tag resolved
+ * recursively until a non-tag object is referenced.
  */
 public class Ref {
 	/** Location where a {@link Ref} is stored. */
@@ -119,19 +124,24 @@ public boolean isPacked() {
 
 	private ObjectId peeledObjectId;
 
+	private final String origName;
+
 	/**
 	 * Create a new ref pairing.
 	 * 
 	 * @param st
 	 *            method used to store this ref.
+	 * @param origName
+	 *            The name used to resolve this ref
 	 * @param refName
 	 *            name of this ref.
 	 * @param id
 	 *            current value of the ref. May be null to indicate a ref that
 	 *            does not exist yet.
 	 */
-	public Ref(final Storage st, final String refName, final ObjectId id) {
+	public Ref(final Storage st, final String origName, final String refName, final ObjectId id) {
 		storage = st;
+		this.origName = origName;
 		name = refName;
 		objectId = id;
 	}
@@ -146,19 +156,56 @@ public Ref(final Storage st, final String refName, final ObjectId id) {
 	 * @param id
 	 *            current value of the ref. May be null to indicate a ref that
 	 *            does not exist yet.
+	 */
+	public Ref(final Storage st, final String refName, final ObjectId id) {
+		this(st, refName, refName, id);
+	}
+
+	/**
+	 * Create a new ref pairing.
+	 * 
+	 * @param st
+	 *            method used to store this ref.
+	 * @param origName
+	 *            The name used to resolve this ref
+	 * @param refName
+	 *            name of this ref.
+	 * @param id
+	 *            current value of the ref. May be null to indicate a ref that
+	 *            does not exist yet.
 	 * @param peel
 	 *            peeled value of the ref's tag. May be null if this is not a
 	 *            tag or the peeled value is not known.
 	 */
-	public Ref(final Storage st, final String refName, final ObjectId id,
+	public Ref(final Storage st, final String origName, final String refName, final ObjectId id,
 			final ObjectId peel) {
 		storage = st;
+		this.origName = origName;
 		name = refName;
 		objectId = id;
 		peeledObjectId = peel;
 	}
 
 	/**
+	 * Create a new ref pairing.
+	 * 
+	 * @param st
+	 *            method used to store this ref.
+	 * @param refName
+	 *            name of this ref.
+	 * @param id
+	 *            current value of the ref. May be null to indicate a ref that
+	 *            does not exist yet.
+	 * @param peel
+	 *            peeled value of the ref's tag. May be null if this is not a
+	 *            tag or the peeled value is not known.
+	 */
+	public Ref(final Storage st, final String refName, final ObjectId id,
+			final ObjectId peel) {
+		this(st, refName, refName, id, peel);
+	}
+
+	/**
 	 * What this ref is called within the repository.
 	 * 
 	 * @return name of this ref.
@@ -168,6 +215,13 @@ public String getName() {
 	}
 
 	/**
+	 * @return the originally resolved name
+	 */
+	public String getOrigName() {
+		return origName;
+	}
+
+	/**
 	 * Cached value of this ref.
 	 * 
 	 * @return the value of this ref at the last time we read it.
@@ -200,6 +254,9 @@ public Storage getStorage() {
 	}
 
 	public String toString() {
-		return "Ref[" + name + "=" + ObjectId.toString(getObjectId()) + "]";
+		String o = "";
+		if (!origName.equals(name))
+			o = "(" + origName + ")";
+		return "Ref[" + o + name + "=" + ObjectId.toString(getObjectId()) + "]";
 	}
 }
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 5c1f060..5a1b85f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -51,6 +51,7 @@
 import java.util.Map;
 
 import org.spearce.jgit.errors.ObjectWritingException;
+import org.spearce.jgit.lib.Ref.Storage;
 import org.spearce.jgit.util.FS;
 import org.spearce.jgit.util.NB;
 
@@ -135,8 +136,8 @@ RefUpdate newUpdate(final String name) throws IOException {
 		return new RefUpdate(this, r, fileForRef(r.getName()));
 	}
 
-	void stored(final String name, final ObjectId id, final long time) {
-		looseRefs.put(name, new Ref(Ref.Storage.LOOSE, name, id));
+	void stored(final String origName, final String name, final ObjectId id, final long time) {
+		looseRefs.put(name, new Ref(Ref.Storage.LOOSE, origName, name, id));
 		looseRefsMTime.put(name, time);
 		setModified();
 		db.fireRefsMaybeChanged();
@@ -222,12 +223,12 @@ private void readLooseRefs(final Map<String, Ref> avail,
 			final String entName = ent.getName();
 			if (".".equals(entName) || "..".equals(entName))
 				continue;
-			readOneLooseRef(avail, prefix + entName, ent);
+			readOneLooseRef(avail, prefix + entName, prefix + entName, ent);
 		}
 	}
 
 	private void readOneLooseRef(final Map<String, Ref> avail,
-			final String refName, final File ent) {
+			final String origName, final String refName, final File ent) {
 		// Unchanged and cached? Don't read it again.
 		//
 		Ref ref = looseRefs.get(refName);
@@ -270,7 +271,7 @@ private void readOneLooseRef(final Map<String, Ref> avail,
 					return;
 				}
 
-				ref = new Ref(Ref.Storage.LOOSE, refName, id);
+				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id);
 				looseRefs.put(ref.getName(), ref);
 				looseRefsMTime.put(ref.getName(), ent.lastModified());
 				avail.put(ref.getName(), ref);
@@ -293,27 +294,35 @@ private File fileForRef(final String name) {
 		return new File(gitDir, name);
 	}
 
-	private Ref readRefBasic(final String name, final int depth)
+	private Ref readRefBasic(final String name, final int depth) throws IOException {
+		return readRefBasic(name, name, depth);
+	}
+
+	private Ref readRefBasic(final String origName, final String name, final int depth)
 			throws IOException {
 		// Prefer loose ref to packed ref as the loose
 		// file can be more up-to-date than a packed one.
 		//
-		Ref ref = looseRefs.get(name);
+		Ref ref = looseRefs.get(origName);
 		final File loose = fileForRef(name);
 		final long mtime = loose.lastModified();
 		if (ref != null) {
 			Long cachedlastModified = looseRefsMTime.get(name);
 			if (cachedlastModified != null && cachedlastModified == mtime)
 				return ref;
-			looseRefs.remove(name);
-			looseRefsMTime.remove(name);
+			looseRefs.remove(origName);
+			looseRefsMTime.remove(origName);
 		}
 
 		if (mtime == 0) {
 			// If last modified is 0 the file does not exist.
 			// Try packed cache.
 			//
-			return packedRefs.get(name);
+			ref = packedRefs.get(name);
+			if (ref != null)
+				if (!ref.getOrigName().equals(origName))
+					ref = new Ref(Storage.LOOSE_PACKED, origName, name, ref.getObjectId());
+			return ref;
 		}
 
 		final String line;
@@ -324,7 +333,7 @@ private Ref readRefBasic(final String name, final int depth)
 		}
 
 		if (line == null || line.length() == 0)
-			return new Ref(Ref.Storage.LOOSE, name, null);
+			return new Ref(Ref.Storage.LOOSE, origName, name, null);
 
 		if (line.startsWith("ref: ")) {
 			if (depth >= 5) {
@@ -333,12 +342,16 @@ private Ref readRefBasic(final String name, final int depth)
 			}
 
 			final String target = line.substring("ref: ".length());
-			final Ref r = readRefBasic(target, depth + 1);
+			Ref r = readRefBasic(target, target, depth + 1);
 			Long cachedMtime = looseRefsMTime.get(name);
 			if (cachedMtime != null && cachedMtime != mtime)
 				setModified();
 			looseRefsMTime.put(name, mtime);
-			return r != null ? r : new Ref(Ref.Storage.LOOSE, target, null);
+			if (r == null)
+				return new Ref(Ref.Storage.LOOSE, origName, target, null);
+			if (!origName.equals(r.getName()))
+				r = new Ref(Ref.Storage.LOOSE_PACKED, origName, r.getName(), r.getObjectId(), r.getPeeledObjectId());
+			return r; 
 		}
 
 		setModified();
@@ -350,7 +363,7 @@ private Ref readRefBasic(final String name, final int depth)
 			throw new IOException("Not a ref: " + name + ": " + line);
 		}
 
-		ref = new Ref(Ref.Storage.LOOSE, name, id);
+		ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
 		looseRefs.put(name, ref);
 		looseRefsMTime.put(name, mtime);
 		return ref;
@@ -384,7 +397,7 @@ private void refreshPackedRefs() {
 
 						final ObjectId id = ObjectId.fromString(p.substring(1));
 						last = new Ref(Ref.Storage.PACKED, last.getName(), last
-								.getObjectId(), id);
+								.getName(), last.getObjectId(), id);
 						newPackedRefs.put(last.getName(), last);
 						continue;
 					}
@@ -392,7 +405,7 @@ private void refreshPackedRefs() {
 					final int sp = p.indexOf(' ');
 					final ObjectId id = ObjectId.fromString(p.substring(0, sp));
 					final String name = new String(p.substring(sp + 1));
-					last = new Ref(Ref.Storage.PACKED, name, id);
+					last = new Ref(Ref.Storage.PACKED, name, name, id);
 					newPackedRefs.put(last.getName(), last);
 				}
 			} finally {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 86b44c5..235c2fd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -131,9 +131,6 @@
 	/** Repository the ref is stored in. */
 	private final RefDatabase db;
 
-	/** Name of the ref. */
-	private final String name;
-
 	/** Location of the loose file holding the value of this ref. */
 	private final File looseFile;
 
@@ -160,7 +157,6 @@
 	RefUpdate(final RefDatabase r, final Ref ref, final File f) {
 		db = r;
 		this.ref = ref;
-		name = ref.getName();
 		oldValue = ref.getObjectId();
 		looseFile = f;
 	}
@@ -171,7 +167,7 @@ RefUpdate(final RefDatabase r, final Ref ref, final File f) {
 	 * @return name of this ref.
 	 */
 	public String getName() {
-		return name;
+		return ref.getName();
 	}
 
 	/**
@@ -349,9 +345,9 @@ public Result delete() throws IOException {
 	 * @throws IOException
 	 */
 	public Result delete(final RevWalk walk) throws IOException {
-		if (name.startsWith(Constants.R_HEADS)) {
+		if (getName().startsWith(Constants.R_HEADS)) {
 			final Ref head = db.readRef(Constants.HEAD);
-			if (head != null && name.equals(head.getName()))
+			if (head != null && getName().equals(head.getName()))
 				return Result.REJECTED_CURRENT_BRANCH;
 		}
 
@@ -373,7 +369,7 @@ private Result updateImpl(final RevWalk walk, final Store store)
 		if (!lock.lock())
 			return Result.LOCK_FAILURE;
 		try {
-			oldValue = db.idOf(name);
+			oldValue = db.idOf(getName());
 			if (oldValue == null)
 				return store.store(lock, Result.NEW);
 
@@ -428,7 +424,7 @@ else if (status == Result.NEW)
 				getName());
 		if (!lock.commit())
 			return Result.LOCK_FAILURE;
-		db.stored(name, newValue, lock.getCommitLastModified());
+		db.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());
 		return status;
 	}
 
-- 
1.6.0.3.578.g6a50

[EGIT PATCH 4/7] Handle peeling of loose refs.

[Robin Rosenberg]

For packed refs we got peeling 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.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java |   22 +++++++++++----
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   28 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/Repository.java       |   13 +++++++++
 3 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
index 2f102af..1a6cc4c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
@@ -140,10 +140,7 @@ public boolean isPacked() {
 	 *            does not exist yet.
 	 */
 	public Ref(final Storage st, final String origName, final String refName, final ObjectId id) {
-		storage = st;
-		this.origName = origName;
-		name = refName;
-		objectId = id;
+		this(st, origName, refName, id, ObjectId.zeroId());
 	}
 
 	/**
@@ -158,7 +155,7 @@ public Ref(final Storage st, final String origName, final String refName, final 
 	 *            does not exist yet.
 	 */
 	public Ref(final Storage st, final String refName, final ObjectId id) {
-		this(st, refName, refName, id);
+		this(st, refName, refName, id, ObjectId.zeroId());
 	}
 
 	/**
@@ -175,7 +172,7 @@ public Ref(final Storage st, final String refName, final ObjectId id) {
 	 *            does not exist yet.
 	 * @param peel
 	 *            peeled value of the ref's tag. May be null if this is not a
-	 *            tag or the peeled value is not known.
+	 *            tag or the zero id if the peeled value is not known.
 	 */
 	public Ref(final Storage st, final String origName, final String refName, final ObjectId id,
 			final ObjectId peel) {
@@ -238,10 +235,19 @@ public ObjectId getObjectId() {
 	 *         refer to an annotated tag.
 	 */
 	public ObjectId getPeeledObjectId() {
+		if (peeledObjectId == ObjectId.zeroId())
+			return null;
 		return peeledObjectId;
 	}
 
 	/**
+	 * @return whether the Ref represents a peeled tag
+	 */
+	public boolean isPeeled() {
+		return peeledObjectId != null && peeledObjectId != ObjectId.zeroId();
+	}
+
+	/**
 	 * How was this ref obtained?
 	 * <p>
 	 * The current storage model of a Ref may influence how the ref must be
@@ -259,4 +265,8 @@ public String toString() {
 			o = "(" + origName + ")";
 		return "Ref[" + o + name + "=" + ObjectId.toString(getObjectId()) + "]";
 	}
+
+	void setPeeledObjectId(final ObjectId id) {
+		peeledObjectId = id;
+	}
 }
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..0d73191 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,8 @@ private void readOneLooseRef(final Map<String, Ref> avail,
 					return;
 				}
 
-				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id);
+				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id, null); // unpeeled
+
 				looseRefs.put(ref.getName(), ref);
 				looseRefsMTime.put(ref.getName(), ent.lastModified());
 				avail.put(ref.getName(), ref);
@@ -288,6 +289,28 @@ private void readOneLooseRef(final Map<String, Ref> avail,
 		}
 	}
 
+	Ref peel(final Ref ref) {
+		if (ref.isPeeled())
+			return ref;
+		try {
+			Object tt = db.mapObject(ref.getObjectId(), ref.getName());
+			if (tt != null && tt instanceof Tag) {
+				Tag t = (Tag)tt;
+				while (t != null && t.getType().equals(Constants.TYPE_TAG))
+					t = db.mapTag(t.getTag(), t.getObjId());
+				if (t != null)
+					ref.setPeeledObjectId(t.getObjId());
+				else
+					ref.setPeeledObjectId(null);
+			} else
+				ref.setPeeledObjectId(ref.getObjectId());
+		} catch (IOException e) {
+			// Serious error. Caller knows a ref should never be null
+			ref.setPeeledObjectId(null);
+		}
+		return ref;
+	}
+
 	private File fileForRef(final String name) {
 		if (name.startsWith(REFS_SLASH))
 			return new File(refsDir, name.substring(REFS_SLASH.length()));
@@ -364,6 +387,9 @@ private Ref readRefBasic(final String origName, final String name, final int dep
 		}
 
 		ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
+
+		looseRefs.put(origName, ref);
+		ref = new Ref(Ref.Storage.LOOSE, origName, id);
 		looseRefs.put(name, ref);
 		looseRefsMTime.put(name, mtime);
 		return ref;
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 26748e2..4d6e6fd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -939,6 +939,19 @@ public String getBranch() throws IOException {
 	}
 
 	/**
+	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
+	 * the peeled id is set to {@link ObjectId#zeroId()}
+	 * 
+	 * @param ref
+	 *            The ref to peel
+	 * @return The same, an updated ref with peeled info or a new instance with
+	 *         more information
+	 */
+	public Ref peel(final Ref ref) {
+		return refs.peel(ref);
+	}
+
+	/**
 	 * @return true if HEAD points to a StGit patch.
 	 */
 	public boolean isStGitMode() {
-- 
1.6.0.3.578.g6a50

[EGIT PATCH 5/7] Add a method to get refs by object Id

[Robin Rosenberg]

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

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 4d6e6fd..82a7454 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -47,10 +47,13 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.Vector;
 
 import org.spearce.jgit.errors.IncorrectObjectTypeException;
@@ -952,6 +955,37 @@ public Ref peel(final Ref ref) {
 	}
 
 	/**
+	 * @return a map with all objects referenced by a peeled ref.
+	 */
+	public Map<AnyObjectId, Set<Ref>> getAllRefsByPeeledObjectId() {
+		Map<String, Ref> allRefs = getAllRefs();
+		Map<AnyObjectId, Set<Ref>> ret = new HashMap<AnyObjectId, Set<Ref>>(allRefs.size());
+		for (Ref ref : allRefs.values()) {
+			if (ref == null)
+				continue;
+			if (!ref.isPeeled()) {
+				ref = peel(ref);
+				allRefs.put(ref.getOrigName(), ref);
+			}
+			AnyObjectId target = ref.getPeeledObjectId();
+			if (target == null)
+				target = ref.getObjectId();
+			// We assume most Sets here are singletons
+			Set<Ref> oset = ret.put(target, Collections.singleton(ref));
+			if (oset != null) {
+				// that was not the case (rare)
+				if (oset.size() == 1) {
+					// Was a read-only singleton, we must copy to a new Set
+					oset = new HashSet<Ref>(oset);
+				}
+				ret.put(target, oset);
+				oset.add(ref);
+			}
+		}
+		return ret;
+	}
+
+	/**
 	 * @return true if HEAD points to a StGit patch.
 	 */
 	public boolean isStGitMode() {
-- 
1.6.0.3.578.g6a50

[EGIT PATCH 6/7] Add tags to the graphical history display.

[Robin Rosenberg]

Both the SWT (Eclipse) drawing and Swing versions are updated.
The coloring and shapes are intentionally not the same as for gitk.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../egit/ui/internal/history/SWTCommit.java        |    5 +-
 .../egit/ui/internal/history/SWTPlotRenderer.java  |   71 +++++++++++++++++++-
 .../spearce/egit/ui/internal/history/SWTWalk.java  |    2 +-
 .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |   46 +++++++++++++
 .../spearce/jgit/revplot/AbstractPlotRenderer.java |   23 ++++++-
 .../src/org/spearce/jgit/revplot/PlotCommit.java   |    8 ++-
 .../src/org/spearce/jgit/revplot/PlotWalk.java     |   60 ++++++++++++++++-
 7 files changed, 207 insertions(+), 8 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTCommit.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTCommit.java
index fa0d25d..2341fbd 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTCommit.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTCommit.java
@@ -10,12 +10,13 @@
 import org.eclipse.swt.widgets.Widget;
 import org.spearce.jgit.lib.AnyObjectId;
 import org.spearce.jgit.revplot.PlotCommit;
+import org.spearce.jgit.lib.Ref;
 
 class SWTCommit extends PlotCommit<SWTCommitList.SWTLane> {
 	Widget widget;
 
-	SWTCommit(final AnyObjectId id) {
-		super(id);
+	SWTCommit(final AnyObjectId id, final Ref[] tags) {
+		super(id, tags);
 	}
 
 	@Override
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
index c4ee282..b008df7 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
@@ -15,7 +15,10 @@
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Event;
 import org.eclipse.swt.widgets.TableItem;
+import org.eclipse.ui.themes.ColorUtil;
 import org.spearce.egit.ui.internal.history.SWTCommitList.SWTLane;
+import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.revplot.AbstractPlotRenderer;
 import org.spearce.jgit.revplot.PlotCommit;
 
@@ -26,7 +29,13 @@
 
 	private final Color sys_gray;
 
-	private Color sys_darkblue;
+	private final Color sys_darkblue;
+
+	private final Color sys_yellow;
+
+	private final Color sys_green;
+
+	private final Color sys_white;
 
 	GC g;
 
@@ -43,6 +52,9 @@ SWTPlotRenderer(final Display d) {
 		sys_black = d.getSystemColor(SWT.COLOR_BLACK);
 		sys_gray = d.getSystemColor(SWT.COLOR_GRAY);
 		sys_darkblue = d.getSystemColor(SWT.COLOR_DARK_BLUE);
+		sys_yellow = d.getSystemColor(SWT.COLOR_YELLOW);
+		sys_green = d.getSystemColor(SWT.COLOR_GREEN);
+		sys_white = d.getSystemColor(SWT.COLOR_WHITE);
 	}
 
 	void paint(final Event event) {
@@ -92,7 +104,64 @@ protected void drawText(final String msg, final int x, final int y) {
 		g.drawString(msg, cellX + x, cellY + texty, true);
 	}
 
+	@Override
+	protected int drawLabel(int x, int y, Ref ref) {
+		String txt;
+		String name = ref.getOrigName();
+		if (name.startsWith(Constants.R_HEADS)) {
+			g.setBackground(sys_green);
+			txt = name.substring(Constants.R_HEADS.length());
+		} else if (name.startsWith(Constants.R_REMOTES)){
+			g.setBackground(sys_gray);
+			txt = name.substring(Constants.R_REMOTES.length());
+		} else if (name.startsWith(Constants.R_TAGS)){
+			g.setBackground(sys_yellow);
+			txt = name.substring(Constants.R_TAGS.length());
+		} else {
+			// Whatever this would be
+			g.setBackground(sys_white);
+			if (name.startsWith(Constants.R_REFS))
+				txt = name.substring(Constants.R_REFS.length());
+			else
+				txt = name; // HEAD and such
+		}
+
+		// Make peeled objects, i.e. via annotated tags come out in a paler color
+		Color peeledColor = null;
+		if (ref.getPeeledObjectId() == null || !ref.getPeeledObjectId().equals(ref.getObjectId())) {
+			peeledColor = new Color(g.getDevice(), ColorUtil.blend(g.getBackground().getRGB(), sys_white.getRGB()));
+			g.setBackground(peeledColor);
+		}
+
+		if (txt.length() > 12)
+			txt = txt.substring(0,11) + "\u2026"; // ellipsis "…" (in UTF-8)
+
+		Point textsz = g.stringExtent(txt);
+		int arc = textsz.y/2;
+		final int texty = (y * 2 - textsz.y) / 2;
+
+		// Draw backgrounds
+		g.fillRoundRectangle(x + 1, cellY + texty -1, textsz.x + 3, textsz.y + 1, arc, arc);
+		g.setForeground(sys_black);
+		g.drawString(txt, x + 2, cellY + texty, true);
+		g.setLineWidth(2);
+
+		// And a two color shaded border, blend with whatever background there already is
+		g.setAlpha(128);
+		g.setForeground(sys_gray);
+		g.drawRoundRectangle(x, cellY + texty -2, textsz.x + 5, textsz.y + 3, arc, arc);
+		g.setLineWidth(2);
+		g.setForeground(sys_black);
+		g.drawRoundRectangle(x + 1, cellY + texty -1, textsz.x + 3, textsz.y + 1, arc, arc);
+		g.setAlpha(255);
+
+		if (peeledColor != null)
+			peeledColor.dispose();
+		return 8 + textsz.x;
+	}
+
 	protected Color laneColor(final SWTLane myLane) {
 		return myLane != null ? myLane.color : sys_black;
 	}
+
 }
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTWalk.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTWalk.java
index 527d284..57039b5 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTWalk.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTWalk.java
@@ -19,6 +19,6 @@ SWTWalk(final Repository repo) {
 
 	@Override
 	protected RevCommit createCommit(final AnyObjectId id) {
-		return new SWTCommit(id);
+		return new SWTCommit(id, getTags(id));
 	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
index b6b715c..5dcddf5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
@@ -44,6 +44,8 @@
 
 import org.spearce.jgit.awtui.CommitGraphPane.GraphCellRender;
 import org.spearce.jgit.awtui.SwingCommitList.SwingLane;
+import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.revplot.AbstractPlotRenderer;
 import org.spearce.jgit.revplot.PlotCommit;
 
@@ -134,4 +136,48 @@ void paintTriangleDown(final int cx, final int y, final int h) {
 		g.drawPolygon(triangle);
 	}
 
+	@Override
+	protected int drawLabel(int x, int y, Ref ref) {
+		String txt;
+		String name = ref.getOrigName();
+		if (name.startsWith(Constants.R_HEADS)) {
+			g.setBackground(Color.GREEN);
+			txt = name.substring(Constants.R_HEADS.length());
+		} else if (name.startsWith(Constants.R_REMOTES)){
+			g.setBackground(Color.LIGHT_GRAY);
+			txt = name.substring(Constants.R_REMOTES.length());
+		} else if (name.startsWith(Constants.R_TAGS)){
+			g.setBackground(Color.YELLOW);
+			txt = name.substring(Constants.R_TAGS.length());
+		} else {
+			// Whatever this would be
+			g.setBackground(Color.WHITE);
+			if (name.startsWith(Constants.R_REFS))
+				txt = name.substring(Constants.R_REFS.length());
+			else
+				txt = name; // HEAD and such
+		}
+		if (ref.getPeeledObjectId() != null) {
+			float[] colorComponents = g.getBackground().getRGBColorComponents(null);
+			colorComponents[0] *= 0.9;
+			colorComponents[1] *= 0.9;
+			colorComponents[2] *= 0.9;
+			g.setBackground(new Color(colorComponents[0],colorComponents[1],colorComponents[2]));
+		}
+		if (txt.length() > 12)
+			txt = txt.substring(0,11) + "\u2026"; // ellipsis "…" (in UTF-8)
+
+		final int texth = g.getFontMetrics().getHeight();
+		int textw = g.getFontMetrics().stringWidth(txt);
+		g.setColor(g.getBackground());
+		int arcHeight = texth/4;
+		int y0 = y - texth/2 + (cell.getHeight() - texth)/2;
+		g.fillRoundRect(x , y0, textw + arcHeight*2, texth -1, arcHeight, arcHeight);
+		g.setColor(g.getColor().darker());
+		g.drawRoundRect(x, y0, textw + arcHeight*2, texth -1 , arcHeight, arcHeight);
+		g.setColor(Color.BLACK);
+		g.drawString(txt, x + arcHeight, y0 + texth - g.getFontMetrics().getDescent());
+
+		return arcHeight * 3 + textw;
+	}
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java
index f175c9d..911dd68 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java
@@ -37,6 +37,7 @@
 
 package org.spearce.jgit.revplot;
 
+import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.revwalk.RevFlag;
 
 /**
@@ -140,11 +141,29 @@ protected void paintCommit(final PlotCommit<TLane> commit, final int h) {
 		else
 			drawCommitDot(dotX, dotY, dotSize, dotSize);
 
+		int textx = Math.max(maxCenter + LANE_WIDTH / 2, dotX + dotSize) + 8;
+		int n = commit.refs == null ? 0 : commit.refs.length;
+		for (int i = 0; i < n; ++i) {
+			textx += drawLabel(textx + dotSize, h/2, commit.refs[i]);
+		}
+
 		final String msg = commit.getShortMessage();
-		final int textx = Math.max(maxCenter + LANE_WIDTH / 2, dotX + dotSize) + 8;
-		drawText(msg, textx, h / 2);
+		drawText(msg, textx + dotSize + n*2, h / 2);
 	}
 
+	/**
+	 * Draw a decoration for the Ref ref at x,y
+	 *
+	 * @param x
+	 *            left
+	 * @param y
+	 *            top
+	 * @param ref
+	 *            A peeled ref
+	 * @return width of label in pixels
+	 */
+	protected abstract int drawLabel(int x, int y, Ref ref);
+
 	private int computeDotSize(final int h) {
 		int d = (int) (Math.min(h, LANE_WIDTH) * 0.50f);
 		d += (d & 1);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java
index 5a5ef1e..c885a44 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java
@@ -39,6 +39,7 @@
 
 import org.spearce.jgit.lib.AnyObjectId;
 import org.spearce.jgit.revwalk.RevCommit;
+import org.spearce.jgit.lib.Ref;
 
 /**
  * A commit reference to a commit in the DAG.
@@ -58,14 +59,19 @@
 
 	PlotCommit[] children;
 
+	final Ref[] refs;
+
 	/**
 	 * Create a new commit.
 	 * 
 	 * @param id
 	 *            the identity of this commit.
+	 * @param tags
+	 *            the tags associated with this commit, null for no tags
 	 */
-	protected PlotCommit(final AnyObjectId id) {
+	protected PlotCommit(final AnyObjectId id, final Ref[] tags) {
 		super(id);
+		this.refs = tags;
 		passingLanes = NO_LANES;
 		children = NO_CHILDREN;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java
index e5e8aba..8801850 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java
@@ -37,14 +37,33 @@
 
 package org.spearce.jgit.revplot;
 
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.Map;
+import java.util.Set;
+
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Commit;
+import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.Tag;
 import org.spearce.jgit.revwalk.RevCommit;
 import org.spearce.jgit.revwalk.RevSort;
 import org.spearce.jgit.revwalk.RevWalk;
 
 /** Specialized RevWalk for visualization of a commit graph. */
 public class PlotWalk extends RevWalk {
+
+	private Map<AnyObjectId, Set<Ref>> reverseRefMap;
+
+	@Override
+	public void dispose() {
+		super.dispose();
+		reverseRefMap.clear();
+	}
+
 	/**
 	 * Create a new revision walker for a given repository.
 	 * 
@@ -54,6 +73,7 @@
 	public PlotWalk(final Repository repo) {
 		super(repo);
 		super.sort(RevSort.TOPO, true);
+		reverseRefMap = repo.getAllRefsByPeeledObjectId();
 	}
 
 	@Override
@@ -65,6 +85,44 @@ public void sort(final RevSort s, final boolean use) {
 
 	@Override
 	protected RevCommit createCommit(final AnyObjectId id) {
-		return new PlotCommit(id);
+		return new PlotCommit(id, getTags(id));
+	}
+
+	protected Ref[] getTags(final AnyObjectId commitId) {
+		Collection<Ref> list = reverseRefMap.get(commitId);
+		Ref[] tags;
+		if (list == null)
+			tags = null;
+		else {
+			tags = list.toArray(new Ref[list.size()]);
+			Arrays.sort(tags, new PlotRefComparator());
+		}
+		return tags;
+	}
+
+	class PlotRefComparator implements Comparator<Ref> {
+		public int compare(Ref o1, Ref o2) {
+			try {
+				Object obj1 = getRepository().mapObject(o1.getObjectId(), o1.getName());
+				Object obj2 = getRepository().mapObject(o2.getObjectId(), o2.getName());
+				long t1 = timeof(obj1);
+				long t2 = timeof(obj2);
+				if (t1 > t2)
+					return -1;
+				if (t1 < t2)
+					return 1;
+				return 0;
+			} catch (IOException e) {
+				// ignore
+				return 0;
+			}
+		}
+		long timeof(Object o) {
+			if (o instanceof Commit)
+				return ((Commit)o).getCommitter().getWhen().getTime();
+			if (o instanceof Tag)
+				return ((Tag)o).getTagger().getWhen().getTime();
+			return 0;
+		}
 	}
 }
-- 
1.6.0.3.578.g6a50

[EGIT PATCH 7/7] Add decorate option to log program

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/pgm/Log.java              |   33 ++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Log.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Log.java
index e3a32c8..bb424d4 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Log.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Log.java
@@ -40,11 +40,19 @@
 
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
 import java.util.TimeZone;
 
+import org.kohsuke.args4j.Option;
+import org.spearce.jgit.lib.AnyObjectId;
 import org.spearce.jgit.lib.PersonIdent;
+import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.revwalk.RevCommit;
+import org.spearce.jgit.revwalk.RevWalk;
 
 @Command(common = true, usage = "View commit history")
 class Log extends RevWalkTextBuiltin {
@@ -52,14 +60,39 @@
 
 	private final DateFormat fmt;
 
+	private Map<AnyObjectId, Set<Ref>> allRefsByPeeledObjectId;
+
+	@Option(name="--decorate", usage="Show ref names matching commits")
+	private boolean decorate;
+
 	Log() {
 		fmt = new SimpleDateFormat("EEE MMM dd HH:mm:ss yyyy ZZZZZ", Locale.US);
 	}
 
 	@Override
+	protected RevWalk createWalk() {
+		RevWalk ret = super.createWalk();
+		if (decorate)
+			allRefsByPeeledObjectId = getRepository().getAllRefsByPeeledObjectId();
+		return ret;
+	}
+
+	@Override
 	protected void show(final RevCommit c) throws Exception {
 		out.print("commit ");
 		c.getId().copyTo(outbuffer, out);
+		if (decorate) {
+			Collection<Ref> list = allRefsByPeeledObjectId.get(c.copy());
+			if (list != null) {
+				out.print(" (");
+				for (Iterator<Ref> i = list.iterator(); i.hasNext(); ) {
+					out.print(i.next().getOrigName());
+					if (i.hasNext())
+						out.print(" ");
+				}
+				out.print(")");
+			}
+		}
 		out.println();
 
 		final PersonIdent author = c.getAuthorIdent();
-- 
1.6.0.3.578.g6a50

Re: [EGIT PATCH 1/7] Test the origName part of the key vs the ref itself

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -127,4 +129,12 @@ public void testDeleteEmptyDirs() throws IOException {
>  	private void assertExists(final boolean expected, final String name) {
>  		assertEquals(expected, new File(db.getDirectory(), name).exists());
>  	}
> +
> +	public void testRefKeySameAsOrigName() {
> +		Map<String, Ref> allRefs = db.getAllRefs();
> +		for (Entry<String, Ref> e : allRefs.entrySet()) {
> +			assertEquals(e.getKey(), e.getValue().getOrigName());
> +
> +		}
> +	}

Hmm, doesn't this have to go after "Keep original ref name ..."
so getOrigName() is actually defined?

-- 
Shawn.

Re: [EGIT PATCH 4/7] Handle peeling of loose refs.

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -238,10 +235,19 @@ public ObjectId getObjectId() {
>  	 *         refer to an annotated tag.
>  	 */
>  	public ObjectId getPeeledObjectId() {
> +		if (peeledObjectId == ObjectId.zeroId())
> +			return null;
>  		return peeledObjectId;
>  	}
>  
>  	/**
> +	 * @return whether the Ref represents a peeled tag
> +	 */
> +	public boolean isPeeled() {
> +		return peeledObjectId != null && peeledObjectId != ObjectId.zeroId();
> +	}

Huh.  So peeledObjectId == ObjectId.zeroId() means what, exactly?
That we tried to peel this but we cannot because its not a tag?

What does a packed-refs record which has no peel data but which
is in a packed-refs with-peeled file have for peeledObjectId?
null or ObjectId.zeroId()?

> @@ -288,6 +289,28 @@ private void readOneLooseRef(final Map<String, Ref> avail,
>  		}
>  	}
>  
> +	Ref peel(final Ref ref) {
> +		if (ref.isPeeled())
> +			return ref;

I think you mean something more like:

	if (ref.peeledObjectId != null)
		return ref;

as the ref is already peeled, or at least attempted.

> +		try {
> +			Object tt = db.mapObject(ref.getObjectId(), ref.getName());
> +			if (tt != null && tt instanceof Tag) {
> +				Tag t = (Tag)tt;
> +				while (t != null && t.getType().equals(Constants.TYPE_TAG))
> +					t = db.mapTag(t.getTag(), t.getObjId());
> +				if (t != null)
> +					ref.setPeeledObjectId(t.getObjId());
> +				else
> +					ref.setPeeledObjectId(null);
> +			} else
> +				ref.setPeeledObjectId(ref.getObjectId());
> +		} catch (IOException e) {
> +			// Serious error. Caller knows a ref should never be null
> +			ref.setPeeledObjectId(null);
> +		}
> +		return ref;

This whole block is simpler to write as:

	try {
		Object target = db.mapObject(ref.getObjectId(), ref.getName());
		while (target instanceof Tag) {
			final Tag tag = (Tag)target;
			ref.setPeeledObjectId(tag.getObjId());
			if (Constants.TYPE_TAG.equals(tag.getType()))
				target = db.mapObject(tag.getObjId(), ref.getName());
			else
				break;
		}
	} catch (IOException e) {
		// Ignore a read error.  Callers will also get the same error
		// if they try to use the result of getPeeledObjectId.
	}

> 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 26748e2..4d6e6fd 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -939,6 +939,19 @@ public String getBranch() throws IOException {
>  	}
>  
>  	/**
> +	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
> +	 * the peeled id is set to {@link ObjectId#zeroId()}

But applications never see ObjectId.zeroId() because you earlier defined
getPeeledObjectId() to return null in that case.  So why is this part of
the documentation relevant? 

> +	 * 
> +	 * @param ref
> +	 *            The ref to peel
> +	 * @return The same, an updated ref with peeled info or a new instance with
> +	 *         more information
> +	 */
> +	public Ref peel(final Ref ref) {
> +		return refs.peel(ref);

Can we tighten this contract?  Are we always going to update the
current ref in-place, or are we going to return the caller a new
Ref instance?  I'd like it to be consistent one way or the other.

If we are updating in-place then the caller needs to realize there
may be some cache synchronization issues when using multiple threads
to access the same Ref instances and peeling them.  I.e. they may
need to go through their own synchornization barrier to ensure the
processor caches are coherent.

Given that almost everywhere else we treat the Ref as immutable
I'm tempted to say this should always return a new Ref object if we
peeled; but return the parameter if the parameter is already peeled
(or is known to be unpeelable, e.g. a branch head).

-- 
Shawn.

Re: [EGIT PATCH 5/7] Add a method to get refs by object Id

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -952,6 +955,37 @@ public Ref peel(final Ref ref) {
>  	}
>  
>  	/**
> +	 * @return a map with all objects referenced by a peeled ref.
> +	 */
> +	public Map<AnyObjectId, Set<Ref>> getAllRefsByPeeledObjectId() {
> +		Map<String, Ref> allRefs = getAllRefs();
> +		Map<AnyObjectId, Set<Ref>> ret = new HashMap<AnyObjectId, Set<Ref>>(allRefs.size());
> +		for (Ref ref : allRefs.values()) {
> +			if (ref == null)
> +				continue;

How did we get a null Ref inside the allRefs collection?

> +			if (!ref.isPeeled()) {
> +				ref = peel(ref);
> +				allRefs.put(ref.getOrigName(), ref);

Hmm.  Mutating a HashMap while you are traversing it with an Iterator
is *not* a good idea.  Its a ConcurrentModificationException waiting
to happen.

-- 
Shawn.

Re: [EGIT PATCH 6/7] Add tags to the graphical history display.

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -92,7 +104,64 @@ protected void drawText(final String msg, final int x, final int y) {
>  		g.drawString(msg, cellX + x, cellY + texty, true);
>  	}

My SWTPlotRenderer doesn't have this context line.  Am I missing
a patch in the series?
  
> +	@Override
> +	protected int drawLabel(int x, int y, Ref ref) {

-- 
Shawn.

Re: [EGIT PATCH 6/7] Add tags to the graphical history display.

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Both the SWT (Eclipse) drawing and Swing versions are updated.
> The coloring and shapes are intentionally not the same as for gitk.

Despite my comments about the series, I really like the rendering
in SWT.  I'm looking forward to getting this in-tree, but I would
like to get a better definition of the Ref peeling API.
 
-- 
Shawn.

Re: [EGIT PATCH 1/7] Test the origName part of the key vs the ref itself

[Robin Rosenberg]

tisdag 11 november 2008 19:05:20 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > @@ -127,4 +129,12 @@ public void testDeleteEmptyDirs() throws IOException {
> >  	private void assertExists(final boolean expected, final String name) {
> >  		assertEquals(expected, new File(db.getDirectory(), name).exists());
> >  	}
> > +
> > +	public void testRefKeySameAsOrigName() {
> > +		Map<String, Ref> allRefs = db.getAllRefs();
> > +		for (Entry<String, Ref> e : allRefs.entrySet()) {
> > +			assertEquals(e.getKey(), e.getValue().getOrigName());
> > +
> > +		}
> > +	}
> 
> Hmm, doesn't this have to go after "Keep original ref name ..."
> so getOrigName() is actually defined?

Oops, Seems we should try to build each new commit before accepting
a push. Shouldn't be too hard.

-- robin

Re: [EGIT PATCH 4/7] Handle peeling of loose refs.

[Robin Rosenberg]

tisdag 11 november 2008 19:23:58 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > @@ -238,10 +235,19 @@ public ObjectId getObjectId() {
> >  	 *         refer to an annotated tag.
> >  	 */
> >  	public ObjectId getPeeledObjectId() {
> > +		if (peeledObjectId == ObjectId.zeroId())
> > +			return null;
> >  		return peeledObjectId;
> >  	}
> >  
> >  	/**
> > +	 * @return whether the Ref represents a peeled tag
> > +	 */
> > +	public boolean isPeeled() {
> > +		return peeledObjectId != null && peeledObjectId != ObjectId.zeroId();
> > +	}
> 
> Huh.  So peeledObjectId == ObjectId.zeroId() means what, exactly?
> That we tried to peel this but we cannot because its not a tag?
Means we haven't tried to peel it yet. I could use an extra flags instead. We may
have thousands of refs but perhaps not zillions.

> 
> What does a packed-refs record which has no peel data but which
> is in a packed-refs with-peeled file have for peeledObjectId?
> null or ObjectId.zeroId()?
> 
> > @@ -288,6 +289,28 @@ private void readOneLooseRef(final Map<String, Ref> avail,
> >  		}
> >  	}
> >  
> > +	Ref peel(final Ref ref) {
> > +		if (ref.isPeeled())
> > +			return ref;
> 
> I think you mean something more like:
> 
> 	if (ref.peeledObjectId != null)
> 		return ref;
> 
> as the ref is already peeled, or at least attempted.
No, if it is a loose ref we did not yet attempt to peel it. null means we have tried
zeroId means we haven't tried (see above).

> 
> > +		try {
> > +			Object tt = db.mapObject(ref.getObjectId(), ref.getName());
> > +			if (tt != null && tt instanceof Tag) {
> > +				Tag t = (Tag)tt;
> > +				while (t != null && t.getType().equals(Constants.TYPE_TAG))
> > +					t = db.mapTag(t.getTag(), t.getObjId());
> > +				if (t != null)
> > +					ref.setPeeledObjectId(t.getObjId());
> > +				else
> > +					ref.setPeeledObjectId(null);
> > +			} else
> > +				ref.setPeeledObjectId(ref.getObjectId());
> > +		} catch (IOException e) {
> > +			// Serious error. Caller knows a ref should never be null
> > +			ref.setPeeledObjectId(null);
> > +		}
> > +		return ref;
> 
> This whole block is simpler to write as:
ok..
>         try {
>                 Object target = db.mapObject(ref.getObjectId(), ref.getName());
>                 while (target instanceof Tag) {
>                         final Tag tag = (Tag)target;
>                         ref.setPeeledObjectId(tag.getObjId());
>                         if (Constants.TYPE_TAG.equals(tag.getType()))
>                                 target = db.mapObject(tag.getObjId(), ref.getName());
>                         else
>                                 break;
>                 }
>         } catch (IOException e) {
>                 // Ignore a read error.  Callers will also get the same error
>                 // if they try to use the result of getPeeledObjectId.
>         }
> 
That gives a different behavior, but which is also ok...

> >  	/**
> > +	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
> > +	 * the peeled id is set to {@link ObjectId#zeroId()}
> 
> But applications never see ObjectId.zeroId() because you earlier defined
> getPeeledObjectId() to return null in that case.  So why is this part of
> the documentation relevant? 
...it's wrong, and thus irrelevant.

> > +	 * 
> > +	 * @param ref
> > +	 *            The ref to peel
> > +	 * @return The same, an updated ref with peeled info or a new instance with
> > +	 *         more information
> > +	 */
> > +	public Ref peel(final Ref ref) {
> > +		return refs.peel(ref);
> 
> Can we tighten this contract?  Are we always going to update the
> current ref in-place, or are we going to return the caller a new
> Ref instance?  I'd like it to be consistent one way or the other.
Ack. 
> 
> If we are updating in-place then the caller needs to realize there
> may be some cache synchronization issues when using multiple threads
> to access the same Ref instances and peeling them.  I.e. they may
> need to go through their own synchornization barrier to ensure the
> processor caches are coherent.
> 
> Given that almost everywhere else we treat the Ref as immutable
> I'm tempted to say this should always return a new Ref object if we
> peeled; but return the parameter if the parameter is already peeled
> (or is known to be unpeelable, e.g. a branch head).

I pick the copy.

-- robin

Re: [EGIT PATCH 6/7] Add tags to the graphical history display.

[Robin Rosenberg]

tisdag 11 november 2008 19:28:16 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > @@ -92,7 +104,64 @@ protected void drawText(final String msg, final int x, final int y) {
> >  		g.drawString(msg, cellX + x, cellY + texty, true);
> >  	}
> 
> My SWTPlotRenderer doesn't have this context line.  Am I missing
> a patch in the series?

>From the "Respect background when drawing history " patch sent to you after the first version
of this series.

-- robin

[EGIT PATCH 1/7 v3] Add constant for "refs/"

[Robin Rosenberg]

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

Here's an update after considering Shawn's comments. 1,2,3 are just reordered.
No 4 and 5 are changed. For patch no 6 and 7 use the ones from the previous
batch.

-- robin

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
index f316881..9613d07 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
@@ -216,6 +216,9 @@
 	/** Prefix for tag refs */
 	public static final String R_TAGS = "refs/tags/";
 
+	/** Prefix for any ref */
+	public static final String R_REFS = "refs/";
+
 	/** Logs folder name */
 	public static final String LOGS = "logs";
 
-- 
1.6.0.3.640.g6331a.dirty

[EGIT PATCH 2/7 v3] Keep original ref name when reading refs

[Robin Rosenberg]

We want to know the original name of refs, not just the target name.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java |   63 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   45 +++++++++-----
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   14 ++---
 3 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
index db94875..2f102af 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
@@ -43,6 +43,11 @@
  * A ref in Git is (more or less) a variable that holds a single object
  * identifier. The object identifier can be any valid Git object (blob, tree,
  * commit, annotated tag, ...).
+ * <p>
+ * The ref name has the attributes of the ref that was asked for as well as
+ * the ref it was resolved to for symbolic refs plus the object id it points
+ * to and (for tags) the peeled target object id, i.e. the tag resolved
+ * recursively until a non-tag object is referenced.
  */
 public class Ref {
 	/** Location where a {@link Ref} is stored. */
@@ -119,19 +124,24 @@ public boolean isPacked() {
 
 	private ObjectId peeledObjectId;
 
+	private final String origName;
+
 	/**
 	 * Create a new ref pairing.
 	 * 
 	 * @param st
 	 *            method used to store this ref.
+	 * @param origName
+	 *            The name used to resolve this ref
 	 * @param refName
 	 *            name of this ref.
 	 * @param id
 	 *            current value of the ref. May be null to indicate a ref that
 	 *            does not exist yet.
 	 */
-	public Ref(final Storage st, final String refName, final ObjectId id) {
+	public Ref(final Storage st, final String origName, final String refName, final ObjectId id) {
 		storage = st;
+		this.origName = origName;
 		name = refName;
 		objectId = id;
 	}
@@ -146,19 +156,56 @@ public Ref(final Storage st, final String refName, final ObjectId id) {
 	 * @param id
 	 *            current value of the ref. May be null to indicate a ref that
 	 *            does not exist yet.
+	 */
+	public Ref(final Storage st, final String refName, final ObjectId id) {
+		this(st, refName, refName, id);
+	}
+
+	/**
+	 * Create a new ref pairing.
+	 * 
+	 * @param st
+	 *            method used to store this ref.
+	 * @param origName
+	 *            The name used to resolve this ref
+	 * @param refName
+	 *            name of this ref.
+	 * @param id
+	 *            current value of the ref. May be null to indicate a ref that
+	 *            does not exist yet.
 	 * @param peel
 	 *            peeled value of the ref's tag. May be null if this is not a
 	 *            tag or the peeled value is not known.
 	 */
-	public Ref(final Storage st, final String refName, final ObjectId id,
+	public Ref(final Storage st, final String origName, final String refName, final ObjectId id,
 			final ObjectId peel) {
 		storage = st;
+		this.origName = origName;
 		name = refName;
 		objectId = id;
 		peeledObjectId = peel;
 	}
 
 	/**
+	 * Create a new ref pairing.
+	 * 
+	 * @param st
+	 *            method used to store this ref.
+	 * @param refName
+	 *            name of this ref.
+	 * @param id
+	 *            current value of the ref. May be null to indicate a ref that
+	 *            does not exist yet.
+	 * @param peel
+	 *            peeled value of the ref's tag. May be null if this is not a
+	 *            tag or the peeled value is not known.
+	 */
+	public Ref(final Storage st, final String refName, final ObjectId id,
+			final ObjectId peel) {
+		this(st, refName, refName, id, peel);
+	}
+
+	/**
 	 * What this ref is called within the repository.
 	 * 
 	 * @return name of this ref.
@@ -168,6 +215,13 @@ public String getName() {
 	}
 
 	/**
+	 * @return the originally resolved name
+	 */
+	public String getOrigName() {
+		return origName;
+	}
+
+	/**
 	 * Cached value of this ref.
 	 * 
 	 * @return the value of this ref at the last time we read it.
@@ -200,6 +254,9 @@ public Storage getStorage() {
 	}
 
 	public String toString() {
-		return "Ref[" + name + "=" + ObjectId.toString(getObjectId()) + "]";
+		String o = "";
+		if (!origName.equals(name))
+			o = "(" + origName + ")";
+		return "Ref[" + o + name + "=" + ObjectId.toString(getObjectId()) + "]";
 	}
 }
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 5c1f060..5a1b85f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -51,6 +51,7 @@
 import java.util.Map;
 
 import org.spearce.jgit.errors.ObjectWritingException;
+import org.spearce.jgit.lib.Ref.Storage;
 import org.spearce.jgit.util.FS;
 import org.spearce.jgit.util.NB;
 
@@ -135,8 +136,8 @@ RefUpdate newUpdate(final String name) throws IOException {
 		return new RefUpdate(this, r, fileForRef(r.getName()));
 	}
 
-	void stored(final String name, final ObjectId id, final long time) {
-		looseRefs.put(name, new Ref(Ref.Storage.LOOSE, name, id));
+	void stored(final String origName, final String name, final ObjectId id, final long time) {
+		looseRefs.put(name, new Ref(Ref.Storage.LOOSE, origName, name, id));
 		looseRefsMTime.put(name, time);
 		setModified();
 		db.fireRefsMaybeChanged();
@@ -222,12 +223,12 @@ private void readLooseRefs(final Map<String, Ref> avail,
 			final String entName = ent.getName();
 			if (".".equals(entName) || "..".equals(entName))
 				continue;
-			readOneLooseRef(avail, prefix + entName, ent);
+			readOneLooseRef(avail, prefix + entName, prefix + entName, ent);
 		}
 	}
 
 	private void readOneLooseRef(final Map<String, Ref> avail,
-			final String refName, final File ent) {
+			final String origName, final String refName, final File ent) {
 		// Unchanged and cached? Don't read it again.
 		//
 		Ref ref = looseRefs.get(refName);
@@ -270,7 +271,7 @@ private void readOneLooseRef(final Map<String, Ref> avail,
 					return;
 				}
 
-				ref = new Ref(Ref.Storage.LOOSE, refName, id);
+				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id);
 				looseRefs.put(ref.getName(), ref);
 				looseRefsMTime.put(ref.getName(), ent.lastModified());
 				avail.put(ref.getName(), ref);
@@ -293,27 +294,35 @@ private File fileForRef(final String name) {
 		return new File(gitDir, name);
 	}
 
-	private Ref readRefBasic(final String name, final int depth)
+	private Ref readRefBasic(final String name, final int depth) throws IOException {
+		return readRefBasic(name, name, depth);
+	}
+
+	private Ref readRefBasic(final String origName, final String name, final int depth)
 			throws IOException {
 		// Prefer loose ref to packed ref as the loose
 		// file can be more up-to-date than a packed one.
 		//
-		Ref ref = looseRefs.get(name);
+		Ref ref = looseRefs.get(origName);
 		final File loose = fileForRef(name);
 		final long mtime = loose.lastModified();
 		if (ref != null) {
 			Long cachedlastModified = looseRefsMTime.get(name);
 			if (cachedlastModified != null && cachedlastModified == mtime)
 				return ref;
-			looseRefs.remove(name);
-			looseRefsMTime.remove(name);
+			looseRefs.remove(origName);
+			looseRefsMTime.remove(origName);
 		}
 
 		if (mtime == 0) {
 			// If last modified is 0 the file does not exist.
 			// Try packed cache.
 			//
-			return packedRefs.get(name);
+			ref = packedRefs.get(name);
+			if (ref != null)
+				if (!ref.getOrigName().equals(origName))
+					ref = new Ref(Storage.LOOSE_PACKED, origName, name, ref.getObjectId());
+			return ref;
 		}
 
 		final String line;
@@ -324,7 +333,7 @@ private Ref readRefBasic(final String name, final int depth)
 		}
 
 		if (line == null || line.length() == 0)
-			return new Ref(Ref.Storage.LOOSE, name, null);
+			return new Ref(Ref.Storage.LOOSE, origName, name, null);
 
 		if (line.startsWith("ref: ")) {
 			if (depth >= 5) {
@@ -333,12 +342,16 @@ private Ref readRefBasic(final String name, final int depth)
 			}
 
 			final String target = line.substring("ref: ".length());
-			final Ref r = readRefBasic(target, depth + 1);
+			Ref r = readRefBasic(target, target, depth + 1);
 			Long cachedMtime = looseRefsMTime.get(name);
 			if (cachedMtime != null && cachedMtime != mtime)
 				setModified();
 			looseRefsMTime.put(name, mtime);
-			return r != null ? r : new Ref(Ref.Storage.LOOSE, target, null);
+			if (r == null)
+				return new Ref(Ref.Storage.LOOSE, origName, target, null);
+			if (!origName.equals(r.getName()))
+				r = new Ref(Ref.Storage.LOOSE_PACKED, origName, r.getName(), r.getObjectId(), r.getPeeledObjectId());
+			return r; 
 		}
 
 		setModified();
@@ -350,7 +363,7 @@ private Ref readRefBasic(final String name, final int depth)
 			throw new IOException("Not a ref: " + name + ": " + line);
 		}
 
-		ref = new Ref(Ref.Storage.LOOSE, name, id);
+		ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
 		looseRefs.put(name, ref);
 		looseRefsMTime.put(name, mtime);
 		return ref;
@@ -384,7 +397,7 @@ private void refreshPackedRefs() {
 
 						final ObjectId id = ObjectId.fromString(p.substring(1));
 						last = new Ref(Ref.Storage.PACKED, last.getName(), last
-								.getObjectId(), id);
+								.getName(), last.getObjectId(), id);
 						newPackedRefs.put(last.getName(), last);
 						continue;
 					}
@@ -392,7 +405,7 @@ private void refreshPackedRefs() {
 					final int sp = p.indexOf(' ');
 					final ObjectId id = ObjectId.fromString(p.substring(0, sp));
 					final String name = new String(p.substring(sp + 1));
-					last = new Ref(Ref.Storage.PACKED, name, id);
+					last = new Ref(Ref.Storage.PACKED, name, name, id);
 					newPackedRefs.put(last.getName(), last);
 				}
 			} finally {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 86b44c5..235c2fd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -131,9 +131,6 @@
 	/** Repository the ref is stored in. */
 	private final RefDatabase db;
 
-	/** Name of the ref. */
-	private final String name;
-
 	/** Location of the loose file holding the value of this ref. */
 	private final File looseFile;
 
@@ -160,7 +157,6 @@
 	RefUpdate(final RefDatabase r, final Ref ref, final File f) {
 		db = r;
 		this.ref = ref;
-		name = ref.getName();
 		oldValue = ref.getObjectId();
 		looseFile = f;
 	}
@@ -171,7 +167,7 @@ RefUpdate(final RefDatabase r, final Ref ref, final File f) {
 	 * @return name of this ref.
 	 */
 	public String getName() {
-		return name;
+		return ref.getName();
 	}
 
 	/**
@@ -349,9 +345,9 @@ public Result delete() throws IOException {
 	 * @throws IOException
 	 */
 	public Result delete(final RevWalk walk) throws IOException {
-		if (name.startsWith(Constants.R_HEADS)) {
+		if (getName().startsWith(Constants.R_HEADS)) {
 			final Ref head = db.readRef(Constants.HEAD);
-			if (head != null && name.equals(head.getName()))
+			if (head != null && getName().equals(head.getName()))
 				return Result.REJECTED_CURRENT_BRANCH;
 		}
 
@@ -373,7 +369,7 @@ private Result updateImpl(final RevWalk walk, final Store store)
 		if (!lock.lock())
 			return Result.LOCK_FAILURE;
 		try {
-			oldValue = db.idOf(name);
+			oldValue = db.idOf(getName());
 			if (oldValue == null)
 				return store.store(lock, Result.NEW);
 
@@ -428,7 +424,7 @@ else if (status == Result.NEW)
 				getName());
 		if (!lock.commit())
 			return Result.LOCK_FAILURE;
-		db.stored(name, newValue, lock.getCommitLastModified());
+		db.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());
 		return status;
 	}
 
-- 
1.6.0.3.640.g6331a.dirty

[EGIT PATCH 3/7 v3] Test the origName part of the key vs the ref itself

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 6e2cfa8..12f9ada 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -39,6 +39,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import org.spearce.jgit.lib.RefUpdate.Result;
 
@@ -127,4 +129,12 @@ public void testDeleteEmptyDirs() throws IOException {
 	private void assertExists(final boolean expected, final String name) {
 		assertEquals(expected, new File(db.getDirectory(), name).exists());
 	}
+
+	public void testRefKeySameAsOrigName() {
+		Map<String, Ref> allRefs = db.getAllRefs();
+		for (Entry<String, Ref> e : allRefs.entrySet()) {
+			assertEquals(e.getKey(), e.getValue().getOrigName());
+
+		}
+	}
 }
-- 
1.6.0.3.640.g6331a.dirty

[EGIT PATCH 4/7 v3] Handle peeling of loose refs.

[Robin Rosenberg]

For packed refs we got peeling 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.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java |   35 ++++++++++++++-----
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   32 ++++++++++++++++--
 .../src/org/spearce/jgit/lib/Repository.java       |   13 +++++++
 .../spearce/jgit/transport/BasePackConnection.java |    2 +-
 .../spearce/jgit/transport/TransportAmazonS3.java  |    2 +-
 .../org/spearce/jgit/transport/TransportHttp.java  |    2 +-
 .../org/spearce/jgit/transport/TransportSftp.java  |    2 +-
 .../jgit/transport/WalkRemoteObjectDatabase.java   |    2 +-
 8 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
index 2f102af..0e98f46 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java
@@ -126,6 +126,8 @@ public boolean isPacked() {
 
 	private final String origName;
 
+	private final boolean peeled;
+
 	/**
 	 * Create a new ref pairing.
 	 * 
@@ -140,10 +142,7 @@ public boolean isPacked() {
 	 *            does not exist yet.
 	 */
 	public Ref(final Storage st, final String origName, final String refName, final ObjectId id) {
-		storage = st;
-		this.origName = origName;
-		name = refName;
-		objectId = id;
+		this(st, origName, refName, id, null, false);
 	}
 
 	/**
@@ -158,7 +157,7 @@ public Ref(final Storage st, final String origName, final String refName, final 
 	 *            does not exist yet.
 	 */
 	public Ref(final Storage st, final String refName, final ObjectId id) {
-		this(st, refName, refName, id);
+		this(st, refName, refName, id, null, false);
 	}
 
 	/**
@@ -175,15 +174,18 @@ public Ref(final Storage st, final String refName, final ObjectId id) {
 	 *            does not exist yet.
 	 * @param peel
 	 *            peeled value of the ref's tag. May be null if this is not a
-	 *            tag or the peeled value is not known.
+	 *            tag or not yet peeled (in which case the next parameter should be null)
+	 * @param peeled
+	 * 			  true if peel represents a the peeled value of the object
 	 */
 	public Ref(final Storage st, final String origName, final String refName, final ObjectId id,
-			final ObjectId peel) {
+			final ObjectId peel, final boolean peeled) {
 		storage = st;
 		this.origName = origName;
 		name = refName;
 		objectId = id;
 		peeledObjectId = peel;
+		this.peeled = peeled;
 	}
 
 	/**
@@ -199,10 +201,12 @@ public Ref(final Storage st, final String origName, final String refName, final 
 	 * @param peel
 	 *            peeled value of the ref's tag. May be null if this is not a
 	 *            tag or the peeled value is not known.
+	 * @param peeled
+	 * 			  true if peel represents a the peeled value of the object
 	 */
 	public Ref(final Storage st, final String refName, final ObjectId id,
-			final ObjectId peel) {
-		this(st, refName, refName, id, peel);
+			final ObjectId peel, boolean peeled) {
+		this(st, refName, refName, id, peel, peeled);
 	}
 
 	/**
@@ -238,10 +242,19 @@ public ObjectId getObjectId() {
 	 *         refer to an annotated tag.
 	 */
 	public ObjectId getPeeledObjectId() {
+		if (!peeled)
+			return null;
 		return peeledObjectId;
 	}
 
 	/**
+	 * @return whether the Ref represents a peeled tag
+	 */
+	public boolean isPeeled() {
+		return peeled;
+	}
+
+	/**
 	 * How was this ref obtained?
 	 * <p>
 	 * The current storage model of a Ref may influence how the ref must be
@@ -259,4 +272,8 @@ public String toString() {
 			o = "(" + origName + ")";
 		return "Ref[" + o + name + "=" + ObjectId.toString(getObjectId()) + "]";
 	}
+
+	void setPeeledObjectId(final ObjectId id) {
+		peeledObjectId = id;
+	}
 }
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..494aecb 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,8 @@ private void readOneLooseRef(final Map<String, Ref> avail,
 					return;
 				}
 
-				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id);
+				ref = new Ref(Ref.Storage.LOOSE, origName, refName, id, null, false); // unpeeled
+
 				looseRefs.put(ref.getName(), ref);
 				looseRefsMTime.put(ref.getName(), ent.lastModified());
 				avail.put(ref.getName(), ref);
@@ -288,6 +289,28 @@ private void readOneLooseRef(final Map<String, Ref> avail,
 		}
 	}
 
+	Ref peel(final Ref ref) {
+		if (ref.isPeeled())
+			return ref;
+		ObjectId peeled = null;
+		try {
+			Object target = db.mapObject(ref.getObjectId(), ref.getName());
+			while (target instanceof Tag) {
+				final Tag tag = (Tag)target;
+				peeled = tag.getObjId();
+				if (Constants.TYPE_TAG.equals(tag.getType()))
+					target = db.mapObject(tag.getObjId(), ref.getName());
+				else
+					break;
+			}
+		} catch (IOException e) {
+			// Ignore a read error.  Callers will also get the same error
+			// if they try to use the result of getPeeledObjectId.
+		}
+		return new Ref(ref.getStorage(), ref.getName(), ref.getObjectId(), peeled, true);
+
+	}
+
 	private File fileForRef(final String name) {
 		if (name.startsWith(REFS_SLASH))
 			return new File(refsDir, name.substring(REFS_SLASH.length()));
@@ -350,7 +373,7 @@ private Ref readRefBasic(final String origName, final String name, final int dep
 			if (r == null)
 				return new Ref(Ref.Storage.LOOSE, origName, target, null);
 			if (!origName.equals(r.getName()))
-				r = new Ref(Ref.Storage.LOOSE_PACKED, origName, r.getName(), r.getObjectId(), r.getPeeledObjectId());
+				r = new Ref(Ref.Storage.LOOSE_PACKED, origName, r.getName(), r.getObjectId(), r.getPeeledObjectId(), true);
 			return r; 
 		}
 
@@ -364,6 +387,9 @@ private Ref readRefBasic(final String origName, final String name, final int dep
 		}
 
 		ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
+
+		looseRefs.put(origName, ref);
+		ref = new Ref(Ref.Storage.LOOSE, origName, id);
 		looseRefs.put(name, ref);
 		looseRefsMTime.put(name, mtime);
 		return ref;
@@ -397,7 +423,7 @@ private void refreshPackedRefs() {
 
 						final ObjectId id = ObjectId.fromString(p.substring(1));
 						last = new Ref(Ref.Storage.PACKED, last.getName(), last
-								.getName(), last.getObjectId(), id);
+								.getName(), last.getObjectId(), id, true);
 						newPackedRefs.put(last.getName(), last);
 						continue;
 					}
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 26748e2..4d6e6fd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -939,6 +939,19 @@ public String getBranch() throws IOException {
 	}
 
 	/**
+	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
+	 * the peeled id is set to {@link ObjectId#zeroId()}
+	 * 
+	 * @param ref
+	 *            The ref to peel
+	 * @return The same, an updated ref with peeled info or a new instance with
+	 *         more information
+	 */
+	public Ref peel(final Ref ref) {
+		return refs.peel(ref);
+	}
+
+	/**
 	 * @return true if HEAD points to a StGit patch.
 	 */
 	public boolean isStGitMode() {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
index e5fc040..e9df30e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
@@ -176,7 +176,7 @@ private void readAdvertisedRefsImpl() throws IOException {
 					throw duplicateAdvertisement(name + "^{}");
 
 				avail.put(name, new Ref(Ref.Storage.NETWORK, name, prior
-						.getObjectId(), id));
+						.getObjectId(), id, true));
 			} else {
 				final Ref prior;
 				prior = avail.put(name, new Ref(Ref.Storage.NETWORK, name, id));
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
index f9df36d..9f1b516 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
@@ -300,7 +300,7 @@ private Ref readRef(final TreeMap<String, Ref> avail, final String rn)
 				if (r == null)
 					return null;
 				r = new Ref(r.getStorage(), rn, r.getObjectId(), r
-						.getPeeledObjectId());
+						.getPeeledObjectId(), r.isPeeled());
 				avail.put(r.getName(), r);
 				return r;
 			}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java
index 1357e58..fe4a437 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java
@@ -237,7 +237,7 @@ FileStream open(final String path) throws IOException {
 						throw duplicateAdvertisement(name + "^{}");
 
 					avail.put(name, new Ref(Ref.Storage.NETWORK, name, prior
-							.getObjectId(), id));
+							.getObjectId(), id, true));
 				} else {
 					final Ref prior = avail.put(name, new Ref(
 							Ref.Storage.NETWORK, name, id));
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
index 78f4ad8..544e77c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
@@ -428,7 +428,7 @@ private Ref readRef(final TreeMap<String, Ref> avail,
 					r = avail.get(p);
 				if (r != null) {
 					r = new Ref(loose(r), name, r.getObjectId(), r
-							.getPeeledObjectId());
+							.getPeeledObjectId(), true);
 					avail.put(name, r);
 				}
 				return r;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
index 54dd581..a4f8961 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
@@ -438,7 +438,7 @@ private void readPackedRefsImpl(final Map<String, Ref> avail,
 					throw new TransportException("Peeled line before ref.");
 				final ObjectId id = ObjectId.fromString(line + 1);
 				last = new Ref(Ref.Storage.PACKED, last.getName(), last
-						.getObjectId(), id);
+						.getObjectId(), id, true);
 				avail.put(last.getName(), last);
 				continue;
 			}
-- 
1.6.0.3.640.g6331a.dirty

[EGIT PATCH 5/7 v3] Add a method to get refs by object Id

[Robin Rosenberg]

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

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 4d6e6fd..5088150 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -47,10 +47,13 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.Vector;
 
 import org.spearce.jgit.errors.IncorrectObjectTypeException;
@@ -952,6 +955,33 @@ public Ref peel(final Ref ref) {
 	}
 
 	/**
+	 * @return a map with all objects referenced by a peeled ref.
+	 */
+	public Map<AnyObjectId, Set<Ref>> getAllRefsByPeeledObjectId() {
+		Map<String, Ref> allRefs = getAllRefs();
+		Map<AnyObjectId, Set<Ref>> ret = new HashMap<AnyObjectId, Set<Ref>>(allRefs.size());
+		for (Ref ref : allRefs.values()) {
+			if (!ref.isPeeled())
+				ref = peel(ref);
+			AnyObjectId target = ref.getPeeledObjectId();
+			if (target == null)
+				target = ref.getObjectId();
+			// We assume most Sets here are singletons
+			Set<Ref> oset = ret.put(target, Collections.singleton(ref));
+			if (oset != null) {
+				// that was not the case (rare)
+				if (oset.size() == 1) {
+					// Was a read-only singleton, we must copy to a new Set
+					oset = new HashSet<Ref>(oset);
+				}
+				ret.put(target, oset);
+				oset.add(ref);
+			}
+		}
+		return ret;
+	}
+
+	/**
 	 * @return true if HEAD points to a StGit patch.
 	 */
 	public boolean isStGitMode() {
-- 
1.6.0.3.640.g6331a.dirty

Re: [EGIT PATCH 4/7 v3] Handle peeling of loose refs.

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> For packed refs we got peeling 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.

I merged your series, but I squashed the following into the patch
I am replying to:
 
 .../src/org/spearce/jgit/lib/Repository.java       |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

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 5088150..c953531 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -942,13 +942,17 @@ public String getBranch() throws IOException {
 	}
 
 	/**
-	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
-	 * the peeled id is set to {@link ObjectId#zeroId()}
+	 * Peel a possibly unpeeled ref and updates it.
+	 * <p>
+	 * If the ref cannot be peeled (as it does not refer to an annotated tag)
+	 * the peeled id stays null, but {@link Ref#isPeeled()} will be true.
 	 * 
 	 * @param ref
 	 *            The ref to peel
-	 * @return The same, an updated ref with peeled info or a new instance with
-	 *         more information
+	 * @return <code>ref</code> if <code>ref.isPeeled()</code> is true; else a
+	 *         new Ref object representing the same data as Ref, but isPeeled()
+	 *         will be true and getPeeledObjectId will contain the peeled object
+	 *         (or null).
 	 */
 	public Ref peel(final Ref ref) {
 		return refs.peel(ref);
-- 
1.6.0.4.969.g58a38

-- 
Shawn.