Page MenuHomePhabricator

D9453.diff
No OneTemporary

D9453.diff

diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php
--- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php
+++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php
@@ -62,41 +62,16 @@
$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
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -688,7 +688,7 @@
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 @@
$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 @@
$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 @@
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
--- 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;

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 19, 7:41 PM (21 h, 50 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6720262
Default Alt Text
D9453.diff (5 KB)

Event Timeline