Git development
 help / color / mirror / Atom feed
* [JGIT PATCH 12/15] Correctly handle hunk headers such as "@@ -0,0 +1 @@"
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-12-git-send-email-spearce@spearce.org>

Sometimes these are created for single line file creations.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/patch/HunkHeader.java     |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
index 4fd9bae..c3bd642 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
@@ -126,10 +126,20 @@ void parseHeader(final int end) {
 		final MutableInteger ptr = new MutableInteger();
 		ptr.value = nextLF(buf, startOffset, ' ');
 		oldStartLine = -parseBase10(buf, ptr.value, ptr);
-		oldLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		if (buf[ptr.value] == ',')
+			oldLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		else {
+			oldLineCount = oldStartLine;
+			oldStartLine = 0;
+		}
 
 		newStartLine = parseBase10(buf, ptr.value + 1, ptr);
-		newLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		if (buf[ptr.value] == ',')
+			newLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		else {
+			newLineCount = newStartLine;
+			newStartLine = 0;
+		}
 	}
 
 	int parseBody(final Patch script, final int end) {
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 10/15] Record patch parsing errors for later inspection by applications
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-10-git-send-email-spearce@spearce.org>

Errors identified while reading a patch script are now collected
into FormatError objects within the errors collection of a Patch.
These can be inspected to determine if a common form of breakage
is found within the patch script once its basic metadata is read.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/PatchErrorTest.java |  174 ++++++++++++++++++++
 .../tst/org/spearce/jgit/patch/PatchTest.java      |    4 +
 .../spearce/jgit/patch/testError_BodyTooLong.patch |   17 ++
 .../jgit/patch/testError_DisconnectedHunk.patch    |   30 ++++
 .../jgit/patch/testError_GarbageBetweenFiles.patch |   33 ++++
 .../patch/testError_GitBinaryNoForwardHunk.patch   |   10 +
 .../jgit/patch/testError_TruncatedNew.patch        |   15 ++
 .../jgit/patch/testError_TruncatedOld.patch        |   15 ++
 .../src/org/spearce/jgit/patch/FormatError.java    |   95 +++++++++++
 .../src/org/spearce/jgit/patch/HunkHeader.java     |   20 ++-
 .../src/org/spearce/jgit/patch/Patch.java          |   38 ++++-
 11 files changed, 441 insertions(+), 10 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
new file mode 100644
index 0000000..3d7e6b2
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.patch;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import junit.framework.TestCase;
+
+public class PatchErrorTest extends TestCase {
+	public void testError_DisconnectedHunk() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		{
+			final FileHeader fh = p.getFiles().get(0);
+			assertEquals(
+					"org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java",
+					fh.getNewName());
+			assertEquals(1, fh.getHunks().size());
+		}
+
+		assertEquals(1, p.getErrors().size());
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Hunk disconnected from file", e.getMessage());
+		assertEquals(18, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -109,4 +109,11 @@ assert"));
+	}
+
+	public void testError_TruncatedOld() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		assertEquals(1, p.getErrors().size());
+
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Truncated hunk, at least 1 old lines is missing", e
+				.getMessage());
+		assertEquals(313, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -236,9 +236,9 @@ protected "));
+	}
+
+	public void testError_TruncatedNew() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		assertEquals(1, p.getErrors().size());
+
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Truncated hunk, at least 1 new lines is missing", e
+				.getMessage());
+		assertEquals(313, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -236,9 +236,9 @@ protected "));
+	}
+
+	public void testError_BodyTooLong() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		assertEquals(1, p.getErrors().size());
+
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.WARNING, e.getSeverity());
+		assertEquals("Hunk header 4:11 does not match body line count of 4:12",
+				e.getMessage());
+		assertEquals(349, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -109,4 +109,11 @@ assert"));
+	}
+
+	public void testError_GarbageBetweenFiles() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(2, p.getFiles().size());
+		{
+			final FileHeader fh = p.getFiles().get(0);
+			assertEquals(
+					"org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java",
+					fh.getNewName());
+			assertEquals(1, fh.getHunks().size());
+		}
+		{
+			final FileHeader fh = p.getFiles().get(1);
+			assertEquals(
+					"org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java",
+					fh.getNewName());
+			assertEquals(1, fh.getHunks().size());
+		}
+
+		assertEquals(1, p.getErrors().size());
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.WARNING, e.getSeverity());
+		assertEquals("Unexpected hunk trailer", e.getMessage());
+		assertEquals(926, e.getOffset());
+		assertEquals("I AM NOT HERE\n", e.getLineText());
+	}
+
+	public void testError_GitBinaryNoForwardHunk() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(2, p.getFiles().size());
+		{
+			final FileHeader fh = p.getFiles().get(0);
+			assertEquals("org.spearce.egit.ui/icons/toolbar/fetchd.png", fh
+					.getNewName());
+			assertSame(FileHeader.PatchType.GIT_BINARY, fh.getPatchType());
+			assertTrue(fh.getHunks().isEmpty());
+			assertNull(fh.getForwardBinaryHunk());
+		}
+		{
+			final FileHeader fh = p.getFiles().get(1);
+			assertEquals("org.spearce.egit.ui/icons/toolbar/fetche.png", fh
+					.getNewName());
+			assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+			assertTrue(fh.getHunks().isEmpty());
+			assertNull(fh.getForwardBinaryHunk());
+		}
+
+		assertEquals(1, p.getErrors().size());
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Missing forward-image in GIT binary patch", e
+				.getMessage());
+		assertEquals(297, e.getOffset());
+		assertEquals("\n", e.getLineText());
+	}
+
+	private Patch parseTestPatchFile() throws IOException {
+		final String patchFile = getName() + ".patch";
+		final InputStream in = getClass().getResourceAsStream(patchFile);
+		if (in == null) {
+			fail("No " + patchFile + " test vector");
+			return null; // Never happens
+		}
+		try {
+			final Patch p = new Patch();
+			p.parse(in);
+			return p;
+		} finally {
+			in.close();
+		}
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
index 453d88e..7c69fff 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
@@ -49,11 +49,13 @@
 	public void testEmpty() {
 		final Patch p = new Patch();
 		assertTrue(p.getFiles().isEmpty());
+		assertTrue(p.getErrors().isEmpty());
 	}
 
 	public void testParse_ConfigCaseInsensitive() throws IOException {
 		final Patch p = parseTestPatchFile();
 		assertEquals(2, p.getFiles().size());
+		assertTrue(p.getErrors().isEmpty());
 
 		final FileHeader fRepositoryConfigTest = p.getFiles().get(0);
 		final FileHeader fRepositoryConfig = p.getFiles().get(1);
@@ -145,6 +147,7 @@ assertSame(FileHeader.PatchType.UNIFIED, fRepositoryConfig
 	public void testParse_NoBinary() throws IOException {
 		final Patch p = parseTestPatchFile();
 		assertEquals(5, p.getFiles().size());
+		assertTrue(p.getErrors().isEmpty());
 
 		for (int i = 0; i < 4; i++) {
 			final FileHeader fh = p.getFiles().get(i);
@@ -179,6 +182,7 @@ public void testParse_GitBinary() throws IOException {
 		final Patch p = parseTestPatchFile();
 		final int[] binsizes = { 359, 393, 372, 404 };
 		assertEquals(5, p.getFiles().size());
+		assertTrue(p.getErrors().isEmpty());
 
 		for (int i = 0; i < 4; i++) {
 			final FileHeader fh = p.getFiles().get(i);
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
new file mode 100644
index 0000000..1d0b1c4
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
@@ -0,0 +1,17 @@
+diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+index da7e704..34ce04a 100644
+--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
++++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+@@ -109,4 +109,11 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+        .getStringList("my", null, "somename")));
+    checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
+  }
++
++ public void test006_readCaseInsensitive() throws IOException {
++   final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
++   RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
++BAD LINE
++   assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
++   assertEquals("", repositoryConfig.getString("foo", null, "bar"));
++ }
+ }
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
new file mode 100644
index 0000000..4762928
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
@@ -0,0 +1,30 @@
+From: A. U. Thor
+
+@@ -109,4 +109,11 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+        .getStringList("my", null, "somename")));
+    checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
+  }
++
++ public void test006_readCaseInsensitive() throws IOException {
++   final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
++   RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
++   assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
++   assertEquals("", repositoryConfig.getString("foo", null, "bar"));
++ }
+ }
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
++   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+      return true;
+-   } else if ("no".equals(n) || "false".equals(n) || "0".equals(n)) {
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
new file mode 100644
index 0000000..163357e
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
@@ -0,0 +1,33 @@
+diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+index da7e704..34ce04a 100644
+--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
++++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+@@ -109,4 +109,11 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+        .getStringList("my", null, "somename")));
+    checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
+  }
++
++ public void test006_readCaseInsensitive() throws IOException {
++   final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
++   RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
++   assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
++   assertEquals("", repositoryConfig.getString("foo", null, "bar"));
++ }
+ }
+I AM NOT HERE
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
++   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+      return true;
+-   } else if ("no".equals(n) || "false".equals(n) || "0".equals(n)) {
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
new file mode 100644
index 0000000..e3f3307
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
@@ -0,0 +1,10 @@
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/pushe.png
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetchd.png b/org.spearce.egit.ui/icons/toolbar/fetchd.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..4433c543f2a52b586a3ed5e31b138244107bc239
+GIT binary patch
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetche.png b/org.spearce.egit.ui/icons/toolbar/fetche.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..0ffeb419e6ab302caa5e58661854b33853dc43dc
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
new file mode 100644
index 0000000..6bbb73d
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
@@ -0,0 +1,15 @@
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
+      return true;
+-   } else if ("no".equals(n) || "false".equals(n) || "0".equals(n)) {
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
new file mode 100644
index 0000000..c8fbdc1
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
@@ -0,0 +1,15 @@
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
++   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+      return true;
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java
new file mode 100644
index 0000000..e6f0a03
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java
@@ -0,0 +1,95 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.patch;
+
+import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.util.RawParseUtils;
+
+/** An error in a patch script */
+public class FormatError {
+	/** Classification of an error. */
+	public static enum Severity {
+		/** The error is unexpected, but can be worked around. */
+		WARNING,
+
+		/** The error indicates the script is severely flawed. */
+		ERROR;
+	}
+
+	private final byte[] buf;
+
+	private final int offset;
+
+	private final Severity severity;
+
+	private final String message;
+
+	FormatError(final byte[] buffer, final int ptr, final Severity sev,
+			final String msg) {
+		buf = buffer;
+		offset = ptr;
+		severity = sev;
+		message = msg;
+	}
+
+	/** @return the severity of the error. */
+	public Severity getSeverity() {
+		return severity;
+	}
+
+	/** @return a message describing the error. */
+	public String getMessage() {
+		return message;
+	}
+
+	/** @return the byte buffer holding the patch script. */
+	public byte[] getBuffer() {
+		return buf;
+	}
+
+	/** @return byte offset within {@link #getBuffer()} where the error is */
+	public int getOffset() {
+		return offset;
+	}
+
+	/** @return line of the patch script the error appears on. */
+	public String getLineText() {
+		final int eol = RawParseUtils.nextLF(buf, offset);
+		return RawParseUtils.decode(Constants.CHARSET, buf, offset, eol);
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
index fc606c3..20dd6e2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
@@ -132,7 +132,7 @@ void parseHeader() {
 		newLineCount = parseBase10(buf, ptr.value + 1, ptr);
 	}
 
-	int parseBody() {
+	int parseBody(final Patch script) {
 		final byte[] buf = file.buf;
 		final int sz = buf.length;
 		int c = nextLF(buf, startOffset), last = c;
@@ -175,9 +175,21 @@ int parseBody() {
 			return last;
 		}
 
-		if (nContext + nDeleted != oldLineCount
-				|| nContext + nAdded != newLineCount) {
-			// TODO report on truncated hunk
+		if (nContext + nDeleted < oldLineCount) {
+			final int missingCount = oldLineCount - (nContext + nDeleted);
+			script.error(buf, startOffset, "Truncated hunk, at least "
+					+ missingCount + " old lines is missing");
+
+		} else if (nContext + nAdded < newLineCount) {
+			final int missingCount = newLineCount - (nContext + nAdded);
+			script.error(buf, startOffset, "Truncated hunk, at least "
+					+ missingCount + " new lines is missing");
+
+		} else if (nContext + nDeleted > oldLineCount
+				|| nContext + nAdded > newLineCount) {
+			script.warn(buf, startOffset, "Hunk header " + oldLineCount + ":"
+					+ newLineCount + " does not match body line count of "
+					+ (nContext + nDeleted) + ":" + (nContext + nAdded));
 		}
 
 		return c;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 56eb327..5cc208c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -69,9 +69,13 @@
 	/** The files, in the order they were parsed out of the input. */
 	private final List<FileHeader> files;
 
+	/** Formatting errors, if any were identified. */
+	private final List<FormatError> errors;
+
 	/** Create an empty patch. */
 	public Patch() {
 		files = new ArrayList<FileHeader>();
+		errors = new ArrayList<FormatError>(0);
 	}
 
 	/**
@@ -93,6 +97,21 @@ public void addFile(final FileHeader fh) {
 	}
 
 	/**
+	 * Add a formatting error to this patch script.
+	 * 
+	 * @param err
+	 *            the error description.
+	 */
+	public void addError(final FormatError err) {
+		errors.add(err);
+	}
+
+	/** @return collection of formatting errors, if any. */
+	public List<FormatError> getErrors() {
+		return errors;
+	}
+
+	/**
 	 * Parse a patch received from an InputStream.
 	 * <p>
 	 * Multiple parse calls on the same instance will concatenate the patch
@@ -149,8 +168,7 @@ private int parseFile(final byte[] buf, int c) {
 				// have missed a file header previously. The hunk
 				// isn't valid without knowing where it comes from.
 				//
-
-				// TODO handle a disconnected hunk fragment
+				error(buf, c, "Hunk disconnected from file");
 				c = nextLF(buf, c);
 				continue;
 			}
@@ -218,6 +236,7 @@ private int parseDiffCC(final byte[] buf, final int startOffset) {
 
 		// TODO Support parsing diff --cc headers
 		// TODO parse diff --cc hunks
+		warn(buf, startOffset, "diff --cc format not supported");
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
@@ -259,12 +278,12 @@ private int parseHunks(final FileHeader fh, int c) {
 			if (match(buf, c, HUNK_HDR) >= 0) {
 				final HunkHeader h = new HunkHeader(fh, c);
 				h.parseHeader();
-				c = h.parseBody();
+				c = h.parseBody(this);
 				h.endOffset = c;
 				fh.addHunk(h);
 				if (c < sz && buf[c] != '@' && buf[c] != 'd'
 						&& match(buf, c, SIG_FOOTER) < 0) {
-					// TODO report on noise between hunks, might be an error
+					warn(buf, c, "Unexpected hunk trailer");
 				}
 				continue;
 			}
@@ -308,8 +327,7 @@ private int parseGitBinary(final FileHeader fh, int c) {
 		if (nEnd < 0) {
 			// Not a binary hunk.
 			//
-
-			// TODO handle invalid binary hunks
+			error(fh.buf, c, "Missing forward-image in GIT binary patch");
 			return c;
 		}
 		c = nEnd;
@@ -327,6 +345,14 @@ private int parseGitBinary(final FileHeader fh, int c) {
 		return c;
 	}
 
+	void warn(final byte[] buf, final int ptr, final String msg) {
+		addError(new FormatError(buf, ptr, FormatError.Severity.WARNING, msg));
+	}
+
+	void error(final byte[] buf, final int ptr, final String msg) {
+		addError(new FormatError(buf, ptr, FormatError.Severity.ERROR, msg));
+	}
+
 	private static boolean matchAny(final byte[] buf, final int c,
 			final byte[][] srcs) {
 		for (final byte[] s : srcs) {
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 11/15] Fix Patch.parse to honor the end point passed in
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-11-git-send-email-spearce@spearce.org>

Otherwise we may over-read the patch script and identify trailer
data as part of a patch when it was requested that we ignore it.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/FileHeaderTest.java |   46 +++++++-----
 .../src/org/spearce/jgit/patch/BinaryHunk.java     |    5 +-
 .../src/org/spearce/jgit/patch/FileHeader.java     |   16 ++--
 .../src/org/spearce/jgit/patch/HunkHeader.java     |    9 +--
 .../src/org/spearce/jgit/patch/Patch.java          |   73 ++++++++++----------
 5 files changed, 77 insertions(+), 72 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
index 4c2140a..36c528e 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
@@ -45,28 +45,32 @@
 public class FileHeaderTest extends TestCase {
 	public void testParseGitFileName_Empty() {
 		final FileHeader fh = data("");
-		assertEquals(-1, fh.parseGitFileName(0));
+		assertEquals(-1, fh.parseGitFileName(0, fh.buf.length));
 		assertNotNull(fh.getHunks());
 		assertTrue(fh.getHunks().isEmpty());
 		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_NoLF() {
-		assertEquals(-1, data("a/ b/").parseGitFileName(0));
+		final FileHeader fh = data("a/ b/");
+		assertEquals(-1, fh.parseGitFileName(0, fh.buf.length));
 	}
 
 	public void testParseGitFileName_NoSecondLine() {
-		assertEquals(-1, data("\n").parseGitFileName(0));
+		final FileHeader fh = data("\n");
+		assertEquals(-1, fh.parseGitFileName(0, fh.buf.length));
 	}
 
 	public void testParseGitFileName_EmptyHeader() {
-		assertEquals(1, data("\n\n").parseGitFileName(0));
+		final FileHeader fh = data("\n\n");
+		assertEquals(1, fh.parseGitFileName(0, fh.buf.length));
 	}
 
 	public void testParseGitFileName_Foo() {
 		final String name = "foo";
 		final FileHeader fh = header(name);
-		assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
+		assertEquals(gitLine(name).length(), fh.parseGitFileName(0,
+				fh.buf.length));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -74,7 +78,7 @@ public void testParseGitFileName_Foo() {
 
 	public void testParseGitFileName_FailFooBar() {
 		final FileHeader fh = data("a/foo b/bar\n-");
-		assertTrue(fh.parseGitFileName(0) > 0);
+		assertTrue(fh.parseGitFileName(0, fh.buf.length) > 0);
 		assertNull(fh.getOldName());
 		assertNull(fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -83,7 +87,8 @@ public void testParseGitFileName_FailFooBar() {
 	public void testParseGitFileName_FooSpBar() {
 		final String name = "foo bar";
 		final FileHeader fh = header(name);
-		assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
+		assertEquals(gitLine(name).length(), fh.parseGitFileName(0,
+				fh.buf.length));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -93,7 +98,8 @@ public void testParseGitFileName_DqFooTabBar() {
 		final String name = "foo\tbar";
 		final String dqName = "foo\\tbar";
 		final FileHeader fh = dqHeader(dqName);
-		assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0));
+		assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0,
+				fh.buf.length));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -103,7 +109,8 @@ public void testParseGitFileName_DqFooSpLfNulBar() {
 		final String name = "foo \n\0bar";
 		final String dqName = "foo \\n\\0bar";
 		final FileHeader fh = dqHeader(dqName);
-		assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0));
+		assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0,
+				fh.buf.length));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -112,7 +119,8 @@ public void testParseGitFileName_DqFooSpLfNulBar() {
 	public void testParseGitFileName_SrcFooC() {
 		final String name = "src/foo/bar/argh/code.c";
 		final FileHeader fh = header(name);
-		assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
+		assertEquals(gitLine(name).length(), fh.parseGitFileName(0,
+				fh.buf.length));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -122,7 +130,7 @@ public void testParseGitFileName_SrcFooCNonStandardPrefix() {
 		final String name = "src/foo/bar/argh/code.c";
 		final String header = "project-v-1.0/" + name + " mydev/" + name + "\n";
 		final FileHeader fh = data(header + "-");
-		assertEquals(header.length(), fh.parseGitFileName(0));
+		assertEquals(header.length(), fh.parseGitFileName(0, fh.buf.length));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
 		assertFalse(fh.hasMetaDataChanges());
@@ -202,12 +210,12 @@ public void testParseRename100_NewStyle() {
 				+ "similarity index 100%\n"
 				+ "rename from a\n"
 				+ "rename to \" c/\\303\\205ngstr\\303\\266m\"\n");
-		int ptr = fh.parseGitFileName(0);
+		int ptr = fh.parseGitFileName(0, fh.buf.length);
 		assertTrue(ptr > 0);
 		assertNull(fh.getOldName()); // can't parse names on a rename
 		assertNull(fh.getNewName());
 
-		ptr = fh.parseGitHeaders(ptr);
+		ptr = fh.parseGitHeaders(ptr, fh.buf.length);
 		assertTrue(ptr > 0);
 
 		assertEquals("a", fh.getOldName());
@@ -231,12 +239,12 @@ public void testParseRename100_OldStyle() {
 				+ "similarity index 100%\n"
 				+ "rename old a\n"
 				+ "rename new \" c/\\303\\205ngstr\\303\\266m\"\n");
-		int ptr = fh.parseGitFileName(0);
+		int ptr = fh.parseGitFileName(0, fh.buf.length);
 		assertTrue(ptr > 0);
 		assertNull(fh.getOldName()); // can't parse names on a rename
 		assertNull(fh.getNewName());
 
-		ptr = fh.parseGitHeaders(ptr);
+		ptr = fh.parseGitHeaders(ptr, fh.buf.length);
 		assertTrue(ptr > 0);
 
 		assertEquals("a", fh.getOldName());
@@ -260,12 +268,12 @@ public void testParseCopy100() {
 				+ "similarity index 100%\n"
 				+ "copy from a\n"
 				+ "copy to \" c/\\303\\205ngstr\\303\\266m\"\n");
-		int ptr = fh.parseGitFileName(0);
+		int ptr = fh.parseGitFileName(0, fh.buf.length);
 		assertTrue(ptr > 0);
 		assertNull(fh.getOldName()); // can't parse names on a copy
 		assertNull(fh.getNewName());
 
-		ptr = fh.parseGitHeaders(ptr);
+		ptr = fh.parseGitHeaders(ptr, fh.buf.length);
 		assertTrue(ptr > 0);
 
 		assertEquals("a", fh.getOldName());
@@ -391,9 +399,9 @@ public void testParseAbbrIndexLine_NoMode() {
 	}
 
 	private static void assertParse(final FileHeader fh) {
-		int ptr = fh.parseGitFileName(0);
+		int ptr = fh.parseGitFileName(0, fh.buf.length);
 		assertTrue(ptr > 0);
-		ptr = fh.parseGitHeaders(ptr);
+		ptr = fh.parseGitHeaders(ptr, fh.buf.length);
 		assertTrue(ptr > 0);
 	}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java b/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
index ce35286..3e07ec4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
@@ -91,7 +91,7 @@ public int getSize() {
 		return length;
 	}
 
-	int parseHunk(int ptr) {
+	int parseHunk(int ptr, final int end) {
 		final byte[] buf = file.buf;
 
 		if (match(buf, ptr, LITERAL) >= 0) {
@@ -115,8 +115,7 @@ int parseHunk(int ptr) {
 		// encoded information in this hunk. To save time we don't do a
 		// validation of the binary data at this point.
 		//
-		final int sz = buf.length;
-		while (ptr < sz) {
+		while (ptr < end) {
 			final boolean empty = buf[ptr] == '\n';
 			ptr = nextLF(buf, ptr);
 			if (empty)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index a6ff4a6..a58e978 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -291,12 +291,14 @@ public BinaryHunk getReverseBinaryHunk() {
 	 * 
 	 * @param ptr
 	 *            first character after the "diff --git " or "diff --cc " part.
+	 * @param end
+	 *            one past the last position to parse.
 	 * @return first character after the LF at the end of the line; -1 on error.
 	 */
-	int parseGitFileName(int ptr) {
+	int parseGitFileName(int ptr, final int end) {
 		final int eol = nextLF(buf, ptr);
 		final int bol = ptr;
-		if (eol >= buf.length) {
+		if (eol >= end) {
 			return -1;
 		}
 
@@ -353,9 +355,8 @@ int parseGitFileName(int ptr) {
 		return eol;
 	}
 
-	int parseGitHeaders(int ptr) {
-		final int sz = buf.length;
-		while (ptr < sz) {
+	int parseGitHeaders(int ptr, final int end) {
+		while (ptr < end) {
 			final int eol = nextLF(buf, ptr);
 			if (match(buf, ptr, HUNK_HDR) >= 0) {
 				// First hunk header; break out and parse them later.
@@ -428,9 +429,8 @@ int parseGitHeaders(int ptr) {
 		return ptr;
 	}
 
-	int parseTraditionalHeaders(int ptr) {
-		final int sz = buf.length;
-		while (ptr < sz) {
+	int parseTraditionalHeaders(int ptr, final int end) {
+		while (ptr < end) {
 			final int eol = nextLF(buf, ptr);
 			if (match(buf, ptr, HUNK_HDR) >= 0) {
 				// First hunk header; break out and parse them later.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
index 20dd6e2..4fd9bae 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
@@ -119,7 +119,7 @@ public int getLinesAdded() {
 		return nAdded;
 	}
 
-	void parseHeader() {
+	void parseHeader(final int end) {
 		// Parse "@@ -236,9 +236,9 @@ protected boolean"
 		//
 		final byte[] buf = file.buf;
@@ -132,15 +132,14 @@ void parseHeader() {
 		newLineCount = parseBase10(buf, ptr.value + 1, ptr);
 	}
 
-	int parseBody(final Patch script) {
+	int parseBody(final Patch script, final int end) {
 		final byte[] buf = file.buf;
-		final int sz = buf.length;
 		int c = nextLF(buf, startOffset), last = c;
 
 		nDeleted = 0;
 		nAdded = 0;
 
-		SCAN: for (; c < sz; last = c, c = nextLF(buf, c)) {
+		SCAN: for (; c < end; last = c, c = nextLF(buf, c)) {
 			switch (buf[c]) {
 			case ' ':
 			case '\n':
@@ -163,7 +162,7 @@ int parseBody(final Patch script) {
 			}
 		}
 
-		if (last < sz && nContext + nDeleted - 1 == oldLineCount
+		if (last < end && nContext + nDeleted - 1 == oldLineCount
 				&& nContext + nAdded == newLineCount
 				&& match(buf, last, Patch.SIG_FOOTER) >= 0) {
 			// This is an extremely common occurrence of "corruption".
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 5cc208c..9aca22d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -157,12 +157,11 @@ public void parse(final InputStream is) throws IOException {
 	 */
 	public void parse(final byte[] buf, int ptr, final int end) {
 		while (ptr < end)
-			ptr = parseFile(buf, ptr);
+			ptr = parseFile(buf, ptr, end);
 	}
 
-	private int parseFile(final byte[] buf, int c) {
-		final int sz = buf.length;
-		while (c < sz) {
+	private int parseFile(final byte[] buf, int c, final int end) {
+		while (c < end) {
 			if (match(buf, c, HUNK_HDR) >= 0) {
 				// If we find a disconnected hunk header we might
 				// have missed a file header previously. The hunk
@@ -176,19 +175,19 @@ private int parseFile(final byte[] buf, int c) {
 			// Valid git style patch?
 			//
 			if (match(buf, c, DIFF_GIT) >= 0)
-				return parseDiffGit(buf, c);
+				return parseDiffGit(buf, c, end);
 			if (match(buf, c, DIFF_CC) >= 0)
-				return parseDiffCC(buf, c);
+				return parseDiffCC(buf, c, end);
 
 			// Junk between files? Leading junk? Traditional
 			// (non-git generated) patch?
 			//
 			final int n = nextLF(buf, c);
-			if (n >= sz) {
+			if (n >= end) {
 				// Patches cannot be only one line long. This must be
 				// trailing junk that we should ignore.
 				//
-				return sz;
+				return end;
 			}
 
 			if (n - c < 6) {
@@ -204,10 +203,10 @@ private int parseFile(final byte[] buf, int c) {
 				// a "@@ -0,0" smelling line next. We only check the "@@ -".
 				//
 				final int f = nextLF(buf, n);
-				if (f >= sz)
-					return sz;
+				if (f >= end)
+					return end;
 				if (match(buf, f, HUNK_HDR) >= 0)
-					return parseTraditionalPatch(buf, c);
+					return parseTraditionalPatch(buf, c, end);
 			}
 
 			c = n;
@@ -215,53 +214,53 @@ private int parseFile(final byte[] buf, int c) {
 		return c;
 	}
 
-	private int parseDiffGit(final byte[] buf, final int startOffset) {
-		final FileHeader fh = new FileHeader(buf, startOffset);
-		int ptr = fh.parseGitFileName(startOffset + DIFF_GIT.length);
+	private int parseDiffGit(final byte[] buf, final int start, final int end) {
+		final FileHeader fh = new FileHeader(buf, start);
+		int ptr = fh.parseGitFileName(start + DIFF_GIT.length, end);
 		if (ptr < 0)
-			return skipFile(buf, startOffset);
+			return skipFile(buf, start, end);
 
-		ptr = fh.parseGitHeaders(ptr);
-		ptr = parseHunks(fh, ptr);
+		ptr = fh.parseGitHeaders(ptr, end);
+		ptr = parseHunks(fh, ptr, end);
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
 	}
 
-	private int parseDiffCC(final byte[] buf, final int startOffset) {
-		final FileHeader fh = new FileHeader(buf, startOffset);
-		int ptr = fh.parseGitFileName(startOffset + DIFF_CC.length);
+	private int parseDiffCC(final byte[] buf, final int start, final int end) {
+		final FileHeader fh = new FileHeader(buf, start);
+		int ptr = fh.parseGitFileName(start + DIFF_CC.length, end);
 		if (ptr < 0)
-			return skipFile(buf, startOffset);
+			return skipFile(buf, start, end);
 
 		// TODO Support parsing diff --cc headers
 		// TODO parse diff --cc hunks
-		warn(buf, startOffset, "diff --cc format not supported");
+		warn(buf, start, "diff --cc format not supported");
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
 	}
 
-	private int parseTraditionalPatch(final byte[] buf, final int startOffset) {
-		final FileHeader fh = new FileHeader(buf, startOffset);
-		int ptr = fh.parseTraditionalHeaders(startOffset);
-		ptr = parseHunks(fh, ptr);
+	private int parseTraditionalPatch(final byte[] buf, final int start,
+			final int end) {
+		final FileHeader fh = new FileHeader(buf, start);
+		int ptr = fh.parseTraditionalHeaders(start, end);
+		ptr = parseHunks(fh, ptr, end);
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
 	}
 
-	private static int skipFile(final byte[] buf, int ptr) {
+	private static int skipFile(final byte[] buf, int ptr, final int end) {
 		ptr = nextLF(buf, ptr);
 		if (match(buf, ptr, OLD_NAME) >= 0)
 			ptr = nextLF(buf, ptr);
 		return ptr;
 	}
 
-	private int parseHunks(final FileHeader fh, int c) {
+	private int parseHunks(final FileHeader fh, int c, final int end) {
 		final byte[] buf = fh.buf;
-		final int sz = buf.length;
-		while (c < sz) {
+		while (c < end) {
 			// If we see a file header at this point, we have all of the
 			// hunks for our current file. We should stop and report back
 			// with this position so it can be parsed again later.
@@ -277,11 +276,11 @@ private int parseHunks(final FileHeader fh, int c) {
 
 			if (match(buf, c, HUNK_HDR) >= 0) {
 				final HunkHeader h = new HunkHeader(fh, c);
-				h.parseHeader();
-				c = h.parseBody(this);
+				h.parseHeader(end);
+				c = h.parseBody(this, end);
 				h.endOffset = c;
 				fh.addHunk(h);
-				if (c < sz && buf[c] != '@' && buf[c] != 'd'
+				if (c < end && buf[c] != '@' && buf[c] != 'd'
 						&& match(buf, c, SIG_FOOTER) < 0) {
 					warn(buf, c, "Unexpected hunk trailer");
 				}
@@ -291,7 +290,7 @@ private int parseHunks(final FileHeader fh, int c) {
 			final int eol = nextLF(buf, c);
 			if (fh.getHunks().isEmpty() && match(buf, c, GIT_BINARY) >= 0) {
 				fh.patchType = FileHeader.PatchType.GIT_BINARY;
-				return parseGitBinary(fh, eol);
+				return parseGitBinary(fh, eol, end);
 			}
 
 			if (fh.getHunks().isEmpty() && BIN_TRAILER.length < eol - c
@@ -321,9 +320,9 @@ private int parseHunks(final FileHeader fh, int c) {
 		return c;
 	}
 
-	private int parseGitBinary(final FileHeader fh, int c) {
+	private int parseGitBinary(final FileHeader fh, int c, final int end) {
 		final BinaryHunk postImage = new BinaryHunk(fh, c);
-		final int nEnd = postImage.parseHunk(c);
+		final int nEnd = postImage.parseHunk(c, end);
 		if (nEnd < 0) {
 			// Not a binary hunk.
 			//
@@ -335,7 +334,7 @@ private int parseGitBinary(final FileHeader fh, int c) {
 		fh.forwardBinaryHunk = postImage;
 
 		final BinaryHunk preImage = new BinaryHunk(fh, c);
-		final int oEnd = preImage.parseHunk(c);
+		final int oEnd = preImage.parseHunk(c, end);
 		if (oEnd >= 0) {
 			c = oEnd;
 			preImage.endOffset = c;
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 09/15] Parse "GIT binary patch" style patch metadata
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-9-git-send-email-spearce@spearce.org>

Git can produce binary patches that carry the full content or
delta information encoded in a base-85 text payload.  We parse
the headers out and produce a BinaryHunk wrapper for these so
applications can identify that the hunk exists, but we do not
attempt to decode the binary information.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/PatchTest.java      |   42 ++++++
 .../spearce/jgit/patch/testParse_GitBinary.patch   |  135 ++++++++++++++++++++
 .../src/org/spearce/jgit/patch/BinaryHunk.java     |  128 +++++++++++++++++++
 .../src/org/spearce/jgit/patch/FileHeader.java     |   16 +++
 .../src/org/spearce/jgit/patch/Patch.java          |   32 +++++
 5 files changed, 353 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_GitBinary.patch
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
index bf37063..453d88e 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
@@ -43,6 +43,7 @@
 import junit.framework.TestCase;
 
 import org.spearce.jgit.lib.FileMode;
+import org.spearce.jgit.lib.ObjectId;
 
 public class PatchTest extends TestCase {
 	public void testEmpty() {
@@ -157,6 +158,45 @@ assertTrue(fh.getNewName().startsWith(
 			assertSame(FileHeader.PatchType.BINARY, fh.getPatchType());
 			assertTrue(fh.getHunks().isEmpty());
 			assertTrue(fh.hasMetaDataChanges());
+
+			assertNull(fh.getForwardBinaryHunk());
+			assertNull(fh.getReverseBinaryHunk());
+		}
+
+		final FileHeader fh = p.getFiles().get(4);
+		assertEquals("org.spearce.egit.ui/plugin.xml", fh.getNewName());
+		assertSame(FileHeader.ChangeType.MODIFY, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertFalse(fh.hasMetaDataChanges());
+		assertEquals("ee8a5a0", fh.getNewId().name());
+		assertNull(fh.getForwardBinaryHunk());
+		assertNull(fh.getReverseBinaryHunk());
+		assertEquals(1, fh.getHunks().size());
+		assertEquals(272, fh.getHunks().get(0).getOldStartLine());
+	}
+
+	public void testParse_GitBinary() throws IOException {
+		final Patch p = parseTestPatchFile();
+		final int[] binsizes = { 359, 393, 372, 404 };
+		assertEquals(5, p.getFiles().size());
+
+		for (int i = 0; i < 4; i++) {
+			final FileHeader fh = p.getFiles().get(i);
+			assertSame(FileHeader.ChangeType.ADD, fh.getChangeType());
+			assertNotNull(fh.getOldId());
+			assertNotNull(fh.getNewId());
+			assertEquals(ObjectId.zeroId().name(), fh.getOldId().name());
+			assertSame(FileMode.REGULAR_FILE, fh.getNewMode());
+			assertTrue(fh.getNewName().startsWith(
+					"org.spearce.egit.ui/icons/toolbar/"));
+			assertSame(FileHeader.PatchType.GIT_BINARY, fh.getPatchType());
+			assertTrue(fh.getHunks().isEmpty());
+			assertTrue(fh.hasMetaDataChanges());
+
+			assertNotNull(fh.getForwardBinaryHunk());
+			assertNotNull(fh.getReverseBinaryHunk());
+			assertEquals(binsizes[i], fh.getForwardBinaryHunk().getSize());
+			assertEquals(0, fh.getReverseBinaryHunk().getSize());
 		}
 
 		final FileHeader fh = p.getFiles().get(4);
@@ -165,6 +205,8 @@ assertTrue(fh.getNewName().startsWith(
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 		assertFalse(fh.hasMetaDataChanges());
 		assertEquals("ee8a5a0", fh.getNewId().name());
+		assertNull(fh.getForwardBinaryHunk());
+		assertNull(fh.getReverseBinaryHunk());
 		assertEquals(1, fh.getHunks().size());
 		assertEquals(272, fh.getHunks().get(0).getOldStartLine());
 	}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_GitBinary.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_GitBinary.patch
new file mode 100644
index 0000000..ab7b235
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_GitBinary.patch
@@ -0,0 +1,135 @@
+From 8363f12135a7d0ff0b5fea7d5a35d294c0479518 Mon Sep 17 00:00:00 2001
+From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
+Date: Tue, 23 Sep 2008 22:19:19 +0200
+Subject: [PATCH] Push and fetch icons for the toolbar
+
+Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
+Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
+---
+ org.spearce.egit.ui/icons/toolbar/fetchd.png |  Bin 0 -> 359 bytes
+ org.spearce.egit.ui/icons/toolbar/fetche.png |  Bin 0 -> 393 bytes
+ org.spearce.egit.ui/icons/toolbar/pushd.png  |  Bin 0 -> 372 bytes
+ org.spearce.egit.ui/icons/toolbar/pushe.png  |  Bin 0 -> 404 bytes
+ org.spearce.egit.ui/plugin.xml               |   32 ++++++++++++++-----------
+ 5 files changed, 18 insertions(+), 14 deletions(-)
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/fetchd.png
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/fetche.png
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/pushd.png
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/pushe.png
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetchd.png b/org.spearce.egit.ui/icons/toolbar/fetchd.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..4433c543f2a52b586a3ed5e31b138244107bc239
+GIT binary patch
+literal 359
+zcmV-t0hs=YP)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV0003lNkl<ZIE|%~
+zF{;8q6h+U7BBn4qm_`dxuy6}@!NzsCfLTKp;5yPsN=p|I(rKe&p~M74Bq5ObRRW{%
+zje4sYc=ybGk2gYV%`a+{hvjnl6{!Lo4hNJ{A44jDl#(C_K2BmJ09dV7A3>VW2}+n!
+zO1rKr0DRxqm&=7b&q>piUayDaIKlvw2}+e_&-3(PFkmzq(Q380-|r+zg6Da9KA(#Q
+zPa2QMl^Gz*GK3HmMPby1VQ9_(U^bf>W`H=3+3)uhMZxuY<#al++wJIfyXFi47K?>p
+z2GCj~rNp*vI-L&fb{osGh@!|$@ci;S2_arv_(sil1xb<+1Odb0kjLY}`F!ShJaW6;
+z>Lr*?r);;|7ihoV2SC*MjhoFzuh;A9KAB9aMXCk(Pk$Loi}X0uxcmSB002ovPDHLk
+FV1lxPoI3yj
+
+literal 0
+HcmV?d00001
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetche.png b/org.spearce.egit.ui/icons/toolbar/fetche.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..0ffeb419e6ab302caa5e58661854b33853dc43dc
+GIT binary patch
+literal 393
+zcmV;40e1e0P)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV0003{Nkl<ZIE|%}
+z%Su8~6o$WjJd|h&)kLg8qatclklHDbhzOh%4SI>*L3{^aqJcvRqCsRQ6~SndH7ExK
+zk=KcyE?#y6(aupYt$(ujUi|ChXYD1Vl>A3Z=W-tL|B2KE=%pm#uoxNA1!yxqxEMW&
+zB>{jQO^yT+ogtn_{8Ep$Aq3h-C?o|y>g-6?-!e46K4}{7I2X6^?w$w$wKqXWo#uE<
+zlN$@u$mIiCW0N$hIYc2#Jf_L5pe_`875HfeP>nhW1zLv1R!iSvNdTZ7`q(*62#YbV
+zQhB;#V#z_Hl;tD;jPm%3!!_Fv=xqj&EpW_lqPo^m>_wFE9KxQ3t1@8v1#@h(gk?2k
+zU%h_@BTD_vVB{6b=^Lij^3<ya#!DI7eU*yg9xg#(&qL<HX{n_QH=dOmU|OU>Dkk>j
+n^=YB|UiI3T3toz$0fY1nZ1068v8@+b00000NkvXXu0mjfWwNMg
+
+literal 0
+HcmV?d00001
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/pushd.png b/org.spearce.egit.ui/icons/toolbar/pushd.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..22c3f7bf17b8b5a931c7527d768af13607b03bce
+GIT binary patch
+literal 372
+zcmV-)0gL{LP)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV0003yNkl<ZIE|%~
+zF{%PF6h%)&8w)Kgg)~~o6c%=-wG$iH;R0NP3veAG$Sh>A(?&%>K?5S92zkHxGyeyN
+zQSnxRymxZ%OQJ-CZ<LQ0VHnEcaNzNHaJ${8)oOIRUG)l}M1;v?B8^6aVzEe}P~dX8
+zV6A1h+tKg$Ga)&E`~8km3g?`+IiJs8M#ur2PA68Y70x-1$0OVAmgRCusZ@FoAR=h3
+zDVNJsDix~LD)oBZD;$r<sngD7(Utm(zh18y4u?;WOu&C>t%;)O$w?l-T1yl~1VO;{
+zdS$=gv)ODopU<8HfZ1#YAcMg`B@Q~B4vWRYJJDL}%|UCO8Yd6XZnu?)$aFeQidwCf
+z_mE--u|}hjN&o=H7-fukIg4hqnKXNVchu|kh_lCf`xbzwX88RJ-{>O;Y5D>6^@Sy#
+SDlMe|0000<MNUMnLSTZnn4{zX
+
+literal 0
+HcmV?d00001
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/pushe.png b/org.spearce.egit.ui/icons/toolbar/pushe.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..1f99f0a702983d674f15eedae5f1218f0a30b5a0
+GIT binary patch
+literal 404
+zcmV;F0c-w=P)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00047Nkl<ZIE|%~
+z%Su8~6o$WjJO~|4C>w|sG%6%VHSk(UL<B+v4SI>*L41dvA&3Z?G>L*BB*n0rWDTl8
+zL8Pb?Jzc!)CSJ}#$rF8}#a?Uu`(JCbg_M&2pmu`H$+oP&=V*R^(bPY1%&ibu+ZV$G
+zgp`tt<A@B;jw3Z6E&C{q>NBF4=c=f%6i@vsq5!CR9fSfc-IT0lZ>^0`E2vbS?r{1v
+z8l^m+g%^~^H#FDePyq!%wm_SSqPmu`yJKk6QAXzdroz+R(7<gg074jZz1Vo3Dy2y#
+zMW2W=)MJ~7I|%3fPE-KBpis_UGqzZuUe(cG%h>L#RCJHY0YK_74TR+C&ZX!&h^>3c
+zJvdA^W^@l;f6eS*z&I*^D|{frVpE>&7273F76LY=;y1$BWF(Q0qALI}5jqkZAq&fh
+y^_oorR)}l`>CE22@+$y+&Cvb}|KU##2Jr)k?t0Dap2#Es0000<MNUMnLSTZgH?cGT
+
+literal 0
+HcmV?d00001
+
+diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
+index 7c98688..ee8a5a0 100644
+--- a/org.spearce.egit.ui/plugin.xml
++++ b/org.spearce.egit.ui/plugin.xml
+@@ -272,22 +272,26 @@
+         </separator>
+ 	    </menu>
+ 		<action
+-		       class="org.spearce.egit.ui.internal.actions.FetchAction"
+-		       id="org.spearce.egit.ui.actionfetch"
+-		       label="%FetchAction_label"
+-		       style="push"
+-		       menubarPath="org.spearce.egit.ui.gitmenu/repo"
+-		       toolbarPath="org.spearce.egit.ui"
+-		       tooltip="%FetchAction_tooltip">
++        class="org.spearce.egit.ui.internal.actions.FetchAction"
++        disabledIcon="icons/toolbar/fetchd.png"
++        icon="icons/toolbar/fetche.png"
++        id="org.spearce.egit.ui.actionfetch"
++        label="%FetchAction_label"
++        menubarPath="org.spearce.egit.ui.gitmenu/repo"
++        style="push"
++        toolbarPath="org.spearce.egit.ui"
++        tooltip="%FetchAction_tooltip">
+ 		</action>
+ 		<action
+-		       class="org.spearce.egit.ui.internal.actions.PushAction"
+-		       id="org.spearce.egit.ui.actionpush"
+-		       label="%PushAction_label"
+-		       style="push"
+-		       menubarPath="org.spearce.egit.ui.gitmenu/repo"
+-		       toolbarPath="org.spearce.egit.ui"
+-		       tooltip="%PushAction_tooltip">
++        class="org.spearce.egit.ui.internal.actions.PushAction"
++        disabledIcon="icons/toolbar/pushd.png"
++        icon="icons/toolbar/pushe.png"
++        id="org.spearce.egit.ui.actionpush"
++        label="%PushAction_label"
++        menubarPath="org.spearce.egit.ui.gitmenu/repo"
++        style="push"
++        toolbarPath="org.spearce.egit.ui"
++        tooltip="%PushAction_tooltip">
+ 		</action>
+ 		<action
+ 		       class="org.spearce.egit.ui.internal.actions.BranchAction"
+-- 
+1.6.1.rc2.306.ge5d5e
+
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java b/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
new file mode 100644
index 0000000..ce35286
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.patch;
+
+import static org.spearce.jgit.lib.Constants.encodeASCII;
+import static org.spearce.jgit.util.RawParseUtils.match;
+import static org.spearce.jgit.util.RawParseUtils.nextLF;
+import static org.spearce.jgit.util.RawParseUtils.parseBase10;
+
+/** Part of a "GIT binary patch" to describe the pre-image or post-image */
+public class BinaryHunk {
+	private static final byte[] LITERAL = encodeASCII("literal ");
+
+	private static final byte[] DELTA = encodeASCII("delta ");
+
+	/** Type of information stored in a binary hunk. */
+	public static enum Type {
+		/** The full content is stored, deflated. */
+		LITERAL_DEFLATED,
+
+		/** A Git pack-style delta is stored, deflated. */
+		DELTA_DEFLATED;
+	}
+
+	private final FileHeader file;
+
+	/** Offset within {@link #file}.buf to the "literal" or "delta " line. */
+	final int startOffset;
+
+	/** Position 1 past the end of this hunk within {@link #file}'s buf. */
+	int endOffset;
+
+	/** Type of the data meaning. */
+	private Type type;
+
+	/** Inflated length of the data. */
+	private int length;
+
+	BinaryHunk(final FileHeader fh, final int offset) {
+		file = fh;
+		startOffset = offset;
+	}
+
+	/** @return header for the file this hunk applies to */
+	public FileHeader getFileHeader() {
+		return file;
+	}
+
+	/** @return type of this binary hunk */
+	public Type getType() {
+		return type;
+	}
+
+	/** @return inflated size of this hunk's data */
+	public int getSize() {
+		return length;
+	}
+
+	int parseHunk(int ptr) {
+		final byte[] buf = file.buf;
+
+		if (match(buf, ptr, LITERAL) >= 0) {
+			type = Type.LITERAL_DEFLATED;
+			length = parseBase10(buf, ptr + LITERAL.length, null);
+
+		} else if (match(buf, ptr, DELTA) >= 0) {
+			type = Type.DELTA_DEFLATED;
+			length = parseBase10(buf, ptr + LITERAL.length, null);
+
+		} else {
+			// Not a valid binary hunk. Signal to the caller that
+			// we cannot parse any further and that this line should
+			// be treated otherwise.
+			//
+			return -1;
+		}
+		ptr = nextLF(buf, ptr);
+
+		// Skip until the first blank line; that is the end of the binary
+		// encoded information in this hunk. To save time we don't do a
+		// validation of the binary data at this point.
+		//
+		final int sz = buf.length;
+		while (ptr < sz) {
+			final boolean empty = buf[ptr] == '\n';
+			ptr = nextLF(buf, ptr);
+			if (empty)
+				break;
+		}
+
+		return ptr;
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index bf8d23a..a6ff4a6 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -158,6 +158,12 @@
 	/** The hunks of this file */
 	private List<HunkHeader> hunks;
 
+	/** If {@link #patchType} is {@link PatchType#GIT_BINARY}, the new image */
+	BinaryHunk forwardBinaryHunk;
+
+	/** If {@link #patchType} is {@link PatchType#GIT_BINARY}, the old image */
+	BinaryHunk reverseBinaryHunk;
+
 	FileHeader(final byte[] b, final int offset) {
 		buf = b;
 		startOffset = offset;
@@ -270,6 +276,16 @@ void addHunk(final HunkHeader h) {
 		hunks.add(h);
 	}
 
+	/** @return if a {@link PatchType#GIT_BINARY}, the new-image delta/literal */
+	public BinaryHunk getForwardBinaryHunk() {
+		return forwardBinaryHunk;
+	}
+
+	/** @return if a {@link PatchType#GIT_BINARY}, the old-image delta/literal */
+	public BinaryHunk getReverseBinaryHunk() {
+		return reverseBinaryHunk;
+	}
+
 	/**
 	 * Parse a "diff --git" or "diff --cc" line.
 	 * 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 164ad96..56eb327 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -62,6 +62,8 @@
 
 	private static final byte[] BIN_TRAILER = encodeASCII(" differ\n");
 
+	private static final byte[] GIT_BINARY = encodeASCII("GIT binary patch\n");
+
 	static final byte[] SIG_FOOTER = encodeASCII("-- \n");
 
 	/** The files, in the order they were parsed out of the input. */
@@ -268,6 +270,11 @@ private int parseHunks(final FileHeader fh, int c) {
 			}
 
 			final int eol = nextLF(buf, c);
+			if (fh.getHunks().isEmpty() && match(buf, c, GIT_BINARY) >= 0) {
+				fh.patchType = FileHeader.PatchType.GIT_BINARY;
+				return parseGitBinary(fh, eol);
+			}
+
 			if (fh.getHunks().isEmpty() && BIN_TRAILER.length < eol - c
 					&& match(buf, eol - BIN_TRAILER.length, BIN_TRAILER) >= 0
 					&& matchAny(buf, c, BIN_HEADERS)) {
@@ -295,6 +302,31 @@ private int parseHunks(final FileHeader fh, int c) {
 		return c;
 	}
 
+	private int parseGitBinary(final FileHeader fh, int c) {
+		final BinaryHunk postImage = new BinaryHunk(fh, c);
+		final int nEnd = postImage.parseHunk(c);
+		if (nEnd < 0) {
+			// Not a binary hunk.
+			//
+
+			// TODO handle invalid binary hunks
+			return c;
+		}
+		c = nEnd;
+		postImage.endOffset = c;
+		fh.forwardBinaryHunk = postImage;
+
+		final BinaryHunk preImage = new BinaryHunk(fh, c);
+		final int oEnd = preImage.parseHunk(c);
+		if (oEnd >= 0) {
+			c = oEnd;
+			preImage.endOffset = c;
+			fh.reverseBinaryHunk = preImage;
+		}
+
+		return c;
+	}
+
 	private static boolean matchAny(final byte[] buf, final int c,
 			final byte[][] srcs) {
 		for (final byte[] s : srcs) {
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 07/15] Set empty patches with no Git metadata to PatchType.BINARY
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-7-git-send-email-spearce@spearce.org>

If a patch has no Git specific metadata and it has no hunks then
it is very likely a binary patch with a "Binary files ... differ"
warning message in a different language, or the message has been
mangled by an editor.  We should consider such patches to be the
same as a binary patch, as there is nothing here to perform an
action on.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/FileHeaderTest.java |   18 ++++++++++++++++++
 .../tst/org/spearce/jgit/patch/PatchTest.java      |    2 ++
 .../src/org/spearce/jgit/patch/FileHeader.java     |    5 +++++
 .../src/org/spearce/jgit/patch/Patch.java          |   18 ++++++++++++++----
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
index d8696a9..4c2140a 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
@@ -48,6 +48,7 @@ public void testParseGitFileName_Empty() {
 		assertEquals(-1, fh.parseGitFileName(0));
 		assertNotNull(fh.getHunks());
 		assertTrue(fh.getHunks().isEmpty());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_NoLF() {
@@ -68,6 +69,7 @@ public void testParseGitFileName_Foo() {
 		assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_FailFooBar() {
@@ -75,6 +77,7 @@ public void testParseGitFileName_FailFooBar() {
 		assertTrue(fh.parseGitFileName(0) > 0);
 		assertNull(fh.getOldName());
 		assertNull(fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_FooSpBar() {
@@ -83,6 +86,7 @@ public void testParseGitFileName_FooSpBar() {
 		assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_DqFooTabBar() {
@@ -92,6 +96,7 @@ public void testParseGitFileName_DqFooTabBar() {
 		assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_DqFooSpLfNulBar() {
@@ -101,6 +106,7 @@ public void testParseGitFileName_DqFooSpLfNulBar() {
 		assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_SrcFooC() {
@@ -109,6 +115,7 @@ public void testParseGitFileName_SrcFooC() {
 		assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseGitFileName_SrcFooCNonStandardPrefix() {
@@ -118,6 +125,7 @@ public void testParseGitFileName_SrcFooCNonStandardPrefix() {
 		assertEquals(header.length(), fh.parseGitFileName(0));
 		assertEquals(name, fh.getOldName());
 		assertSame(fh.getOldName(), fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 	}
 
 	public void testParseUnicodeName_NewFile() {
@@ -135,6 +143,7 @@ public void testParseUnicodeName_NewFile() {
 
 		assertSame(FileHeader.ChangeType.ADD, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertTrue(fh.hasMetaDataChanges());
 
 		assertNull(fh.getOldMode());
 		assertSame(FileMode.REGULAR_FILE, fh.getNewMode());
@@ -159,6 +168,7 @@ public void testParseUnicodeName_DeleteFile() {
 
 		assertSame(FileHeader.ChangeType.DELETE, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertTrue(fh.hasMetaDataChanges());
 
 		assertSame(FileMode.REGULAR_FILE, fh.getOldMode());
 		assertNull(fh.getNewMode());
@@ -177,6 +187,7 @@ public void testParseModeChange() {
 
 		assertSame(FileHeader.ChangeType.MODIFY, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertTrue(fh.hasMetaDataChanges());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -204,6 +215,7 @@ public void testParseRename100_NewStyle() {
 
 		assertSame(FileHeader.ChangeType.RENAME, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertTrue(fh.hasMetaDataChanges());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -232,6 +244,7 @@ public void testParseRename100_OldStyle() {
 
 		assertSame(FileHeader.ChangeType.RENAME, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertTrue(fh.hasMetaDataChanges());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -260,6 +273,7 @@ public void testParseCopy100() {
 
 		assertSame(FileHeader.ChangeType.COPY, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertTrue(fh.hasMetaDataChanges());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -282,6 +296,7 @@ public void testParseFullIndexLine_WithMode() {
 
 		assertSame(FileMode.REGULAR_FILE, fh.getOldMode());
 		assertSame(FileMode.REGULAR_FILE, fh.getNewMode());
+		assertFalse(fh.hasMetaDataChanges());
 
 		assertNotNull(fh.getOldId());
 		assertNotNull(fh.getNewId());
@@ -302,6 +317,7 @@ public void testParseFullIndexLine_NoMode() {
 
 		assertEquals("a", fh.getOldName());
 		assertEquals("a", fh.getNewName());
+		assertFalse(fh.hasMetaDataChanges());
 
 		assertNull(fh.getOldMode());
 		assertNull(fh.getNewMode());
@@ -330,6 +346,7 @@ public void testParseAbbrIndexLine_WithMode() {
 
 		assertSame(FileMode.REGULAR_FILE, fh.getOldMode());
 		assertSame(FileMode.REGULAR_FILE, fh.getNewMode());
+		assertFalse(fh.hasMetaDataChanges());
 
 		assertNotNull(fh.getOldId());
 		assertNotNull(fh.getNewId());
@@ -358,6 +375,7 @@ public void testParseAbbrIndexLine_NoMode() {
 
 		assertNull(fh.getOldMode());
 		assertNull(fh.getNewMode());
+		assertFalse(fh.hasMetaDataChanges());
 
 		assertNotNull(fh.getOldId());
 		assertNotNull(fh.getNewId());
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
index 833bf5d..bf37063 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
@@ -156,12 +156,14 @@ assertTrue(fh.getNewName().startsWith(
 					"org.spearce.egit.ui/icons/toolbar/"));
 			assertSame(FileHeader.PatchType.BINARY, fh.getPatchType());
 			assertTrue(fh.getHunks().isEmpty());
+			assertTrue(fh.hasMetaDataChanges());
 		}
 
 		final FileHeader fh = p.getFiles().get(4);
 		assertEquals("org.spearce.egit.ui/plugin.xml", fh.getNewName());
 		assertSame(FileHeader.ChangeType.MODIFY, fh.getChangeType());
 		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertFalse(fh.hasMetaDataChanges());
 		assertEquals("ee8a5a0", fh.getNewId().name());
 		assertEquals(1, fh.getHunks().size());
 		assertEquals(272, fh.getHunks().get(0).getOldStartLine());
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index 4bb6b7e..bf8d23a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -250,6 +250,11 @@ public PatchType getPatchType() {
 		return patchType;
 	}
 
+	/** @return true if this patch modifies metadata about a file */
+	public boolean hasMetaDataChanges() {
+		return changeType != ChangeType.MODIFY || newMode != oldMode;
+	}
+
 	/** @return hunks altering this file; in order of appearance in patch */
 	public List<HunkHeader> getHunks() {
 		if (hunks == null)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 6e9ae77..c940a00 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -245,13 +245,13 @@ private int parseHunks(final FileHeader fh, final byte[] buf, int c) {
 			// with this position so it can be parsed again later.
 			//
 			if (match(buf, c, DIFF_GIT) >= 0)
-				return c;
+				break;
 			if (match(buf, c, DIFF_CC) >= 0)
-				return c;
+				break;
 			if (match(buf, c, OLD_NAME) >= 0)
-				return c;
+				break;
 			if (match(buf, c, NEW_NAME) >= 0)
-				return c;
+				break;
 
 			if (match(buf, c, HUNK_HDR) >= 0) {
 				final HunkHeader h = new HunkHeader(fh, c);
@@ -281,6 +281,16 @@ private int parseHunks(final FileHeader fh, final byte[] buf, int c) {
 			//
 			c = eol;
 		}
+
+		if (fh.getHunks().isEmpty()
+				&& fh.getPatchType() == FileHeader.PatchType.UNIFIED
+				&& !fh.hasMetaDataChanges()) {
+			// Hmm, an empty patch? If there is no metadata here we
+			// really have a binary patch that we didn't notice above.
+			//
+			fh.patchType = FileHeader.PatchType.BINARY;
+		}
+
 		return c;
 	}
 
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 08/15] Always use the FileHeader buffer during Patch.parseHunks
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-8-git-send-email-spearce@spearce.org>

This ensures that if the code is ever refactored in the future to
permit different entry path into parseHunks that we won't read the
wrong buffer and potentially setup an invalid FileHeader.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/patch/Patch.java          |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index c940a00..164ad96 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -202,7 +202,7 @@ private int parseDiffGit(final byte[] buf, final int startOffset) {
 			return skipFile(buf, startOffset);
 
 		ptr = fh.parseGitHeaders(ptr);
-		ptr = parseHunks(fh, buf, ptr);
+		ptr = parseHunks(fh, ptr);
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
@@ -224,7 +224,7 @@ private int parseDiffCC(final byte[] buf, final int startOffset) {
 	private int parseTraditionalPatch(final byte[] buf, final int startOffset) {
 		final FileHeader fh = new FileHeader(buf, startOffset);
 		int ptr = fh.parseTraditionalHeaders(startOffset);
-		ptr = parseHunks(fh, buf, ptr);
+		ptr = parseHunks(fh, ptr);
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
@@ -237,7 +237,8 @@ private static int skipFile(final byte[] buf, int ptr) {
 		return ptr;
 	}
 
-	private int parseHunks(final FileHeader fh, final byte[] buf, int c) {
+	private int parseHunks(final FileHeader fh, int c) {
+		final byte[] buf = fh.buf;
 		final int sz = buf.length;
 		while (c < sz) {
 			// If we see a file header at this point, we have all of the
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 06/15] Test for non-git binary files and mark them as PatchType.BINARY
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-6-git-send-email-spearce@spearce.org>

By marking non-git binary patches as binary callers can examine
the patch type to determine if there is meaningful content on
this particular file.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/PatchTest.java      |   26 ++++++
 .../spearce/jgit/patch/testParse_NoBinary.patch    |   83 ++++++++++++++++++++
 .../src/org/spearce/jgit/patch/FileHeader.java     |    2 +-
 .../src/org/spearce/jgit/patch/Patch.java          |   26 ++++++-
 4 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_NoBinary.patch

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
index 3afe0a1..833bf5d 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
@@ -141,6 +141,32 @@ assertSame(FileHeader.PatchType.UNIFIED, fRepositoryConfig
 		}
 	}
 
+	public void testParse_NoBinary() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(5, p.getFiles().size());
+
+		for (int i = 0; i < 4; i++) {
+			final FileHeader fh = p.getFiles().get(i);
+			assertSame(FileHeader.ChangeType.ADD, fh.getChangeType());
+			assertNotNull(fh.getOldId());
+			assertNotNull(fh.getNewId());
+			assertEquals("0000000", fh.getOldId().name());
+			assertSame(FileMode.REGULAR_FILE, fh.getNewMode());
+			assertTrue(fh.getNewName().startsWith(
+					"org.spearce.egit.ui/icons/toolbar/"));
+			assertSame(FileHeader.PatchType.BINARY, fh.getPatchType());
+			assertTrue(fh.getHunks().isEmpty());
+		}
+
+		final FileHeader fh = p.getFiles().get(4);
+		assertEquals("org.spearce.egit.ui/plugin.xml", fh.getNewName());
+		assertSame(FileHeader.ChangeType.MODIFY, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+		assertEquals("ee8a5a0", fh.getNewId().name());
+		assertEquals(1, fh.getHunks().size());
+		assertEquals(272, fh.getHunks().get(0).getOldStartLine());
+	}
+
 	private Patch parseTestPatchFile() throws IOException {
 		final String patchFile = getName() + ".patch";
 		final InputStream in = getClass().getResourceAsStream(patchFile);
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_NoBinary.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_NoBinary.patch
new file mode 100644
index 0000000..684b13c
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_NoBinary.patch
@@ -0,0 +1,83 @@
+From 8363f12135a7d0ff0b5fea7d5a35d294c0479518 Mon Sep 17 00:00:00 2001
+From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
+Date: Tue, 23 Sep 2008 22:19:19 +0200
+Subject: [PATCH] Push and fetch icons for the toolbar
+
+Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
+Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
+---
+ org.spearce.egit.ui/icons/toolbar/fetchd.png |  Bin 0 -> 359 bytes
+ org.spearce.egit.ui/icons/toolbar/fetche.png |  Bin 0 -> 393 bytes
+ org.spearce.egit.ui/icons/toolbar/pushd.png  |  Bin 0 -> 372 bytes
+ org.spearce.egit.ui/icons/toolbar/pushe.png  |  Bin 0 -> 404 bytes
+ org.spearce.egit.ui/plugin.xml               |   32 ++++++++++++++-----------
+ 5 files changed, 18 insertions(+), 14 deletions(-)
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/fetchd.png
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/fetche.png
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/pushd.png
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/pushe.png
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetchd.png b/org.spearce.egit.ui/icons/toolbar/fetchd.png
+new file mode 100644
+index 0000000..4433c54
+Binary files /dev/null and b/org.spearce.egit.ui/icons/toolbar/fetchd.png differ
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetche.png b/org.spearce.egit.ui/icons/toolbar/fetche.png
+new file mode 100644
+index 0000000..0ffeb41
+Binary files /dev/null and b/org.spearce.egit.ui/icons/toolbar/fetche.png differ
+diff --git a/org.spearce.egit.ui/icons/toolbar/pushd.png b/org.spearce.egit.ui/icons/toolbar/pushd.png
+new file mode 100644
+index 0000000..22c3f7b
+Binary files /dev/null and b/org.spearce.egit.ui/icons/toolbar/pushd.png differ
+diff --git a/org.spearce.egit.ui/icons/toolbar/pushe.png b/org.spearce.egit.ui/icons/toolbar/pushe.png
+new file mode 100644
+index 0000000..1f99f0a
+Binary files /dev/null and b/org.spearce.egit.ui/icons/toolbar/pushe.png differ
+diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
+index 7c98688..ee8a5a0 100644
+--- a/org.spearce.egit.ui/plugin.xml
++++ b/org.spearce.egit.ui/plugin.xml
+@@ -272,22 +272,26 @@
+         </separator>
+ 	    </menu>
+ 		<action
+-		       class="org.spearce.egit.ui.internal.actions.FetchAction"
+-		       id="org.spearce.egit.ui.actionfetch"
+-		       label="%FetchAction_label"
+-		       style="push"
+-		       menubarPath="org.spearce.egit.ui.gitmenu/repo"
+-		       toolbarPath="org.spearce.egit.ui"
+-		       tooltip="%FetchAction_tooltip">
++        class="org.spearce.egit.ui.internal.actions.FetchAction"
++        disabledIcon="icons/toolbar/fetchd.png"
++        icon="icons/toolbar/fetche.png"
++        id="org.spearce.egit.ui.actionfetch"
++        label="%FetchAction_label"
++        menubarPath="org.spearce.egit.ui.gitmenu/repo"
++        style="push"
++        toolbarPath="org.spearce.egit.ui"
++        tooltip="%FetchAction_tooltip">
+ 		</action>
+ 		<action
+-		       class="org.spearce.egit.ui.internal.actions.PushAction"
+-		       id="org.spearce.egit.ui.actionpush"
+-		       label="%PushAction_label"
+-		       style="push"
+-		       menubarPath="org.spearce.egit.ui.gitmenu/repo"
+-		       toolbarPath="org.spearce.egit.ui"
+-		       tooltip="%PushAction_tooltip">
++        class="org.spearce.egit.ui.internal.actions.PushAction"
++        disabledIcon="icons/toolbar/pushd.png"
++        icon="icons/toolbar/pushe.png"
++        id="org.spearce.egit.ui.actionpush"
++        label="%PushAction_label"
++        menubarPath="org.spearce.egit.ui.gitmenu/repo"
++        style="push"
++        toolbarPath="org.spearce.egit.ui"
++        tooltip="%PushAction_tooltip">
+ 		</action>
+ 		<action
+ 		       class="org.spearce.egit.ui.internal.actions.BranchAction"
+-- 
+1.6.1.rc2.306.ge5d5e
+
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index 5ff7cb7..4bb6b7e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -153,7 +153,7 @@
 	private AbbreviatedObjectId newId;
 
 	/** Type of patch used to modify this file */
-	private PatchType patchType;
+	PatchType patchType;
 
 	/** The hunks of this file */
 	private List<HunkHeader> hunks;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index d59635a..6e9ae77 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -57,6 +57,11 @@
 
 	private static final byte[] DIFF_CC = encodeASCII("diff --cc ");
 
+	private static final byte[][] BIN_HEADERS = new byte[][] {
+			encodeASCII("Binary files "), encodeASCII("Files "), };
+
+	private static final byte[] BIN_TRAILER = encodeASCII(" differ\n");
+
 	static final byte[] SIG_FOOTER = encodeASCII("-- \n");
 
 	/** The files, in the order they were parsed out of the input. */
@@ -261,11 +266,30 @@ private int parseHunks(final FileHeader fh, final byte[] buf, int c) {
 				continue;
 			}
 
+			final int eol = nextLF(buf, c);
+			if (fh.getHunks().isEmpty() && BIN_TRAILER.length < eol - c
+					&& match(buf, eol - BIN_TRAILER.length, BIN_TRAILER) >= 0
+					&& matchAny(buf, c, BIN_HEADERS)) {
+				// The patch is a binary file diff, with no deltas.
+				//
+				fh.patchType = FileHeader.PatchType.BINARY;
+				return eol;
+			}
+
 			// Skip this line and move to the next. Its probably garbage
 			// after the last hunk of a file.
 			//
-			c = nextLF(buf, c);
+			c = eol;
 		}
 		return c;
 	}
+
+	private static boolean matchAny(final byte[] buf, final int c,
+			final byte[][] srcs) {
+		for (final byte[] s : srcs) {
+			if (match(buf, c, s) >= 0)
+				return true;
+		}
+		return false;
+	}
 }
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 05/15] Define FileHeader.PatchType to report the style of patch used
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-5-git-send-email-spearce@spearce.org>

Patches in a Git world come in at least three flavors:

  * Traditional unified patch
  * Binary patch with no data ("Binary files a/a and b/a differ")
  * Git binary patch with forward and reverse deltas

The PatchType indicates which of these flavors a given FileHeader
is looking at.  Right now we assume UNIFIED by default as that is
the most common form used.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/FileHeaderTest.java |    6 +++++
 .../tst/org/spearce/jgit/patch/PatchTest.java      |    4 +++
 .../src/org/spearce/jgit/patch/FileHeader.java     |   21 ++++++++++++++++++++
 3 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
index 4094a5c..d8696a9 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
@@ -134,6 +134,7 @@ public void testParseUnicodeName_NewFile() {
 		assertEquals("\u00c5ngstr\u00f6m", fh.getNewName());
 
 		assertSame(FileHeader.ChangeType.ADD, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 
 		assertNull(fh.getOldMode());
 		assertSame(FileMode.REGULAR_FILE, fh.getNewMode());
@@ -157,6 +158,7 @@ public void testParseUnicodeName_DeleteFile() {
 		assertSame(FileHeader.DEV_NULL, fh.getNewName());
 
 		assertSame(FileHeader.ChangeType.DELETE, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 
 		assertSame(FileMode.REGULAR_FILE, fh.getOldMode());
 		assertNull(fh.getNewMode());
@@ -174,6 +176,7 @@ public void testParseModeChange() {
 		assertEquals("a b", fh.getNewName());
 
 		assertSame(FileHeader.ChangeType.MODIFY, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -200,6 +203,7 @@ public void testParseRename100_NewStyle() {
 		assertEquals(" c/\u00c5ngstr\u00f6m", fh.getNewName());
 
 		assertSame(FileHeader.ChangeType.RENAME, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -227,6 +231,7 @@ public void testParseRename100_OldStyle() {
 		assertEquals(" c/\u00c5ngstr\u00f6m", fh.getNewName());
 
 		assertSame(FileHeader.ChangeType.RENAME, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
@@ -254,6 +259,7 @@ public void testParseCopy100() {
 		assertEquals(" c/\u00c5ngstr\u00f6m", fh.getNewName());
 
 		assertSame(FileHeader.ChangeType.COPY, fh.getChangeType());
+		assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
 
 		assertNull(fh.getOldId());
 		assertNull(fh.getNewId());
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
index 8ddaadc..3afe0a1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
@@ -70,6 +70,8 @@ assertEquals(
 
 		assertEquals("da7e704", fRepositoryConfigTest.getOldId().name());
 		assertEquals("34ce04a", fRepositoryConfigTest.getNewId().name());
+		assertSame(FileHeader.PatchType.UNIFIED, fRepositoryConfigTest
+				.getPatchType());
 		assertSame(FileMode.REGULAR_FILE, fRepositoryConfigTest.getOldMode());
 		assertSame(FileMode.REGULAR_FILE, fRepositoryConfigTest.getNewMode());
 		assertEquals(1, fRepositoryConfigTest.getHunks().size());
@@ -90,6 +92,8 @@ assertEquals(
 
 		assertEquals("45c2f8a", fRepositoryConfig.getOldId().name());
 		assertEquals("3291bba", fRepositoryConfig.getNewId().name());
+		assertSame(FileHeader.PatchType.UNIFIED, fRepositoryConfig
+				.getPatchType());
 		assertSame(FileMode.REGULAR_FILE, fRepositoryConfig.getOldMode());
 		assertSame(FileMode.REGULAR_FILE, fRepositoryConfig.getNewMode());
 		assertEquals(3, fRepositoryConfig.getHunks().size());
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index b49cb7b..5ff7cb7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -107,6 +107,18 @@
 		COPY;
 	}
 
+	/** Type of patch used by this file. */
+	public static enum PatchType {
+		/** A traditional unified diff style patch of a text file. */
+		UNIFIED,
+
+		/** An empty patch with a message "Binary files ... differ" */
+		BINARY,
+
+		/** A Git binary patch, holding pre and post image deltas */
+		GIT_BINARY;
+	}
+
 	/** Buffer holding the patch data for this file. */
 	final byte[] buf;
 
@@ -140,6 +152,9 @@
 	/** ObjectId listed on the index line for the new (post-image) */
 	private AbbreviatedObjectId newId;
 
+	/** Type of patch used to modify this file */
+	private PatchType patchType;
+
 	/** The hunks of this file */
 	private List<HunkHeader> hunks;
 
@@ -147,6 +162,7 @@ FileHeader(final byte[] b, final int offset) {
 		buf = b;
 		startOffset = offset;
 		changeType = ChangeType.MODIFY; // unless otherwise designated
+		patchType = PatchType.UNIFIED;
 	}
 
 	/**
@@ -229,6 +245,11 @@ public AbbreviatedObjectId getNewId() {
 		return newId;
 	}
 
+	/** @return style of patch used to modify this file */
+	public PatchType getPatchType() {
+		return patchType;
+	}
+
 	/** @return hunks altering this file; in order of appearance in patch */
 	public List<HunkHeader> getHunks() {
 		if (hunks == null)
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 04/15] Add lineMap computer to RawParseUtils to index locations of line starts
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-4-git-send-email-spearce@spearce.org>

Some algorithms like a diff or patch require efficient lookup of lines
within a source file, using a 1 based line number.  lineMap produces a
list of offsets within the file where each line starts, providing O(1)
lookup time to locate those positions.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/util/IntListTest.java     |   24 ++++++
 .../jgit/util/RawParseUtils_LineMapTest.java       |   88 ++++++++++++++++++++
 .../src/org/spearce/jgit/util/IntList.java         |   15 ++++
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   31 +++++++
 4 files changed, 158 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_LineMapTest.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
index f943075..c470d55 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
@@ -103,6 +103,30 @@ public void testAdd_LargeGroup() {
 		}
 	}
 
+	public void testFillTo0() {
+		final IntList i = new IntList();
+		i.fillTo(0, Integer.MIN_VALUE);
+		assertEquals(0, i.size());
+	}
+
+	public void testFillTo1() {
+		final IntList i = new IntList();
+		i.fillTo(1, Integer.MIN_VALUE);
+		assertEquals(1, i.size());
+		i.add(0);
+		assertEquals(Integer.MIN_VALUE, i.get(0));
+		assertEquals(0, i.get(1));
+	}
+
+	public void testFillTo100() {
+		final IntList i = new IntList();
+		i.fillTo(100, Integer.MIN_VALUE);
+		assertEquals(100, i.size());
+		i.add(3);
+		assertEquals(Integer.MIN_VALUE, i.get(99));
+		assertEquals(3, i.get(100));
+	}
+
 	public void testClear() {
 		final IntList i = new IntList();
 		final int n = 5;
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_LineMapTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_LineMapTest.java
new file mode 100644
index 0000000..3f562a4
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_LineMapTest.java
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.util;
+
+import java.io.UnsupportedEncodingException;
+
+import junit.framework.TestCase;
+
+public class RawParseUtils_LineMapTest extends TestCase {
+	public void testEmpty() {
+		final IntList map = RawParseUtils.lineMap(new byte[] {}, 0, 0);
+		assertNotNull(map);
+		assertEquals(1, map.size());
+		assertEquals(Integer.MIN_VALUE, map.get(0));
+	}
+
+	public void testOneBlankLine() {
+		final IntList map = RawParseUtils.lineMap(new byte[] { '\n' }, 0, 1);
+		assertEquals(2, map.size());
+		assertEquals(Integer.MIN_VALUE, map.get(0));
+		assertEquals(0, map.get(1));
+	}
+
+	public void testTwoLineFooBar() throws UnsupportedEncodingException {
+		final byte[] buf = "foo\nbar\n".getBytes("ISO-8859-1");
+		final IntList map = RawParseUtils.lineMap(buf, 0, buf.length);
+		assertEquals(3, map.size());
+		assertEquals(Integer.MIN_VALUE, map.get(0));
+		assertEquals(0, map.get(1));
+		assertEquals(4, map.get(2));
+	}
+
+	public void testTwoLineNoLF() throws UnsupportedEncodingException {
+		final byte[] buf = "foo\nbar".getBytes("ISO-8859-1");
+		final IntList map = RawParseUtils.lineMap(buf, 0, buf.length);
+		assertEquals(3, map.size());
+		assertEquals(Integer.MIN_VALUE, map.get(0));
+		assertEquals(0, map.get(1));
+		assertEquals(4, map.get(2));
+	}
+
+	public void testFourLineBlanks() throws UnsupportedEncodingException {
+		final byte[] buf = "foo\n\n\nbar\n".getBytes("ISO-8859-1");
+		final IntList map = RawParseUtils.lineMap(buf, 0, buf.length);
+		assertEquals(5, map.size());
+		assertEquals(Integer.MIN_VALUE, map.get(0));
+		assertEquals(0, map.get(1));
+		assertEquals(4, map.get(2));
+		assertEquals(5, map.get(3));
+		assertEquals(6, map.get(4));
+	}
+
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java b/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java
index 1445f88..2824074 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java
@@ -93,6 +93,21 @@ public void add(final int n) {
 		entries[count++] = n;
 	}
 
+	/**
+	 * Pad the list with entries.
+	 * 
+	 * @param toIndex
+	 *            index position to stop filling at. 0 inserts no filler. 1
+	 *            ensures the list has a size of 1, adding <code>val</code> if
+	 *            the list is currently empty.
+	 * @param val
+	 *            value to insert into padded positions.
+	 */
+	public void fillTo(int toIndex, final int val) {
+		while (count < toIndex)
+			add(val);
+	}
+
 	private void grow() {
 		final int[] n = new int[(entries.length + 16) * 3 / 2];
 		System.arraycopy(entries, 0, n, 0, count);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index 8896d38..6143188 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -266,6 +266,37 @@ public static final int nextLF(final byte[] b, int ptr, final char chrA) {
 	}
 
 	/**
+	 * Index the region between <code>[ptr, end)</code> to find line starts.
+	 * <p>
+	 * The returned list is 1 indexed. Index 0 contains
+	 * {@link Integer#MIN_VALUE} to pad the list out.
+	 * <p>
+	 * Using a 1 indexed list means that line numbers can be directly accessed
+	 * from the list, so <code>list.get(1)</code> (aka get line 1) returns
+	 * <code>ptr</code>.
+	 * 
+	 * @param buf
+	 *            buffer to scan.
+	 * @param ptr
+	 *            position within the buffer corresponding to the first byte of
+	 *            line 1.
+	 * @param end
+	 *            1 past the end of the content within <code>buf</code>.
+	 * @return a line map indexing the start position of each line.
+	 */
+	public static final IntList lineMap(final byte[] buf, int ptr, int end) {
+		// Experimentally derived from multiple source repositories
+		// the average number of bytes/line is 36. Its a rough guess
+		// to initially size our map close to the target.
+		//
+		final IntList map = new IntList((end - ptr) / 36);
+		map.fillTo(1, Integer.MIN_VALUE);
+		for (; ptr < end; ptr = nextLF(buf, ptr))
+			map.add(ptr);
+		return map;
+	}
+
+	/**
 	 * Locate the "author " header line data.
 	 * 
 	 * @param b
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 02/15] Add tests for TemporaryBuffer
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-2-git-send-email-spearce@spearce.org>

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/util/TemporaryBufferTest.java |  374 ++++++++++++++++++++
 .../tst/org/spearce/jgit/util/TestRng.java         |   61 ++++
 .../src/org/spearce/jgit/util/TemporaryBuffer.java |    4 +-
 3 files changed, 437 insertions(+), 2 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/TemporaryBufferTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/TestRng.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/TemporaryBufferTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/TemporaryBufferTest.java
new file mode 100644
index 0000000..e532d98
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/TemporaryBufferTest.java
@@ -0,0 +1,374 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.util;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+
+import junit.framework.TestCase;
+
+public class TemporaryBufferTest extends TestCase {
+	public void testEmpty() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		try {
+			b.close();
+			assertEquals(0, b.length());
+			final byte[] r = b.toByteArray();
+			assertNotNull(r);
+			assertEquals(0, r.length);
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testOneByte() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte test = (byte) new TestRng(getName()).nextInt();
+		try {
+			b.write(test);
+			b.close();
+			assertEquals(1, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(1, r.length);
+				assertEquals(test, r[0]);
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(1, r.length);
+				assertEquals(test, r[0]);
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testOneBlock_BulkWrite() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.Block.SZ);
+		try {
+			b.write(test, 0, 2);
+			b.write(test, 2, 4);
+			b.write(test, 6, test.length - 6 - 2);
+			b.write(test, test.length - 2, 2);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testOneBlockAndHalf_BulkWrite() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.Block.SZ * 3 / 2);
+		try {
+			b.write(test, 0, 2);
+			b.write(test, 2, 4);
+			b.write(test, 6, test.length - 6 - 2);
+			b.write(test, test.length - 2, 2);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testOneBlockAndHalf_SingleWrite() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.Block.SZ * 3 / 2);
+		try {
+			for (int i = 0; i < test.length; i++)
+				b.write(test[i]);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testOneBlockAndHalf_Copy() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.Block.SZ * 3 / 2);
+		try {
+			final ByteArrayInputStream in = new ByteArrayInputStream(test);
+			b.write(in.read());
+			b.copy(in);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testLarge_SingleWrite() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.DEFAULT_IN_CORE_LIMIT * 3);
+		try {
+			b.write(test);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testInCoreLimit_SwitchOnAppendByte() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.DEFAULT_IN_CORE_LIMIT + 1);
+		try {
+			b.write(test, 0, test.length - 1);
+			b.write(test[test.length - 1]);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testInCoreLimit_SwitchBeforeAppendByte() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.DEFAULT_IN_CORE_LIMIT * 3);
+		try {
+			b.write(test, 0, test.length - 1);
+			b.write(test[test.length - 1]);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testInCoreLimit_SwitchOnCopy() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final byte[] test = new TestRng(getName())
+				.nextBytes(TemporaryBuffer.DEFAULT_IN_CORE_LIMIT * 2);
+		try {
+			final ByteArrayInputStream in = new ByteArrayInputStream(test,
+					TemporaryBuffer.DEFAULT_IN_CORE_LIMIT, test.length
+							- TemporaryBuffer.DEFAULT_IN_CORE_LIMIT);
+			b.write(test, 0, TemporaryBuffer.DEFAULT_IN_CORE_LIMIT);
+			b.copy(in);
+			b.close();
+			assertEquals(test.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(test.length, r.length);
+				assertTrue(Arrays.equals(test, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testDestroyWhileOpen() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		try {
+			b.write(new TestRng(getName())
+					.nextBytes(TemporaryBuffer.DEFAULT_IN_CORE_LIMIT * 2));
+		} finally {
+			b.destroy();
+		}
+	}
+
+	public void testRandomWrites() throws IOException {
+		final TemporaryBuffer b = new TemporaryBuffer();
+		final TestRng rng = new TestRng(getName());
+		final int max = TemporaryBuffer.DEFAULT_IN_CORE_LIMIT * 2;
+		final byte[] expect = new byte[max];
+		try {
+			int written = 0;
+			boolean onebyte = true;
+			while (written < max) {
+				if (onebyte) {
+					final byte v = (byte) rng.nextInt();
+					b.write(v);
+					expect[written++] = v;
+				} else {
+					final int len = Math
+							.min(rng.nextInt() & 127, max - written);
+					final byte[] tmp = rng.nextBytes(len);
+					b.write(tmp, 0, len);
+					System.arraycopy(tmp, 0, expect, written, len);
+					written += len;
+				}
+				onebyte = !onebyte;
+			}
+			assertEquals(expect.length, written);
+			b.close();
+
+			assertEquals(expect.length, b.length());
+			{
+				final byte[] r = b.toByteArray();
+				assertNotNull(r);
+				assertEquals(expect.length, r.length);
+				assertTrue(Arrays.equals(expect, r));
+			}
+			{
+				final ByteArrayOutputStream o = new ByteArrayOutputStream();
+				b.writeTo(o, null);
+				o.close();
+				final byte[] r = o.toByteArray();
+				assertEquals(expect.length, r.length);
+				assertTrue(Arrays.equals(expect, r));
+			}
+		} finally {
+			b.destroy();
+		}
+	}
+
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/TestRng.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/TestRng.java
new file mode 100644
index 0000000..d74a534
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/TestRng.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.util;
+
+/** Toy RNG to ensure we get predictable numbers during unit tests. */
+public class TestRng {
+	private int next;
+
+	public TestRng(final String seed) {
+		next = 0;
+		for (int i = 0; i < seed.length(); i++)
+			next = next * 11 + seed.charAt(i);
+	}
+
+	public byte[] nextBytes(final int cnt) {
+		final byte[] r = new byte[cnt];
+		for (int i = 0; i < cnt; i++)
+			r[i] = (byte) nextInt();
+		return r;
+	}
+
+	public int nextInt() {
+		next = next * 1103515245 + 12345;
+		return next;
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
index 8f91246..6267fb5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
@@ -60,7 +60,7 @@
  * after this stream has been properly closed by {@link #close()}.
  */
 public class TemporaryBuffer extends OutputStream {
-	private static final int DEFAULT_IN_CORE_LIMIT = 1024 * 1024;
+	static final int DEFAULT_IN_CORE_LIMIT = 1024 * 1024;
 
 	/** Chain of data, if we are still completely in-core; otherwise null. */
 	private ArrayList<Block> blocks;
@@ -297,7 +297,7 @@ public void destroy() {
 		}
 	}
 
-	private static class Block {
+	static class Block {
 		static final int SZ = 8 * 1024;
 
 		final byte[] buffer = new byte[SZ];
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer>
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-3-git-send-email-spearce@spearce.org>

Java's generic container types can only handle reference values,
which means making a List of ints requires boxing each value. A
boxed int typically requires at least 12 bytes more space per
value over an unboxed int, as it has an additional object header
and a cell to hold the object reference.

IntList uses an int[] internally to hold the values, rather than
an Object[] like List<Integer> would use.

We don't conform to the List (or even Collection) APIs as doing
so would require that we box return values, which is even less
efficient than just using ArrayList<Integer>, because we would
be boxing every return value each time it is accessed.  Instead
we use an API that smells the same, so there is some finger feel
to using the class.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/util/IntListTest.java     |  132 ++++++++++++++++++++
 .../src/org/spearce/jgit/util/IntList.java         |  113 +++++++++++++++++
 2 files changed, 245 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/IntList.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
new file mode 100644
index 0000000..f943075
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.util;
+
+import junit.framework.TestCase;
+
+public class IntListTest extends TestCase {
+	public void testEmpty_DefaultCapacity() {
+		final IntList i = new IntList();
+		assertEquals(0, i.size());
+		try {
+			i.get(0);
+			fail("Accepted 0 index on empty list");
+		} catch (ArrayIndexOutOfBoundsException e) {
+			assertTrue(true);
+		}
+	}
+
+	public void testEmpty_SpecificCapacity() {
+		final IntList i = new IntList(5);
+		assertEquals(0, i.size());
+		try {
+			i.get(0);
+			fail("Accepted 0 index on empty list");
+		} catch (ArrayIndexOutOfBoundsException e) {
+			assertTrue(true);
+		}
+	}
+
+	public void testAdd_SmallGroup() {
+		final IntList i = new IntList();
+		final int n = 5;
+		for (int v = 0; v < n; v++)
+			i.add(10 + v);
+		assertEquals(n, i.size());
+
+		for (int v = 0; v < n; v++)
+			assertEquals(10 + v, i.get(v));
+		try {
+			i.get(n);
+			fail("Accepted out of bound index on list");
+		} catch (ArrayIndexOutOfBoundsException e) {
+			assertTrue(true);
+		}
+	}
+
+	public void testAdd_ZeroCapacity() {
+		final IntList i = new IntList(0);
+		assertEquals(0, i.size());
+		i.add(1);
+		assertEquals(1, i.get(0));
+	}
+
+	public void testAdd_LargeGroup() {
+		final IntList i = new IntList();
+		final int n = 500;
+		for (int v = 0; v < n; v++)
+			i.add(10 + v);
+		assertEquals(n, i.size());
+
+		for (int v = 0; v < n; v++)
+			assertEquals(10 + v, i.get(v));
+		try {
+			i.get(n);
+			fail("Accepted out of bound index on list");
+		} catch (ArrayIndexOutOfBoundsException e) {
+			assertTrue(true);
+		}
+	}
+
+	public void testClear() {
+		final IntList i = new IntList();
+		final int n = 5;
+		for (int v = 0; v < n; v++)
+			i.add(10 + v);
+		assertEquals(n, i.size());
+
+		i.clear();
+		assertEquals(0, i.size());
+		try {
+			i.get(0);
+			fail("Accepted 0 index on empty list");
+		} catch (ArrayIndexOutOfBoundsException e) {
+			assertTrue(true);
+		}
+	}
+
+	public void testToString() {
+		final IntList i = new IntList();
+		i.add(1);
+		assertEquals("[1]", i.toString());
+		i.add(13);
+		i.add(5);
+		assertEquals("[1, 13, 5]", i.toString());
+	}
+
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java b/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java
new file mode 100644
index 0000000..1445f88
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/IntList.java
@@ -0,0 +1,113 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * 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.util;
+
+/** A more efficient List<Integer> using a primitive integer array. */
+public class IntList {
+	private int[] entries;
+
+	private int count;
+
+	/** Create an empty list with a default capacity. */
+	public IntList() {
+		this(10);
+	}
+
+	/**
+	 * Create an empty list with the specified capacity.
+	 * 
+	 * @param capacity
+	 *            number of entries the list can initially hold.
+	 */
+	public IntList(final int capacity) {
+		entries = new int[capacity];
+	}
+
+	/** @return number of entries in this list */
+	public int size() {
+		return count;
+	}
+
+	/**
+	 * @param i
+	 *            index to read, must be in the range [0, {@link #size()}).
+	 * @return the number at the specified index
+	 * @throws ArrayIndexOutOfBoundsException
+	 *             the index outside the valid range
+	 */
+	public int get(final int i) {
+		if (count <= i)
+			throw new ArrayIndexOutOfBoundsException(i);
+		return entries[i];
+	}
+
+	/** Empty this list */
+	public void clear() {
+		count = 0;
+	}
+
+	/**
+	 * Add an entry to the end of the list.
+	 * 
+	 * @param n
+	 *            the number to add.
+	 */
+	public void add(final int n) {
+		if (count == entries.length)
+			grow();
+		entries[count++] = n;
+	}
+
+	private void grow() {
+		final int[] n = new int[(entries.length + 16) * 3 / 2];
+		System.arraycopy(entries, 0, n, 0, count);
+		entries = n;
+	}
+
+	public String toString() {
+		final StringBuilder r = new StringBuilder();
+		r.append('[');
+		for (int i = 0; i < count; i++) {
+			if (i > 0)
+				r.append(", ");
+			r.append(entries[i]);
+		}
+		r.append(']');
+		return r.toString();
+	}
+}
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 01/15] Correct use of TemporaryBuffer in Patch
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-1-git-send-email-spearce@spearce.org>

We need to destroy the buffer in case it created a
temporary file to hold the content.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/patch/Patch.java          |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 165058d..d59635a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -105,9 +105,13 @@ public void parse(final InputStream is) throws IOException {
 
 	private static byte[] readFully(final InputStream is) throws IOException {
 		final TemporaryBuffer b = new TemporaryBuffer();
-		b.copy(is);
-		final byte[] buf = b.toByteArray();
-		return buf;
+		try {
+			b.copy(is);
+			b.close();
+			return b.toByteArray();
+		} finally {
+			b.destroy();
+		}
 	}
 
 	/**
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 00/15] More patch parsing support
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

Unit test improvements, introduction of IntList (which Dscho also
needs for his diff code, its probably a drop-in replacement for him),
and support for parsing the meta data of binary patches.

I'm writing the "diff --cc" parser right now but the unit tests
for it still fail so I'm not including those changes just yet.


Shawn O. Pearce (15):
  Correct use of TemporaryBuffer in Patch
  Add tests for TemporaryBuffer
  Add IntList as a more efficient representation of List<Integer>
  Add lineMap computer to RawParseUtils to index locations of line
    starts
  Define FileHeader.PatchType to report the style of patch used
  Test for non-git binary files and mark them as PatchType.BINARY
  Set empty patches with no Git metadata to PatchType.BINARY
  Always use the FileHeader buffer during Patch.parseHunks
  Parse "GIT binary patch" style patch metadata
  Record patch parsing errors for later inspection by applications
  Fix Patch.parse to honor the end point passed in
  Correctly handle hunk headers such as "@@ -0,0 +1 @@"
  Patch parse test comparing "git log -p" output to "git log --numstat"
  Abstract the hunk header testing into a method
  Treat "diff --combined" the same as "diff --cc"

 .../spearce/jgit/patch/EGitPatchHistoryTest.java   |  221 ++++++++++++
 .../tst/org/spearce/jgit/patch/FileHeaderTest.java |   70 +++-
 .../tst/org/spearce/jgit/patch/PatchErrorTest.java |  174 +++++++++
 .../tst/org/spearce/jgit/patch/PatchTest.java      |   78 ++++
 .../spearce/jgit/patch/testError_BodyTooLong.patch |   17 +
 .../jgit/patch/testError_DisconnectedHunk.patch    |   30 ++
 .../jgit/patch/testError_GarbageBetweenFiles.patch |   33 ++
 .../patch/testError_GitBinaryNoForwardHunk.patch   |   10 +
 .../jgit/patch/testError_TruncatedNew.patch        |   15 +
 .../jgit/patch/testError_TruncatedOld.patch        |   15 +
 .../spearce/jgit/patch/testParse_GitBinary.patch   |  135 +++++++
 .../spearce/jgit/patch/testParse_NoBinary.patch    |   83 +++++
 .../tst/org/spearce/jgit/util/IntListTest.java     |  156 ++++++++
 .../jgit/util/RawParseUtils_LineMapTest.java       |   88 +++++
 .../org/spearce/jgit/util/TemporaryBufferTest.java |  374 ++++++++++++++++++++
 .../tst/org/spearce/jgit/util/TestRng.java         |   61 ++++
 .../src/org/spearce/jgit/patch/BinaryHunk.java     |  127 +++++++
 .../src/org/spearce/jgit/patch/FileHeader.java     |   94 +++++-
 .../src/org/spearce/jgit/patch/FormatError.java    |   95 +++++
 .../src/org/spearce/jgit/patch/HunkHeader.java     |   41 ++-
 .../src/org/spearce/jgit/patch/Patch.java          |  197 ++++++++---
 .../src/org/spearce/jgit/util/IntList.java         |  128 +++++++
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   31 ++
 .../src/org/spearce/jgit/util/TemporaryBuffer.java |    4 +-
 24 files changed, 2187 insertions(+), 90 deletions(-)
 create mode 100644 org.spearce.jgit.test/exttst/org/spearce/jgit/patch/EGitPatchHistoryTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_GitBinary.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testParse_NoBinary.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/IntListTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_LineMapTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/TemporaryBufferTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/TestRng.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/IntList.java

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Daniel Barkalow @ 2008-12-12  2:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyen Thai Ngoc Duy, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <7vy6ym9nm8.fsf@gitster.siamese.dyndns.org>

On Thu, 11 Dec 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> >> That's the point. CE_VALID does not define checkout area while
> >> CE_NO_CHECKOUT does.  If an entry is CE_VALID, it is still in checkout
> >> area. But if it is CE_NO_CHECKOUT, "git grep" should ignore that path.
> >> core.defaultsparse has nothing to do here.
> >
> > My point is that the index cannot tell git grep whether it should search a 
> > path if the path isn't in the index.
> 
> Let's step back a bit.  I think "git grep" that stays silent outside of
> the checkout area when used to grep in the work tree or in the index is a
> mistake.
> 
> The problem "sparse checkout" attempts to address is not this:
> 
>     I ran "git init && git add ." in /usr/src by mistake.  There is no
>     reason for coreutils that is in /usr/src/coreutils and gnucash that is
>     in /usr/src/gnucash to share the same development history nor their
>     should be any ordering between commits in these two independent
>     projects.  I should have done N separate "init & add" independently at
>     one level deeper in the directory hierarchy, but I am too lazy to
>     filter branch the resulting mess now.
> 
> At least, it should not be that, at least to me.
> 
> "Sparse" is "I am not going to modify the files in these areas, and I know
> they do not need to be present for my purposes (e.g. build), so I do not
> need copies in the work tree."  It still works on the whole tree structure
> recorded in the commit, but gives you a way to work inside a sparsely
> populated work tree, iow, without checking everything out.

There's the meta question of: "Do people who have declared that they 
aren't going to modify or build with some files want their searches to 
tell them about those files?"

Say I'm the "tr" guy, and I care about the build system, library code, and 
"tr.c", and I run "make tr"; my sparse checkout doesn't include "head.c", 
and I totally ignore all the other stuff that's in coreutils. Maybe I want 
"git grep" to exclude the other stuff.

I don't really have a firm position on whether "git grep" should ignore 
"head.c" or not, but I think it should be consistent between "git grep" 
and "git grep origin/next", and I think that, if origin/next has a new 
"foot.c" that isn't in the current branch to by marked as NO_CHECKOUT, it 
should be skipped if "tail.c" (which is in my current branch) is skipped.

> So "git grep -e frotz Documentation/", whether you only check out
> Documentation or the whole tree, should grep only in Documentation area,
> and "git grep -e frotz" should grep in the whole tree, even if you happen
> to have a sparse checkout.  By definition, a sparse checkout has no
> modifications outside the checkout area, so whenever grep wants to look
> for strings outside the checkout area it should pretend as if the same
> content as what the index records is in the work tree.  This is consistent
> with the way how "git diff" in a sparsely checked out work tree should
> behave.

"git diff" is an ambiguous model for "git grep". It equally describes 
the behavior of "git diff" to say that it treats files outside the 
checkout area as matching the index or to say that it never lists files 
outside the checkout area. On the other hand, there is the question of 
whether "git diff branch1 branch2" shows differences that are outside the 
checkout area, and whether "git log" shows commits that only change things 
outside the checkout area, and "git grep" should match the behavior of 
these.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: git-gui: Warn when username and e-mail address is unconfigured?
From: jidanni @ 2008-12-12  2:37 UTC (permalink / raw)
  To: git
In-Reply-To: <b9fd99020812051218o1c337148u12b8e190c60f32eb@mail.gmail.com>

On git-commit-tree(1)
  DIAGNOSTICS
       You don't exist. Go away!
           The passwd(5) gecos field couldn't be read

It should say that null is OK though.
jidanni:x:1000:1000:,,,:/home/jidanni:/bin/bash
gives
 Committer: jidanni <jidanni@jidanni.org>
which is what I want. No old fashioned English name for me.

^ permalink raw reply

* Re: user-manual.html invalid HTML
From: Jeff King @ 2008-12-12  2:30 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87skouzc4w.fsf@jidanni.org>

On Fri, Dec 12, 2008 at 04:32:15AM +0800, jidanni@jidanni.org wrote:

> Please see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507476
> Which it turns out didn't get forwarded to git@vger.kernel.org after all. 

The versions I build locally have:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
    "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">

in each HTML file, which is added by asciidoc.  Maybe the package you
are looking at was built with an older version of asciidoc that doesn't
do this (I don't actually know the history of this feature, but it seems
to me that this is something asciidoc should be doing, not git).

-Peff

^ permalink raw reply

* Re: git log --numstat disagrees with git apply --numstat
From: Jeff King @ 2008-12-12  2:21 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081212020857.GB23128@sigill.intra.peff.net>

On Thu, Dec 11, 2008 at 09:08:58PM -0500, Jeff King wrote:

> > which is probably just due to different xdi settings being used between
> > the two codepaths. I haven't looked closely to see which different
> > options we are feeding to xdiff.
> 
> Ah. Doing this gives me the 68/12 answer for "git log --numstat":

BTW, I got a little confused looking at the parameters to xdi_diff_outf,
since ecb gets passed in full of random garbage. I don't know if this
cleanup is worth applying:

-- >8 --
remove xecb parameter to xdi_diff_outf

It is pointless to pass this parameter in instead of just
declaring it locally inside the function because:

  1. We overwrite every member of the struct inside the
     function anyway, so we ignore anything passed in.

  2. The contents after we return point to a local variable
     that has gone out of scope, so it is wrong to look at
     them.

So it is just making the interface more complex to have it
there, and it looks like a potential error in the callers to
be passing a completely uninitialized variable.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c            |   15 +++++----------
 xdiff-interface.c |    9 +++++----
 xdiff-interface.h |    2 +-
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/diff.c b/diff.c
index af822c1..1801aef 100644
--- a/diff.c
+++ b/diff.c
@@ -403,7 +403,6 @@ static void diff_words_show(struct diff_words_data *diff_words)
 {
 	xpparam_t xpp;
 	xdemitconf_t xecfg;
-	xdemitcb_t ecb;
 	mmfile_t minus, plus;
 	int i;
 
@@ -428,7 +427,7 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	xpp.flags = XDF_NEED_MINIMAL;
 	xecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;
 	xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
-		      &xpp, &xecfg, &ecb);
+		      &xpp, &xecfg);
 	free(minus.ptr);
 	free(plus.ptr);
 	diff_words->minus.text.size = diff_words->plus.text.size = 0;
@@ -1436,7 +1435,6 @@ static void builtin_diff(const char *name_a,
 		const char *diffopts = getenv("GIT_DIFF_OPTS");
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
-		xdemitcb_t ecb;
 		struct emit_callback ecbdata;
 		const struct userdiff_funcname *pe;
 
@@ -1484,7 +1482,7 @@ static void builtin_diff(const char *name_a,
 			ecbdata.diff_words->file = o->file;
 		}
 		xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,
-			      &xpp, &xecfg, &ecb);
+			      &xpp, &xecfg);
 		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			free_diff_words_data(&ecbdata);
 		if (textconv_one)
@@ -1535,13 +1533,12 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 		/* Crazy xdl interfaces.. */
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
-		xdemitcb_t ecb;
 
 		memset(&xpp, 0, sizeof(xpp));
 		memset(&xecfg, 0, sizeof(xecfg));
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
-			      &xpp, &xecfg, &ecb);
+			      &xpp, &xecfg);
 	}
 
  free_and_return:
@@ -1582,14 +1579,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 		/* Crazy xdl interfaces.. */
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
-		xdemitcb_t ecb;
 
 		memset(&xpp, 0, sizeof(xpp));
 		memset(&xecfg, 0, sizeof(xecfg));
 		xecfg.ctxlen = 1; /* at least one context line */
 		xpp.flags = XDF_NEED_MINIMAL;
 		xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
-			      &xpp, &xecfg, &ecb);
+			      &xpp, &xecfg);
 
 		if ((data.ws_rule & WS_TRAILING_SPACE) &&
 		    data.trailing_blanks_start) {
@@ -3009,7 +3005,6 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
 	for (i = 0; i < q->nr; i++) {
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
-		xdemitcb_t ecb;
 		mmfile_t mf1, mf2;
 		struct diff_filepair *p = q->queue[i];
 		int len1, len2;
@@ -3071,7 +3066,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
 		xecfg.ctxlen = 3;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
-			      &xpp, &xecfg, &ecb);
+			      &xpp, &xecfg);
 	}
 
 	git_SHA1_Final(sha1, &ctx);
diff --git a/xdiff-interface.c b/xdiff-interface.c
index d782f06..d0d60fa 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -140,18 +140,19 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
 int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
 		  xdiff_emit_consume_fn fn, void *consume_callback_data,
 		  xpparam_t const *xpp,
-		  xdemitconf_t const *xecfg, xdemitcb_t *xecb)
+		  xdemitconf_t const *xecfg)
 {
 	int ret;
 	struct xdiff_emit_state state;
+	xdemitcb_t xecb;
 
 	memset(&state, 0, sizeof(state));
 	state.consume = fn;
 	state.consume_callback_data = consume_callback_data;
-	xecb->outf = xdiff_outf;
-	xecb->priv = &state;
+	xecb.outf = xdiff_outf;
+	xecb.priv = &state;
 	strbuf_init(&state.remainder, 0);
-	ret = xdi_diff(mf1, mf2, xpp, xecfg, xecb);
+	ret = xdi_diff(mf1, mf2, xpp, xecfg, &xecb);
 	strbuf_release(&state.remainder);
 	return ret;
 }
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 7352b9a..491037d 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -10,7 +10,7 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
 int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
 		  xdiff_emit_consume_fn fn, void *consume_callback_data,
 		  xpparam_t const *xpp,
-		  xdemitconf_t const *xecfg, xdemitcb_t *xecb);
+		  xdemitconf_t const *xecfg);
 int xdi_diff_hunks(mmfile_t *mf1, mmfile_t *mf2,
 		   xdiff_emit_hunk_consume_fn fn, void *consume_callback_data,
 		   xpparam_t const *xpp, xdemitconf_t *xecfg);
-- 
1.6.1.rc2.307.gb39e0.dirty

^ permalink raw reply related

* Re: git log --numstat disagrees with git apply --numstat
From: Jeff King @ 2008-12-12  2:08 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081212015254.GA23128@sigill.intra.peff.net>

On Thu, Dec 11, 2008 at 08:52:55PM -0500, Jeff King wrote:

> It looks like it is just a place where two different valid diffs can be
> constructed:
> [...]
> which is probably just due to different xdi settings being used between
> the two codepaths. I haven't looked closely to see which different
> options we are feeding to xdiff.

Ah. Doing this gives me the 68/12 answer for "git log --numstat":

diff --git a/diff.c b/diff.c
index af822c1..5f314ce 100644
--- a/diff.c
+++ b/diff.c
@@ -1539,6 +1539,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 
 		memset(&xpp, 0, sizeof(xpp));
 		memset(&xecfg, 0, sizeof(xecfg));
+		xecfg.ctxlen = o->context;
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
 			      &xpp, &xecfg, &ecb);

I guess it is slightly less efficient (since we just throw away the
context lines anyway), but it is nice to count the exact same patch
that "git log -p" would produce.

-Peff

^ permalink raw reply related

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Jakub Narebski @ 2008-12-12  2:03 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <87zlj2xm35.wl%seb@cine7.net>

On Fri, 12 Dec 2008, Sébastien Cevey wrote:
> At Fri, 12 Dec 2008 01:13:45 +0100, Jakub Narebski wrote:
> 
> > I just tried, it works but we first need to sort @projects by
> > category.
> > 
> > I don't understand.
> > [...]
> > I propose to change it to:
> 
> Well in my previous iteration of the patch (v3), the printing of
> projects with categories is done using:
> 
>   foreach my $cat (sort keys %categories) {
> 
> So everything was already sorted by category (and then by whichever
> property you picked inside each category).  You seemed okay with it,
> but requested that I documented that behaviour in the commit log.

But this does not mean that sorting by categories is necessary, or
even wanted (see below). This foreach _sorts_ by categories as primary
key using kind of bucket sort algorithm.
 
> To maintain the same result with your proposed change (which is what I
> submitted in my patch), we need to sort by categories first (AFAIK
> Perl sort retains the original order inside equivalence classes of
> comparison key?), otherwise splice(projlist, from, to) doesn't return 
> the expected subset.

Perl requires "use sort 'stable';" pragma to ensure stable sort.

And no, we don't need to sort by categories first.  Let me explain
in more detail a bit.

Let us assume that $from and $to is actually used to divide projects
list into categories (which goal is incompatible with searching
projects, limiting to given tag/tagset and hiding forked as it is done
now, at the display time; it has to be done _before_ pagination).
They are used to display first page, i.e. repositories numbered 1..N
in current ordering, or N..2*N, or 2*N..3*N to show next pages. Let us
have the following project list:

  Project       Category
  ---------------------------
  1             a
  2             b
  3             b
  4             a

If categories are not shown, and page limit is 2, then project would
be displayed like this:


  page 1          page 2
  ------          ------
  1               3
  2               4

Now _without_ sorting by category upfront, those pages would look like
the following if grouping by category is enabled:

A.page 1          page 2
  ------          ------
  [a]             [a]
  1               4
  [b]             [b]
  2               3

What is not visible in this example is that projects inside category
would be sorted by given order.


Now if you would sort by categories _before_ pagination, like you
(from what I understand) proposed, you would have (assuming that
you used "use sort 'stable'" inside block):

  Project       Category
  ---------------------------
  1             a
  4             a
  2             b
  3             b

Pagination would then look like the following:

B.page 1          page 2
  ------          ------
  [a]             [b]
  1               2
  4               3


Now which result you consider correct depends on the point of view.
First is sort, paginate, sort; second is sort, sort, paginate.
First have first N repositories in given order on first page, perhaps
reordered a bit by categories, second doesn't have this feature.
I think that the case A is more correct, but you might disagree.


Let us change example a bit:

  Project       Category
  ---------------------------
  1             b
  2             a
  3             a
  4             b

A.page 1          page 2
  ------          ------
  [a]             [a]
  2               3
  [b]             [b]
  1               4

B.page 1          page 2
  ------          ------
  [a]             [b]
  2               1
  3               4

P.S. It is IMHO better to use

 	for (my $i = $from; $i <= $to; $i++) {

than the style which is not used elsewhere in gitweb, from what
I remember

 	foreach my $i ($from..$to) {

I might also be inefficient as it generates temporary array which
might be quite big; I don't know if Perl 5.8.x, the oldest version
one can sensibly use with gitweb I think, has this bug or not.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: git log --numstat disagrees with git apply --numstat
From: Jeff King @ 2008-12-12  1:52 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081211235337.GK32487@spearce.org>

On Thu, Dec 11, 2008 at 03:53:37PM -0800, Shawn O. Pearce wrote:

> I've found a case where git apply --numstat and git log --numstat
> produce different results for the same commit.

"git apply" doesn't actually do the diff; it just calculates based on
what it sees in the patch. So the real issue is "git log -p" and "git
log --numstat" produce different patches. And you can see it by
instrumenting like this:

diff --git a/diff.c b/diff.c
index af822c1..fce93db 100644
--- a/diff.c
+++ b/diff.c
@@ -726,6 +726,8 @@ static void diffstat_consume(void *priv, char *line, unsigned long len)
 	struct diffstat_t *diffstat = priv;
 	struct diffstat_file *x = diffstat->files[diffstat->nr - 1];
 
+	fwrite(line, 1, len, stderr);
+
 	if (line[0] == '+')
 		x->added++;
 	else if (line[0] == '-')

and then comparing what diffstat_consume gets versus the patch,
something like:

    what="9bda5ece org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java"
    git log -p -1 --pretty=format: $what | grep '^[-+@]' >a
    git log --numstat -1 --pretty=format: $what 2>b >/dev/null
    diff -u a b

It looks like it is just a place where two different valid diffs can be
constructed:

    +-
    +-              for (final RevCommit p : c.parents) {
    +-                      if ((p.flags & SEEN) != 0)
     +              for (;;) {
     +                      final RevCommit c = pending.pop();
     +                      if (c == null)
    @@ -67,19 +68,20 @@
     +                              p.flags |= SEEN;
     +                              pending.add(p);
     +                      }
    --              for (final RevCommit p : c.parents) {
    --                      if ((p.flags & SEEN) != 0)
    ++

which is probably just due to different xdi settings being used between
the two codepaths. I haven't looked closely to see which different
options we are feeding to xdiff.

-Peff

^ permalink raw reply related

* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Junio C Hamano @ 2008-12-12  1:41 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Nguyen Thai Ngoc Duy, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <alpine.LNX.1.00.0812111520490.19665@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

>> That's the point. CE_VALID does not define checkout area while
>> CE_NO_CHECKOUT does.  If an entry is CE_VALID, it is still in checkout
>> area. But if it is CE_NO_CHECKOUT, "git grep" should ignore that path.
>> core.defaultsparse has nothing to do here.
>
> My point is that the index cannot tell git grep whether it should search a 
> path if the path isn't in the index.

Let's step back a bit.  I think "git grep" that stays silent outside of
the checkout area when used to grep in the work tree or in the index is a
mistake.

The problem "sparse checkout" attempts to address is not this:

    I ran "git init && git add ." in /usr/src by mistake.  There is no
    reason for coreutils that is in /usr/src/coreutils and gnucash that is
    in /usr/src/gnucash to share the same development history nor their
    should be any ordering between commits in these two independent
    projects.  I should have done N separate "init & add" independently at
    one level deeper in the directory hierarchy, but I am too lazy to
    filter branch the resulting mess now.

At least, it should not be that, at least to me.

"Sparse" is "I am not going to modify the files in these areas, and I know
they do not need to be present for my purposes (e.g. build), so I do not
need copies in the work tree."  It still works on the whole tree structure
recorded in the commit, but gives you a way to work inside a sparsely
populated work tree, iow, without checking everything out.

So "git grep -e frotz Documentation/", whether you only check out
Documentation or the whole tree, should grep only in Documentation area,
and "git grep -e frotz" should grep in the whole tree, even if you happen
to have a sparse checkout.  By definition, a sparse checkout has no
modifications outside the checkout area, so whenever grep wants to look
for strings outside the checkout area it should pretend as if the same
content as what the index records is in the work tree.  This is consistent
with the way how "git diff" in a sparsely checked out work tree should
behave.

If you understand that, it is clear what "git grep -e frotz HEAD^" should
do.  No checkout area is involved.

^ permalink raw reply

* Re: [RFC] cgit in git?
From: Todd Zullinger @ 2008-12-12  0:15 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, Seth Vidal, Git Mailing List
In-Reply-To: <8c5c35580812111537v1144c9fdy19f2a3cc56e2a04f@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]

Lars Hjemli wrote:
> On Thu, Dec 11, 2008 at 23:35, Junio C Hamano <gitster@pobox.com>
> wrote:
>> Doesn't cgit bind git.git as a subproject at the source level?  I
>> would expect that the most natural release tarball for such a
>> project would be a single tarball that has both the superproject
>> itself _and_ the submodules it contains already extracted, iow, the
>> state of your tree after you run "make get-git".
>
> Your expectation makes sense to me, thanks for elaborating.
> 
> Seth: would such a self-contained tarball solve the problems on your
> end?

(I'm not Seth, nor do I play him on TV -- though I have been offered
his role in a small town production of "How the Grinch Stole
Christmas"... ;)

The downside to this is that cgit would be duplicating the git
sources, and thus, so would any distribution packages.  If there is a
bug in git, both the git and cgit packages would need to be updated to
fix it.

Basically, Fedora tries hard to use system libraries rather than
having applications include their own local copies.  (I recall some
zlib vulnerabilities years back that required way too many packages to
be rebuilt, since they each included their own copy of zlib.)

Obviously, since git is not intended to be used as a library, this
doesn't exactly match that situation.  But cgit is using git as a
library at the moment and if we could find a way to only have one copy
of the git sources to maintain, that'd be ideal from a distribution /
packaging perspective.  I do understand that it might not be as ideal
from either git or cgit developer / maintainer perspective, so the
consideration you're giving the issue is very much appreciated.

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I never vote for anyone; I always vote against.
    -- W.C. Fields


[-- Attachment #2: Type: application/pgp-signature, Size: 542 bytes --]

^ permalink raw reply

* Re: [PATCH v3] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Junio C Hamano @ 2008-12-12  1:17 UTC (permalink / raw)
  To: Fabian Franz; +Cc: git, j.sixt, hjemli, git
In-Reply-To: <20081212002101.292020@gmx.net>

"Fabian Franz" <FabianFranz@gmx.de> writes:

>> Fabian Franz <git@fabian-franz.de> writes:
>> 
>> > However I have both cases: Stable development, where I need one special
>> > version and "wild" development, where I always want the newest published
>> > one.
>> 
>> I do not think supporting both styles of development is a bad idea.
>> 
>> However, use of 0{40} in the index and the resulting commit object in the
>> superproject means that this is a project-wide decision, not your personal
>> preference.  It is not implausible that you would want to do a wild
>> expeeriment in your own clone of a project that uses the "Stable
>> development" approach (hence the upstream never would want to have 0{40}
>> gitlink in its commits).
>
> Yes, but at the same time I might want to record it permanently as a project decision or play at my own with it ...
>
> So both styles should be supported.

While I think they both _could_ have uses, I do not necessarily agree with
your "should be".  First of all, I am not sure project wide 0{40} really
makes sense.

By creating such a commit in your superproject, you are essentially
claiming that you will work with _any_ future version of the subproject,
which is rather absurd.

And using 0{40} in trees and in the index to mark it is not really
necessary, and here is why.

You could tell the participants that you do not care the exact version by
storing 0{40} in the trees and the index, but in order for you to tell
them the tip of which branch of the subproject to use, you need to give
that information (i.e. branch name) to them as well.  Obviously there is
not enough space to put that information in gitlink (we could make room
and I have another implementation in mind but that will be a more involved
change so for a moment let's not go there).  The infomation will come
somewhere out-of-band, not in trees nor in the index.  And at that point,
the presense of such an out-of-band information itself is a good enough
cue that such a path in the superproject is for the "wilder" style of
development with the submodule.

Such an out-of-band information is necessary to use submodules in
distributed development already (iow, the commit object name in gitlink is
not enough), and we already have a Porcelain convention for that.  The
canonical repository URL for each submodule path is distributed as part of
the superproject in .gitmodules.  I would imagine that the message from
the project that says "we expect you to use 'wilder' development style
with this submodule, and use the tip of frotz branch here", if it ever
makes sense, can be recorded in .gitmodules as well.

When updating (or initializing) a submodule, we can check .gitmodules, and
iff it is the "wilder" kind, we can set assume-unchanged in the index and
run "cd there && git fetch $remote $branch && git checkout FETCH_HEAD^0"
or whatever you did in your patch.

If the supermodule did not work well with the updated submodule in such a
checkout, at least you have one commit that you can reset your submodule
checkout to, if you do not wipe that information with 0{40} in the trees
and in the index.  The commit recorded in the gitlink can serve as the
"project wide" suggested version to use, even in "wilder" development
style that also suggests to use "tip of that branch".

^ permalink raw reply

* Re: [PATCH v3] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Junio C Hamano @ 2008-12-12  0:59 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, Fabian Franz, git, j.sixt
In-Reply-To: <8c5c35580812111631k54657bdcme8f048c77b6765eb@mail.gmail.com>

"Lars Hjemli" <hjemli@gmail.com> writes:

> On Thu, Dec 11, 2008 at 21:42, Junio C Hamano <gitster@pobox.com> wrote:
>> I wonder if you can just set "assume unchanged" bit for the subproject
>> gitlink in the index to achieve the same goal.
>
> Using assume-unchanged works, in the sense that the modification to
> the submodule is not detected in the containing repo. But running `git
> submodule update` will checkout the sha1 recorded in HEAD, and I
> suspect Fabian wants something like the hypothetical command `git
> submodule update -b [branch]` which could do `(cd sub && git fetch &&
> git reset --hard origin/$branch)`.

Yeah, that would *also* make sense, but I think that is orthogonal issue.

You can update the state of the checkouts of subproject repositories in
any way you want.  Doing so however makes "git commit -a" inconvenient to
use without assume-unchanged.  The magic 0{40} which Fabian's patch
addresses the same issue in a different way.

Although I would probably detach the head at that point, rather than
resetting whatever branch happens to be checked out:

	( cd sub && git fetch && git checkout origin/$branch^0 )

We also need to make sure that whatever we do we should not break
workflows that do not check out submodules that are uninteresting.  So
doing the above unconditionally to all the submodules is out.  In such a
sparsely populated superproject, "cd sub" would go to an empty directory,
and "git fetch" step would error out.

I did not read Fabian's patch too deeply, and do not remember what checks
it did before running "git pull".  Perhaps it pulled unconditionally?

^ permalink raw reply

* Re: [PATCH] autodetect number of CPUs by default when using threads
From: Nicolas Pitre @ 2008-12-12  0:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v63lqcptn.fsf@gitster.siamese.dyndns.org>

On Thu, 11 Dec 2008, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > I've spent quite a while wondering why repacking in one repo was faster 
> > than repacking in a clone of that repo on the same machine.  So let's 
> > display how many threads are actually used.
> >
> > We have comprehensive test in Makefile to determine if threads are 
> > available, just to not use them by default.  I think that code has 
> > proven itself for long enough now not to let people benefit from it.
> 
> Hmm, it does appear that distros compile with THREADED_DELTA_SEARCH turned
> on (I only checked Fedora and Debian), and I tend to agree with "long
> enough" but "proven itself" feels bit too strong a statement.
> 
> I think defaulting to autodetect is a good change.  I do not like to add
> new output to stderr deep in -rc, though.
> 
> Can we park this in 'next' and move it to 'master' after 1.6.1?

Sure.  This is probably best for next.


Nicolas

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox