From b43f84d6dc457e2f3df80470cfba5e116f20d04d Mon Sep 17 00:00:00 2001 From: vrana Date: Fri, 28 Sep 2012 14:36:47 -0700 Subject: [PATCH] Read rename information from DB instead of repository Summary: We already have this information for all VCSes, there's no point in parsing it again. We currently support it only for Git and I remember there were some bugs in it. It should also be faster. Test Plan: Blamed previous revision of a file which was moved in SVN. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3564 --- src/__phutil_library_map__.php | 9 +- .../DiffusionBrowseFileController.php | 6 +- .../query/DiffusionRenameHistoryQuery.php | 112 ++++++++++++++++++ .../DiffusionGitRenameHistoryQuery.php | 88 -------------- .../DiffusionMercurialRenameHistoryQuery.php | 27 ----- .../DiffusionRenameHistoryQuery.php | 51 -------- .../DiffusionSvnRenameHistoryQuery.php | 27 ----- 7 files changed, 116 insertions(+), 204 deletions(-) create mode 100644 src/applications/diffusion/query/DiffusionRenameHistoryQuery.php delete mode 100644 src/applications/diffusion/query/renamehistory/DiffusionGitRenameHistoryQuery.php delete mode 100644 src/applications/diffusion/query/renamehistory/DiffusionMercurialRenameHistoryQuery.php delete mode 100644 src/applications/diffusion/query/renamehistory/DiffusionRenameHistoryQuery.php delete mode 100644 src/applications/diffusion/query/renamehistory/DiffusionSvnRenameHistoryQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7616cca30c..4133b07d8b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -349,7 +349,6 @@ phutil_register_library_map(array( 'DiffusionGitLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php', 'DiffusionGitMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionGitMergedCommitsQuery.php', 'DiffusionGitRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php', - 'DiffusionGitRenameHistoryQuery' => 'applications/diffusion/query/renamehistory/DiffusionGitRenameHistoryQuery.php', 'DiffusionGitRequest' => 'applications/diffusion/request/DiffusionGitRequest.php', 'DiffusionGitTagListQuery' => 'applications/diffusion/query/taglist/DiffusionGitTagListQuery.php', 'DiffusionHistoryController' => 'applications/diffusion/controller/DiffusionHistoryController.php', @@ -372,7 +371,6 @@ phutil_register_library_map(array( 'DiffusionMercurialLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php', 'DiffusionMercurialMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionMercurialMergedCommitsQuery.php', 'DiffusionMercurialRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php', - 'DiffusionMercurialRenameHistoryQuery' => 'applications/diffusion/query/renamehistory/DiffusionMercurialRenameHistoryQuery.php', 'DiffusionMercurialRequest' => 'applications/diffusion/request/DiffusionMercurialRequest.php', 'DiffusionMercurialTagListQuery' => 'applications/diffusion/query/taglist/DiffusionMercurialTagListQuery.php', 'DiffusionMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionMergedCommitsQuery.php', @@ -385,7 +383,7 @@ phutil_register_library_map(array( 'DiffusionPathValidateController' => 'applications/diffusion/controller/DiffusionPathValidateController.php', 'DiffusionQuery' => 'applications/diffusion/query/DiffusionQuery.php', 'DiffusionRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php', - 'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/renamehistory/DiffusionRenameHistoryQuery.php', + 'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php', 'DiffusionRepositoryController' => 'applications/diffusion/controller/DiffusionRepositoryController.php', 'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php', 'DiffusionRepositoryTag' => 'applications/diffusion/DiffusionRepositoryTag.php', @@ -402,7 +400,6 @@ phutil_register_library_map(array( 'DiffusionSvnLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php', 'DiffusionSvnMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionSvnMergedCommitsQuery.php', 'DiffusionSvnRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php', - 'DiffusionSvnRenameHistoryQuery' => 'applications/diffusion/query/renamehistory/DiffusionSvnRenameHistoryQuery.php', 'DiffusionSvnRequest' => 'applications/diffusion/request/DiffusionSvnRequest.php', 'DiffusionSvnTagListQuery' => 'applications/diffusion/query/taglist/DiffusionSvnTagListQuery.php', 'DiffusionSymbolController' => 'applications/diffusion/controller/DiffusionSymbolController.php', @@ -1546,7 +1543,6 @@ phutil_register_library_map(array( 'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery', 'DiffusionGitMergedCommitsQuery' => 'DiffusionMergedCommitsQuery', 'DiffusionGitRawDiffQuery' => 'DiffusionRawDiffQuery', - 'DiffusionGitRenameHistoryQuery' => 'DiffusionRenameHistoryQuery', 'DiffusionGitRequest' => 'DiffusionRequest', 'DiffusionGitTagListQuery' => 'DiffusionTagListQuery', 'DiffusionHistoryController' => 'DiffusionController', @@ -1569,7 +1565,6 @@ phutil_register_library_map(array( 'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery', 'DiffusionMercurialMergedCommitsQuery' => 'DiffusionMergedCommitsQuery', 'DiffusionMercurialRawDiffQuery' => 'DiffusionRawDiffQuery', - 'DiffusionMercurialRenameHistoryQuery' => 'DiffusionRenameHistoryQuery', 'DiffusionMercurialRequest' => 'DiffusionRequest', 'DiffusionMercurialTagListQuery' => 'DiffusionTagListQuery', 'DiffusionMergedCommitsQuery' => 'DiffusionQuery', @@ -1577,7 +1572,6 @@ phutil_register_library_map(array( 'DiffusionPathQueryTestCase' => 'PhabricatorTestCase', 'DiffusionPathValidateController' => 'DiffusionController', 'DiffusionRawDiffQuery' => 'DiffusionQuery', - 'DiffusionRenameHistoryQuery' => 'DiffusionQuery', 'DiffusionRepositoryController' => 'DiffusionController', 'DiffusionSetupException' => 'AphrontUsageException', 'DiffusionSvnBrowseQuery' => 'DiffusionBrowseQuery', @@ -1591,7 +1585,6 @@ phutil_register_library_map(array( 'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery', 'DiffusionSvnMergedCommitsQuery' => 'DiffusionMergedCommitsQuery', 'DiffusionSvnRawDiffQuery' => 'DiffusionRawDiffQuery', - 'DiffusionSvnRenameHistoryQuery' => 'DiffusionRenameHistoryQuery', 'DiffusionSvnRequest' => 'DiffusionRequest', 'DiffusionSvnTagListQuery' => 'DiffusionTagListQuery', 'DiffusionSymbolController' => 'DiffusionController', diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index e5eeae6a41..96048cadf0 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -754,8 +754,8 @@ final class DiffusionBrowseFileController extends DiffusionController { $parent->getCommitIdentifier()); if ($grandparent) { - $rename_query = DiffusionRenameHistoryQuery::newFromDiffusionRequest( - $drequest); + $rename_query = new DiffusionRenameHistoryQuery(); + $rename_query->setRequest($drequest); $rename_query->setOldCommit($grandparent->getCommitIdentifier()); $old_filename = $rename_query->loadOldFilename(); $was_created = $rename_query->getWasCreated(); @@ -783,7 +783,7 @@ final class DiffusionBrowseFileController extends DiffusionController { $path = $drequest->getPath(); $renamed = null; if ($old_filename !== null && - $old_filename !== $path) { + $old_filename !== '/'.$path) { $renamed = $path; $path = $old_filename; } diff --git a/src/applications/diffusion/query/DiffusionRenameHistoryQuery.php b/src/applications/diffusion/query/DiffusionRenameHistoryQuery.php new file mode 100644 index 0000000000..cb2db9eb7c --- /dev/null +++ b/src/applications/diffusion/query/DiffusionRenameHistoryQuery.php @@ -0,0 +1,112 @@ +wasCreated; + } + + public function setRequest(DiffusionRequest $request) { + $this->request = $request; + return $this; + } + + public function setOldCommit($old_commit) { + $this->oldCommit = $old_commit; + return $this; + } + + public function getOldCommit() { + return $this->oldCommit; + } + + final public function loadOldFilename() { + $drequest = $this->request; + $repository_id = $drequest->getRepository()->getID(); + $conn_r = id(new PhabricatorRepository())->establishConnection('r'); + + $commit_id = $this->loadCommitId($this->oldCommit); + $old_commit_sequence = $this->loadCommitSequence($commit_id); + + $path = '/'.$drequest->getPath(); + $commit_id = $this->loadCommitId($drequest->getCommit()); + + do { + $commit_sequence = $this->loadCommitSequence($commit_id); + $change = queryfx_one( + $conn_r, + 'SELECT pc.changeType, pc.targetCommitID, tp.path + FROM %T p + JOIN %T pc ON p.id = pc.pathID + LEFT JOIN %T tp ON pc.targetPathID = tp.id + WHERE p.pathHash = %s + AND pc.repositoryID = %d + AND pc.changeType IN (%d, %d) + AND pc.commitSequence BETWEEN %d AND %d + ORDER BY pc.commitSequence DESC + LIMIT 1', + PhabricatorRepository::TABLE_PATH, + PhabricatorRepository::TABLE_PATHCHANGE, + PhabricatorRepository::TABLE_PATH, + md5($path), + $repository_id, + ArcanistDiffChangeType::TYPE_MOVE_HERE, + ArcanistDiffChangeType::TYPE_ADD, + $old_commit_sequence, + $commit_sequence); + if ($change) { + if ($change['changeType'] == ArcanistDiffChangeType::TYPE_ADD) { + $this->wasCreated = true; + return $path; + } + $commit_id = $change['targetCommitID']; + $path = $change['path']; + } + } while ($change && $path); + + return $path; + } + + private function loadCommitId($commit_identifier) { + $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( + 'repositoryID = %d AND commitIdentifier = %s', + $this->request->getRepository()->getID(), + $commit_identifier); + return $commit->getID(); + } + + private function loadCommitSequence($commit_id) { + $conn_r = id(new PhabricatorRepository())->establishConnection('r'); + $path_change = queryfx_one( + $conn_r, + 'SELECT commitSequence + FROM %T + WHERE repositoryID = %d AND commitID = %d + LIMIT 1', + PhabricatorRepository::TABLE_PATHCHANGE, + $this->request->getRepository()->getID(), + $commit_id); + return reset($path_change); + } + +} diff --git a/src/applications/diffusion/query/renamehistory/DiffusionGitRenameHistoryQuery.php b/src/applications/diffusion/query/renamehistory/DiffusionGitRenameHistoryQuery.php deleted file mode 100644 index d46b264e82..0000000000 --- a/src/applications/diffusion/query/renamehistory/DiffusionGitRenameHistoryQuery.php +++ /dev/null @@ -1,88 +0,0 @@ -getRequest(); - - $repository = $drequest->getRepository(); - list($err, $stdout) = $repository->execLocalCommand( - 'log --format=%s --follow --find-copies-harder -M -C --summary '. - '%s..%s -- %s', - '%x20', - $this->getOldCommit(), - $drequest->getCommit(), - $drequest->getPath()); - - if ($err) { - return null; - } - - $lines = explode("\n", $stdout); - $lines = array_map('trim', $lines); - $lines = array_filter($lines); - - $name = null; - foreach ($lines as $line) { - list($action, $info) = explode(' ', $line, 2); - switch ($action) { - case 'rename': - // We support these cases: - // rename README => README.txt (100%) - // rename src/README => README (100%) - // rename src/{README => README.txt} (100%) - // rename {resources => rsrc}/README (100%) - // rename src/{aphront => }/README (100%) - // rename src/{ => aphront}/README (100%) - // rename src/{docs => ducks}/README (100%) - $matches = null; - $ok = preg_match( - '/^(.*){(.*) => (.*)}(.*) \([0-9%]+\)$/', - $info, - $matches); - if ($ok) { - $name = $matches[1].ltrim($matches[2].$matches[4], '/'); - } else { - $ok = preg_match( - '/^(.*) => (.*) \([0-9%]+\)$/', - $info, - $matches); - if (!$ok) { - throw new Exception( - "Unparseable git log --summary line: {$line}."); - } - $name = $matches[1]; - } - - break; - case 'create': - // create mode 100644 - $this->setWasCreated(true); - break; - default: - // Anything else we care about? - break; - } - } - - return $name; - } - -} diff --git a/src/applications/diffusion/query/renamehistory/DiffusionMercurialRenameHistoryQuery.php b/src/applications/diffusion/query/renamehistory/DiffusionMercurialRenameHistoryQuery.php deleted file mode 100644 index 3f1d928b3b..0000000000 --- a/src/applications/diffusion/query/renamehistory/DiffusionMercurialRenameHistoryQuery.php +++ /dev/null @@ -1,27 +0,0 @@ -wasCreated = $was_created; - return $this; - } - - public function getWasCreated() { - return $this->wasCreated; - } - - public function setOldCommit($old_commit) { - $this->oldCommit = $old_commit; - return $this; - } - - public function getOldCommit() { - return $this->oldCommit; - } - - final public static function newFromDiffusionRequest( - DiffusionRequest $request) { - return self::newQueryObject(__CLASS__, $request); - } - - final public function loadOldFilename() { - return $this->executeQuery(); - } - -} diff --git a/src/applications/diffusion/query/renamehistory/DiffusionSvnRenameHistoryQuery.php b/src/applications/diffusion/query/renamehistory/DiffusionSvnRenameHistoryQuery.php deleted file mode 100644 index a173a3c15c..0000000000 --- a/src/applications/diffusion/query/renamehistory/DiffusionSvnRenameHistoryQuery.php +++ /dev/null @@ -1,27 +0,0 @@ -