From 46fe94db9f8ceed308ab589360d5add581ab3ed9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 15 Mar 2014 11:25:49 -0700 Subject: [PATCH] Support `git-format-patch` format in diff parser Summary: Fixes T4063. The `git format-patch` command produces a special header and footer which we need to detect, strip, and parse. Test Plan: - Added and ran unit tests. - Submitted a diff with `git format-patch HEAD^ --stdout | arc diff --raw`. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4063 Differential Revision: https://secure.phabricator.com/D8547 --- src/parser/ArcanistDiffParser.php | 62 +++++++++++++++++++ .../__tests__/ArcanistDiffParserTestCase.php | 14 +++++ .../__tests__/diff/git-format-patch.gitdiff | 21 +++++++ src/workflow/ArcanistDiffWorkflow.php | 1 + 4 files changed, 98 insertions(+) create mode 100644 src/parser/__tests__/diff/git-format-patch.gitdiff diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php index df548bab..d3b509bf 100644 --- a/src/parser/ArcanistDiffParser.php +++ b/src/parser/ArcanistDiffParser.php @@ -193,6 +193,22 @@ final class ArcanistDiffParser { throw new Exception("Can't parse an empty diff!"); } + // Detect `git-format-patch`, by looking for a "---" line somewhere in + // the file and then a footer with Git version number, which looks like + // this: + // + // -- + // 1.8.4.2 + // + // Note that `git-format-patch` adds a space after the "--", but we don't + // require it when detecting patches, as trailing whitespace can easily be + // lost in transit. + $detect_patch = '/^---$.*^-- ?[\s\d.]+\z/ms'; + $message = null; + if (preg_match($detect_patch, $diff)) { + list($message, $diff) = $this->stripGitFormatPatch($diff); + } + $this->didStartParse($diff); // Strip off header comments. While `patch` allows comments anywhere in the @@ -203,6 +219,14 @@ final class ArcanistDiffParser { $line = $this->nextLine(); } + if (strlen($message)) { + // If we found a message during pre-parse steps, add it to the resulting + // changes here. + $change = $this->buildChange(null) + ->setType(ArcanistDiffChangeType::TYPE_MESSAGE) + ->setMetadata('message', $message); + } + do { $patterns = array( // This is a normal SVN text change, probably from "svn diff". @@ -1346,4 +1370,42 @@ final class ArcanistDiffParser { return array($old, $new); } + + /** + * Strip the header and footer off a `git-format-patch` diff. + * + * Returns a parseable normal diff and a textual commit message. + */ + private function stripGitFormatPatch($diff) { + + // We can parse this by splitting it into two pieces over and over again + // along different section dividers: + // + // 1. Mail headers. + // 2. ("\n\n") + // 3. Mail body. + // 4. ("---") + // 5. Diff stat section. + // 6. ("\n\n") + // 7. Actual diff body. + // 8. ("--") + // 9. Patch footer. + + list($head, $tail) = preg_split("/^---$/m", $diff, 2); + list($mail_headers, $mail_body) = explode("\n\n", $head, 2); + list($body, $foot) = preg_split('/^-- ?$/m', $tail, 2); + list($stat, $diff) = explode("\n\n", $body, 2); + + // Rebuild the commit message by putting the subject line back on top of it, + // if we can find one. + $matches = null; + $pattern = '/^Subject: (?:\[PATCH\] )?(.*)$/mi'; + if (preg_match($pattern, $mail_headers, $matches)) { + $mail_body = $matches[1]."\n\n".$mail_body; + $mail_body = rtrim($mail_body); + } + + return array($mail_body, $diff); + } + } diff --git a/src/parser/__tests__/ArcanistDiffParserTestCase.php b/src/parser/__tests__/ArcanistDiffParserTestCase.php index d90988cc..7be06394 100644 --- a/src/parser/__tests__/ArcanistDiffParserTestCase.php +++ b/src/parser/__tests__/ArcanistDiffParserTestCase.php @@ -575,6 +575,20 @@ EOTEXT case 'svnlook-delete.svndiff': $this->assertEqual(1, count($changes)); break; + case 'git-format-patch.gitdiff': + $this->assertEqual(2, count($changes)); + + $change = array_shift($changes); + $this->assertEqual( + ArcanistDiffChangeType::TYPE_MESSAGE, + $change->getType()); + $this->assertEqual("WIP", $change->getMetadata('message')); + + $change = array_shift($changes); + $this->assertEqual( + ArcanistDiffChangeType::TYPE_CHANGE, + $change->getType()); + break; default: throw new Exception("No test block for diff file {$diff_file}."); break; diff --git a/src/parser/__tests__/diff/git-format-patch.gitdiff b/src/parser/__tests__/diff/git-format-patch.gitdiff new file mode 100644 index 00000000..f69e7edd --- /dev/null +++ b/src/parser/__tests__/diff/git-format-patch.gitdiff @@ -0,0 +1,21 @@ +From 9c68ca45aafefc1f9b9f49a7b380d6159d3e93c1 Mon Sep 17 00:00:00 2001 +From: epriestley +Date: Sat, 15 Mar 2014 08:54:34 -0700 +Subject: [PATCH] WIP + +--- + number_j.txt | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/number_j.txt b/number_j.txt +index bd44aa2..d64876d 100644 +--- a/number_j.txt ++++ b/number_j.txt +@@ -138,3 +138,4 @@ j + j + j + j ++j +-- +1.8.4.2 + diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index bbfac224..fe02de89 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -1574,6 +1574,7 @@ EOTEXT )); } } + $old_message = $template; $included = array();