diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ff75e15f6c..a7f8058250 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -481,7 +481,6 @@ phutil_register_library_map(array( 'DiffusionCommitHash' => 'applications/diffusion/data/DiffusionCommitHash.php', 'DiffusionCommitHookEngine' => 'applications/diffusion/engine/DiffusionCommitHookEngine.php', 'DiffusionCommitHookRejectException' => 'applications/diffusion/exception/DiffusionCommitHookRejectException.php', - 'DiffusionCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionCommitParentsQuery.php', 'DiffusionCommitQuery' => 'applications/diffusion/query/DiffusionCommitQuery.php', 'DiffusionCommitRef' => 'applications/diffusion/data/DiffusionCommitRef.php', 'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php', @@ -494,7 +493,6 @@ phutil_register_library_map(array( 'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionFileContentQuery.php', 'DiffusionGitBranch' => 'applications/diffusion/data/DiffusionGitBranch.php', 'DiffusionGitBranchTestCase' => 'applications/diffusion/data/__tests__/DiffusionGitBranchTestCase.php', - 'DiffusionGitCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php', 'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php', 'DiffusionGitRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php', 'DiffusionGitRequest' => 'applications/diffusion/request/DiffusionGitRequest.php', @@ -512,9 +510,9 @@ phutil_register_library_map(array( 'DiffusionLowLevelCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php', 'DiffusionLowLevelGitRefQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php', 'DiffusionLowLevelMercurialBranchesQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php', + 'DiffusionLowLevelParentsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php', 'DiffusionLowLevelQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php', 'DiffusionLowLevelResolveRefsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php', - 'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php', 'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php', 'DiffusionMercurialRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php', 'DiffusionMercurialRequest' => 'applications/diffusion/request/DiffusionMercurialRequest.php', @@ -572,7 +570,6 @@ phutil_register_library_map(array( 'DiffusionSetupException' => 'applications/diffusion/exception/DiffusionSetupException.php', 'DiffusionSubversionWireProtocol' => 'applications/diffusion/protocol/DiffusionSubversionWireProtocol.php', 'DiffusionSubversionWireProtocolTestCase' => 'applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php', - 'DiffusionSvnCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php', 'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php', 'DiffusionSvnRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php', 'DiffusionSvnRequest' => 'applications/diffusion/request/DiffusionSvnRequest.php', @@ -2871,7 +2868,6 @@ phutil_register_library_map(array( 'DiffusionCommitHash' => 'Phobject', 'DiffusionCommitHookEngine' => 'Phobject', 'DiffusionCommitHookRejectException' => 'Exception', - 'DiffusionCommitParentsQuery' => 'DiffusionQuery', 'DiffusionCommitQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DiffusionCommitRef' => 'Phobject', 'DiffusionCommitTagsController' => 'DiffusionController', @@ -2882,7 +2878,6 @@ phutil_register_library_map(array( 'DiffusionExternalController' => 'DiffusionController', 'DiffusionFileContentQuery' => 'DiffusionQuery', 'DiffusionGitBranchTestCase' => 'PhabricatorTestCase', - 'DiffusionGitCommitParentsQuery' => 'DiffusionCommitParentsQuery', 'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionGitRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionGitRequest' => 'DiffusionRequest', @@ -2899,9 +2894,9 @@ phutil_register_library_map(array( 'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelMercurialBranchesQuery' => 'DiffusionLowLevelQuery', + 'DiffusionLowLevelParentsQuery' => 'DiffusionLowLevelQuery', 'DiffusionLowLevelQuery' => 'Phobject', 'DiffusionLowLevelResolveRefsQuery' => 'DiffusionLowLevelQuery', - 'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery', 'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionMercurialRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionMercurialRequest' => 'DiffusionRequest', @@ -2957,7 +2952,6 @@ phutil_register_library_map(array( 'DiffusionSetupException' => 'AphrontUsageException', 'DiffusionSubversionWireProtocol' => 'Phobject', 'DiffusionSubversionWireProtocolTestCase' => 'PhabricatorTestCase', - 'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery', 'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionSvnRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionSvnRequest' => 'DiffusionRequest', diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitparentsquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitparentsquery_Method.php index f0315e46f2..82ff91a0ce 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitparentsquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitparentsquery_Method.php @@ -7,12 +7,12 @@ final class ConduitAPI_diffusion_commitparentsquery_Method extends ConduitAPI_diffusion_abstractquery_Method { public function getMethodDescription() { - return - 'Commit parent(s) information for a commit in a repository.'; + return pht( + "Get the commit identifiers for a commit's parent or parents."); } public function defineReturnType() { - return 'array'; + return 'list'; } protected function defineCustomParamTypes() { @@ -22,10 +22,12 @@ final class ConduitAPI_diffusion_commitparentsquery_Method } protected function getResult(ConduitAPIRequest $request) { - $drequest = $this->getDiffusionRequest(); + $repository = $this->getRepository($request); - $query = DiffusionCommitParentsQuery::newFromDiffusionRequest($drequest); - $parents = $query->loadParents(); - return $parents; + return id(new DiffusionLowLevelParentsQuery()) + ->setRepository($repository) + ->withIdentifier($request->getValue('commit')) + ->execute(); } + } diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index 9ca001f21c..f3da2d3583 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -857,17 +857,16 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { // NOTE: We need to get the grandparent so we can capture filename changes // in the parent. - $parent = $this->loadParentRevisionOf($before); + $parent = $this->loadParentCommitOf($before); $old_filename = null; $was_created = false; if ($parent) { - $grandparent = $this->loadParentRevisionOf( - $parent->getCommitIdentifier()); + $grandparent = $this->loadParentCommitOf($parent); if ($grandparent) { $rename_query = new DiffusionRenameHistoryQuery(); $rename_query->setRequest($drequest); - $rename_query->setOldCommit($grandparent->getCommitIdentifier()); + $rename_query->setOldCommit($grandparent); $rename_query->setViewer($request->getUser()); $old_filename = $rename_query->loadOldFilename(); $was_created = $rename_query->getWasCreated(); @@ -955,7 +954,7 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { return $line; } - private function loadParentRevisionOf($commit) { + private function loadParentCommitOf($commit) { $drequest = $this->getDiffusionRequest(); $user = $this->getRequest()->getUser(); @@ -963,7 +962,7 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { array( 'user' => $user, 'repository' => $drequest->getRepository(), - 'commit' => $commit, + 'commit' => $commit, )); $parents = DiffusionQuery::callConduitWithDiffusionRequest( @@ -971,7 +970,8 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController { $before_req, 'diffusion.commitparentsquery', array( - 'commit' => $commit)); + 'commit' => $commit, + )); return head($parents); } diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 0060108cee..557338a46e 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -102,6 +102,14 @@ final class DiffusionCommitController extends DiffusionController { 'diffusion.commitparentsquery', array('commit' => $drequest->getCommit())); + if ($parents) { + $parents = id(new DiffusionCommitQuery()) + ->setViewer($user) + ->withRepository($repository) + ->withIdentifiers($parents) + ->execute(); + } + $headsup_view = id(new PHUIHeaderView()) ->setHeader(nonempty($commit->getSummary(), pht('Commit Detail'))); diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php new file mode 100644 index 0000000000..b7ecf5a5c4 --- /dev/null +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php @@ -0,0 +1,84 @@ +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->loadGitParents(); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $result = $this->loadMercurialParents(); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $result = $this->loadSubversionParents(); + break; + default: + throw new Exception(pht('Unsupported repository type "%s"!', $type)); + } + + return $result; + } + + private function loadGitParents() { + $repository = $this->getRepository(); + + list($stdout) = $repository->execxLocalCommand( + 'log -n 1 --format=%s %s', + '%P', + $this->identifier); + + return preg_split('/\s+/', trim($stdout)); + } + + private function loadMercurialParents() { + $repository = $this->getRepository(); + + list($stdout) = $repository->execxLocalCommand( + 'log --debug --limit 1 --template={parents} --rev %s', + $this->identifier); + $stdout = PhabricatorRepository::filterMercurialDebugOutput($stdout); + + $hashes = preg_split('/\s+/', trim($stdout)); + foreach ($hashes as $key => $value) { + // Mercurial parents look like "23:ad9f769d6f786fad9f76d9a" -- we want + // to strip out the local rev part. + list($local, $global) = explode(':', $value); + $hashes[$key] = $global; + + // With --debug we get 40-character hashes but also get the "000000..." + // hash for missing parents; ignore it. + if (preg_match('/^0+$/', $global)) { + unset($hashes[$key]); + } + } + + return $hashes; + } + + private function loadSubversionParents() { + $n = (int)$this->identifier; + if ($n > 1) { + $ids = array($n - 1); + } else { + $ids = array(); + } + + return $ids; + } + +} diff --git a/src/applications/diffusion/query/parents/DiffusionCommitParentsQuery.php b/src/applications/diffusion/query/parents/DiffusionCommitParentsQuery.php deleted file mode 100644 index a8a9e53af7..0000000000 --- a/src/applications/diffusion/query/parents/DiffusionCommitParentsQuery.php +++ /dev/null @@ -1,14 +0,0 @@ -executeQuery(); - } - -} diff --git a/src/applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php b/src/applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php deleted file mode 100644 index ba10921865..0000000000 --- a/src/applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php +++ /dev/null @@ -1,19 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - list($stdout) = $repository->execxLocalCommand( - 'log -n 1 --format=%s %s', - '%P', - $drequest->getStableCommitName()); - - $hashes = preg_split('/\s+/', trim($stdout)); - - return self::loadCommitsByIdentifiers($hashes, $drequest); - } -} diff --git a/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php b/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php deleted file mode 100644 index dc9f900ce8..0000000000 --- a/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php +++ /dev/null @@ -1,31 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - list($stdout) = $repository->execxLocalCommand( - 'log --debug --limit 1 --template={parents} --rev %s', - $drequest->getStableCommitName()); - $stdout = PhabricatorRepository::filterMercurialDebugOutput($stdout); - - $hashes = preg_split('/\s+/', trim($stdout)); - foreach ($hashes as $key => $value) { - // Mercurial parents look like "23:ad9f769d6f786fad9f76d9a" -- we want - // to strip out the local rev part. - list($local, $global) = explode(':', $value); - $hashes[$key] = $global; - - // With --debug we get 40-character hashes but also get the "000000..." - // hash for missing parents; ignore it. - if (preg_match('/^0+$/', $global)) { - unset($hashes[$key]); - } - } - - return self::loadCommitsByIdentifiers($hashes, $drequest); - } -} diff --git a/src/applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php b/src/applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php deleted file mode 100644 index 8b2ae329b2..0000000000 --- a/src/applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php +++ /dev/null @@ -1,22 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - // TODO: With merge properties in recent versions of SVN, can we do - // a better job of this? - - $n = $drequest->getStableCommitName(); - if ($n > 1) { - $ids = array($n - 1); - } else { - $ids = array(); - } - - return self::loadCommitsByIdentifiers($ids, $drequest); - } -} diff --git a/src/applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php b/src/applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php index 40b4bd0be8..e222e33e0b 100644 --- a/src/applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php +++ b/src/applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php @@ -82,15 +82,10 @@ final class WaitForPreviousBuildStepImplementation $call->setUser(PhabricatorUser::getOmnipotentUser()); $parents = $call->execute(); - $hashes = array(); - foreach ($parents as $parent => $obj) { - $hashes[] = $parent; - } - $parents = id(new DiffusionCommitQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withRepository($commit->getRepository()) - ->withIdentifiers($hashes) + ->withIdentifiers($parents) ->execute(); $blockers = array(); diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 51de4ee37b..aa1295523d 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -205,15 +205,20 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker DifferentialRevision $revision, $actor_phid) { + $viewer = PhabricatorUser::getOmnipotentUser(); + $drequest = DiffusionRequest::newFromDictionary(array( - 'user' => PhabricatorUser::getOmnipotentUser(), - 'initFromConduit' => false, + 'user' => $viewer, 'repository' => $this->repository, - 'commit' => $this->commit->getCommitIdentifier(), )); - $raw_diff = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest) - ->loadRawDiff(); + $raw_diff = DiffusionQuery::callConduitWithDiffusionRequest( + $viewer, + $drequest, + 'diffusion.rawdiffquery', + array( + 'commit' => $this->commit->getCommitIdentifier(), + )); // TODO: Support adds, deletes and moves under SVN. if (strlen($raw_diff)) { @@ -248,10 +253,15 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $diff->setArcanistProjectPHID($arcanist_project->getPHID()); } - $parents = DiffusionCommitParentsQuery::newFromDiffusionRequest($drequest) - ->loadParents(); + $parents = DiffusionQuery::callConduitWithDiffusionRequest( + $viewer, + $drequest, + 'diffusion.commitparentsquery', + array( + 'commit' => $this->commit->getCommitIdentifier(), + )); if ($parents) { - $diff->setSourceControlBaseRevision(head_key($parents)); + $diff->setSourceControlBaseRevision(head($parents)); } // TODO: Attach binary files.