{"thread":{"id":"15795","subject":"(unknown)","startedAt":"2008-10-05T23:36:36Z","lastAt":"2008-10-06T22:43:48Z","messageCount":14,"participants":["Robin Rosenberg","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"92371","messageId":"1223249802-9959-1-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":null,"subject":"(unknown)","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:36Z","receivedAt":"2008-10-05T23:36:36Z","isPatch":false,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"This series decorates the graphical and text (jgit log) history listings\nwith tags. For the text command it is optional.\n\nReviewers may want to pay special attention to the changes in the Ref class.\n\n-- robin\n"},{"id":"92373","messageId":"1223249802-9959-2-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"1223249802-9959-1-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 1/6] Keep original ref name when reading refs","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:37Z","receivedAt":"2008-10-05T23:36:37Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"We want to know the original name of refs, not just the target name.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java |   63 +++++++++++++++++++-\n .../src/org/spearce/jgit/lib/RefDatabase.java      |   45 +++++++++-----\n .../src/org/spearce/jgit/lib/RefUpdate.java        |   14 ++---\n 3 files changed, 94 insertions(+), 28 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java\nindex db94875..2f102af 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java\n@@ -43,6 +43,11 @@\n  * A ref in Git is (more or less) a variable that holds a single object\n  * identifier. The object identifier can be any valid Git object (blob, tree,\n  * commit, annotated tag, ...).\n+ * <p>\n+ * The ref name has the attributes of the ref that was asked for as well as\n+ * the ref it was resolved to for symbolic refs plus the object id it points\n+ * to and (for tags) the peeled target object id, i.e. the tag resolved\n+ * recursively until a non-tag object is referenced.\n  */\n public class Ref {\n \t/** Location where a {@link Ref} is stored. */\n@@ -119,19 +124,24 @@ public boolean isPacked() {\n \n \tprivate ObjectId peeledObjectId;\n \n+\tprivate final String origName;\n+\n \t/**\n \t * Create a new ref pairing.\n \t * \n \t * @param st\n \t *            method used to store this ref.\n+\t * @param origName\n+\t *            The name used to resolve this ref\n \t * @param refName\n \t *            name of this ref.\n \t * @param id\n \t *            current value of the ref. May be null to indicate a ref that\n \t *            does not exist yet.\n \t */\n-\tpublic Ref(final Storage st, final String refName, final ObjectId id) {\n+\tpublic Ref(final Storage st, final String origName, final String refName, final ObjectId id) {\n \t\tstorage = st;\n+\t\tthis.origName = origName;\n \t\tname = refName;\n \t\tobjectId = id;\n \t}\n@@ -146,19 +156,56 @@ public Ref(final Storage st, final String refName, final ObjectId id) {\n \t * @param id\n \t *            current value of the ref. May be null to indicate a ref that\n \t *            does not exist yet.\n+\t */\n+\tpublic Ref(final Storage st, final String refName, final ObjectId id) {\n+\t\tthis(st, refName, refName, id);\n+\t}\n+\n+\t/**\n+\t * Create a new ref pairing.\n+\t * \n+\t * @param st\n+\t *            method used to store this ref.\n+\t * @param origName\n+\t *            The name used to resolve this ref\n+\t * @param refName\n+\t *            name of this ref.\n+\t * @param id\n+\t *            current value of the ref. May be null to indicate a ref that\n+\t *            does not exist yet.\n \t * @param peel\n \t *            peeled value of the ref's tag. May be null if this is not a\n \t *            tag or the peeled value is not known.\n \t */\n-\tpublic Ref(final Storage st, final String refName, final ObjectId id,\n+\tpublic Ref(final Storage st, final String origName, final String refName, final ObjectId id,\n \t\t\tfinal ObjectId peel) {\n \t\tstorage = st;\n+\t\tthis.origName = origName;\n \t\tname = refName;\n \t\tobjectId = id;\n \t\tpeeledObjectId = peel;\n \t}\n \n \t/**\n+\t * Create a new ref pairing.\n+\t * \n+\t * @param st\n+\t *            method used to store this ref.\n+\t * @param refName\n+\t *            name of this ref.\n+\t * @param id\n+\t *            current value of the ref. May be null to indicate a ref that\n+\t *            does not exist yet.\n+\t * @param peel\n+\t *            peeled value of the ref's tag. May be null if this is not a\n+\t *            tag or the peeled value is not known.\n+\t */\n+\tpublic Ref(final Storage st, final String refName, final ObjectId id,\n+\t\t\tfinal ObjectId peel) {\n+\t\tthis(st, refName, refName, id, peel);\n+\t}\n+\n+\t/**\n \t * What this ref is called within the repository.\n \t * \n \t * @return name of this ref.\n@@ -168,6 +215,13 @@ public String getName() {\n \t}\n \n \t/**\n+\t * @return the originally resolved name\n+\t */\n+\tpublic String getOrigName() {\n+\t\treturn origName;\n+\t}\n+\n+\t/**\n \t * Cached value of this ref.\n \t * \n \t * @return the value of this ref at the last time we read it.\n@@ -200,6 +254,9 @@ public Storage getStorage() {\n \t}\n \n \tpublic String toString() {\n-\t\treturn \"Ref[\" + name + \"=\" + ObjectId.toString(getObjectId()) + \"]\";\n+\t\tString o = \"\";\n+\t\tif (!origName.equals(name))\n+\t\t\to = \"(\" + origName + \")\";\n+\t\treturn \"Ref[\" + o + name + \"=\" + ObjectId.toString(getObjectId()) + \"]\";\n \t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\nindex 5c1f060..5a1b85f 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n@@ -51,6 +51,7 @@\n import java.util.Map;\n \n import org.spearce.jgit.errors.ObjectWritingException;\n+import org.spearce.jgit.lib.Ref.Storage;\n import org.spearce.jgit.util.FS;\n import org.spearce.jgit.util.NB;\n \n@@ -135,8 +136,8 @@ RefUpdate newUpdate(final String name) throws IOException {\n \t\treturn new RefUpdate(this, r, fileForRef(r.getName()));\n \t}\n \n-\tvoid stored(final String name, final ObjectId id, final long time) {\n-\t\tlooseRefs.put(name, new Ref(Ref.Storage.LOOSE, name, id));\n+\tvoid stored(final String origName, final String name, final ObjectId id, final long time) {\n+\t\tlooseRefs.put(name, new Ref(Ref.Storage.LOOSE, origName, name, id));\n \t\tlooseRefsMTime.put(name, time);\n \t\tsetModified();\n \t\tdb.fireRefsMaybeChanged();\n@@ -222,12 +223,12 @@ private void readLooseRefs(final Map<String, Ref> avail,\n \t\t\tfinal String entName = ent.getName();\n \t\t\tif (\".\".equals(entName) || \"..\".equals(entName))\n \t\t\t\tcontinue;\n-\t\t\treadOneLooseRef(avail, prefix + entName, ent);\n+\t\t\treadOneLooseRef(avail, prefix + entName, prefix + entName, ent);\n \t\t}\n \t}\n \n \tprivate void readOneLooseRef(final Map<String, Ref> avail,\n-\t\t\tfinal String refName, final File ent) {\n+\t\t\tfinal String origName, final String refName, final File ent) {\n \t\t// Unchanged and cached? Don't read it again.\n \t\t//\n \t\tRef ref = looseRefs.get(refName);\n@@ -270,7 +271,7 @@ private void readOneLooseRef(final Map<String, Ref> avail,\n \t\t\t\t\treturn;\n \t\t\t\t}\n \n-\t\t\t\tref = new Ref(Ref.Storage.LOOSE, refName, id);\n+\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id);\n \t\t\t\tlooseRefs.put(ref.getName(), ref);\n \t\t\t\tlooseRefsMTime.put(ref.getName(), ent.lastModified());\n \t\t\t\tavail.put(ref.getName(), ref);\n@@ -293,27 +294,35 @@ private File fileForRef(final String name) {\n \t\treturn new File(gitDir, name);\n \t}\n \n-\tprivate Ref readRefBasic(final String name, final int depth)\n+\tprivate Ref readRefBasic(final String name, final int depth) throws IOException {\n+\t\treturn readRefBasic(name, name, depth);\n+\t}\n+\n+\tprivate Ref readRefBasic(final String origName, final String name, final int depth)\n \t\t\tthrows IOException {\n \t\t// Prefer loose ref to packed ref as the loose\n \t\t// file can be more up-to-date than a packed one.\n \t\t//\n-\t\tRef ref = looseRefs.get(name);\n+\t\tRef ref = looseRefs.get(origName);\n \t\tfinal File loose = fileForRef(name);\n \t\tfinal long mtime = loose.lastModified();\n \t\tif (ref != null) {\n \t\t\tLong cachedlastModified = looseRefsMTime.get(name);\n \t\t\tif (cachedlastModified != null && cachedlastModified == mtime)\n \t\t\t\treturn ref;\n-\t\t\tlooseRefs.remove(name);\n-\t\t\tlooseRefsMTime.remove(name);\n+\t\t\tlooseRefs.remove(origName);\n+\t\t\tlooseRefsMTime.remove(origName);\n \t\t}\n \n \t\tif (mtime == 0) {\n \t\t\t// If last modified is 0 the file does not exist.\n \t\t\t// Try packed cache.\n \t\t\t//\n-\t\t\treturn packedRefs.get(name);\n+\t\t\tref = packedRefs.get(name);\n+\t\t\tif (ref != null)\n+\t\t\t\tif (!ref.getOrigName().equals(origName))\n+\t\t\t\t\tref = new Ref(Storage.LOOSE_PACKED, origName, name, ref.getObjectId());\n+\t\t\treturn ref;\n \t\t}\n \n \t\tfinal String line;\n@@ -324,7 +333,7 @@ private Ref readRefBasic(final String name, final int depth)\n \t\t}\n \n \t\tif (line == null || line.length() == 0)\n-\t\t\treturn new Ref(Ref.Storage.LOOSE, name, null);\n+\t\t\treturn new Ref(Ref.Storage.LOOSE, origName, name, null);\n \n \t\tif (line.startsWith(\"ref: \")) {\n \t\t\tif (depth >= 5) {\n@@ -333,12 +342,16 @@ private Ref readRefBasic(final String name, final int depth)\n \t\t\t}\n \n \t\t\tfinal String target = line.substring(\"ref: \".length());\n-\t\t\tfinal Ref r = readRefBasic(target, depth + 1);\n+\t\t\tRef r = readRefBasic(target, target, depth + 1);\n \t\t\tLong cachedMtime = looseRefsMTime.get(name);\n \t\t\tif (cachedMtime != null && cachedMtime != mtime)\n \t\t\t\tsetModified();\n \t\t\tlooseRefsMTime.put(name, mtime);\n-\t\t\treturn r != null ? r : new Ref(Ref.Storage.LOOSE, target, null);\n+\t\t\tif (r == null)\n+\t\t\t\treturn new Ref(Ref.Storage.LOOSE, origName, target, null);\n+\t\t\tif (!origName.equals(r.getName()))\n+\t\t\t\tr = new Ref(Ref.Storage.LOOSE_PACKED, origName, r.getName(), r.getObjectId(), r.getPeeledObjectId());\n+\t\t\treturn r; \n \t\t}\n \n \t\tsetModified();\n@@ -350,7 +363,7 @@ private Ref readRefBasic(final String name, final int depth)\n \t\t\tthrow new IOException(\"Not a ref: \" + name + \": \" + line);\n \t\t}\n \n-\t\tref = new Ref(Ref.Storage.LOOSE, name, id);\n+\t\tref = new Ref(Ref.Storage.LOOSE, origName, name, id);\n \t\tlooseRefs.put(name, ref);\n \t\tlooseRefsMTime.put(name, mtime);\n \t\treturn ref;\n@@ -384,7 +397,7 @@ private void refreshPackedRefs() {\n \n \t\t\t\t\t\tfinal ObjectId id = ObjectId.fromString(p.substring(1));\n \t\t\t\t\t\tlast = new Ref(Ref.Storage.PACKED, last.getName(), last\n-\t\t\t\t\t\t\t\t.getObjectId(), id);\n+\t\t\t\t\t\t\t\t.getName(), last.getObjectId(), id);\n \t\t\t\t\t\tnewPackedRefs.put(last.getName(), last);\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n@@ -392,7 +405,7 @@ private void refreshPackedRefs() {\n \t\t\t\t\tfinal int sp = p.indexOf(' ');\n \t\t\t\t\tfinal ObjectId id = ObjectId.fromString(p.substring(0, sp));\n \t\t\t\t\tfinal String name = new String(p.substring(sp + 1));\n-\t\t\t\t\tlast = new Ref(Ref.Storage.PACKED, name, id);\n+\t\t\t\t\tlast = new Ref(Ref.Storage.PACKED, name, name, id);\n \t\t\t\t\tnewPackedRefs.put(last.getName(), last);\n \t\t\t\t}\n \t\t\t} finally {\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\nindex 86b44c5..235c2fd 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\n@@ -131,9 +131,6 @@\n \t/** Repository the ref is stored in. */\n \tprivate final RefDatabase db;\n \n-\t/** Name of the ref. */\n-\tprivate final String name;\n-\n \t/** Location of the loose file holding the value of this ref. */\n \tprivate final File looseFile;\n \n@@ -160,7 +157,6 @@\n \tRefUpdate(final RefDatabase r, final Ref ref, final File f) {\n \t\tdb = r;\n \t\tthis.ref = ref;\n-\t\tname = ref.getName();\n \t\toldValue = ref.getObjectId();\n \t\tlooseFile = f;\n \t}\n@@ -171,7 +167,7 @@ RefUpdate(final RefDatabase r, final Ref ref, final File f) {\n \t * @return name of this ref.\n \t */\n \tpublic String getName() {\n-\t\treturn name;\n+\t\treturn ref.getName();\n \t}\n \n \t/**\n@@ -349,9 +345,9 @@ public Result delete() throws IOException {\n \t * @throws IOException\n \t */\n \tpublic Result delete(final RevWalk walk) throws IOException {\n-\t\tif (name.startsWith(Constants.R_HEADS)) {\n+\t\tif (getName().startsWith(Constants.R_HEADS)) {\n \t\t\tfinal Ref head = db.readRef(Constants.HEAD);\n-\t\t\tif (head != null && name.equals(head.getName()))\n+\t\t\tif (head != null && getName().equals(head.getName()))\n \t\t\t\treturn Result.REJECTED_CURRENT_BRANCH;\n \t\t}\n \n@@ -373,7 +369,7 @@ private Result updateImpl(final RevWalk walk, final Store store)\n \t\tif (!lock.lock())\n \t\t\treturn Result.LOCK_FAILURE;\n \t\ttry {\n-\t\t\toldValue = db.idOf(name);\n+\t\t\toldValue = db.idOf(getName());\n \t\t\tif (oldValue == null)\n \t\t\t\treturn store.store(lock, Result.NEW);\n \n@@ -428,7 +424,7 @@ else if (status == Result.NEW)\n \t\t\t\tgetName());\n \t\tif (!lock.commit())\n \t\t\treturn Result.LOCK_FAILURE;\n-\t\tdb.stored(name, newValue, lock.getCommitLastModified());\n+\t\tdb.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());\n \t\treturn status;\n \t}\n \n-- \n1.6.0.1.310.gf789d0.dirty\n"},{"id":"92372","messageId":"1223249802-9959-3-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"1223249802-9959-2-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 2/6] Peel annotated tags when getting all refs","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:38Z","receivedAt":"2008-10-05T23:36:38Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"For packed refs we got this automatically from packed-refs,\nbut for loose tags we have to follow the tags and get the leaf\nobject in order to comply with the documentation.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../src/org/spearce/jgit/lib/RefDatabase.java      |   24 ++++++++++++++++++-\n 1 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\nindex 5a1b85f..1ee70d9 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n@@ -271,7 +271,16 @@ private void readOneLooseRef(final Map<String, Ref> avail,\n \t\t\t\t\treturn;\n \t\t\t\t}\n \n-\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id);\n+\t\t\t\tObject tt = db.mapObject(id, refName);\n+\t\t\t\tif (tt != null && tt instanceof Tag) {\n+\t\t\t\t\tTag t = (Tag)tt;\n+\t\t\t\t\twhile (t != null && t.getType().equals(Constants.TYPE_TAG))\n+\t\t\t\t\t\tt = db.mapTag(t.getTag(), t.getObjId());\n+\t\t\t\t\tif (t != null)\n+\t\t\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id, t.getObjId());\n+\t\t\t\t} else\n+\t\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id);\n+\n \t\t\t\tlooseRefs.put(ref.getName(), ref);\n \t\t\t\tlooseRefsMTime.put(ref.getName(), ent.lastModified());\n \t\t\t\tavail.put(ref.getName(), ref);\n@@ -363,7 +372,18 @@ private Ref readRefBasic(final String origName, final String name, final int dep\n \t\t\tthrow new IOException(\"Not a ref: \" + name + \": \" + line);\n \t\t}\n \n-\t\tref = new Ref(Ref.Storage.LOOSE, origName, name, id);\n+\t\tObject tt = db.mapObject(id, origName);\n+\t\tif (tt != null && tt instanceof Tag) {\n+\t\t\tTag t = (Tag)tt;\n+\t\t\twhile (t != null && t.getType().equals(Constants.TYPE_TAG))\n+\t\t\t\tt = db.mapTag(t.getTag(), t.getObjId());\n+\t\t\tif (t != null)\n+\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, name, id, t.getObjId());\n+\t\t} else\n+\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, name, id);\n+\n+\t\tlooseRefs.put(origName, ref);\n+\t\tref = new Ref(Ref.Storage.LOOSE, origName, id);\n \t\tlooseRefs.put(name, ref);\n \t\tlooseRefsMTime.put(name, mtime);\n \t\treturn ref;\n-- \n1.6.0.1.310.gf789d0.dirty\n"},{"id":"92377","messageId":"1223249802-9959-4-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"1223249802-9959-3-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 3/6] Add a method to get refs by object Id","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:39Z","receivedAt":"2008-10-05T23:36:39Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../src/org/spearce/jgit/lib/Repository.java       |   28 ++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\nindex dfce1b8..3fc5236 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n@@ -47,6 +47,7 @@\n import java.io.IOException;\n import java.util.ArrayList;\n import java.util.Collection;\n+import java.util.Collections;\n import java.util.HashMap;\n import java.util.LinkedList;\n import java.util.List;\n@@ -939,6 +940,33 @@ public String getBranch() throws IOException {\n \t}\n \n \t/**\n+\t * @return a map with all objects referenced by a peeled ref.\n+\t */\n+\tpublic Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {\n+\t\tMap<String, Ref> allRefs = getAllRefs();\n+\t\tHashMap<AnyObjectId, List<Ref>> ret = new HashMap<AnyObjectId, List<Ref>>(allRefs.size());\n+\t\tfor (Map.Entry<String,Ref> e : allRefs.entrySet()) {\n+\t\t\tRef ref = e.getValue();\n+\t\t\tAnyObjectId target = ref.getPeeledObjectId();\n+\t\t\tif (target == null)\n+\t\t\t\ttarget = ref.getObjectId();\n+\t\t\tList<Ref> list = ret.get(target);\n+\t\t\tif (list == null) {\n+\t\t\t\tlist = Collections.singletonList(ref);\n+\t\t\t} else {\n+\t\t\t\tif (list.size() == 1) {\n+\t\t\t\t\tArrayList<Ref> list2 = new ArrayList<Ref>(2);\n+\t\t\t\t\tlist2.add(list.get(0));\n+\t\t\t\t\tlist = list2;\n+\t\t\t\t}\n+\t\t\t\tlist.add(ref);\n+\t\t\t}\n+\t\t\tret.put(target, list);\n+\t\t}\n+\t\treturn ret;\n+\t}\n+\n+\t/**\n \t * @return true if HEAD points to a StGit patch.\n \t */\n \tpublic boolean isStGitMode() {\n-- \n1.6.0.1.310.gf789d0.dirty\n"},{"id":"92375","messageId":"1223249802-9959-5-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"1223249802-9959-4-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 4/6] Add tags to the graphical history display.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:40Z","receivedAt":"2008-10-05T23:36:40Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Both the SWT (Eclipse) drawing and Swing versions are updated.\nThe coloring and shapes are intentionally not the same as for gitk.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../egit/core/internal/storage/KidWalk.java        |    3 +-\n .../egit/ui/internal/history/SWTCommit.java        |    5 +-\n .../egit/ui/internal/history/SWTPlotRenderer.java  |   64 +++++++++++++++++++-\n .../spearce/egit/ui/internal/history/SWTWalk.java  |    5 +-\n .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |   46 ++++++++++++++\n .../spearce/jgit/revplot/AbstractPlotRenderer.java |   18 +++++-\n .../src/org/spearce/jgit/revplot/PlotCommit.java   |    8 ++-\n .../src/org/spearce/jgit/revplot/PlotWalk.java     |   56 ++++++++++++++++-\n .../src/org/spearce/jgit/revwalk/RevWalk.java      |   19 +++++-\n 9 files changed, 209 insertions(+), 15 deletions(-)\n\ndiff --git a/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/KidWalk.java b/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/KidWalk.java\nindex 6b8f468..b337efe 100644\n--- a/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/KidWalk.java\n+++ b/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/KidWalk.java\n@@ -11,6 +11,7 @@\n import org.spearce.jgit.lib.AnyObjectId;\n import org.spearce.jgit.lib.Repository;\n import org.spearce.jgit.revwalk.RevCommit;\n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.revwalk.RevWalk;\n \n class KidWalk extends RevWalk {\n@@ -19,7 +20,7 @@ KidWalk(final Repository repo) {\n \t}\n \n \t@Override\n-\tprotected RevCommit createCommit(final AnyObjectId id) {\n+\tprotected RevCommit createCommit(final AnyObjectId id, final Ref[] tags) {\n \t\treturn new KidCommit(id);\n \t}\n }\ndiff --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\nindex fa0d25d..2341fbd 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTCommit.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTCommit.java\n@@ -10,12 +10,13 @@\n import org.eclipse.swt.widgets.Widget;\n import org.spearce.jgit.lib.AnyObjectId;\n import org.spearce.jgit.revplot.PlotCommit;\n+import org.spearce.jgit.lib.Ref;\n \n class SWTCommit extends PlotCommit<SWTCommitList.SWTLane> {\n \tWidget widget;\n \n-\tSWTCommit(final AnyObjectId id) {\n-\t\tsuper(id);\n+\tSWTCommit(final AnyObjectId id, final Ref[] tags) {\n+\t\tsuper(id, tags);\n \t}\n \n \t@Override\ndiff --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\nindex 23ec255..56d5842 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java\n@@ -15,7 +15,10 @@\n import org.eclipse.swt.widgets.Display;\n import org.eclipse.swt.widgets.Event;\n import org.eclipse.swt.widgets.TableItem;\n+import org.eclipse.ui.themes.ColorUtil;\n import org.spearce.egit.ui.internal.history.SWTCommitList.SWTLane;\n+import org.spearce.jgit.lib.Constants;\n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.revplot.AbstractPlotRenderer;\n import org.spearce.jgit.revplot.PlotCommit;\n \n@@ -26,7 +29,13 @@\n \n \tprivate final Color sys_gray;\n \n-\tprivate Color sys_darkblue;\n+\tprivate final Color sys_darkblue;\n+\n+\tprivate final Color sys_yellow;\n+\n+\tprivate final Color sys_green;\n+\n+\tprivate final Color sys_white;\n \n \tGC g;\n \n@@ -43,6 +52,9 @@ SWTPlotRenderer(final Display d) {\n \t\tsys_black = d.getSystemColor(SWT.COLOR_BLACK);\n \t\tsys_gray = d.getSystemColor(SWT.COLOR_GRAY);\n \t\tsys_darkblue = d.getSystemColor(SWT.COLOR_DARK_BLUE);\n+\t\tsys_yellow = d.getSystemColor(SWT.COLOR_YELLOW);\n+\t\tsys_green = d.getSystemColor(SWT.COLOR_GREEN);\n+\t\tsys_white = d.getSystemColor(SWT.COLOR_WHITE);\n \t}\n \n \tvoid paint(final Event event) {\n@@ -92,7 +104,57 @@ protected void drawText(final String msg, final int x, final int y) {\n \t\tg.drawString(msg, cellX + x, cellY + texty);\n \t}\n \n+\t@Override\n+\tprotected int drawLabel(int x, int y, Ref ref) {\n+\t\tString txt;\n+\t\tString name = ref.getOrigName();\n+\t\tif (name.startsWith(Constants.R_HEADS)) {\n+\t\t\tg.setBackground(sys_green);\n+\t\t\ttxt = name.substring(Constants.R_HEADS.length());\n+\t\t} else if (name.startsWith(Constants.R_REMOTES)){\n+\t\t\tg.setBackground(sys_gray);\n+\t\t\ttxt = name.substring(Constants.R_REMOTES.length());\n+\t\t} else if (name.startsWith(Constants.R_TAGS)){\n+\t\t\tg.setBackground(sys_yellow);\n+\t\t\ttxt = name.substring(Constants.R_TAGS.length());\n+\t\t} else {\n+\t\t\t// Whatever this would be\n+\t\t\tg.setBackground(sys_white);\n+\t\t\tif (name.startsWith(Constants.R_REFS))\n+\t\t\t\ttxt = name.substring(Constants.R_REFS.length());\n+\t\t\telse\n+\t\t\t\ttxt = name; // HEAD and such\n+\t\t}\n+\t\tColor peeledColor = null;\n+\t\tif (ref.getPeeledObjectId() != null) {\n+\t\t\tpeeledColor = new Color(g.getDevice(), ColorUtil.blend(g.getBackground().getRGB(), sys_white.getRGB()));\n+\t\t\tg.setBackground(peeledColor);\n+\t\t}\n+\t\tif (txt.length() > 12)\n+\t\t\ttxt = txt.substring(0,11) + \"\\u2026\"; // ellipsis \"…\" (in UTF-8)\n+\n+\t\tPoint testsz = g.stringExtent(txt);\n+\t\tfinal int texty = (y * 2 - testsz.y) / 2;\n+\t\tg.setForeground(sys_black);\n+\t\tg.drawString(txt, x + 2, cellY + texty);\n+\t\tg.setLineWidth(2);\n+\t\tColor blend1 = new Color(g.getDevice(), ColorUtil.blend(g.getBackground().getRGB(), sys_gray.getRGB()));\n+\t\tg.setForeground(blend1);\n+\t\tg.drawRoundRectangle(x, cellY + texty -2, testsz.x + 3, testsz.y + 3, testsz.y/4, testsz.y/4);\n+\t\tg.setLineWidth(2);\n+\t\tColor blend2 = new Color(g.getDevice(), ColorUtil.blend(g.getBackground().getRGB(), sys_black.getRGB()));\n+\t\tg.setForeground(blend2);\n+\t\tg.drawRoundRectangle(x + 1, cellY + texty -1, testsz.x + 1, testsz.y + 1, testsz.y/4, testsz.y/4);\n+\n+\t\tblend1.dispose();\n+\t\tblend2.dispose();\n+\t\tif (peeledColor != null)\n+\t\t\tpeeledColor.dispose();\n+\t\treturn 8 + testsz.x;\n+\t}\n+\n \tprotected Color laneColor(final SWTLane myLane) {\n \t\treturn myLane != null ? myLane.color : sys_black;\n \t}\n+\n }\ndiff --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\nindex 527d284..bc347db 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTWalk.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTWalk.java\n@@ -11,6 +11,7 @@\n import org.spearce.jgit.lib.Repository;\n import org.spearce.jgit.revplot.PlotWalk;\n import org.spearce.jgit.revwalk.RevCommit;\n+import org.spearce.jgit.lib.Ref;\n \n class SWTWalk extends PlotWalk {\n \tSWTWalk(final Repository repo) {\n@@ -18,7 +19,7 @@ SWTWalk(final Repository repo) {\n \t}\n \n \t@Override\n-\tprotected RevCommit createCommit(final AnyObjectId id) {\n-\t\treturn new SWTCommit(id);\n+\tprotected RevCommit createCommit(final AnyObjectId id, final Ref[] tags) {\n+\t\treturn new SWTCommit(id, tags);\n \t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java\nindex b6b715c..5dcddf5 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java\n@@ -44,6 +44,8 @@\n \n import org.spearce.jgit.awtui.CommitGraphPane.GraphCellRender;\n import org.spearce.jgit.awtui.SwingCommitList.SwingLane;\n+import org.spearce.jgit.lib.Constants;\n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.revplot.AbstractPlotRenderer;\n import org.spearce.jgit.revplot.PlotCommit;\n \n@@ -134,4 +136,48 @@ void paintTriangleDown(final int cx, final int y, final int h) {\n \t\tg.drawPolygon(triangle);\n \t}\n \n+\t@Override\n+\tprotected int drawLabel(int x, int y, Ref ref) {\n+\t\tString txt;\n+\t\tString name = ref.getOrigName();\n+\t\tif (name.startsWith(Constants.R_HEADS)) {\n+\t\t\tg.setBackground(Color.GREEN);\n+\t\t\ttxt = name.substring(Constants.R_HEADS.length());\n+\t\t} else if (name.startsWith(Constants.R_REMOTES)){\n+\t\t\tg.setBackground(Color.LIGHT_GRAY);\n+\t\t\ttxt = name.substring(Constants.R_REMOTES.length());\n+\t\t} else if (name.startsWith(Constants.R_TAGS)){\n+\t\t\tg.setBackground(Color.YELLOW);\n+\t\t\ttxt = name.substring(Constants.R_TAGS.length());\n+\t\t} else {\n+\t\t\t// Whatever this would be\n+\t\t\tg.setBackground(Color.WHITE);\n+\t\t\tif (name.startsWith(Constants.R_REFS))\n+\t\t\t\ttxt = name.substring(Constants.R_REFS.length());\n+\t\t\telse\n+\t\t\t\ttxt = name; // HEAD and such\n+\t\t}\n+\t\tif (ref.getPeeledObjectId() != null) {\n+\t\t\tfloat[] colorComponents = g.getBackground().getRGBColorComponents(null);\n+\t\t\tcolorComponents[0] *= 0.9;\n+\t\t\tcolorComponents[1] *= 0.9;\n+\t\t\tcolorComponents[2] *= 0.9;\n+\t\t\tg.setBackground(new Color(colorComponents[0],colorComponents[1],colorComponents[2]));\n+\t\t}\n+\t\tif (txt.length() > 12)\n+\t\t\ttxt = txt.substring(0,11) + \"\\u2026\"; // ellipsis \"…\" (in UTF-8)\n+\n+\t\tfinal int texth = g.getFontMetrics().getHeight();\n+\t\tint textw = g.getFontMetrics().stringWidth(txt);\n+\t\tg.setColor(g.getBackground());\n+\t\tint arcHeight = texth/4;\n+\t\tint y0 = y - texth/2 + (cell.getHeight() - texth)/2;\n+\t\tg.fillRoundRect(x , y0, textw + arcHeight*2, texth -1, arcHeight, arcHeight);\n+\t\tg.setColor(g.getColor().darker());\n+\t\tg.drawRoundRect(x, y0, textw + arcHeight*2, texth -1 , arcHeight, arcHeight);\n+\t\tg.setColor(Color.BLACK);\n+\t\tg.drawString(txt, x + arcHeight, y0 + texth - g.getFontMetrics().getDescent());\n+\n+\t\treturn arcHeight * 3 + textw;\n+\t}\n }\n\\ No newline at end of file\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java\nindex f175c9d..603547b 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/AbstractPlotRenderer.java\n@@ -37,6 +37,7 @@\n \n package org.spearce.jgit.revplot;\n \n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.revwalk.RevFlag;\n \n /**\n@@ -140,11 +141,24 @@ protected void paintCommit(final PlotCommit<TLane> commit, final int h) {\n \t\telse\n \t\t\tdrawCommitDot(dotX, dotY, dotSize, dotSize);\n \n+\t\tint textx = Math.max(maxCenter + LANE_WIDTH / 2, dotX + dotSize) + 8;\n+\t\tint n = commit.refs == null ? 0 : commit.refs.length;\n+\t\tfor (int i = 0; i < n; ++i) {\n+\t\t\ttextx += drawLabel(textx + dotSize, h/2, commit.refs[i]);\n+\t\t}\n+\n \t\tfinal String msg = commit.getShortMessage();\n-\t\tfinal int textx = Math.max(maxCenter + LANE_WIDTH / 2, dotX + dotSize) + 8;\n-\t\tdrawText(msg, textx, h / 2);\n+\t\tdrawText(msg, textx + dotSize + n*2, h / 2);\n \t}\n \n+\t/** FIXME: supply text \n+\t * @param x \n+\t * @param y \n+\t * @param ref TODO\n+\t * @return TODO\n+\t */\n+\tprotected abstract int drawLabel(int x, int y, Ref ref);\n+\n \tprivate int computeDotSize(final int h) {\n \t\tint d = (int) (Math.min(h, LANE_WIDTH) * 0.50f);\n \t\td += (d & 1);\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java\nindex 5a5ef1e..fac89f5 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java\n@@ -39,6 +39,7 @@\n \n import org.spearce.jgit.lib.AnyObjectId;\n import org.spearce.jgit.revwalk.RevCommit;\n+import org.spearce.jgit.lib.Ref;\n \n /**\n  * A commit reference to a commit in the DAG.\n@@ -58,14 +59,19 @@\n \n \tPlotCommit[] children;\n \n+\tRef[] refs;\n+\n \t/**\n \t * Create a new commit.\n \t * \n \t * @param id\n \t *            the identity of this commit.\n+\t * @param tags\n+\t *            the tags associated with this commit, null for no tags\n \t */\n-\tprotected PlotCommit(final AnyObjectId id) {\n+\tprotected PlotCommit(final AnyObjectId id, final Ref[] tags) {\n \t\tsuper(id);\n+\t\tthis.refs = tags;\n \t\tpassingLanes = NO_LANES;\n \t\tchildren = NO_CHILDREN;\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java\nindex e5e8aba..6e253f4 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotWalk.java\n@@ -37,14 +37,26 @@\n \n package org.spearce.jgit.revplot;\n \n+import java.io.IOException;\n+import java.util.Collections;\n+import java.util.Comparator;\n+import java.util.List;\n+import java.util.Map;\n+\n import org.spearce.jgit.lib.AnyObjectId;\n+import org.spearce.jgit.lib.Commit;\n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.lib.Repository;\n+import org.spearce.jgit.lib.Tag;\n import org.spearce.jgit.revwalk.RevCommit;\n import org.spearce.jgit.revwalk.RevSort;\n import org.spearce.jgit.revwalk.RevWalk;\n \n /** Specialized RevWalk for visualization of a commit graph. */\n public class PlotWalk extends RevWalk {\n+\n+\tMap<AnyObjectId, List<Ref>> reverseRefMap;\n+\n \t/**\n \t * Create a new revision walker for a given repository.\n \t * \n@@ -54,6 +66,7 @@\n \tpublic PlotWalk(final Repository repo) {\n \t\tsuper(repo);\n \t\tsuper.sort(RevSort.TOPO, true);\n+\t\treverseRefMap = repo.getAllRefsByPeeledObjectId();\n \t}\n \n \t@Override\n@@ -64,7 +77,46 @@ public void sort(final RevSort s, final boolean use) {\n \t}\n \n \t@Override\n-\tprotected RevCommit createCommit(final AnyObjectId id) {\n-\t\treturn new PlotCommit(id);\n+\tprotected RevCommit createCommit(final AnyObjectId id, final Ref[] tags) {\n+\t\treturn new PlotCommit(id, tags);\n+\t}\n+\n+\t@Override\n+\tprotected Ref[] getTags(final AnyObjectId commitId) {\n+\t\tList<Ref> list = reverseRefMap.get(commitId);\n+\t\tRef[] tags;\n+\t\tif (list == null)\n+\t\t\ttags = null;\n+\t\telse {\n+\t\t\tif (list != null && list.size() > 1) {\n+\t\t\t\tCollections.sort(list, new Comparator<Ref>() {\n+\t\t\t\t\tpublic int compare(Ref o1, Ref o2) {\n+\t\t\t\t\t\ttry {\n+\t\t\t\t\t\t\tObject obj1 = getRepository().mapObject(o1.getObjectId(), o1.getName());\n+\t\t\t\t\t\t\tObject obj2 = getRepository().mapObject(o2.getObjectId(), o2.getName());\n+\t\t\t\t\t\t\tlong t1 = timeof(obj1);\n+\t\t\t\t\t\t\tlong t2 = timeof(obj2);\n+\t\t\t\t\t\t\tif (t1 > t2)\n+\t\t\t\t\t\t\t\treturn -1;\n+\t\t\t\t\t\t\tif (t1 < t2)\n+\t\t\t\t\t\t\t\treturn 1;\n+\t\t\t\t\t\t\treturn 0;\n+\t\t\t\t\t\t} catch (IOException e) {\n+\t\t\t\t\t\t\t// ignore\n+\t\t\t\t\t\t\treturn 0;\n+\t\t\t\t\t\t}\n+\t\t\t\t\t}\n+\t\t\t\t\tlong timeof(Object o) {\n+\t\t\t\t\t\tif (o instanceof Commit)\n+\t\t\t\t\t\t\treturn ((Commit)o).getCommitter().getWhen().getTime();\n+\t\t\t\t\t\tif (o instanceof Tag)\n+\t\t\t\t\t\t\treturn ((Tag)o).getTagger().getWhen().getTime();\n+\t\t\t\t\t\treturn 0;\n+\t\t\t\t\t}\n+\t\t\t\t});\n+\t\t\t}\n+\t\t\ttags = list.toArray(new Ref[list.size()]);\n+\t\t}\n+\t\treturn tags;\n \t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\nindex d7e4c58..41d57c6 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n@@ -53,6 +53,7 @@\n import org.spearce.jgit.lib.ObjectId;\n import org.spearce.jgit.lib.ObjectIdSubclassMap;\n import org.spearce.jgit.lib.ObjectLoader;\n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.lib.Repository;\n import org.spearce.jgit.lib.WindowCursor;\n import org.spearce.jgit.revwalk.filter.RevFilter;\n@@ -541,7 +542,7 @@ public RevTree lookupTree(final AnyObjectId id) {\n \tpublic RevCommit lookupCommit(final AnyObjectId id) {\n \t\tRevCommit c = (RevCommit) objects.get(id);\n \t\tif (c == null) {\n-\t\t\tc = createCommit(id);\n+\t\t\tc = createCommit(id, getTags(id));\n \t\t\tobjects.add(c);\n \t\t}\n \t\treturn c;\n@@ -564,7 +565,7 @@ public RevObject lookupAny(final AnyObjectId id, final int type) {\n \t\tif (r == null) {\n \t\t\tswitch (type) {\n \t\t\tcase Constants.OBJ_COMMIT:\n-\t\t\t\tr = createCommit(id);\n+\t\t\t\tr = createCommit(id, getTags(id));\n \t\t\t\tbreak;\n \t\t\tcase Constants.OBJ_TREE:\n \t\t\t\tr = new RevTree(id);\n@@ -687,7 +688,7 @@ public RevObject parseAny(final AnyObjectId id)\n \t\t\tfinal int type = ldr.getType();\n \t\t\tswitch (type) {\n \t\t\tcase Constants.OBJ_COMMIT: {\n-\t\t\t\tfinal RevCommit c = createCommit(ldr.getId());\n+\t\t\t\tfinal RevCommit c = createCommit(ldr.getId(), getTags(ldr.getId()));\n \t\t\t\tc.parseCanonical(this, data);\n \t\t\t\tr = c;\n \t\t\t\tbreak;\n@@ -718,6 +719,14 @@ public RevObject parseAny(final AnyObjectId id)\n \t}\n \n \t/**\n+\t * @param commitId\n+\t * @return the list of refs associated with a commit, possibly filtered\n+\t */\n+\tprotected Ref[] getTags(final AnyObjectId commitId) {\n+\t\treturn null; // Don't get tags in the basic case\n+\t}\n+\n+\t/**\n \t * Ensure the object's content has been parsed.\n \t * <p>\n \t * This method only returns successfully if the object exists and was parsed\n@@ -1008,9 +1017,11 @@ private boolean isNotStarted() {\n \t * \n \t * @param id\n \t *            the object this walker requires a commit reference for.\n+\t * @param tags\n+\t *            tags attached to the commit\n \t * @return a new unparsed reference for the object.\n \t */\n-\tprotected RevCommit createCommit(final AnyObjectId id) {\n+\tprotected RevCommit createCommit(final AnyObjectId id, final Ref[] tags) {\n \t\treturn new RevCommit(id);\n \t}\n \n-- \n1.6.0.1.310.gf789d0.dirty\n"},{"id":"92374","messageId":"1223249802-9959-6-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"1223249802-9959-5-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 5/6] Add decorate option to log program","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:41Z","receivedAt":"2008-10-05T23:36:41Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../src/org/spearce/jgit/pgm/Log.java              |   32 ++++++++++++++++++++\n 1 files changed, 32 insertions(+), 0 deletions(-)\n\ndiff --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\nindex e16387b..378d9e0 100644\n--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Log.java\n+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Log.java\n@@ -40,10 +40,17 @@\n \n import java.text.DateFormat;\n import java.text.SimpleDateFormat;\n+import java.util.Iterator;\n+import java.util.List;\n+import java.util.Map;\n import java.util.TimeZone;\n \n+import org.kohsuke.args4j.Option;\n+import org.spearce.jgit.lib.AnyObjectId;\n import org.spearce.jgit.lib.PersonIdent;\n+import org.spearce.jgit.lib.Ref;\n import org.spearce.jgit.revwalk.RevCommit;\n+import org.spearce.jgit.revwalk.RevWalk;\n \n @Command(common = true, usage = \"View commit history\")\n class Log extends RevWalkTextBuiltin {\n@@ -51,14 +58,39 @@\n \n \tprivate final DateFormat fmt;\n \n+\tprivate Map<AnyObjectId, List<Ref>> allRefsByPeeledObjectId;\n+\n+\t@Option(name=\"--decorate\", usage=\"Show ref names matching commits\")\n+\tprivate boolean decorate;\n+\n \tLog() {\n \t\tfmt = new SimpleDateFormat(\"EEE MMM dd HH:mm:ss yyyy ZZZZZ\");\n \t}\n \n \t@Override\n+\tprotected org.spearce.jgit.revwalk.RevWalk createWalk() {\n+\t\tRevWalk ret = super.createWalk();\n+\t\tif (decorate)\n+\t\t\tallRefsByPeeledObjectId = getRepository().getAllRefsByPeeledObjectId();\n+\t\treturn ret;\n+\t}\n+\n+\t@Override\n \tprotected void show(final RevCommit c) throws Exception {\n \t\tout.print(\"commit \");\n \t\tc.getId().copyTo(outbuffer, out);\n+\t\tif (decorate) {\n+\t\t\tList<Ref> list = allRefsByPeeledObjectId.get(c.copy());\n+\t\t\tif (list != null) {\n+\t\t\t\tout.print(\" (\");\n+\t\t\t\tfor (Iterator<Ref> i = list.iterator(); i.hasNext(); ) {\n+\t\t\t\t\tout.print(i.next().getOrigName());\n+\t\t\t\t\tif (i.hasNext())\n+\t\t\t\t\t\tout.print(\" \");\n+\t\t\t\t}\n+\t\t\t\tout.print(\")\");\n+\t\t\t}\n+\t\t}\n \t\tout.println();\n \n \t\tfinal PersonIdent author = c.getAuthorIdent();\n-- \n1.6.0.1.310.gf789d0.dirty\n"},{"id":"92376","messageId":"1223249802-9959-7-git-send-email-robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"1223249802-9959-6-git-send-email-robin.rosenberg@dewire.com","subject":"[EGIT PATCH 6/6] Comment the getId method and hint for copy to actually get an ObjectId","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-05T23:36:42Z","receivedAt":"2008-10-05T23:36:42Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../src/org/spearce/jgit/revwalk/RevObject.java    |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\nindex 451205c..2209c04 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java\n@@ -67,6 +67,7 @@ abstract void parse(RevWalk walk) throws MissingObjectException,\n \n \t/**\n \t * Get the name of this object.\n+\t * See {@link #copy()} to really get a copy\n \t * \n \t * @return unique hash of this object.\n \t */\n-- \n1.6.0.1.310.gf789d0.dirty\n"},{"id":"92395","messageId":"20081006074303.GB27516@spearce.org","threadId":"15795","inReplyTo":"1223249802-9959-3-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 2/6] Peel annotated tags when getting all refs","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-06T07:43:03Z","receivedAt":"2008-10-06T07:43:03Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"FWIW I'm still going through the series.  But this jumped out at me.\n\nRobin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> For packed refs we got this automatically from packed-refs,\n> but for loose tags we have to follow the tags and get the leaf\n> object in order to comply with the documentation.\n...\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n> index 5a1b85f..1ee70d9 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n> @@ -271,7 +271,16 @@ private void readOneLooseRef(final Map<String, Ref> avail,\n>  \t\t\t\t\treturn;\n>  \t\t\t\t}\n>  \n> -\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id);\n> +\t\t\t\tObject tt = db.mapObject(id, refName);\n> +\t\t\t\tif (tt != null && tt instanceof Tag) {\n> +\t\t\t\t\tTag t = (Tag)tt;\n\nOwwww.\n\nI don't want to be doing peeling of loose annotated tags anytime\nwe loop through the loose objects.  That is just painful and very\nexpensive.  I'd rather the peeling be caused on demand by callers\nwho need it, but if its done through a method on Repository then\nwe can push the peeled ObjectId back into the RefDatabase cache.\n\nI'm thinking more like:\n\n\tRepository db = ...;\n\t...\n\tObjectId p = ref.getPeeledObjectId()\n\tif (p == null)\n\t\tp = db.peel(ref)\n\n-- \nShawn.\n"},{"id":"92396","messageId":"20081006080834.GC27516@spearce.org","threadId":"15795","inReplyTo":"1223249802-9959-5-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 4/6] Add tags to the graphical history display.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-06T08:08:34Z","receivedAt":"2008-10-06T08:08:34Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"FYI, my comments don't fully cover this patch yet.\n\nRobin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java\n> index 5a5ef1e..fac89f5 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/revplot/PlotCommit.java\n> @@ -58,14 +59,19 @@\n>  \n>  \tPlotCommit[] children;\n>  \n> +\tRef[] refs;\n> +\n>  \t/**\n>  \t * Create a new commit.\n>  \t * \n>  \t * @param id\n>  \t *            the identity of this commit.\n> +\t * @param tags\n> +\t *            the tags associated with this commit, null for no tags\n>  \t */\n> -\tprotected PlotCommit(final AnyObjectId id) {\n> +\tprotected PlotCommit(final AnyObjectId id, final Ref[] tags) {\n>  \t\tsuper(id);\n> +\t\tthis.refs = tags;\n\nthis.refs isn't final.  There is no reason to be adding it to the\nconstructor and having this ripple effect all the way up into the\nRevWalk class.\n\nMaybe PlotWalk should override next() and only set PlotCommit.refs on\nthings returned from super.next()?  This way we only do tag lookup\non commits that the filtering rules have said should be shown in\nto the application, but the refs should still be inserted prior to\nthe application seeing the RevCommit.\n\n> @@ -54,6 +66,7 @@\n>  \tpublic PlotWalk(final Repository repo) {\n>  \t\tsuper(repo);\n>  \t\tsuper.sort(RevSort.TOPO, true);\n> +\t\treverseRefMap = repo.getAllRefsByPeeledObjectId();\n\nI wonder if this shouldn't be done as part of the StartGenerator\n(or something like it but specialized for PlotWalk).  I only say\nthat because a reused PlotWalk may want to make sure its ref map\nis current with the repository its about to walk against.\n\n> +\t@Override\n> +\tprotected Ref[] getTags(final AnyObjectId commitId) {\n> +\t\tList<Ref> list = reverseRefMap.get(commitId);\n> +\t\tRef[] tags;\n> +\t\tif (list == null)\n> +\t\t\ttags = null;\n> +\t\telse {\n> +\t\t\tif (list != null && list.size() > 1) {\n> +\t\t\t\tCollections.sort(list, new Comparator<Ref>() {\n> +\t\t\t\t\tpublic int compare(Ref o1, Ref o2) {\n> +\t\t\t\t\t\ttry {\n> +\t\t\t\t\t\t\tObject obj1 = getRepository().mapObject(o1.getObjectId(), o1.getName());\n> +\t\t\t\t\t\t\tObject obj2 = getRepository().mapObject(o2.getObjectId(), o2.getName());\n> +\t\t\t\t\t\t\tlong t1 = timeof(obj1);\n> +\t\t\t\t\t\t\tlong t2 = timeof(obj2);\n> +\t\t\t\t\t\t\tif (t1 > t2)\n> +\t\t\t\t\t\t\t\treturn -1;\n> +\t\t\t\t\t\t\tif (t1 < t2)\n> +\t\t\t\t\t\t\t\treturn 1;\n> +\t\t\t\t\t\t\treturn 0;\n> +\t\t\t\t\t\t} catch (IOException e) {\n> +\t\t\t\t\t\t\t// ignore\n> +\t\t\t\t\t\t\treturn 0;\n> +\t\t\t\t\t\t}\n> +\t\t\t\t\t}\n> +\t\t\t\t\tlong timeof(Object o) {\n> +\t\t\t\t\t\tif (o instanceof Commit)\n> +\t\t\t\t\t\t\treturn ((Commit)o).getCommitter().getWhen().getTime();\n> +\t\t\t\t\t\tif (o instanceof Tag)\n> +\t\t\t\t\t\t\treturn ((Tag)o).getTagger().getWhen().getTime();\n> +\t\t\t\t\t\treturn 0;\n\nYou are inside of a PlotWalk, which extends RevWalk, which has very\naggressive caching of RevCommit and RevTag data instances, and very\nfast parsers.  Much faster than the legacy Commit and Tag classes.\n\nI think we should use the RevCommit and RevTag classes to handle\nsorting here.  RevCommit already has the committer time cached\nand stored in an int.  RevCommit probably should learn to do the\nsame for its \"tagger\" field.  The tiny extra bloat (1 word) that\nadds to a RevTag instance is worth it when we consider implementing\nsomething like this and/or git-describe where sorting tags by their\ndates is useful.\n\n\n> +\t\t\ttags = list.toArray(new Ref[list.size()]);\n\nI wonder if using a Ref[] here even makes sense given that the data\nis stored in a List<Ref>.  I use RevCommit[] inside of RevCommit\ngenerally because the number of entries in the array is 1 or 2 and\nthe array is smaller than say an ArrayList.\n\nIn hindsight those RevCommit[] probably should be a List<RevCommit>\nwith different list implementations based on the number of parents\nneeded:\n\n\t0 parents:  Collections.emptyList()\n\t1 parent:   Collections.singletonList()\n\t2 parents:  some especialized AbstractList subclass\n\t3 parents:  ArrayList\n\nI think it would actually use less memory per instance, which is\na huge bonus, but we'd pay a downcasting penalty on each access.\nHotSpot is supposed to be really good at removing the downcast\npenalty from say java.util.List, but I don't if it can beat a typed\narray access.\n\nSorry I got off on a bit of a tangent here.  I'm just trying to\npoint out that the primary reason I've usd an array before is\nprobably moot here since the data is already in a List.\n\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n> index d7e4c58..41d57c6 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n> @@ -53,6 +53,7 @@\n\nIMHO this class shouldn't need to be modified.\n\n> @@ -541,7 +542,7 @@ public RevTree lookupTree(final AnyObjectId id) {\n>  \tpublic RevCommit lookupCommit(final AnyObjectId id) {\n>  \t\tRevCommit c = (RevCommit) objects.get(id);\n>  \t\tif (c == null) {\n> -\t\t\tc = createCommit(id);\n> +\t\t\tc = createCommit(id, getTags(id));\n\nThis code is performance critical to commit parsing.  Trying to\nlookup tags for every commit we evaluate, especially ones that we\nwill never show in the UI (because they are uninteresting) but that\nwe still need to parse in order to derive the merge base is just\na huge waste of time.\n\nSame applies for the lookupAny and parseAny methods.\n\n-- \nShawn.\n"},{"id":"92397","messageId":"20081006081554.GD27516@spearce.org","threadId":"15795","inReplyTo":"1223249802-9959-4-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 3/6] Add a method to get refs by object Id","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-06T08:15:54Z","receivedAt":"2008-10-06T08:15:54Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> index dfce1b8..3fc5236 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> @@ -939,6 +940,33 @@ public String getBranch() throws IOException {\n>  \t}\n>  \n>  \t/**\n> +\t * @return a map with all objects referenced by a peeled ref.\n> +\t */\n> +\tpublic Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {\n\nDo we really want to promise List here?  Can we make it just\nCollection instead?\n\n> +\t\tMap<String, Ref> allRefs = getAllRefs();\n> +\t\tHashMap<AnyObjectId, List<Ref>> ret = new HashMap<AnyObjectId, List<Ref>>(allRefs.size());\n> +\t\tfor (Map.Entry<String,Ref> e : allRefs.entrySet()) {\n> +\t\t\tRef ref = e.getValue();\n\nI think this is cleaner:\n\n\tfor (Ref ref : allRefs.values()) {\n\nas you never use the key.\n\n> +\t\t\tAnyObjectId target = ref.getPeeledObjectId();\n> +\t\t\tif (target == null)\n> +\t\t\t\ttarget = ref.getObjectId();\n> +\t\t\tList<Ref> list = ret.get(target);\n> +\t\t\tif (list == null) {\n> +\t\t\t\tlist = Collections.singletonList(ref);\n> +\t\t\t} else {\n> +\t\t\t\tif (list.size() == 1) {\n> +\t\t\t\t\tArrayList<Ref> list2 = new ArrayList<Ref>(2);\n> +\t\t\t\t\tlist2.add(list.get(0));\n> +\t\t\t\t\tlist = list2;\n> +\t\t\t\t}\n> +\t\t\t\tlist.add(ref);\n> +\t\t\t}\n> +\t\t\tret.put(target, list);\n\nHmm.  Putting the list every time is pointless.  This is may run\nfaster because we (on average) only do one hash lookup per target,\nnot 2:\n\n\tList<Ref> nl = Collections.singletonList(ref);\n\tList<Ref> ol = ret.put(target, nl);\n\tif (ol != null) {\n\t\tif (ol.size() == 1) {\n\t\t\tnl = new ArrayList<Ref>(2);\n\t\t\tnl.add(ol.get(0));\n\t\t\tnl.add(ref);\n\t\t\tret.put(target, nl);\n\t\t} else {\n\t\t\tol.add(ref)\n\t\t\tret.put(target, ol);\n\t\t}\n\t}\n\nThe trick is that most targets have exactly one Ref, so the first\nret.put will return null and we'll never worry about the expansion\ncases.  In the rare cases where more than one Ref points at the\nsame object we'll degrade into doing two hash lookups per target,\nwhich is no worse than what you have right now.\n\n-- \nShawn.\n"},{"id":"92456","messageId":"200810062358.44954.robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"20081006080834.GC27516@spearce.org","subject":"Re: [EGIT PATCH 4/6] Add tags to the graphical history display.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-06T21:58:44Z","receivedAt":"2008-10-06T21:58:44Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 6 oktober 2008 10.08.34 skrev Shawn O. Pearce:\n> FYI, my comments don't fully cover this patch yet.\n> \n> > -\tprotected PlotCommit(final AnyObjectId id) {\n> > +\tprotected PlotCommit(final AnyObjectId id, final Ref[] tags) {\n> >  \t\tsuper(id);\n> > +\t\tthis.refs = tags;\n> \n> this.refs isn't final.  There is no reason to be adding it to the\n> constructor and having this ripple effect all the way up into the\n> RevWalk class.\nEh, it doesn't, ecept I added a methos to RevWalk than can return tags,\nwhich it doesn't unless the information is attached.\n\n> Maybe PlotWalk should override next() and only set PlotCommit.refs on\n> things returned from super.next()?  This way we only do tag lookup\n> on commits that the filtering rules have said should be shown in\n> to the application, but the refs should still be inserted prior to\n> the application seeing the RevCommit.\n\nI missed something here I think. Thanks. Maybe I thought... doesn't matter.\nI'll rewrite the changes in and related to this completely.\n\n> > @@ -54,6 +66,7 @@\n> >  \tpublic PlotWalk(final Repository repo) {\n> >  \t\tsuper(repo);\n> >  \t\tsuper.sort(RevSort.TOPO, true);\n> > +\t\treverseRefMap = repo.getAllRefsByPeeledObjectId();\n> \n> I wonder if this shouldn't be done as part of the StartGenerator\n> (or something like it but specialized for PlotWalk).  I only say\n> that because a reused PlotWalk may want to make sure its ref map\n> is current with the repository its about to walk against.\nnoted.\n\n> You are inside of a PlotWalk, which extends RevWalk, which has very\n> aggressive caching of RevCommit and RevTag data instances, and very\n> fast parsers.  Much faster than the legacy Commit and Tag classes.\nMaybe we should rid us of them completely and make new commit from\nthese classes of possible more lightweight ones. The old classes were\nstraightforward but are on overtime now.\n\n> I think we should use the RevCommit and RevTag classes to handle\n> sorting here.  RevCommit already has the committer time cached\n> and stored in an int.  RevCommit probably should learn to do the\n> same for its \"tagger\" field.  The tiny extra bloat (1 word) that\n> adds to a RevTag instance is worth it when we consider implementing\n> something like this and/or git-describe where sorting tags by their\n> dates is useful.\n\nAs you noted earlier we usually have at most one ref per commit to sort.\nNot much to optimize for speed here with current repos. For a one-element \nlist the compare routine wil not even be invoked.\n\n> \n> > +\t\t\ttags = list.toArray(new Ref[list.size()]);\n> \n> I wonder if using a Ref[] here even makes sense given that the data\n> is stored in a List<Ref>.  I use RevCommit[] inside of RevCommit\n> generally because the number of entries in the array is 1 or 2 and\n> the array is smaller than say an ArrayList.\n> \n> In hindsight those RevCommit[] probably should be a List<RevCommit>\n> with different list implementations based on the number of parents\n> needed:\n> \n> \t0 parents:  Collections.emptyList()\n> \t1 parent:   Collections.singletonList()\n> \t2 parents:  some especialized AbstractList subclass\n> \t3 parents:  ArrayList\n> \n> I think it would actually use less memory per instance, which is\n> a huge bonus, but we'd pay a downcasting penalty on each access.\n> HotSpot is supposed to be really good at removing the downcast\n> penalty from say java.util.List, but I don't if it can beat a typed\n> array access.\nI guess measuring is the right way. For these small arrays, my bet\nis that List is way faster because we do not need any bounds checking.\nHotspot is reasonably good at optimizing those away, but that won't\nhelp for much small arrays.\n\n> Sorry I got off on a bit of a tangent here.  I'm just trying to\n> point out that the primary reason I've usd an array before is\n> probably moot here since the data is already in a List.\nCould it be that arrays are often better then lists.\n\n> > diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n> > index d7e4c58..41d57c6 100644\n> > --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n> > +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n> > @@ -53,6 +53,7 @@\n> \n> IMHO this class shouldn't need to be modified.\n> \n> > @@ -541,7 +542,7 @@ public RevTree lookupTree(final AnyObjectId id) {\n> >  \tpublic RevCommit lookupCommit(final AnyObjectId id) {\n> >  \t\tRevCommit c = (RevCommit) objects.get(id);\n> >  \t\tif (c == null) {\n> > -\t\t\tc = createCommit(id);\n> > +\t\t\tc = createCommit(id, getTags(id));\n> \n> This code is performance critical to commit parsing.  Trying to\n> lookup tags for every commit we evaluate, especially ones that we\n> will never show in the UI (because they are uninteresting) but that\n> we still need to parse in order to derive the merge base is just\n> a huge waste of time.\n> \n> Same applies for the lookupAny and parseAny methods.\n> \nAck.\n\n-- robin\n"},{"id":"92458","messageId":"20081006221421.GA13687@spearce.org","threadId":"15795","inReplyTo":"200810062358.44954.robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 4/6] Add tags to the graphical history display.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-06T22:14:21Z","receivedAt":"2008-10-06T22:14:21Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> måndagen den 6 oktober 2008 10.08.34 skrev Shawn O. Pearce:\n> > Maybe PlotWalk should override next() [...]\n> \n> I missed something here I think. Thanks. Maybe I thought... doesn't matter.\n> I'll rewrite the changes in and related to this completely.\n\nOK, looking forward to the changes.\n \n> > > @@ -54,6 +66,7 @@\n> > >  \tpublic PlotWalk(final Repository repo) {\n> > >  \t\tsuper(repo);\n> > >  \t\tsuper.sort(RevSort.TOPO, true);\n> > > +\t\treverseRefMap = repo.getAllRefsByPeeledObjectId();\n> > \n> > I wonder if this shouldn't be done as part of the StartGenerator\n> > (or something like it but specialized for PlotWalk).  I only say\n> > that because a reused PlotWalk may want to make sure its ref map\n> > is current with the repository its about to walk against.\n>\n> noted.\n\nBe careful.  StartGenerator is package access only and I think\nPlotWalk is in a different package.\n\nIMHO binding into the StartGenerator (by subclassing it and replacing\nit) feels like the right thing to me, but it means making a bunch\nof changes to RevWalk and StartGenerator to make that type public\nand allow PlotWalk to inject a different subclass of StartGenerator\nfor itself.\n\nThe StartGenerator trick is very useful though; it allows the RevWalk\nto perform init activity only on the first invocation of next()\nand then removes the init code entirely from the call path.  It is a\nvirtual dispatch, but I figured its a virtual dispatch anyway due to\nthe way the other generators are chained together based the filters\nso we at least avoid a \"if (first) {...}\" test on every call to next.\n \n> > You are inside of a PlotWalk, which extends RevWalk, which has very\n> > aggressive caching of RevCommit and RevTag data instances, and very\n> > fast parsers.  Much faster than the legacy Commit and Tag classes.\n>\n> Maybe we should rid us of them completely and make new commit from\n> these classes of possible more lightweight ones. The old classes were\n> straightforward but are on overtime now.\n\nI'm trying to go in that direction.  However there are three reasons\nwhy we aren't ready yet:\n\n- Commits can only be made by the old-style Commit class.\n- Tags can only be made by the old-style Tag class.\n- Trees can only be made by the old-style Tree class.\n\nI have some experimental code in my merge branch to solve the last\none by teaching DirCache how to write out the index and update the\ncache trees with the ObjectIds of the newly created trees, but I\nhave not had a chance to clean the patches up with test cases and\ndocs to submit to the git ML yet.\n\nSo I'm actively working on fixing the last item.\n\nOh, and of course existing callers have to be converted.  But I am\ntrying to avoid introducing new callers.\n \n> As you noted earlier we usually have at most one ref per commit to sort.\n> Not much to optimize for speed here with current repos. For a one-element \n> list the compare routine wil not even be invoked.\n\nOh, good point.\n \n> > In hindsight those RevCommit[] probably should be a List<RevCommit>\n> > with different list implementations based on the number of parents\n> > needed:\n> > \n> > \t0 parents:  Collections.emptyList()\n> > \t1 parent:   Collections.singletonList()\n> > \t2 parents:  some especialized AbstractList subclass\n> > \t3 parents:  ArrayList\n>\n> I guess measuring is the right way. For these small arrays, my bet\n> is that List is way faster because we do not need any bounds checking.\n> Hotspot is reasonably good at optimizing those away, but that won't\n> help for much small arrays.\n\nYup.  Its something to explore.  But I lack the time right now so\nI'm going going to be making any changes and measuring it anytime\nsoon.  ;-)\n\nMaybe someone who wants to get involved can look at this.  Or any\nother number of issues we still have open.\n\nBut without measurements to back up the code either way I won't be\nmaking changes to what we have right now.\n \n-- \nShawn.\n"},{"id":"92461","messageId":"200810070037.53841.robin.rosenberg@dewire.com","threadId":"15795","inReplyTo":"20081006081554.GD27516@spearce.org","subject":"Re: [EGIT PATCH 3/6] Add a method to get refs by object Id","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-06T22:37:53Z","receivedAt":"2008-10-06T22:37:53Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:\n> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> > diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> > index dfce1b8..3fc5236 100644\n> > --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> > +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> > @@ -939,6 +940,33 @@ public String getBranch() throws IOException {\n> >  \t}\n> >  \n> >  \t/**\n> > +\t * @return a map with all objects referenced by a peeled ref.\n> > +\t */\n> > +\tpublic Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {\n> \n> Do we really want to promise List here?  Can we make it just\n> Collection instead?\nSure. Our promise is actually slightly better, it is Set, but Java doesn't have a suitable class for that.\n\n> \n> > +\t\tMap<String, Ref> allRefs = getAllRefs();\n> > +\t\tHashMap<AnyObjectId, List<Ref>> ret = new HashMap<AnyObjectId, List<Ref>>(allRefs.size());\n> > +\t\tfor (Map.Entry<String,Ref> e : allRefs.entrySet()) {\n> > +\t\t\tRef ref = e.getValue();\n> \n> I think this is cleaner:\n> \n> \tfor (Ref ref : allRefs.values()) {\n> \n> as you never use the key.\nYes. I was thinking it might be less efficient, but the JDK implementation looks quite well optimized in 1.6 at least\nso values() is slightly faster.\n\n> > +\t\t\tAnyObjectId target = ref.getPeeledObjectId();\n> > +\t\t\tif (target == null)\n> > +\t\t\t\ttarget = ref.getObjectId();\n> > +\t\t\tList<Ref> list = ret.get(target);\n> > +\t\t\tif (list == null) {\n> > +\t\t\t\tlist = Collections.singletonList(ref);\n> > +\t\t\t} else {\n> > +\t\t\t\tif (list.size() == 1) {\n> > +\t\t\t\t\tArrayList<Ref> list2 = new ArrayList<Ref>(2);\n> > +\t\t\t\t\tlist2.add(list.get(0));\n> > +\t\t\t\t\tlist = list2;\n> > +\t\t\t\t}\n> > +\t\t\t\tlist.add(ref);\n> > +\t\t\t}\n> > +\t\t\tret.put(target, list);\n> \n> Hmm.  Putting the list every time is pointless.  This is may run\n> faster because we (on average) only do one hash lookup per target,\n> not 2:\n> \n> \tList<Ref> nl = Collections.singletonList(ref);\n> \tList<Ref> ol = ret.put(target, nl);\n> \tif (ol != null) {\n> \t\tif (ol.size() == 1) {\n> \t\t\tnl = new ArrayList<Ref>(2);\n> \t\t\tnl.add(ol.get(0));\n> \t\t\tnl.add(ref);\n> \t\t\tret.put(target, nl);\n> \t\t} else {\n> \t\t\tol.add(ref)\n> \t\t\tret.put(target, ol);\n> \t\t}\n> \t}\n\nok, I guess one just has has to include the comment on why for the casual reader. \n\n-- robin\n"},{"id":"92462","messageId":"20081006224348.GB13687@spearce.org","threadId":"15795","inReplyTo":"200810070037.53841.robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH 3/6] Add a method to get refs by object Id","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-06T22:43:48Z","receivedAt":"2008-10-06T22:43:48Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:\n> > Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> > >  \n> > >  \t/**\n> > > +\t * @return a map with all objects referenced by a peeled ref.\n> > > +\t */\n> > > +\tpublic Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {\n> > \n> > Do we really want to promise List here?  Can we make it just\n> > Collection instead?\n>\n> Sure. Our promise is actually slightly better, it is Set, but Java doesn't have a suitable class for that.\n\njava.util.Set?  java.util.HashSet?\n\nWhat am I missing?\n \n> > > +\t\tfor (Map.Entry<String,Ref> e : allRefs.entrySet()) {\n> > > +\t\t\tRef ref = e.getValue();\n> > \n> > I think this is cleaner:\n> > \n> > \tfor (Ref ref : allRefs.values()) {\n> > \n> > as you never use the key.\n>\n> Yes. I was thinking it might be less efficient, but the JDK implementation looks quite well optimized in 1.6 at least\n> so values() is slightly faster.\n\nI wasn't concerned about performance, I was going for readability.\nThis loop is probably not performance critical as its done once\nearly when the PlotWalk starts.  I just found that grabbing the\nentrySet when all you care about is the values was odd.\n \n> > \tList<Ref> nl = Collections.singletonList(ref);\n> > \tList<Ref> ol = ret.put(target, nl);\n> > \tif (ol != null) {\n> > \t\tif (ol.size() == 1) {\n> > \t\t\tnl = new ArrayList<Ref>(2);\n> > \t\t\tnl.add(ol.get(0));\n> > \t\t\tnl.add(ref);\n> > \t\t\tret.put(target, nl);\n> > \t\t} else {\n> > \t\t\tol.add(ref)\n> > \t\t\tret.put(target, ol);\n> > \t\t}\n> > \t}\n> \n> ok, I guess one just has has to include the comment on why for the casual reader. \n\nI'm probably too used to this pattern of \"put, then test\" because\nI use it a lot when the odds of put returning null are very high.\nIts an odd idiom.  I think most developers wouldn't write it.\nSo I guess a comment of some sort is probably a good idea if you\nchose to use this mess.  ;-)\n\n-- \nShawn.\n"}]}