* [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>
---
| 18 ++++++++++++++++++
.../tst/org/spearce/jgit/patch/PatchTest.java | 2 ++
| 5 +++++
.../src/org/spearce/jgit/patch/Patch.java | 18 ++++++++++++++----
4 files changed, 39 insertions(+), 4 deletions(-)
--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());
--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 ++++++++++++++++++++
| 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
+
--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>
---
| 6 +++++
.../tst/org/spearce/jgit/patch/PatchTest.java | 4 +++
| 21 ++++++++++++++++++++
3 files changed, 31 insertions(+), 0 deletions(-)
--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());
--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
* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Sébastien Cevey @ 2008-12-12 0:40 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <200812120113.47051.jnareb@gmail.com>
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.
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.
--
Sébastien Cevey / inso.cc
^ permalink raw reply
* Re: [PATCH v3] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Lars Hjemli @ 2008-12-12 0:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fabian Franz, git, j.sixt
In-Reply-To: <7vbpvicuk2.fsf@gitster.siamese.dyndns.org>
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)`.
>
> Or is there more to it?
>
Something like this (probably mangled) patch is needed for 'submodule
status' to behave sensibly when the assume-unchanged bit is turned on
for a submodule path.
diff --git a/git-submodule.sh b/git-submodule.sh
index 2f47e06..375dfbf 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -588,21 +588,21 @@ cmd_status()
do
name=$(module_name "$path") || exit
url=$(git config submodule."$name".url)
+ orgsha1=$(git ls-tree HEAD "$path" | cut -d ' ' -f 3 | cut -f 1)
if test -z "$url" || ! test -d "$path"/.git -o -f "$path"/.git
then
say "-$sha1 $path"
continue;
fi
+ if test -z "$cached"
+ then
+ sha1=$(unset GIT_DIR; cd "$path" && git
rev-parse --verify HEAD)
+ fi
set_name_rev "$path" "$sha1"
- if git diff-files --quiet -- "$path"
+ if test "$sha1" = "$orgsha1"
then
say " $sha1 $path$revname"
else
- if test -z "$cached"
- then
- sha1=$(unset GIT_DIR; cd "$path" &&
git rev-parse --verify HEAD)
- set_name_rev "$path" "$sha1"
- fi
say "+$sha1 $path$revname"
fi
done
^ permalink raw reply related
* Re: [PATCH v3] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Fabian Franz @ 2008-12-12 0:21 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: j.sixt, hjemli, git
In-Reply-To: <7vbpvicuk2.fsf@gitster.siamese.dyndns.org>
> Fabian Franz <git@fabian-franz.de> writes:
>
> > Submodules currently only allow tracking a specific revision
> > and each update in a submodule leads to a new commit in the
> > master repository. However some users may want to always track
> > the newest revision of a specific (named) tag or branch or HEAD.
> > For example the user might want to track a staging branch in all
> > submodules.
>
> I initially liked the direction this is going, but I think the above
> rationale and the change to use 0{40} have impedance mismatch. Your
> change is not a good way to go about "some users may want". I'll discuss
> more on this below.
I do agree. I did like the idea of HEAD.gitlink also better and I even more like the assume-unchanged version, which works.
> It seems to me that what you are really after is to let you change the
> state of the subproject checkout in whatever way and have "git commit -a"
> in the superproject ignore that change.
>
> I wonder if you can just set "assume unchanged" bit for the subproject
> gitlink in the index to achieve the same goal.
>
> Or is there more to it?
Nope, that is it. I just did not knew that flag.
> > 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.
> For example, suppose the project uses "Stable development" approach, and
> records the v1.0.0 of submodule at "sub/" in the superproject. You are a
> contributor to that project, and would want to help them futureproof the
> superproject code to be forward compatible with the upcoming v1.2.0
> release of the subproject. What would you do?
>
> * have a clone of superproject, with v1.0.0 submodule bound at "sub/";
>
> * go to "sub/", fetch and checkout v1.2.0-rc2;
>
> * go up, build using the updated submodule, see many failures in
> supermodule build;
>
> * fix them up in a way that can work with both v1.0.0 and v1.2.0 of the
> submodule, while making commits in logical steps, in the supermodule.
>
> And you do not want to record the fact that you used v1.2.0-rc2 of the
> submodule at "sub/" in the commits you make in the supermodule, as you
> would want to label these commits as "futureproof for upcoming submodule
> v1.2.0".
>
> But you cannot use your 0{40} trick, as sending that to the upstream of
> the superproject would break their "Stable development" policy.
>
> I wonder if you can just set "assume unchanged" bit for the subproject
> gitlink in the index to achieve the same goal. That would be a local
> operation, the gitlink would still point at v1.0.0 version of submodule,
> and "git commit -a" in the superproject won't make commits that flips
> everybody else's copy to use v1.2.0-rc2 of submodule.
Yes, that works. I tried it. I am now gonna change the patch to use this new approach and also re-think the workflow I want to support.
> > @@ -118,6 +118,10 @@ update::
> > If the submodule is not yet initialized, and you just want to use the
> > setting as stored in .gitmodules, you can automatically initialize the
> > submodule with the --init option.
> > ++
> > +If you used --track or set the "track" option in .gitmodules this will
> > +automatically pull the newest updates from remote instead of tracking a
> > +specific revision.
>
> "automatically pull" in the sense that it always goes to the remote, fetch
> and merge? That sounds horribly broken. You can never work disconnected?
Uhm, the same is already true for git submodule update. Before it checkouts the new branch it always does a fetch.
However I think what I really want (without) scripting via foreach is:
git checkout staging
In .gitmodules is from a personal (own branch) or project wide decision the fact documented that the submodules do track staging and one stable tag for example.
[module1]
track = staging
[module2]
track = staging
[module3]
track = stable-v1.0.0
And now I just do git submodule update and it fetches and afterwards checks out my local branch of staging and fast-forwards it.
However I do agree that in that workflow you always have to go online and that is not good.
But in my case I also want a developer to just be able to change to
[module1]
track = feature_1
[module2]
track = feature_1
[module3]
track = stable-v1.0.0
to work in a feature branch in two submodules at once.
So I am gonna rethink this design.
> > @@ -159,6 +163,10 @@ OPTIONS
> > --branch::
> > Branch of repository to add as submodule.
> >
> > +-t::
> > +--track::
> > + Branch/Tag/HEAD of repository to track in a submodule.
> > +
>
> How does the branch parameter to the --track option interact with the
> branch parameter to the --branch option? Does an end user typically set
> them to the same branch? Or would these parameters almost always point at
> different branchesof the remote repository? What are the reasons for the
> end user to choose one parameter value for the --branch option and a
> different parameter value for the --track option?
--branch is always checking out just the branch on the initial add and then the branch head is of course recorded as commit in the index.
However I found that option just helpful for creating a first initial commit already in a branch.
For later submodule init or updates this has no effect.
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 2f47e06..16df528 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > ...
> > @@ -197,12 +203,14 @@ cmd_add()
> > (unset GIT_DIR; cd "$path" && git checkout -f -q ${branch:+-b
> "$branch" "origin/$branch"}) ||
> > die "Unable to checkout submodule '$path'"
> > fi
> > + test -n "$track" && printf '160000
> 0000000000000000000000000000000000000000\t%s\n' "$path" | git update-index --index-info
> >
>
> You have many overlong lines due to the 0{40} constant string in your
> patch. Have a
>
> null_sha1=0000000000000000000000000000000000000000
>
> at the beginning of the script, and rewrite the above like this:
>
> test -n "$track" &&
> printf '160000 %s\t%s\n' "$null_sha1" "$path" |
> git update-index --index-info
>
> or even like this:
>
> if test -n "$track"
> then
> printf '160000 %s\t%s\n' "$null_sha1" "$path" |
> git update-index --index-info
> fi
>
> Use of $null_sha1 throughout the script will make things easier to read
> and at the same time make it less error prone as well for "git submodule"
> developers.
Okay, thanks that is a good practice.
> > @@ -345,11 +357,29 @@ cmd_update()
> > then
> > force="-f"
> > fi
> > + pull=
> > + if [ "$sha1" = "0000000000000000000000000000000000000000" ]
> > + then
> > + track=$(git config submodule."$name".track)
> > + : ${track:="master"}
>
> In the v2 patch this used to point at "HEAD". What made you change your
> mind?
Because at the moment HEAD would be created as local branhc, which is horribly broken.
> > + # if the local branch does not yet exist, create it
> > + ( unset GIT_DIR; cd "$path"; git-show-ref --heads --tags -q
> "$track" || git branch --track "$track" "origin/$track" )
>
> No error checking?
>
> (
> unset GIT_DIR;
> cd "$path" &&
> git show-ref --heads --tags -q "$track" ||
> git branch --track "$track" "origin/$track"
> ) || barf
>
> The ';' after unset is intentional; some shells reports failure when you
> unset an unset variable.
Okay, thanks. I didn't know that.
>
> > + sha1="$track"
> > + pull=1
>
> I tend to prefer booleans in shell scripts spelled like boolean, e.g.
>
> pull=yes
Good idea.
>
> > + fi
> > +
> > (unset GIT_DIR; cd "$path" && git-fetch &&
> > git-checkout $force -q "$sha1") ||
> > die "Unable to checkout '$sha1' in submodule path '$path'"
> >
> > say "Submodule path '$path': checked out '$sha1'"
> > +
> > + if [ "$pull" = "1" ]
> > + then
> > + # Now pull new updates from origin
> > + ( unset GIT_DIR; cd "$path"; git-pull ) || die "Unable to pull in
> submodule path '$path'"
>
> No error checking?
In v3 there should be ... And I even see error checking in the above ...
>
> > + fi
> > +
> > fi
> > done
> > }
> > @@ -596,7 +626,8 @@ cmd_status()
> > set_name_rev "$path" "$sha1"
> > if git diff-files --quiet -- "$path"
> > then
> > - say " $sha1 $path$revname"
> > + track=$(git config submodule."$name".track)
> > + say " $sha1 $path$revname${track:+ (tracking "$track")}"
> > else
> > if test -z "$cached"
> > then
> > diff --git a/read-cache.c b/read-cache.c
> > index 8579663..0c14b68 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -137,6 +137,11 @@ static int ce_compare_gitlink(struct cache_entry
> *ce)
> > */
> > if (resolve_gitlink_ref(ce->name, "HEAD", sha1) < 0)
> > return 0;
> > +
> > + // To be able to track newest revision
> > + if (is_null_sha1(ce->sha1))
> > + return 0;
> > +
>
> I think the comment is wrong, as it is not about newness at all.
>
> /* ignore changes in the submodule path */
>
> would be more appropriate.
Yes, I do agree. I am now changing first to --assume-unchanged so this is made unnecessary.
I'll write a PATCH/RFC next.
Thank you for your detailed feedback,
Best Wishes,
Fabian
^ permalink raw reply
* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Jakub Narebski @ 2008-12-12 0:13 UTC (permalink / raw)
To: Sébastien Cevey
Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <878wqmz3pl.wl%seb@cine7.net>
On Fri, 12 Dec 2008, Sébastien Cevey wrote:
> At Fri, 5 Dec 2008 11:45:08 +0100, Jakub Narebski wrote:
>
>>>> Nice... but perhaps it would be better to simply pass $from / $to to
>>>> build_projlist_by_category function, and have in %categories only
>>>> projects which are shown...
>>>
>>> Ah you're right, we could do that, I hadn't thought of it. Sounds
>>> cleaner than the $from/$to dance I did in this patch.
>>
>> [...] we can simply pass $from and $to to
>> build_projlist_by_category(), and use loop $from..$to there.
>
> I just tried, it works but we first need to sort @projects by
> category.
I don't understand. You have the following code:
+# returns a hash of categories, containing the list of project
+# belonging to each category
+sub build_projlist_by_category {
+ my $projlist = shift;
+ my %categories;
+
+ for my $pr (@$projlist) {
+ push @{$categories{ $pr->{'category'} }}, $pr;
+ }
+
+ return %categories;
+}
I propose to change it to:
+# returns a hash of categories, containing the list of project
+# belonging to each category
+sub build_projlist_by_category {
+ my ($projlist, $from, $to) = shift; # <<<<<<<<<<<<<<<<<<<<<<<
+ $from = 0 unless defined $from; # +++++++++++++++++++++++
+ $to = $#$projlist if (!defined $to || $#$projlist < $to); # +++++++++++++++++++++++
+ my %categories;
+
+ for (my $i = $from; $i <= $to; $i++) { # <<<<<<<<<<<<<<<<<<<<<<<
+ $pr = $projlist->[$i]; # +++++++++++++++++++++++
+ push @{$categories{ $pr->{'category'} }}, $pr;
+ }
+
+ return %categories;
+}
And of course update callers to pass $from and $to parameters.
This code doesn't change _anything_ beside the fact that it can operate
only on part of @$projlist.
But as you have noticed some kinds of limiting shown project list size
make sense only if done _after_ dividing into categories. Some but not
all. For example page length limiting after sorting by name,
description, owner or age (sorting which takes place before dividing
into categories) makes sense if it is done _before_ categorization:
we want to show e.g. 100 freshest projects, and not up to 100 projects
from first categories, with last category showing maybe only freshest
projects in this category.
Besides making selection of projects to show operate on list of projects
instead of being done just before display is IMHO better, more flexible,
and more robust solution.
> I'm gonna re-send the three patches as a new version.
Thanks in advance.
--
Jakub Narebski
Poland
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox