git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [JGIT PATCH 00/12] Extensions in core needed by PackWriter
@ 2008-06-02 21:24 Marek Zawirski
  2008-06-02 21:24 ` [JGIT PATCH 01/12] Format PackFile class Marek Zawirski
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Hello,

Here is my first GSoC series - some work from the last week.
It's actually not a PackWriter, but some changes in existing jgit core
related to PackWriting. Some of these added methods/refactors are not yet
used within this series, but are used in my dirty branch in PackWriter
which is under-development, even somewhat usable.

Series start with formatting stuff, as some old files were not
appropriatelly formatted.

This series is also available at my corechanges branch:
http://repo.or.cz/w/egit/zawir.git?a=shortlog;h=refs/heads/corechanges
It's based on Shawn's bsd branch, with new BSD-style license, but I can
rebase if really needed.

If you want to track some PackWriter (itself) development you may want to
have a look at my dirty branch:
http://repo.or.cz/w/egit/zawir.git?a=shortlog;h=refs/heads/dirty

That's all. Although Shawn already reviewed some old version of this patches,
I'm still interested in your comments.

Marek Zawirski (12):
  Format PackFile class
  Format PackIndex class
  Format PackIndexV1 class
  Add getType() method to RevObject hierarchy
  Replace instanceof in WalkFetchConnection with getType()
  Move PackFile.SIGNATURE to Constants.PACK_SIGNATURE
  Add overload of fromRaw() in MutableObjectId accepting int[]
  Copying constructor of MutableObjectId
  Add getSize() method to ObjectIdSubclassMap
  Add getObjectCount() method to PackFile
  Entries iterator in PackIndex and indirectly PackFile
  Add PackIndex specific tests, currently only iterators tests

 .../tst/org/spearce/jgit/lib/PackIndexTest.java    |  152 ++++++++++++++++++++
 .../tst/org/spearce/jgit/lib/PackIndexV1Test.java  |   54 +++++++
 .../tst/org/spearce/jgit/lib/PackIndexV2Test.java  |   54 +++++++
 ...-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2 |  Bin 0 -> 1296 bytes
 ...-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2 |  Bin 0 -> 2976 bytes
 .../src/org/spearce/jgit/lib/Constants.java        |    8 +
 .../src/org/spearce/jgit/lib/MutableObjectId.java  |   50 +++++++
 .../org/spearce/jgit/lib/ObjectIdSubclassMap.java  |    9 ++
 .../src/org/spearce/jgit/lib/PackFile.java         |   55 +++++--
 .../src/org/spearce/jgit/lib/PackIndex.java        |   86 +++++++++++-
 .../src/org/spearce/jgit/lib/PackIndexV1.java      |   45 +++++-
 .../src/org/spearce/jgit/lib/PackIndexV2.java      |   36 +++++
 .../src/org/spearce/jgit/revwalk/RevBlob.java      |    6 +
 .../src/org/spearce/jgit/revwalk/RevCommit.java    |    5 +
 .../src/org/spearce/jgit/revwalk/RevObject.java    |    8 +
 .../src/org/spearce/jgit/revwalk/RevTag.java       |    5 +
 .../src/org/spearce/jgit/revwalk/RevTree.java      |    6 +
 .../src/org/spearce/jgit/transport/IndexPack.java  |    8 +-
 .../jgit/transport/WalkFetchConnection.java        |   36 +++--
 19 files changed, 576 insertions(+), 47 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV1Test.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV2Test.java
 create mode 100644 org.spearce.jgit.test/tst/pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2
 create mode 100644 org.spearce.jgit.test/tst/pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [JGIT PATCH 01/12] Format PackFile class
  2008-06-02 21:24 [JGIT PATCH 00/12] Extensions in core needed by PackWriter Marek Zawirski
@ 2008-06-02 21:24 ` Marek Zawirski
  2008-06-02 21:24   ` [JGIT PATCH 02/12] Format PackIndex class Marek Zawirski
  2008-06-02 22:15 ` [JGIT PATCH 00/12] Extensions in core needed by PackWriter Johannes Schindelin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/PackFile.java         |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index 23a175c..ccff47d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -45,9 +45,9 @@ import java.util.zip.DataFormatException;
 import org.spearce.jgit.util.NB;
 
 /**
- * A Git version 2 pack file representation. A pack file contains
- * Git objects in delta packed format yielding high compression of
- * lots of object where some objects are similar.
+ * A Git version 2 pack file representation. A pack file contains Git objects in
+ * delta packed format yielding high compression of lots of object where some
+ * objects are similar.
  */
 public class PackFile {
 	private static final byte[] SIGNATURE = { 'P', 'A', 'C', 'K' };
@@ -58,7 +58,7 @@ public class PackFile {
 
 	/**
 	 * Construct a reader for an existing, pre-indexed packfile.
-	 *
+	 * 
 	 * @param parentRepo
 	 *            Git repository holding this pack file
 	 * @param idxFile
@@ -188,13 +188,11 @@ public class PackFile {
 		if (idx.getObjectCount() != objectCnt)
 			throw new IOException("Pack index"
 					+ " object count mismatch; expected " + objectCnt
-					+ " found " + idx.getObjectCount() + ": "
-					+ pack.getName());
+					+ " found " + idx.getObjectCount() + ": " + pack.getName());
 	}
 
 	private PackedObjectLoader reader(final WindowCursor curs,
-			final long objOffset)
-			throws IOException {
+			final long objOffset) throws IOException {
 		long pos = objOffset;
 		int p = 0;
 		final byte[] ib = curs.tempId;
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 02/12] Format PackIndex class
  2008-06-02 21:24 ` [JGIT PATCH 01/12] Format PackFile class Marek Zawirski
@ 2008-06-02 21:24   ` Marek Zawirski
  2008-06-02 21:24     ` [JGIT PATCH 03/12] Format PackIndexV1 class Marek Zawirski
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/PackIndex.java        |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
index 5834ca6..104c361 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
@@ -61,7 +61,7 @@ public abstract class PackIndex {
 	 * implementation for that format will be constructed and returned to the
 	 * caller. The file may or may not be held open by the returned instance.
 	 * </p>
-	 *
+	 * 
 	 * @param idxFile
 	 *            existing pack .idx to read.
 	 * @return access implementation for the requested file.
@@ -118,7 +118,7 @@ public abstract class PackIndex {
 
 	/**
 	 * Obtain the total number of objects described by this index.
-	 *
+	 * 
 	 * @return number of objects in this index, and likewise in the associated
 	 *         pack that this index was generated from.
 	 */
@@ -126,7 +126,7 @@ public abstract class PackIndex {
 
 	/**
 	 * Locate the file offset position for the requested object.
-	 *
+	 * 
 	 * @param objId
 	 *            name of the object to locate within the pack.
 	 * @return offset of the object's header and compressed content; -1 if the
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 03/12] Format PackIndexV1 class
  2008-06-02 21:24   ` [JGIT PATCH 02/12] Format PackIndex class Marek Zawirski
@ 2008-06-02 21:24     ` Marek Zawirski
  2008-06-02 21:24       ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Marek Zawirski
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/PackIndexV1.java      |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java
index 84f13d2..cfd18da 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java
@@ -64,9 +64,9 @@ class PackIndexV1 extends PackIndex {
 		for (int k = 0; k < idxHeader.length; k++) {
 			int n;
 			if (k == 0) {
-				n = (int)(idxHeader[k]);
+				n = (int) (idxHeader[k]);
 			} else {
-				n = (int)(idxHeader[k]-idxHeader[k-1]);
+				n = (int) (idxHeader[k] - idxHeader[k - 1]);
 			}
 			if (n > 0) {
 				idxdata[k] = new byte[n * (Constants.OBJECT_ID_LENGTH + 4)];
@@ -94,11 +94,11 @@ class PackIndexV1 extends PackIndex {
 			if (cmp < 0)
 				high = mid;
 			else if (cmp == 0) {
-				int b0 = data[pos-4] & 0xff;
-				int b1 = data[pos-3] & 0xff;
-				int b2 = data[pos-2] & 0xff;
-				int b3 = data[pos-1] & 0xff;
-				return (((long)b0) << 24) | ( b1 << 16 ) | ( b2 << 8 ) | (b3);
+				int b0 = data[pos - 4] & 0xff;
+				int b1 = data[pos - 3] & 0xff;
+				int b2 = data[pos - 2] & 0xff;
+				int b3 = data[pos - 1] & 0xff;
+				return (((long) b0) << 24) | (b1 << 16) | (b2 << 8) | (b3);
 			} else
 				low = mid + 1;
 		} while (low < high);
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy
  2008-06-02 21:24     ` [JGIT PATCH 03/12] Format PackIndexV1 class Marek Zawirski
@ 2008-06-02 21:24       ` Marek Zawirski
  2008-06-02 21:24         ` [JGIT PATCH 05/12] Replace instanceof in WalkFetchConnection with getType() Marek Zawirski
  2008-06-06 13:24         ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Robin Rosenberg
  0 siblings, 2 replies; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

This let us avoid using instanceof.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/revwalk/RevBlob.java      |    6 ++++++
 .../src/org/spearce/jgit/revwalk/RevCommit.java    |    5 +++++
 .../src/org/spearce/jgit/revwalk/RevObject.java    |    8 ++++++++
 .../src/org/spearce/jgit/revwalk/RevTag.java       |    5 +++++
 .../src/org/spearce/jgit/revwalk/RevTree.java      |    6 ++++++
 5 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
index f6d34f4..66cdc02 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
@@ -38,6 +38,7 @@
 package org.spearce.jgit.revwalk;
 
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
 
 /** A binary file, or a symbolic link. */
 public class RevBlob extends RevObject {
@@ -55,4 +56,9 @@ public class RevBlob extends RevObject {
 	void parse(final RevWalk walk) {
 		flags |= PARSED;
 	}
+	
+	@Override
+	public int getType() {
+		return Constants.OBJ_BLOB;
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
index 0aa7098..77f1d1a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
@@ -135,6 +135,11 @@ public class RevCommit extends RevObject {
 		buffer = raw;
 		flags |= PARSED;
 	}
+	
+	@Override
+	public int getType() {
+		return Constants.OBJ_COMMIT;
+	}
 
 	static void carryFlags(RevCommit c, final int carry) {
 		for (;;) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
index 86c50b5..451205c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
@@ -42,6 +42,7 @@ import java.io.IOException;
 import org.spearce.jgit.errors.IncorrectObjectTypeException;
 import org.spearce.jgit.errors.MissingObjectException;
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
 
 /** Base object type accessed during revision walking. */
@@ -56,6 +57,13 @@ public abstract class RevObject extends ObjectId {
 
 	abstract void parse(RevWalk walk) throws MissingObjectException,
 			IncorrectObjectTypeException, IOException;
+	
+	/**
+	 * Get Git object type. See {@link Constants}.
+	 * 
+	 * @return object type
+	 */
+	public abstract int getType();
 
 	/**
 	 * Get the name of this object.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
index 668819c..bbb18ee 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
@@ -96,6 +96,11 @@ public class RevTag extends RevObject {
 		flags |= PARSED;
 	}
 
+	@Override
+	public int getType() {
+		return Constants.OBJ_TAG;
+	}
+	
 	/**
 	 * Parse this tag buffer for display.
 	 * 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
index 7ad9be0..e1cd4b5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
@@ -38,6 +38,7 @@
 package org.spearce.jgit.revwalk;
 
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
 
 /** A reference to a tree of subtrees/files. */
 public class RevTree extends RevObject {
@@ -55,4 +56,9 @@ public class RevTree extends RevObject {
 	void parse(final RevWalk walk) {
 		flags |= PARSED;
 	}
+	
+	@Override
+	public int getType() {
+		return Constants.OBJ_TREE;
+	}
 }
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 05/12] Replace instanceof in WalkFetchConnection with getType()
  2008-06-02 21:24       ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Marek Zawirski
@ 2008-06-02 21:24         ` Marek Zawirski
  2008-06-02 21:24           ` [JGIT PATCH 06/12] Move PackFile.SIGNATURE to Constants.PACK_SIGNATURE Marek Zawirski
  2008-06-06 13:24         ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Robin Rosenberg
  1 sibling, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../jgit/transport/WalkFetchConnection.java        |   36 +++++++++++--------
 1 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
index 4edeb93..45c2ded 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
@@ -66,7 +66,6 @@ import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.lib.Repository;
 import org.spearce.jgit.lib.UnpackedObjectLoader;
 import org.spearce.jgit.revwalk.DateRevQueue;
-import org.spearce.jgit.revwalk.RevBlob;
 import org.spearce.jgit.revwalk.RevCommit;
 import org.spearce.jgit.revwalk.RevFlag;
 import org.spearce.jgit.revwalk.RevObject;
@@ -251,20 +250,22 @@ class WalkFetchConnection extends FetchConnection {
 		//
 		obj.dispose();
 
-		if (obj instanceof RevBlob)
-			processBlob(obj);
-
-		else if (obj instanceof RevTree)
+		switch (obj.getType()) {
+		case Constants.OBJ_BLOB:
+			processBlob(obj);	
+			break;
+		case Constants.OBJ_TREE:
 			processTree(obj);
-
-		else if (obj instanceof RevCommit)
+			break;
+		case Constants.OBJ_COMMIT:
 			processCommit(obj);
-
-		else if (obj instanceof RevTag)
+			break;
+		case Constants.OBJ_TAG:
 			processTag(obj);
-
-		else
+			break;
+		default:
 			throw new TransportException("Unknown object type " + obj.getId());
+		}
 
 		// If we had any prior errors fetching this object they are
 		// now resolved, as the object was parsed successfully.
@@ -632,19 +633,24 @@ class WalkFetchConnection extends FetchConnection {
 	}
 
 	private void markLocalObjComplete(RevObject obj) throws IOException {
-		while (obj instanceof RevTag) {
+		while (obj.getType() == Constants.OBJ_TAG) {
 			obj.add(COMPLETE);
 			obj.dispose();
 			obj = ((RevTag) obj).getObject();
 			revWalk.parse(obj);
 		}
 
-		if (obj instanceof RevBlob)
+		switch (obj.getType()) {
+		case Constants.OBJ_BLOB:
 			obj.add(COMPLETE);
-		else if (obj instanceof RevCommit)
+			break;
+		case Constants.OBJ_COMMIT:			
 			pushLocalCommit((RevCommit) obj);
-		else if (obj instanceof RevTree)
+			break;
+		case Constants.OBJ_TREE:
 			markTreeComplete((RevTree) obj);
+			break;
+		}
 	}
 
 	private void markLocalCommitsComplete(final int until)
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 06/12] Move PackFile.SIGNATURE to Constants.PACK_SIGNATURE
  2008-06-02 21:24         ` [JGIT PATCH 05/12] Replace instanceof in WalkFetchConnection with getType() Marek Zawirski
@ 2008-06-02 21:24           ` Marek Zawirski
  2008-06-02 21:24             ` [JGIT PATCH 07/12] Add overload of fromRaw() in MutableObjectId accepting int[] Marek Zawirski
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Move to avoid redundancy in reading and writing packfiles in 3 places.
It seems to be a better place for format-related constant.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/Constants.java        |    8 ++++++++
 .../src/org/spearce/jgit/lib/PackFile.java         |   12 +++++-------
 .../src/org/spearce/jgit/transport/IndexPack.java  |    8 +++-----
 3 files changed, 16 insertions(+), 12 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 d1e8a41..7c2cef9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
@@ -188,6 +188,14 @@ public final class Constants {
 	 */
 	public static final int OBJ_REF_DELTA = 7;
 
+	/**
+	 * Pack file signature that occurs at file header - identifies file as Git
+	 * packfile formatted.
+	 * <p>
+	 * <b>This constant is fixed and is defined by the Git packfile format.</b>
+	 */
+	public static final byte[] PACK_SIGNATURE = { 'P', 'A', 'C', 'K' };
+	
 	/** Native character encoding for commit messages, file names... */
 	public static final String CHARACTER_ENCODING = "UTF-8";
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index ccff47d..b1fbc2a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -50,8 +50,6 @@ import org.spearce.jgit.util.NB;
  * objects are similar.
  */
 public class PackFile {
-	private static final byte[] SIGNATURE = { 'P', 'A', 'C', 'K' };
-
 	private final WindowedFile pack;
 
 	private final PackIndex idx;
@@ -165,17 +163,17 @@ public class PackFile {
 	private void readPackHeader() throws IOException {
 		final WindowCursor curs = new WindowCursor();
 		long position = 0;
-		final byte[] sig = new byte[SIGNATURE.length];
+		final byte[] sig = new byte[Constants.PACK_SIGNATURE.length];
 		final byte[] intbuf = new byte[4];
 		final long vers;
 
-		if (pack.read(position, sig, curs) != SIGNATURE.length)
+		if (pack.read(position, sig, curs) != Constants.PACK_SIGNATURE.length)
 			throw new IOException("Not a PACK file.");
-		for (int k = 0; k < SIGNATURE.length; k++) {
-			if (sig[k] != SIGNATURE[k])
+		for (int k = 0; k < Constants.PACK_SIGNATURE.length; k++) {
+			if (sig[k] != Constants.PACK_SIGNATURE[k])
 				throw new IOException("Not a PACK file.");
 		}
-		position += SIGNATURE.length;
+		position += Constants.PACK_SIGNATURE.length;
 
 		pack.readFully(position, intbuf, curs);
 		vers = NB.decodeUInt32(intbuf, 0);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 0b5c962..bec211c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -74,8 +74,6 @@ public class IndexPack {
 	/** Progress message when computing names of delta compressed objects. */
 	public static final String PROGRESS_RESOLVE_DELTA = "Resolving deltas";
 
-	private static final byte[] SIGNATURE = { 'P', 'A', 'C', 'K' };
-
 	private static final int BUFFER_SIZE = 2048;
 
 	/**
@@ -477,10 +475,10 @@ public class IndexPack {
 	}
 
 	private void readPackHeader() throws IOException {
-		final int hdrln = SIGNATURE.length + 4 + 4;
+		final int hdrln = Constants.PACK_SIGNATURE.length + 4 + 4;
 		final int p = fillFromInput(hdrln);
-		for (int k = 0; k < SIGNATURE.length; k++)
-			if (buf[p + k] != SIGNATURE[k])
+		for (int k = 0; k < Constants.PACK_SIGNATURE.length; k++)
+			if (buf[p + k] != Constants.PACK_SIGNATURE[k])
 				throw new IOException("Not a PACK file.");
 
 		final long vers = NB.decodeUInt32(buf, p + 4);
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 07/12] Add overload of fromRaw() in MutableObjectId accepting int[]
  2008-06-02 21:24           ` [JGIT PATCH 06/12] Move PackFile.SIGNATURE to Constants.PACK_SIGNATURE Marek Zawirski
@ 2008-06-02 21:24             ` Marek Zawirski
  2008-06-02 21:24               ` [JGIT PATCH 08/12] Copying constructor of MutableObjectId Marek Zawirski
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/MutableObjectId.java  |   29 ++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
index b23d36c..954380b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
@@ -74,6 +74,35 @@ public class MutableObjectId extends AnyObjectId {
 	}
 
 	/**
+	 * Convert an ObjectId from binary representation expressed in integers.
+	 * 
+	 * @param ints
+	 *            the raw int buffer to read from. At least 5 integers must be
+	 *            available within this integers array.
+	 */
+	public void fromRaw(final int[] ints) {
+		fromRaw(ints, 0);
+	}
+
+	/**
+	 * Convert an ObjectId from binary representation expressed in integers.
+	 * 
+	 * @param ints
+	 *            the raw int buffer to read from. At least 5 integers after p
+	 *            must be available within this integers array.
+	 * @param p
+	 *            position to read the first integer of data from.
+	 * 
+	 */
+	public void fromRaw(final int[] ints, final int p) {
+		w1 = ints[p];
+		w2 = ints[p + 1];
+		w3 = ints[p + 2];
+		w4 = ints[p + 3];
+		w5 = ints[p + 4];
+	}
+
+	/**
 	 * Convert an ObjectId from hex characters (US-ASCII).
 	 * 
 	 * @param buf
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 08/12] Copying constructor of MutableObjectId
  2008-06-02 21:24             ` [JGIT PATCH 07/12] Add overload of fromRaw() in MutableObjectId accepting int[] Marek Zawirski
@ 2008-06-02 21:24               ` Marek Zawirski
  2008-06-02 21:24                 ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Marek Zawirski
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/MutableObjectId.java  |   21 ++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
index 954380b..f88d8cb 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/MutableObjectId.java
@@ -46,6 +46,27 @@ import org.spearce.jgit.util.NB;
  */
 public class MutableObjectId extends AnyObjectId {
 	/**
+	 * Empty constructor. Initialize object with default (zeros) value.
+	 */
+	public MutableObjectId() {
+		super();
+	}
+
+	/**
+	 * Copying constructor.
+	 * 
+	 * @param src
+	 *            original entry, to copy id from
+	 */
+	MutableObjectId(MutableObjectId src) {
+		this.w1 = src.w1;
+		this.w2 = src.w2;
+		this.w3 = src.w3;
+		this.w4 = src.w4;
+		this.w5 = src.w5;
+	}
+
+	/**
 	 * Convert an ObjectId from raw binary representation.
 	 * 
 	 * @param bs
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap
  2008-06-02 21:24               ` [JGIT PATCH 08/12] Copying constructor of MutableObjectId Marek Zawirski
@ 2008-06-02 21:24                 ` Marek Zawirski
  2008-06-02 21:24                   ` [JGIT PATCH 10/12] Add getObjectCount() method to PackFile Marek Zawirski
  2008-06-06 13:24                   ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Robin Rosenberg
  0 siblings, 2 replies; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Method is based on already existing field.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../org/spearce/jgit/lib/ObjectIdSubclassMap.java  |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
index 79ef5b6..76bc9d9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
@@ -107,6 +107,15 @@ public class ObjectIdSubclassMap<V extends ObjectId> {
 		size++;
 	}
 
+	/**
+	 * Returns number of objects in map.
+	 * 
+	 * @return number of objects in map
+	 */
+	public int size() {
+		return size;
+	}
+
 	private final int index(final AnyObjectId id) {
 		return (id.w1 >>> 1) % obj_hash.length;
 	}
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 10/12] Add getObjectCount() method to PackFile
  2008-06-02 21:24                 ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Marek Zawirski
@ 2008-06-02 21:24                   ` Marek Zawirski
  2008-06-02 21:24                     ` [JGIT PATCH 11/12] Entries iterator in PackIndex and indirectly PackFile Marek Zawirski
  2008-06-06 13:24                   ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Robin Rosenberg
  1 sibling, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Method relies on PackIndex getObjectCount(). Exposed only to package.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/PackFile.java         |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index b1fbc2a..84562aa 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -145,6 +145,16 @@ public class PackFile {
 		pack.close();
 	}
 
+	/**
+	 * Obtain the total number of objects available in this pack. This method
+	 * relies on pack index, giving number of effectively available objects.
+	 * 
+	 * @return number of objects in index of this pack, likewise in this pack
+	 */
+	long getObjectCount() {
+		return idx.getObjectCount();
+	}
+
 	final UnpackedObjectCache.Entry readCache(final long position) {
 		return UnpackedObjectCache.get(pack, position);
 	}
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 11/12] Entries iterator in PackIndex and indirectly PackFile
  2008-06-02 21:24                   ` [JGIT PATCH 10/12] Add getObjectCount() method to PackFile Marek Zawirski
@ 2008-06-02 21:24                     ` Marek Zawirski
  2008-06-02 21:24                       ` [JGIT PATCH 12/12] Add PackIndex specific tests, currently only iterators tests Marek Zawirski
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

New iterators operate on MutableEntry to achieve high performance.
Information about objects (and its offset) in pack is needed in several
places in original git, and it will be also useful here.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/lib/PackFile.java         |   19 +++++-
 .../src/org/spearce/jgit/lib/PackIndex.java        |   80 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/PackIndexV1.java      |   31 ++++++++
 .../src/org/spearce/jgit/lib/PackIndexV2.java      |   36 +++++++++
 4 files changed, 164 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index 84562aa..1b2c167 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -40,6 +40,7 @@ package org.spearce.jgit.lib;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.zip.DataFormatException;
 
 import org.spearce.jgit.util.NB;
@@ -49,7 +50,7 @@ import org.spearce.jgit.util.NB;
  * delta packed format yielding high compression of lots of object where some
  * objects are similar.
  */
-public class PackFile {
+public class PackFile implements Iterable<PackIndex.MutableEntry> {
 	private final WindowedFile pack;
 
 	private final PackIndex idx;
@@ -146,6 +147,22 @@ public class PackFile {
 	}
 
 	/**
+	 * Provide iterator over entries in associated pack index, that should also
+	 * exist in this pack file. Objects returned by such iterator are mutable
+	 * during iteration.
+	 * <p>
+	 * Iterator returns objects in SHA-1 lexicographical order.
+	 * </p>
+	 * 
+	 * @return iterator over entries of associated pack index
+	 * 
+	 * @see PackIndex#iterator()
+	 */
+	public Iterator<PackIndex.MutableEntry> iterator() {
+		return idx.iterator();
+	}
+
+	/**
 	 * Obtain the total number of objects available in this pack. This method
 	 * relies on pack index, giving number of effectively available objects.
 	 * 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
index 104c361..3935d4f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
  *
  * All rights reserved.
  *
@@ -41,6 +42,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.util.Iterator;
 
 import org.spearce.jgit.util.NB;
 
@@ -53,7 +55,7 @@ import org.spearce.jgit.util.NB;
  * by ObjectId.
  * </p>
  */
-public abstract class PackIndex {
+public abstract class PackIndex implements Iterable<PackIndex.MutableEntry> {
 	/**
 	 * Open an existing pack <code>.idx</code> file for reading.
 	 * <p>
@@ -117,6 +119,19 @@ public abstract class PackIndex {
 	}
 
 	/**
+	 * Provide iterator that gives access to index entries. Note, that iterator
+	 * returns reference to mutable object, the same reference in each call -
+	 * for performance reason. If client needs immutable objects, it must copy
+	 * returned object on its own.
+	 * <p>
+	 * Iterator returns objects in SHA-1 lexicographical order.
+	 * </p>
+	 * 
+	 * @return iterator over pack index entries
+	 */
+	public abstract Iterator<MutableEntry> iterator();
+
+	/**
 	 * Obtain the total number of objects described by this index.
 	 * 
 	 * @return number of objects in this index, and likewise in the associated
@@ -134,4 +149,67 @@ public abstract class PackIndex {
 	 *         associated pack.
 	 */
 	abstract long findOffset(AnyObjectId objId);
+
+	/**
+	 * Represent mutable entry of pack index consisting of object id and offset
+	 * in pack (both mutable).
+	 * 
+	 */
+	public static class MutableEntry extends MutableObjectId {
+		private long offset;
+
+		/**
+		 * Empty constructor. Object fields should be filled in later.
+		 */
+		public MutableEntry() {
+			super();
+		}
+
+		/**
+		 * Returns offset for this index object entry
+		 * 
+		 * @return offset of this object in a pack file
+		 */
+		public long getOffset() {
+			return offset;
+		}
+
+		void setOffset(long offset) {
+			this.offset = offset;
+		}
+
+		private MutableEntry(MutableEntry src) {
+			super(src);
+			this.offset = src.offset;
+		}
+
+		/**
+		 * Returns mutable copy of this mutable entry.
+		 * 
+		 * @return copy of this mutable entry
+		 */
+		public MutableEntry cloneEntry() {
+			return new MutableEntry(this);
+		}
+	}
+
+	protected abstract class EntriesIterator implements Iterator<MutableEntry> {
+		protected MutableEntry objectId = new MutableEntry();
+
+		protected long returnedNumber = 0;
+
+		public boolean hasNext() {
+			return returnedNumber < getObjectCount();
+		}
+
+		/**
+		 * Implementation must update {@link #returnedNumber} before returning
+		 * element.
+		 */
+		public abstract MutableEntry next();
+
+		public void remove() {
+			throw new UnsupportedOperationException();
+		}
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java
index cfd18da..b8d9de3 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV1.java
@@ -40,6 +40,8 @@ package org.spearce.jgit.lib;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.spearce.jgit.errors.CorruptObjectException;
 import org.spearce.jgit.util.NB;
@@ -104,4 +106,33 @@ class PackIndexV1 extends PackIndex {
 		} while (low < high);
 		return -1;
 	}
+
+	public Iterator<MutableEntry> iterator() {
+		return new IndexV1Iterator();
+	}
+
+	private class IndexV1Iterator extends EntriesIterator {
+		private int levelOne;
+
+		private int levelTwo;
+
+		public MutableEntry next() {
+			for (; levelOne < idxdata.length; levelOne++) {
+				if (idxdata[levelOne] == null)
+					continue;
+
+				if (levelTwo < idxdata[levelOne].length) {
+					long offset = NB.decodeUInt32(idxdata[levelOne], levelTwo);
+					objectId.setOffset(offset);
+					objectId.fromRaw(idxdata[levelOne], levelTwo + 4);
+					levelTwo += Constants.OBJECT_ID_LENGTH + 4;
+					returnedNumber++;
+					return objectId;
+				} else {
+					levelTwo = 0;
+				}
+			}
+			throw new NoSuchElementException();
+		}
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java
index b1b4d73..9a695ef 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexV2.java
@@ -40,6 +40,8 @@ package org.spearce.jgit.lib;
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.spearce.jgit.util.NB;
 
@@ -174,4 +176,38 @@ class PackIndexV2 extends PackIndex {
 		} while (low < high);
 		return -1;
 	}
+
+	public Iterator<MutableEntry> iterator() {
+		return new EntriesIteratorV2();
+	}
+
+	private class EntriesIteratorV2 extends EntriesIterator {
+		private int levelOne;
+
+		private int levelTWo;
+
+		public MutableEntry next() {
+			for (; levelOne < names.length; levelOne++) {
+				if (levelTWo < names[levelOne].length) {
+					objectId.fromRaw(names[levelOne], levelTWo);
+					int arrayIdx = levelTWo / (Constants.OBJECT_ID_LENGTH / 4)
+							* 4;
+					long offset = NB.decodeUInt32(offset32[levelOne], arrayIdx);
+					if ((offset & IS_O64) != 0) {
+						arrayIdx = (8 * (int) (offset & ~IS_O64));
+						offset = NB.decodeUInt64(offset64, arrayIdx);
+					}
+					objectId.setOffset(offset);
+
+					levelTWo += Constants.OBJECT_ID_LENGTH / 4;
+					returnedNumber++;
+					return objectId;
+				} else {
+					levelTWo = 0;
+				}
+			}
+			throw new NoSuchElementException();
+		}
+	}
+
 }
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH 12/12] Add PackIndex specific tests, currently only iterators tests
  2008-06-02 21:24                     ` [JGIT PATCH 11/12] Entries iterator in PackIndex and indirectly PackFile Marek Zawirski
@ 2008-06-02 21:24                       ` Marek Zawirski
  0 siblings, 0 replies; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 21:24 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Abstract PackIndexTest added, with specializations for v1 and v2
indexes. New dedicated indexes files for v2 tests created.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../tst/org/spearce/jgit/lib/PackIndexTest.java    |  152 ++++++++++++++++++++
 .../tst/org/spearce/jgit/lib/PackIndexV1Test.java  |   54 +++++++
 .../tst/org/spearce/jgit/lib/PackIndexV2Test.java  |   54 +++++++
 ...-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2 |  Bin 0 -> 1296 bytes
 ...-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2 |  Bin 0 -> 2976 bytes
 5 files changed, 260 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV1Test.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV2Test.java
 create mode 100644 org.spearce.jgit.test/tst/pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2
 create mode 100644 org.spearce.jgit.test/tst/pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java
new file mode 100644
index 0000000..c682153
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexTest.java
@@ -0,0 +1,152 @@
+/*
+ * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+import org.spearce.jgit.lib.PackIndex.MutableEntry;
+
+public abstract class PackIndexTest extends RepositoryTestCase {
+
+	PackIndex smallIdx;
+
+	PackIndex denseIdx;
+
+	public void setUp() throws Exception {
+		super.setUp();
+		smallIdx = PackIndex.open(getFileForPack34be9032());
+		denseIdx = PackIndex.open(getFileForPackdf2982f28());
+	}
+
+	/**
+	 * Return file with appropriate index version for prepared pack.
+	 * 
+	 * @return file with index
+	 */
+	public abstract File getFileForPack34be9032();
+
+	/**
+	 * Return file with appropriate index version for prepared pack.
+	 * 
+	 * @return file with index
+	 */
+	public abstract File getFileForPackdf2982f28();
+
+	/**
+	 * Test contracts of Iterator methods and this implementation remove()
+	 * limitations.
+	 */
+	public void testIteratorMethodsContract() {
+		Iterator<PackIndex.MutableEntry> iter = smallIdx.iterator();
+		while (iter.hasNext()) {
+			iter.next();
+		}
+
+		try {
+			iter.next();
+			fail("next() unexpectedly returned element");
+		} catch (NoSuchElementException x) {
+			// expected
+		}
+
+		try {
+			iter.remove();
+			fail("remove() shouldn't be implemented");
+		} catch (UnsupportedOperationException x) {
+			// expected
+		}
+	}
+
+	/**
+	 * Test results of iterator comparing to content of well-known (prepared)
+	 * small index.
+	 */
+	public void testIteratorReturnedValues1() {
+		Iterator<PackIndex.MutableEntry> iter = smallIdx.iterator();
+		assertEquals("4b825dc642cb6eb9a060e54bf8d69288fbee4904", iter.next()
+				.toString());
+		assertEquals("540a36d136cf413e4b064c2b0e0a4db60f77feab", iter.next()
+				.toString());
+		assertEquals("5b6e7c66c276e7610d4a73c70ec1a1f7c1003259", iter.next()
+				.toString());
+		assertEquals("6ff87c4664981e4397625791c8ea3bbb5f2279a3", iter.next()
+				.toString());
+		assertEquals("82c6b885ff600be425b4ea96dee75dca255b69e7", iter.next()
+				.toString());
+		assertEquals("902d5476fa249b7abc9d84c611577a81381f0327", iter.next()
+				.toString());
+		assertEquals("aabf2ffaec9b497f0950352b3e582d73035c2035", iter.next()
+				.toString());
+		assertEquals("c59759f143fb1fe21c197981df75a7ee00290799", iter.next()
+				.toString());
+		assertFalse(iter.hasNext());
+	}
+
+	/**
+	 * Compare offset from iterator entries with output of findOffset() method.
+	 */
+	public void testCompareEntriesOffsetsWithFindOffsets() {
+		for (MutableEntry me : smallIdx) {
+			assertEquals(smallIdx.findOffset(me), me.getOffset());
+		}
+		for (MutableEntry me : denseIdx) {
+			assertEquals(denseIdx.findOffset(me), me.getOffset());
+		}
+	}
+
+	/**
+	 * Test partial results of iterator comparing to content of well-known
+	 * (prepared) dense index, that may need multi-level indexing.
+	 */
+	public void testIteratorReturnedValues2() {
+		Iterator<PackIndex.MutableEntry> iter = denseIdx.iterator();
+		while (!iter.next().toString().equals(
+				"0a3d7772488b6b106fb62813c4d6d627918d9181")) {
+			// just iterating
+		}
+		assertEquals("1004d0d7ac26fbf63050a234c9b88a46075719d3", iter.next()
+				.toString()); // same level-1
+		assertEquals("10da5895682013006950e7da534b705252b03be6", iter.next()
+				.toString()); // same level-1
+		assertEquals("1203b03dc816ccbb67773f28b3c19318654b0bc8", iter.next()
+				.toString());
+	}
+
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV1Test.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV1Test.java
new file mode 100644
index 0000000..dda3ef4
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV1Test.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.File;
+
+public class PackIndexV1Test extends PackIndexTest {
+	@Override
+	public File getFileForPack34be9032() {
+		return new File(new File("tst"),
+				"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idx");
+	}
+
+	@Override
+	public File getFileForPackdf2982f28() {
+		return new File(new File("tst"),
+				"pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idx");
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV2Test.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV2Test.java
new file mode 100644
index 0000000..8267e48
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackIndexV2Test.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.File;
+
+public class PackIndexV2Test extends PackIndexTest {
+	@Override
+	public File getFileForPack34be9032() {
+		return new File(new File("tst"),
+				"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2");
+	}
+
+	@Override
+	public File getFileForPackdf2982f28() {
+		return new File(new File("tst"),
+				"pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2");
+	}
+}
diff --git a/org.spearce.jgit.test/tst/pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2 b/org.spearce.jgit.test/tst/pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2
new file mode 100644
index 0000000000000000000000000000000000000000..1d45fa811a1855a9ac71e5237c8aa91e502e8224
GIT binary patch
literal 1296
zcmexg;-AdGz`z8=qk#AjU<4{9gh6473o`@dsEJvC=1~i?0?i@^vjO$d1G59|p%>-=
z+BG7ucT?;!r_*^m7bHCO{&8(m$M1KZEFoNG7tPK)+Ih42X!CLTZsRZiw>mnnChbt!
z^F&^+;^TY=7k)p;U=*4EqsA>|hMe>Cr0|I+URm#sSE^jxbZkfK{{-$Qs#{)7yZ1cy
zlxlS5^9i~kWxrHrSM8bGa!fG1s?kE8S$)-h{a<fpd)9LXm}=Wa=oT}_D3~6d9{JJv
zxBMd+$;!t2rOV$jXtK{dw9P4^Xl?kR{{l-YO*Yluad-asB!%0wTTA?O9Jh7!G6n{@
zG+>qt24*>6K48)T#v2b%%mOHO4anZ)^4)OqwtbqDxeqp+n<8~%>Q9^4X-{K**qkZ&
S;_1>JC~?!}g1(aBk0JolEO)B_

literal 0
HcmV?d00001

diff --git a/org.spearce.jgit.test/tst/pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2 b/org.spearce.jgit.test/tst/pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.idxV2
new file mode 100644
index 0000000000000000000000000000000000000000..ccbf00e34fb8de324c5fe3f584b80bdb99e02df0
GIT binary patch
literal 2976
zcmb`Jc{J4PAIHCTgRx}KHdlq1j4d}LNmof_JF>(LG1<A6$h2TI4Oy~9nKYP^t!63=
zYLulYB3n$iD0_*r=DPIzIH%6(bbj4#=iKL<*Lgn6`+1(vf6x0<nAKqfK{$agivxo1
z!390;@4yT7RrtPwA7a$MKmas?-$4lK{|{lPe~k#_{{^CO&-YjZy`MoG^k0yGSn@ka
zL0$SYGO({A`vc^lFApgE2PncFt0)1=z=qGLz^?WWY=qt_>Yveo{aZFc?Pt&gJr2<N
z0XooMMfWGr1MTK-(1(1LUjc(3U<m!MF@pRHTOs~}F~r-x!36SEw*N~^LAT0|pTrFG
z-?9^G{|INbPN!$C4Z>L`Ws~j~98nzpsR)Iq>AhXc=~_`Kak}JiK`rYFrUEtfaQGrI
zQ>~Q1zD&jZnaZI`iWV@A2-$tsPc*1PO}wF}M>RDmHAa-X{aLBX(!7>+zFspk!JN-Q
zwo{btnCXoXM|`YD*>;w}wzhPmku_X&<0h$=nj;Y=YPasE%XnD|Hc9?9K4QadJg}@V
zIY%xwsdbJreIT_sf#lkIH*B5_tu|Cx(CMO#)hj*gKDOjLxRSBy9t9hzXV$)VS`l-t
zZ1kvsg(CGXp0MaTRx(4=Yf=f9N$K^8?7p)vM9k;xFEZDS<{hzm>6<GTviPtI8yjY7
zP!barFPpLTvy#rqY!?%`Cf9-NL=O9=7J02gdPtY)^S@U(@r=g5$M-5X>|*a28f_fr
zdnNw1CX$2UXWK?19Dik@qro^~*ph0#n3&ZbX``c7Gjq?P@uSW5z-Vr!6jNFxNM=xY
zETYOjuh6aEl0?O16mj=o5qnwg<;3I2GIy$R5X*S?n@79l=1VHS1la~U1^bHKeO(oQ
zQI))93R844<xFPBp$)}zp`+9cpJQi=RGGR1WO4H_H%tMuyGMW6qUElT-Rq;~aW&c-
zJQyyQ*iCj*HOO4*y4g7TLEhe5o*tvUoMz`4`;OYYo)CLo^FE5F!Jk;vPNj@IRZi31
z-`V$SUf5uT?tfOCAn&EB?Q}dIpJGZNWlkqO3#h7lyI_BAaxs)&T0c}EH_wOK5GWCj
z33Lz>c`{qI4rAu8m>p7}d@5GWAb%yxZHDx?=CMgy<4VlSg_t3a{^MaM4xS<2kf|ls
zE=?uX-HFwjC29@uxaJBr<`+iD%)dxq&x~cX$Kl7V50UVdkzrOB_5LUniius)j0_FW
z32P9=Pz9>rP<~B_k|et3iO0Mg-}?vE)U&nN&X_lJXqZUc6SZ8tc8%MC(?{mZ&T(?Z
z&wCO(-kZs_HrFj<rI~6HMkA9}tmxB$b^O=;%Sty~eP$)g(6|}3eq}vL#YLBqqcJM?
zf{<9{xNbVknClwp$%FC}(aSV0WC@Kw=5?jM-T{~7i&KApvnogj-Gi&>OAYF<IMJ74
zT`k#_ZWpul!E4TJ1dEoY7fq&;Q$<{5%=Tgvj3dWbA80;}9)aiW+{HC<lJUnc_dHZ(
zm4|CN)F+%zu=FoJYkxI5d%)t*-Q4iP_TT@?CwH`L8|}sI)(pN*2zqy$YVZP8V2F}C
zo-}^eP`zVXiTs8%llklnZD1HZ<Wm}m>(?BU;h(nau1x7HJ$Z8Hee&;1yVSe)MCzQt
z*EUQ{m9{I3@GcBJw|;Xlf5-d&cN+<+H?F61JNO3Ho93*O)1V~lQiHM<m#WU3BF@%T
z3mNw=;rtG9Q;R>G&Zw^N+x=1hU8c|l@%{67vmkmDipt9QP1gv|Vf0cyRAQ=8q@?c_
zCE#9gSt5U&P>d9tba0cGlc8T#tNMLH?s&Vk(nQ%`^%X+2(+n$DR`&haQ+%%^>Uehj
z=^Jw1ksM4bufR4xxN$1-&Mpgd?Y3C#^i178%X99nmTh$>u44V-(4z=T-mP%tg6)-d
zu|3RE9lsK-!K)5bgYd}4yc^_BC1<xT-ZAa=1I*G5Te>Ahg*HuhRWX#B5Z@WP%k*t`
z(TI1xIf;C|zUpQ9bE7TuA7r9yt4g--Pau1;MUwm0rr<<)2}b=YDy}ZH#E;maPl6$j
zQ5nw{ir6XwZ_VW|ku}OkYCpLshzPe@OWNKxR21(;67FbE7e7w-jt=nRq#72n`SrTW
z1uqLP7-b$&(!oph_^B@$^|#hM5fj{L(XRQ>UP%ubRZ;i<aCt!xd1#3?O?#A`a6#5Y
zFrAXr@BMUE#!_Ww&g=5?Hqw}ZcYyPpg;thow!&D;$+YrCciB7~!`;Y~{Bgf-tooCz
z5W-}OZQp!v-y>{KkcJLD(=;O>WxupVlwC8|y2ZogB`bU4g|h<ZRHC2vbO_fGDVR=9
z<*UmirY&u7=o(MFtf6M4Qgq{E)|PT+^Q73Jkd7HEMOPLxg}}8VS^m0{gLR=(f15sK
zF#RYi-I_;4k8YNhxhp9Q<*abYC2cmDCO|tMeD1}}V9t80j1tfA12;$1rrYTGxW%Uy
zaVYfKEjM0#IKU%|Daq>yyf(73_9lfhV3^|Wc5k<<Fk@;P{Hr419R(c00?`o!y%*>P
z`-VjjF7PqvO0ZlxkP{I^>Hx%cU>jLrwP=vHAqWrbs~%7o{#xNQu!^APy^kOg5m0`D
zAYvrYfPa(%A1MT9`IEuY9s~_|6Nx?W<^r6{gZn;$wgfD!CD_y%=+(nph0p^JC3zUE
zS~koX3-dex9U5?lGw_c~g0B<=jaV;W54shY`xa;)!CY_Q-A|x#fO;a_1+#I$8+^`?
z!=H-+4~2$YssQeqfHRXYoBYl^A8q!6_j1OT;sSk?0(z_H4h!AoN{ZqUN0dGrKk#Uu
L#~L@=K-~WTG^vnn

literal 0
HcmV?d00001

-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-02 21:24 [JGIT PATCH 00/12] Extensions in core needed by PackWriter Marek Zawirski
  2008-06-02 21:24 ` [JGIT PATCH 01/12] Format PackFile class Marek Zawirski
@ 2008-06-02 22:15 ` Johannes Schindelin
  2008-06-02 22:58   ` Marek Zawirski
  2008-06-06 13:24 ` Robin Rosenberg
  2008-06-10 21:09 ` Robin Rosenberg
  3 siblings, 1 reply; 24+ messages in thread
From: Johannes Schindelin @ 2008-06-02 22:15 UTC (permalink / raw
  To: Marek Zawirski; +Cc: robin.rosenberg, spearce, git

Hi,

On Mon, 2 Jun 2008, Marek Zawirski wrote:

> Series start with formatting stuff, as some old files were not 
> appropriatelly formatted.

You mean line-wrapping, right?  Is there a different 
recommended column/line ratio for JGit than for Git?  Because some of your 
later patches introduce lines longer than 80 columns/line.

>   Add getType() method to RevObject hierarchy

Was the idea not to use instanceof to be able to have multiple "types" per 
object?  I.e. a commit object is of type commit, but also of type 
object...

BTW I really like the iterator implementation.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-02 22:15 ` [JGIT PATCH 00/12] Extensions in core needed by PackWriter Johannes Schindelin
@ 2008-06-02 22:58   ` Marek Zawirski
  2008-06-02 23:43     ` Johannes Schindelin
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-02 22:58 UTC (permalink / raw
  To: Johannes Schindelin; +Cc: robin.rosenberg, spearce, git

Johannes Schindelin wrote:
(...)
> You mean line-wrapping, right?  Is there a different 
> recommended column/line ratio for JGit than for Git?  Because some of your 
> later patches introduce lines longer than 80 columns/line.

Not only line-wrapping in fact. egit/jgit use it's own eclipse formatting template 
that define more than line-wrapping, even some spaces in javadocs as you may have seen.
Actually I don't know is it 80 columns/line (but I suspect it may be), as I just use 
this formatter associated with project by simply pressing some keyboard shortcut. 
Maybe I simply missed some formatting, which patch do you mean? Some things however 
look strange after auto-formatting. While it may even look ugly for somebody I 
wouldn't try to change that by hand, to make other commiters lifes easier allowing 
them also just press "format" button ;)

>>   Add getType() method to RevObject hierarchy
> 
> Was the idea not to use instanceof to be able to have multiple "types" per 
> object?  I.e. a commit object is of type commit, but also of type 
> object...

I'm not sure whether I understand you, but probably it was not what you mean. The 
only idea behind that was to refactor instanceofs to polymorphic getType() calls. It 
allows us mapping, indexing by type, using switches... without tones of code.


> BTW I really like the iterator implementation.

Thanks for looking in.

-- 
Marek Zawirski [zawir]
marek.zawirski@gmail.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-02 22:58   ` Marek Zawirski
@ 2008-06-02 23:43     ` Johannes Schindelin
  0 siblings, 0 replies; 24+ messages in thread
From: Johannes Schindelin @ 2008-06-02 23:43 UTC (permalink / raw
  To: Marek Zawirski; +Cc: robin.rosenberg, spearce, git

Hi,

On Tue, 3 Jun 2008, Marek Zawirski wrote:

> Johannes Schindelin wrote:
> (...)
> > You mean line-wrapping, right?  Is there a different recommended 
> > column/line ratio for JGit than for Git?  Because some of your later 
> > patches introduce lines longer than 80 columns/line.
> 
> Not only line-wrapping in fact. egit/jgit use it's own eclipse 
> formatting template that define more than line-wrapping, even some 
> spaces in javadocs as you may have seen.
>
> Actually I don't know is it 80 columns/line (but I suspect it may be), 
> as I just use this formatter associated with project by simply pressing 
> some keyboard shortcut. Maybe I simply missed some formatting, which 
> patch do you mean? Some things however look strange after 
> auto-formatting. While it may even look ugly for somebody I wouldn't try 
> to change that by hand, to make other commiters lifes easier allowing 
> them also just press "format" button ;)

Ah, thanks for the explanation!

> > >   Add getType() method to RevObject hierarchy
> > 
> > Was the idea not to use instanceof to be able to have multiple "types" 
> > per object?  I.e. a commit object is of type commit, but also of type 
> > object...
> 
> I'm not sure whether I understand you, but probably it was not what you 
> mean. The only idea behind that was to refactor instanceofs to 
> polymorphic getType() calls.

Actually, the result is no longer polymorphic, as every object can have 
only one type now.

> It allows us mapping, indexing by type, using switches... without tones 
> of code.

Hrm, I thought that you added more lines than you deleted.  But hey, I do 
not really know what you want to index, maybe you got a point there.  I 
guess I'll see ;-)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-02 21:24 [JGIT PATCH 00/12] Extensions in core needed by PackWriter Marek Zawirski
  2008-06-02 21:24 ` [JGIT PATCH 01/12] Format PackFile class Marek Zawirski
  2008-06-02 22:15 ` [JGIT PATCH 00/12] Extensions in core needed by PackWriter Johannes Schindelin
@ 2008-06-06 13:24 ` Robin Rosenberg
  2008-06-07  0:06   ` Marek Zawirski
  2008-06-10 21:09 ` Robin Rosenberg
  3 siblings, 1 reply; 24+ messages in thread
From: Robin Rosenberg @ 2008-06-06 13:24 UTC (permalink / raw
  To: Marek Zawirski; +Cc: spearce, git

måndagen den 2 juni 2008 23.24.31 skrev Marek Zawirski:
> Hello,
> 
> Here is my first GSoC series - some work from the last week.
> It's actually not a PackWriter, but some changes in existing jgit core
> related to PackWriting. Some of these added methods/refactors are not yet
> used within this series, but are used in my dirty branch in PackWriter
> which is under-development, even somewhat usable.
> 
> Series start with formatting stuff, as some old files were not
> appropriatelly formatted.
> 
> This series is also available at my corechanges branch:
> http://repo.or.cz/w/egit/zawir.git?a=shortlog;h=refs/heads/corechanges
> It's based on Shawn's bsd branch, with new BSD-style license, but I can
> rebase if really needed.
No, that's fine. 
> 
> If you want to track some PackWriter (itself) development you may want to
> have a look at my dirty branch:
> http://repo.or.cz/w/egit/zawir.git?a=shortlog;h=refs/heads/dirty
> 
> That's all. Although Shawn already reviewed some old version of this patches,
> I'm still interested in your comments.

A well defined set of enhancements with only minor nitpicks (in separate mails)  I'm somewhat reluctant to reformatting patches though. Ideally we'd be using eclipse 3.4 and have it format changed parts automatically on save (or similar feature for other IDE's). I tried format on all classes leading to 118 files changed, but nothing major, just a lot of changes with no relevant to readability, except some cases which only made things worse.  That's a reason why I have not formatted code according to settings, although
the most common reason is probably sloppiness.

-- robin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy
  2008-06-02 21:24       ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Marek Zawirski
  2008-06-02 21:24         ` [JGIT PATCH 05/12] Replace instanceof in WalkFetchConnection with getType() Marek Zawirski
@ 2008-06-06 13:24         ` Robin Rosenberg
  2008-06-07  0:10           ` [JGIT PATCH v2 " Marek Zawirski
  1 sibling, 1 reply; 24+ messages in thread
From: Robin Rosenberg @ 2008-06-06 13:24 UTC (permalink / raw
  To: Marek Zawirski; +Cc: spearce, git

måndagen den 2 juni 2008 23.24.35 skrev Marek Zawirski:
> This let us avoid using instanceof.

Obviously. I think I know why you want to do this and I think it's a
good idea. You might want to explaint why *you* think it's good.

-- robin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap
  2008-06-02 21:24                 ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Marek Zawirski
  2008-06-02 21:24                   ` [JGIT PATCH 10/12] Add getObjectCount() method to PackFile Marek Zawirski
@ 2008-06-06 13:24                   ` Robin Rosenberg
  2008-06-07  0:10                     ` [JGIT PATCH v2 09/12] Add size() " Marek Zawirski
  1 sibling, 1 reply; 24+ messages in thread
From: Robin Rosenberg @ 2008-06-06 13:24 UTC (permalink / raw
  To: Marek Zawirski; +Cc: spearce, git

måndagen den 2 juni 2008 23.24.40 skrev Marek Zawirski:
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
> @@ -107,6 +107,15 @@ public class ObjectIdSubclassMap<V extends ObjectId> {
>  		size++;
>  	}
>  
> +	/**
> +	 * Returns number of objects in map.
> +	 * 
> +	 * @return number of objects in map
> +	 */
> +	public int size() {
> +		return size;
> +	}

Just the @return is usually enough for methods that simply return a value. If you
have both the untagged part of the comment should explain much more. Patch
10 is a good example.

-- robin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-06 13:24 ` Robin Rosenberg
@ 2008-06-07  0:06   ` Marek Zawirski
  2008-06-07  7:16     ` Shawn O. Pearce
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Zawirski @ 2008-06-07  0:06 UTC (permalink / raw
  To: Robin Rosenberg; +Cc: spearce, git

On Fri, Jun 6, 2008 at 3:24 PM, Robin Rosenberg 
<robin.rosenberg@dewire.com> wrote:
> A well defined set of enhancements with only minor nitpicks (in separate mails)

Nice to hear. I'll send v2 of these 2 patches (04 and 09) in a moment. 
With a slightly less place for nitpicks, I hope.

>  I'm somewhat reluctant to reformatting patches though. 

Well, so should I remove these reformatting patches from series? (oouch!)

Actually, when I ran into formatting problem, Shawn suggested that I may 
format PackFile as he was also touching this file - I much appreciated 
this idea. PackIndex* formatting is however my own invention.

So you assume that Eclipse 3.4 will just force us to submit new patches
with specified formatting, but keep old lines formatting untouched, am I
right?
Isn't it somewhat annoying that formatting is inconsistent through
project (especially line width in some places)? And that I almost have
to block formatting shortcut to stop my pre-save/pre-commit habit?;)

Thanks,
-- 
Marek Zawirski / zawir
marek.zawirski@gmail.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [JGIT PATCH v2 04/12] Add getType() method to RevObject hierarchy
  2008-06-06 13:24         ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Robin Rosenberg
@ 2008-06-07  0:10           ` Marek Zawirski
  0 siblings, 0 replies; 24+ messages in thread
From: Marek Zawirski @ 2008-06-07  0:10 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Introduce natural correlation between each concrete RevObject and
existing integer (constant) for that object type. Such an integer
allows us reducing code amount to perform indexing array by type
or mapping by type. We can also use switches instead of instanceof
or write output type directly.
We could have a common code with behavior determined by polymorphic
getType() call.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/jgit/revwalk/RevBlob.java      |    6 ++++++
 .../src/org/spearce/jgit/revwalk/RevCommit.java    |    5 +++++
 .../src/org/spearce/jgit/revwalk/RevObject.java    |    8 ++++++++
 .../src/org/spearce/jgit/revwalk/RevTag.java       |    5 +++++
 .../src/org/spearce/jgit/revwalk/RevTree.java      |    6 ++++++
 5 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
index f6d34f4..66cdc02 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
@@ -38,6 +38,7 @@
 package org.spearce.jgit.revwalk;
 
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
 
 /** A binary file, or a symbolic link. */
 public class RevBlob extends RevObject {
@@ -55,4 +56,9 @@ public class RevBlob extends RevObject {
 	void parse(final RevWalk walk) {
 		flags |= PARSED;
 	}
+	
+	@Override
+	public int getType() {
+		return Constants.OBJ_BLOB;
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
index 0aa7098..77f1d1a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
@@ -135,6 +135,11 @@ public class RevCommit extends RevObject {
 		buffer = raw;
 		flags |= PARSED;
 	}
+	
+	@Override
+	public int getType() {
+		return Constants.OBJ_COMMIT;
+	}
 
 	static void carryFlags(RevCommit c, final int carry) {
 		for (;;) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
index 86c50b5..451205c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
@@ -42,6 +42,7 @@ import java.io.IOException;
 import org.spearce.jgit.errors.IncorrectObjectTypeException;
 import org.spearce.jgit.errors.MissingObjectException;
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
 
 /** Base object type accessed during revision walking. */
@@ -56,6 +57,13 @@ public abstract class RevObject extends ObjectId {
 
 	abstract void parse(RevWalk walk) throws MissingObjectException,
 			IncorrectObjectTypeException, IOException;
+	
+	/**
+	 * Get Git object type. See {@link Constants}.
+	 * 
+	 * @return object type
+	 */
+	public abstract int getType();
 
 	/**
 	 * Get the name of this object.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
index 668819c..bbb18ee 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
@@ -96,6 +96,11 @@ public class RevTag extends RevObject {
 		flags |= PARSED;
 	}
 
+	@Override
+	public int getType() {
+		return Constants.OBJ_TAG;
+	}
+	
 	/**
 	 * Parse this tag buffer for display.
 	 * 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
index 7ad9be0..e1cd4b5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
@@ -38,6 +38,7 @@
 package org.spearce.jgit.revwalk;
 
 import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
 
 /** A reference to a tree of subtrees/files. */
 public class RevTree extends RevObject {
@@ -55,4 +56,9 @@ public class RevTree extends RevObject {
 	void parse(final RevWalk walk) {
 		flags |= PARSED;
 	}
+	
+	@Override
+	public int getType() {
+		return Constants.OBJ_TREE;
+	}
 }
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [JGIT PATCH v2 09/12] Add size() method to ObjectIdSubclassMap
  2008-06-06 13:24                   ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Robin Rosenberg
@ 2008-06-07  0:10                     ` Marek Zawirski
  0 siblings, 0 replies; 24+ messages in thread
From: Marek Zawirski @ 2008-06-07  0:10 UTC (permalink / raw
  To: robin.rosenberg, spearce; +Cc: git, Marek Zawirski

Method is based on already existing field.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../org/spearce/jgit/lib/ObjectIdSubclassMap.java  |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
index 79ef5b6..2a13204 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectIdSubclassMap.java
@@ -107,6 +107,13 @@ public class ObjectIdSubclassMap<V extends ObjectId> {
 		size++;
 	}
 
+	/**
+	 * @return number of objects in map
+	 */
+	public int size() {
+		return size;
+	}
+
 	private final int index(final AnyObjectId id) {
 		return (id.w1 >>> 1) % obj_hash.length;
 	}
-- 
1.5.5.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-07  0:06   ` Marek Zawirski
@ 2008-06-07  7:16     ` Shawn O. Pearce
  0 siblings, 0 replies; 24+ messages in thread
From: Shawn O. Pearce @ 2008-06-07  7:16 UTC (permalink / raw
  To: Marek Zawirski; +Cc: Robin Rosenberg, git

Marek Zawirski <marek.zawirski@gmail.com> wrote:
> On Fri, Jun 6, 2008 at 3:24 PM, Robin Rosenberg 
> > 
> > I'm somewhat reluctant to reformatting patches though. 
> 
> Well, so should I remove these reformatting patches from series? (oouch!)
> 
> Actually, when I ran into formatting problem, Shawn suggested that I may 
> format PackFile as he was also touching this file - I much appreciated 
> this idea. PackIndex* formatting is however my own invention.

I'm in favor of reformatting, at least these two classes.
Editing them without the formatting fixes is insanely annoying.
But bulk reformatting all 118 files at once is nuts.

I'd rather do it one file at a time, when we touch it, and especially
if it is fairly stable and isn't being actively hacked on by others.

> Isn't it somewhat annoying that formatting is inconsistent through
> project (especially line width in some places)? And that I almost have
> to block formatting shortcut to stop my pre-save/pre-commit habit?;)

Yes.  Yes it is.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [JGIT PATCH 00/12] Extensions in core needed by PackWriter
  2008-06-02 21:24 [JGIT PATCH 00/12] Extensions in core needed by PackWriter Marek Zawirski
                   ` (2 preceding siblings ...)
  2008-06-06 13:24 ` Robin Rosenberg
@ 2008-06-10 21:09 ` Robin Rosenberg
  3 siblings, 0 replies; 24+ messages in thread
From: Robin Rosenberg @ 2008-06-10 21:09 UTC (permalink / raw
  To: Marek Zawirski; +Cc: spearce, git

måndagen den 2 juni 2008 23.24.31 skrev Marek Zawirski:
> Hello,
> 
> Here is my first GSoC series - some work from the last week.
> It's actually not a PackWriter, but some changes in existing jgit core
> related to PackWriting. Some of these added methods/refactors are not yet
> used within this series, but are used in my dirty branch in PackWriter
> which is under-development, even somewhat usable.
> 
> Series start with formatting stuff, as some old files were not
> appropriatelly formatted.
> 
> This series is also available at my corechanges branch:
> http://repo.or.cz/w/egit/zawir.git?a=shortlog;h=refs/heads/corechanges
> It's based on Shawn's bsd branch, with new BSD-style license, but I can
> rebase if really needed.
> 
> If you want to track some PackWriter (itself) development you may want to
> have a look at my dirty branch:
> http://repo.or.cz/w/egit/zawir.git?a=shortlog;h=refs/heads/dirty
> 
> That's all. Although Shawn already reviewed some old version of this patches,
> I'm still interested in your comments.

This patchset is in master (merged)

Thanks.

-- robin

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2008-06-10 21:16 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-02 21:24 [JGIT PATCH 00/12] Extensions in core needed by PackWriter Marek Zawirski
2008-06-02 21:24 ` [JGIT PATCH 01/12] Format PackFile class Marek Zawirski
2008-06-02 21:24   ` [JGIT PATCH 02/12] Format PackIndex class Marek Zawirski
2008-06-02 21:24     ` [JGIT PATCH 03/12] Format PackIndexV1 class Marek Zawirski
2008-06-02 21:24       ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Marek Zawirski
2008-06-02 21:24         ` [JGIT PATCH 05/12] Replace instanceof in WalkFetchConnection with getType() Marek Zawirski
2008-06-02 21:24           ` [JGIT PATCH 06/12] Move PackFile.SIGNATURE to Constants.PACK_SIGNATURE Marek Zawirski
2008-06-02 21:24             ` [JGIT PATCH 07/12] Add overload of fromRaw() in MutableObjectId accepting int[] Marek Zawirski
2008-06-02 21:24               ` [JGIT PATCH 08/12] Copying constructor of MutableObjectId Marek Zawirski
2008-06-02 21:24                 ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Marek Zawirski
2008-06-02 21:24                   ` [JGIT PATCH 10/12] Add getObjectCount() method to PackFile Marek Zawirski
2008-06-02 21:24                     ` [JGIT PATCH 11/12] Entries iterator in PackIndex and indirectly PackFile Marek Zawirski
2008-06-02 21:24                       ` [JGIT PATCH 12/12] Add PackIndex specific tests, currently only iterators tests Marek Zawirski
2008-06-06 13:24                   ` [JGIT PATCH 09/12] Add getSize() method to ObjectIdSubclassMap Robin Rosenberg
2008-06-07  0:10                     ` [JGIT PATCH v2 09/12] Add size() " Marek Zawirski
2008-06-06 13:24         ` [JGIT PATCH 04/12] Add getType() method to RevObject hierarchy Robin Rosenberg
2008-06-07  0:10           ` [JGIT PATCH v2 " Marek Zawirski
2008-06-02 22:15 ` [JGIT PATCH 00/12] Extensions in core needed by PackWriter Johannes Schindelin
2008-06-02 22:58   ` Marek Zawirski
2008-06-02 23:43     ` Johannes Schindelin
2008-06-06 13:24 ` Robin Rosenberg
2008-06-07  0:06   ` Marek Zawirski
2008-06-07  7:16     ` Shawn O. Pearce
2008-06-10 21:09 ` Robin Rosenberg

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).