diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php index 22f1bb1e4c..4bb7f1b9a9 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php @@ -62,41 +62,16 @@ final class ConduitAPI_diffusion_branchquery_Method $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); - $refs = id(new DiffusionLowLevelMercurialBranchesQuery()) - ->setRepository($repository) - ->execute(); + $query = id(new DiffusionLowLevelMercurialBranchesQuery()) + ->setRepository($repository); - // If we have a 'contains' query, filter these branches down to just the - // ones which contain the commit. $contains = $request->getValue('contains'); if (strlen($contains)) { - list($branches_raw) = $repository->execxLocalCommand( - 'log --template %s --limit 1 --rev %s --', - '{branches}', - hgsprintf('%s', $contains)); - - $branches_raw = trim($branches_raw); - if (!strlen($branches_raw)) { - $containing_branches = array('default'); - } else { - $containing_branches = explode(' ', $branches_raw); - } - - $containing_branches = array_fuse($containing_branches); - - // NOTE: We get this very slightly wrong: a branch may have multiple - // heads and we'll keep all of the heads of the branch, even if the - // commit is only on some of the heads. This should be rare, is probably - // more clear to users as is, and would potentially be expensive to get - // right since we'd have to do additional checks. - - foreach ($refs as $key => $ref) { - if (empty($containing_branches[$ref->getShortName()])) { - unset($refs[$key]); - } - } + $query->withContainsCommit($contains); } + $refs = $query->execute(); + return $this->processBranchRefs($request, $refs); } diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 430417fcf7..98bd78bc77 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -688,7 +688,7 @@ final class DiffusionCommitHookEngine extends Phobject { foreach (array('old', 'new') as $key) { $futures[$key] = $repository->getLocalCommandFuture( 'heads --template %s', - '{node}\1{branches}\2'); + '{node}\1{branch}\2'); } // Wipe HG_PENDING out of the old environment so we see the pre-commit // state of the repository. @@ -697,7 +697,7 @@ final class DiffusionCommitHookEngine extends Phobject { $futures['commits'] = $repository->getLocalCommandFuture( 'log --rev %s --template %s', hgsprintf('%s:%s', $hg_node, 'tip'), - '{node}\1{branches}\2'); + '{node}\1{branch}\2'); // Resolve all of the futures now. We don't need the 'commits' future yet, // but it simplifies the logic to just get it out of the way. @@ -978,15 +978,8 @@ final class DiffusionCommitHookEngine extends Phobject { $commits_lines = array_filter($commits_lines); $commit_map = array(); foreach ($commits_lines as $commit_line) { - list($node, $branches_raw) = explode("\1", $commit_line); - - if (!strlen($branches_raw)) { - $branches = array('default'); - } else { - $branches = explode(' ', $branches_raw); - } - - $commit_map[$node] = $branches; + list($node, $branch) = explode("\1", $commit_line); + $commit_map[$node] = array($branch); } return $commit_map; @@ -1152,6 +1145,9 @@ final class DiffusionCommitHookEngine extends Phobject { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: return idx($this->gitCommits, $identifier, array()); case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + // NOTE: This will be "the branch the commit was made to", not + // "a list of all branch heads which descend from the commit". + // This is consistent with Mercurial, but possibly confusing. return idx($this->mercurialCommits, $identifier, array()); case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: // Subversion doesn't have branches. diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php index f779794b51..015f0b1c8e 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php @@ -6,21 +6,36 @@ final class DiffusionLowLevelMercurialBranchesQuery extends DiffusionLowLevelQuery { + private $contains; + + public function withContainsCommit($commit) { + $this->contains = $commit; + return $this; + } + protected function executeQuery() { $repository = $this->getRepository(); - // NOTE: `--debug` gives us 40-character hashes. + if ($this->contains !== null) { + $spec = hgsprintf('(descendants(%s) and head())', $this->contains); + } else { + $spec = hgsprintf('head()'); + } + list($stdout) = $repository->execxLocalCommand( - '--debug branches'); - $stdout = PhabricatorRepository::filterMercurialDebugOutput($stdout); + 'log --template %s --rev %s', + '{node}\1{branch}\2', + $spec); $branches = array(); - $lines = ArcanistMercurialParser::parseMercurialBranches($stdout); - foreach ($lines as $name => $spec) { + $lines = explode("\2", $stdout); + $lines = array_filter($lines); + foreach ($lines as $line) { + list($node, $branch) = explode("\1", $line); $branches[] = id(new DiffusionRepositoryRef()) - ->setShortName($name) - ->setCommitIdentifier($spec['rev']); + ->setShortName($branch) + ->setCommitIdentifier($node); } return $branches;