From f750d5f8dc91007a23d5a83786cae3d1d81b6827 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 19 Dec 2013 11:05:06 -0800 Subject: [PATCH] Provide a low-level SVN commit query, and merge the VCS query types Summary: Ref T4195. Even though we use `svnlook` in the hook itself, I need this query elsewhere, so provide it and merge the classes into one which does the right thing. Test Plan: - Used `reparse.php` to reparse messages for Git, SVN and Mercurial commits, using `var_dump()` to examine the commit refs for sanity. - Used `reparse.php` to reparse changes for an SVN commit. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4195 Differential Revision: https://secure.phabricator.com/D7800 --- src/__phutil_library_map__.php | 6 +- .../engine/DiffusionCommitHookEngine.php | 6 +- .../lowlevel/DiffusionLowLevelCommitQuery.php | 137 ++++++++++++++++++ .../DiffusionLowLevelGitCommitQuery.php | 48 ------ .../DiffusionLowLevelMercurialCommitQuery.php | 41 ------ ...habricatorRepositoryCommitParserWorker.php | 22 --- ...rRepositorySvnCommitChangeParserWorker.php | 19 ++- ...RepositoryGitCommitMessageParserWorker.php | 2 +- ...toryMercurialCommitMessageParserWorker.php | 2 +- ...RepositorySvnCommitMessageParserWorker.php | 16 +- 10 files changed, 166 insertions(+), 133 deletions(-) create mode 100644 src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php delete mode 100644 src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php delete mode 100644 src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 80ac8d25a1..a5b0803014 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -507,10 +507,9 @@ phutil_register_library_map(array( 'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php', 'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php', 'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php', - 'DiffusionLowLevelGitCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php', + 'DiffusionLowLevelCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php', 'DiffusionLowLevelGitRefQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php', 'DiffusionLowLevelMercurialBranchesQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php', - 'DiffusionLowLevelMercurialCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php', 'DiffusionLowLevelQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php', 'DiffusionLowLevelResolveRefsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php', 'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php', @@ -2891,10 +2890,9 @@ phutil_register_library_map(array( 'DiffusionLastModifiedController' => 'DiffusionController', 'DiffusionLintController' => 'DiffusionController', 'DiffusionLintDetailsController' => 'DiffusionController', - 'DiffusionLowLevelGitCommitQuery' => 'DiffusionLowLevelQuery', + 'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelMercurialBranchesQuery' => 'DiffusionLowLevelQuery', - 'DiffusionLowLevelMercurialCommitQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelQuery' => 'Phobject', 'DiffusionLowLevelResolveRefsQuery' => 'DiffusionLowLevelQuery', 'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery', diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 3996b9ee6e..be6898d774 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -939,12 +939,8 @@ final class DiffusionCommitHookEngine extends Phobject { $vcs = $repository->getVersionControlSystem(); switch ($vcs) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - return id(new DiffusionLowLevelGitCommitQuery()) - ->setRepository($repository) - ->withIdentifier($identifier) - ->execute(); case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - return id(new DiffusionLowLevelMercurialCommitQuery()) + return id(new DiffusionLowLevelCommitQuery()) ->setRepository($repository) ->withIdentifier($identifier) ->execute(); diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php new file mode 100644 index 0000000000..dc5f3db137 --- /dev/null +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php @@ -0,0 +1,137 @@ +identifier = $identifier; + return $this; + } + + public function executeQuery() { + if (!strlen($this->identifier)) { + throw new Exception( + pht('You must provide an identifier with withIdentifier()!')); + } + + $type = $this->getRepository()->getVersionControlSystem(); + switch ($type) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $result = $this->loadGitCommitRef(); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $result = $this->loadMercurialCommitRef(); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $result = $this->loadSubversionCommitRef(); + break; + default: + throw new Exception(pht('Unsupported repository type "%s"!', $type)); + } + + return $result; + } + + private function loadGitCommitRef() { + $repository = $this->getRepository(); + + // NOTE: %B was introduced somewhat recently in git's history, so pull + // commit message information with %s and %b instead. + + // Even though we pass --encoding here, git doesn't always succeed, so + // we try a little harder, since git *does* tell us what the actual encoding + // is correctly (unless it doesn't; encoding is sometimes empty). + list($info) = $repository->execxLocalCommand( + 'log -n 1 --encoding=%s --format=%s %s --', + 'UTF-8', + implode('%x00', array('%e', '%cn', '%ce', '%an', '%ae', '%s%n%n%b')), + $this->identifier); + + $parts = explode("\0", $info); + $encoding = array_shift($parts); + + foreach ($parts as $key => $part) { + if ($encoding) { + $part = phutil_utf8_convert($part, 'UTF-8', $encoding); + } + $parts[$key] = phutil_utf8ize($part); + if (!strlen($parts[$key])) { + $parts[$key] = null; + } + } + + return id(new DiffusionCommitRef()) + ->setCommitterName($parts[0]) + ->setCommitterEmail($parts[1]) + ->setAuthorName($parts[2]) + ->setAuthorEmail($parts[3]) + ->setMessage($parts[4]); + } + + private function loadMercurialCommitRef() { + $repository = $this->getRepository(); + + list($stdout) = $repository->execxLocalCommand( + 'log --template %s --rev %s', + '{author}\\n{desc}', + hgsprintf('%s', $this->identifier)); + + list($author, $message) = explode("\n", $stdout, 2); + + $author = phutil_utf8ize($author); + $message = phutil_utf8ize($message); + + list($author_name, $author_email) = $this->splitUserIdentifier($author); + + return id(new DiffusionCommitRef()) + ->setAuthorName($author_name) + ->setAuthorEmail($author_email) + ->setMessage($message); + } + + private function loadSubversionCommitRef() { + $repository = $this->getRepository(); + + list($xml) = $repository->execxRemoteCommand( + 'log --xml --limit 1 %s', + $repository->getSubversionPathURI(null, $this->identifier)); + + // Subversion may send us back commit messages which won't parse because + // they have non UTF-8 garbage in them. Slam them into valid UTF-8. + $xml = phutil_utf8ize($xml); + $log = new SimpleXMLElement($xml); + $entry = $log->logentry[0]; + + $author = (string)$entry->author; + $message = (string)$entry->msg; + + list($author_name, $author_email) = $this->splitUserIdentifier($author); + + return id(new DiffusionCommitRef()) + ->setAuthorName($author_name) + ->setAuthorEmail($author_email) + ->setMessage($message); + } + + private function splitUserIdentifier($user) { + $email = new PhutilEmailAddress($user); + + if ($email->getDisplayName() || $email->getDomainName()) { + $user_name = $email->getDisplayName(); + $user_email = $email->getAddress(); + } else { + $user_name = $email->getAddress(); + $user_email = null; + } + + return array($user_name, $user_email); + } + +} diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php deleted file mode 100644 index d5f33c8d64..0000000000 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php +++ /dev/null @@ -1,48 +0,0 @@ -identifier = $identifier; - return $this; - } - - protected function executeQuery() { - $repository = $this->getRepository(); - - // NOTE: %B was introduced somewhat recently in git's history, so pull - // commit message information with %s and %b instead. - - // Even though we pass --encoding here, git doesn't always succeed, so - // we try a little harder, since git *does* tell us what the actual encoding - // is correctly (unless it doesn't; encoding is sometimes empty). - list($info) = $repository->execxLocalCommand( - 'log -n 1 --encoding=%s --format=%s %s --', - 'UTF-8', - implode('%x00', array('%e', '%cn', '%ce', '%an', '%ae', '%s%n%n%b')), - $this->identifier); - - $parts = explode("\0", $info); - $encoding = array_shift($parts); - - foreach ($parts as $key => $part) { - if ($encoding) { - $part = phutil_utf8_convert($part, 'UTF-8', $encoding); - } - $parts[$key] = phutil_utf8ize($part); - if (!strlen($parts[$key])) { - $parts[$key] = null; - } - } - - return id(new DiffusionCommitRef()) - ->setCommitterName($parts[0]) - ->setCommitterEmail($parts[1]) - ->setAuthorName($parts[2]) - ->setAuthorEmail($parts[3]) - ->setMessage($parts[4]); - } - -} diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php deleted file mode 100644 index 7a0d92d35a..0000000000 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php +++ /dev/null @@ -1,41 +0,0 @@ -identifier = $identifier; - return $this; - } - - protected function executeQuery() { - $repository = $this->getRepository(); - - list($stdout) = $repository->execxLocalCommand( - 'log --template %s --rev %s', - '{author}\\n{desc}', - hgsprintf('%s', $this->identifier)); - - list($author, $message) = explode("\n", $stdout, 2); - - $author = phutil_utf8ize($author); - $message = phutil_utf8ize($message); - - $email = new PhutilEmailAddress($author); - if ($email->getDisplayName() || $email->getDomainName()) { - $author_name = $email->getDisplayName(); - $author_email = $email->getAddress(); - } else { - $author_name = $email->getAddress(); - $author_email = null; - } - - return id(new DiffusionCommitRef()) - ->setAuthorName($author_name) - ->setAuthorEmail($author_email) - ->setMessage($message); - } - -} diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php index e75e6022c8..607b80de46 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php @@ -52,28 +52,6 @@ abstract class PhabricatorRepositoryCommitParserWorker PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit); - /** - * This method is kind of awkward here but both the SVN message and - * change parsers use it. - */ - protected function getSVNLogXMLObject($uri, $revision, $verbose = false) { - - if ($verbose) { - $verbose = '--verbose'; - } - - list($xml) = $this->repository->execxRemoteCommand( - "log --xml {$verbose} --limit 1 %s@%d", - $uri, - $revision); - - // Subversion may send us back commit messages which won't parse because - // they have non UTF-8 garbage in them. Slam them into valid UTF-8. - $xml = phutil_utf8ize($xml); - - return new SimpleXMLElement($xml); - } - protected function isBadCommit($full_commit_name) { $repository = new PhabricatorRepository(); diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php index d802a900a2..428780f2f4 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php @@ -29,7 +29,7 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker // Pull the top-level path changes out of "svn log". This is pretty // straightforward; just parse the XML log. - $log = $this->getSVNLogXMLObject($uri, $svn_commit, $verbose = true); + $log = $this->getSVNLogXMLObject($uri, $svn_commit); $entry = $log->logentry[0]; @@ -769,4 +769,21 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker return $parents; } + /** + * This method is kind of awkward here but both the SVN message and + * change parsers use it. + */ + private function getSVNLogXMLObject($uri, $revision) { + list($xml) = $this->repository->execxRemoteCommand( + "log --xml --verbose --limit 1 %s@%d", + $uri, + $revision); + + // Subversion may send us back commit messages which won't parse because + // they have non UTF-8 garbage in them. Slam them into valid UTF-8. + $xml = phutil_utf8ize($xml); + + return new SimpleXMLElement($xml); + } + } diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php index f437ff54fa..b10a835ad1 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php @@ -7,7 +7,7 @@ final class PhabricatorRepositoryGitCommitMessageParserWorker PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit) { - $ref = id(new DiffusionLowLevelGitCommitQuery()) + $ref = id(new DiffusionLowLevelCommitQuery()) ->setRepository($repository) ->withIdentifier($commit->getCommitIdentifier()) ->execute(); diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php index e7b126d871..6084b62bd9 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php @@ -7,7 +7,7 @@ final class PhabricatorRepositoryMercurialCommitMessageParserWorker PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit) { - $ref = id(new DiffusionLowLevelMercurialCommitQuery()) + $ref = id(new DiffusionLowLevelCommitQuery()) ->setRepository($repository) ->withIdentifier($commit->getCommitIdentifier()) ->execute(); diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php index 78654352f6..7aabcd4346 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php @@ -7,17 +7,13 @@ final class PhabricatorRepositorySvnCommitMessageParserWorker PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit) { - $uri = $repository->getSubversionPathURI(); + $ref = id(new DiffusionLowLevelCommitQuery()) + ->setRepository($repository) + ->withIdentifier($commit->getCommitIdentifier()) + ->execute(); - $log = $this->getSVNLogXMLObject( - $uri, - $commit->getCommitIdentifier(), - $verbose = false); - - $entry = $log->logentry[0]; - - $author = (string)$entry->author; - $message = (string)$entry->msg; + $author = $ref->getAuthor(); + $message = $ref->getMessage(); $this->updateCommitData($author, $message);