From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 07/15] Set empty patches with no Git metadata to PatchType.BINARY
Date: Thu, 11 Dec 2008 18:46:13 -0800 [thread overview]
Message-ID: <1229049981-14152-8-git-send-email-spearce@spearce.org> (raw)
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
next prev parent reply other threads:[~2008-12-12 2:48 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-12 2:46 [JGIT PATCH 00/15] More patch parsing support Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 01/15] Correct use of TemporaryBuffer in Patch Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 02/15] Add tests for TemporaryBuffer Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer> Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 04/15] Add lineMap computer to RawParseUtils to index locations of line starts Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 05/15] Define FileHeader.PatchType to report the style of patch used Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 06/15] Test for non-git binary files and mark them as PatchType.BINARY Shawn O. Pearce
2008-12-12 2:46 ` Shawn O. Pearce [this message]
2008-12-12 2:46 ` [JGIT PATCH 08/15] Always use the FileHeader buffer during Patch.parseHunks Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 09/15] Parse "GIT binary patch" style patch metadata Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 10/15] Record patch parsing errors for later inspection by applications Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 11/15] Fix Patch.parse to honor the end point passed in Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 12/15] Correctly handle hunk headers such as "@@ -0,0 +1 @@" Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 13/15] Patch parse test comparing "git log -p" output to "git log --numstat" Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 14/15] Abstract the hunk header testing into a method Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 15/15] Treat "diff --combined" the same as "diff --cc" Shawn O. Pearce
2008-12-12 23:11 ` Robin Rosenberg
2008-12-12 23:18 ` [JGIT PATCH 15/15 v2] " Shawn O. Pearce
[not found] ` <bd6139dc0812120243y2b1a3dddu4975162114280e17@mail.gmail.com>
2008-12-12 15:15 ` [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer> Shawn O. Pearce
2008-12-12 15:33 ` Sverre Rabbelier
2008-12-12 15:41 ` Shawn O. Pearce
2008-12-12 15:50 ` Sverre Rabbelier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1229049981-14152-8-git-send-email-spearce@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).