Page MenuHomePhabricator
Files F77127

D7447.diff
All Users
Actions

D7447.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -491,6 +491,8 @@
'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php',
'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
+ 'DiffusionLowLevelGitRefQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php',
+ 'DiffusionLowLevelQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php',
'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php',
'DiffusionMercurialExpandShortNameQuery' => 'applications/diffusion/query/expandshortname/DiffusionMercurialExpandShortNameQuery.php',
'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php',
@@ -525,6 +527,7 @@
'DiffusionRepositoryEditSubversionController' => 'applications/diffusion/controller/DiffusionRepositoryEditSubversionController.php',
'DiffusionRepositoryListController' => 'applications/diffusion/controller/DiffusionRepositoryListController.php',
'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php',
+ 'DiffusionRepositoryRef' => 'applications/diffusion/data/DiffusionRepositoryRef.php',
'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php',
'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php',
'DiffusionSSHGitReceivePackWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitReceivePackWorkflow.php',
@@ -2684,6 +2687,8 @@
'DiffusionLastModifiedController' => 'DiffusionController',
'DiffusionLintController' => 'DiffusionController',
'DiffusionLintDetailsController' => 'DiffusionController',
+ 'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery',
+ 'DiffusionLowLevelQuery' => 'Phobject',
'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery',
'DiffusionMercurialExpandShortNameQuery' => 'DiffusionExpandShortNameQuery',
'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
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
@@ -27,44 +27,31 @@
$limit = $request->getValue('limit');
$offset = $request->getValue('offset');
- // We need to add 1 in case we pick up HEAD.
- $count = $offset + $limit + 1;
-
- if ($repository->isWorkingCopyBare()) {
- list($stdout) = $repository->execxLocalCommand(
- 'for-each-ref %C --sort=-creatordate --format=%s refs/heads',
- $count ? '--count='.(int)$count : null,
- '%(refname:short) %(objectname)');
- $branch_list = DiffusionGitBranch::parseLocalBranchOutput(
- $stdout);
- } else {
- list($stdout) = $repository->execxLocalCommand(
- 'for-each-ref %C --sort=-creatordate --format=%s refs/remotes',
- $count ? '--count='.(int)$count : null,
- '%(refname:short) %(objectname)');
- $branch_list = DiffusionGitBranch::parseRemoteBranchOutput(
- $stdout,
- $only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
- }
+ $refs = id(new DiffusionLowLevelGitRefQuery())
+ ->setRepository($repository)
+ ->withIsOriginBranch(true)
+ ->execute();
$branches = array();
- foreach ($branch_list as $name => $head) {
- if (!$repository->shouldTrackBranch($name)) {
+ foreach ($refs as $ref) {
+ $branch = id(new DiffusionBranchInformation())
+ ->setName($ref->getShortName())
+ ->setHeadCommitIdentifier($ref->getCommitIdentifier());
+
+ if (!$repository->shouldTrackBranch($branch->getName())) {
continue;
}
- $branch = new DiffusionBranchInformation();
- $branch->setName($name);
- $branch->setHeadCommitIdentifier($head);
$branches[] = $branch->toDictionary();
}
+ // NOTE: We can't apply the offset or limit until here, because we may have
+ // filtered untrackable branches out of the result set.
+
if ($offset) {
$branches = array_slice($branches, $offset);
}
- // We might have too many even after offset slicing, if there was no HEAD
- // for some reason.
if ($limit) {
$branches = array_slice($branches, 0, $limit);
}
diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php
--- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php
+++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php
@@ -25,6 +25,8 @@
$repository = $drequest->getRepository();
$commit = $request->getValue('commit');
+ // NOTE: We can't use DiffusionLowLevelGitRefQuery here because
+ // `git for-each-ref` does not support `--contains`.
if ($repository->isWorkingCopyBare()) {
list($contains) = $repository->execxLocalCommand(
'branch --verbose --no-abbrev --contains %s',
diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php
--- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php
+++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php
@@ -75,55 +75,33 @@
$drequest = $this->getDiffusionRequest();
$repository = $drequest->getRepository();
- $count = $offset + $limit;
-
- list($stdout) = $repository->execxLocalCommand(
- 'for-each-ref %C --sort=-creatordate --format=%s refs/tags',
- $count ? '--count='.(int)$count : null,
- '%(objectname) %(objecttype) %(refname) %(*objectname) %(*objecttype) '.
- '%(subject)%01%(creator)');
-
- $stdout = trim($stdout);
- if (!strlen($stdout)) {
- return array();
- }
+ $refs = id(new DiffusionLowLevelGitRefQuery())
+ ->setRepository($repository)
+ ->withIsTag(true)
+ ->execute();
$tags = array();
- foreach (explode("\n", $stdout) as $line) {
- list($info, $creator) = explode("\1", $line);
- list(
- $objectname,
- $objecttype,
- $refname,
- $refobjectname,
- $refobjecttype,
- $description) = explode(' ', $info, 6);
-
- $matches = null;
- if (!preg_match('/^(.*) ([0-9]+) ([0-9+-]+)$/', $creator, $matches)) {
- // It's possible a tag doesn't have a creator (tagger)
- $author = null;
- $epoch = null;
- } else {
- $author = $matches[1];
- $epoch = $matches[2];
- }
-
- $tag = new DiffusionRepositoryTag();
- $tag->setAuthor($author);
- $tag->setEpoch($epoch);
- $tag->setCommitIdentifier(nonempty($refobjectname, $objectname));
- $tag->setName(preg_replace('@^refs/tags/@', '', $refname));
- $tag->setDescription($description);
- $tag->setType('git/'.$objecttype);
+ foreach ($refs as $ref) {
+ $fields = $ref->getRawFields();
+ $tag = id(new DiffusionRepositoryTag())
+ ->setAuthor($fields['author'])
+ ->setEpoch($fields['epoch'])
+ ->setCommitIdentifier($ref->getCommitIdentifier())
+ ->setName($ref->getShortName())
+ ->setDescription($fields['subject'])
+ ->setType('git/'.$fields['objecttype']);
$tags[] = $tag;
}
if ($offset) {
$tags = array_slice($tags, $offset);
}
+ if ($limit) {
+ $tags = array_slice($tags, 0, $limit);
+ }
+
if ($serialize) {
$tags = mpull($tags, 'toDictionary');
}
diff --git a/src/applications/diffusion/data/DiffusionRepositoryRef.php b/src/applications/diffusion/data/DiffusionRepositoryRef.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/data/DiffusionRepositoryRef.php
@@ -0,0 +1,36 @@
+<?php
+
+final class DiffusionRepositoryRef {
+
+ private $shortName;
+ private $commitIdentifier;
+ private $rawFields;
+
+ public function setRawFields(array $raw_fields) {
+ $this->rawFields = $raw_fields;
+ return $this;
+ }
+
+ public function getRawFields() {
+ return $this->rawFields;
+ }
+
+ public function setCommitIdentifier($commit_identifier) {
+ $this->commitIdentifier = $commit_identifier;
+ return $this;
+ }
+
+ public function getCommitIdentifier() {
+ return $this->commitIdentifier;
+ }
+
+ public function setShortName($short_name) {
+ $this->shortName = $short_name;
+ return $this;
+ }
+
+ public function getShortName() {
+ return $this->shortName;
+ }
+
+}
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php
@@ -0,0 +1,135 @@
+<?php
+
+/**
+ * Execute and parse a low-level Git ref query using `git for-each-ref`. This
+ * is useful for returning a list of tags or branches.
+ *
+ *
+ */
+final class DiffusionLowLevelGitRefQuery extends DiffusionLowLevelQuery {
+
+ private $isTag;
+ private $isOriginBranch;
+
+ public function withIsTag($is_tag) {
+ $this->isTag = $is_tag;
+ return $this;
+ }
+
+ public function withIsOriginBranch($is_origin_branch) {
+ $this->isOriginBranch = $is_origin_branch;
+ return $this;
+ }
+
+ protected function executeQuery() {
+ $repository = $this->getRepository();
+
+ if ($this->isTag && $this->isOriginBranch) {
+ throw new Exception("Specify tags or origin branches, not both!");
+ } else if ($this->isTag) {
+ $prefix = 'refs/tags/';
+ } else if ($this->isOriginBranch) {
+ if ($repository->isWorkingCopyBare()) {
+ $prefix = 'refs/heads/';
+ } else {
+ $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE;
+ $prefix = 'refs/remotes/'.$remote.'/';
+ }
+ } else {
+ throw new Exception("Specify tags or origin branches!");
+ }
+
+ $order = '-creatordate';
+
+ list($stdout) = $repository->execxLocalCommand(
+ 'for-each-ref --sort=%s --format=%s %s',
+ $order,
+ $this->getFormatString(),
+ $prefix);
+
+ $stdout = rtrim($stdout);
+ if (!strlen($stdout)) {
+ return array();
+ }
+
+ // NOTE: Although git supports --count, we can't apply any offset or limit
+ // logic until the very end because we may encounter a HEAD which we want
+ // to discard.
+
+ $lines = explode("\n", $stdout);
+ $results = array();
+ foreach ($lines as $line) {
+ $fields = $this->extractFields($line);
+
+ $creator = $fields['creator'];
+ $matches = null;
+ if (preg_match('/^(.*) ([0-9]+) ([0-9+-]+)$/', $creator, $matches)) {
+ $fields['author'] = $matches[1];
+ $fields['epoch'] = (int)$matches[2];
+ } else {
+ $fields['author'] = null;
+ $fields['epoch'] = null;
+ }
+
+ $commit = nonempty($fields['*objectname'], $fields['objectname']);
+
+ $short = substr($fields['refname'], strlen($prefix));
+ if ($short == 'HEAD') {
+ continue;
+ }
+
+ $ref = id(new DiffusionRepositoryRef())
+ ->setShortName($short)
+ ->setCommitIdentifier($commit)
+ ->setRawFields($fields);
+
+ $results[] = $ref;
+ }
+
+ return $results;
+ }
+
+ /**
+ * List of git `--format` fields we want to grab.
+ */
+ private function getFields() {
+ return array(
+ 'objectname',
+ 'objecttype',
+ 'refname',
+ '*objectname',
+ '*objecttype',
+ 'subject',
+ 'creator',
+ );
+ }
+
+ /**
+ * Get a string for `--format` which enumerates all the fields we want.
+ */
+ private function getFormatString() {
+ $fields = $this->getFields();
+
+ foreach ($fields as $key => $field) {
+ $fields[$key] = '%('.$field.')';
+ }
+
+ return implode("%01", $fields);
+ }
+
+ /**
+ * Parse a line back into fields.
+ */
+ private function extractFields($line) {
+ $fields = $this->getFields();
+ $parts = explode("\1", $line, count($fields));
+
+ $dict = array();
+ foreach ($fields as $index => $field) {
+ $dict[$field] = idx($parts, $index);
+ }
+
+ return $dict;
+ }
+
+}
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php
@@ -0,0 +1,22 @@
+<?php
+
+abstract class DiffusionLowLevelQuery extends Phobject {
+
+ private $repository;
+
+ abstract protected function executeQuery();
+
+ public function setRepository(PhabricatorRepository $repository) {
+ $this->repository = $repository;
+ return $this;
+ }
+
+ public function getRepository() {
+ return $this->repository;
+ }
+
+ public function execute() {
+ return $this->executeQuery();
+ }
+
+}
diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
--- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
+++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
@@ -526,18 +526,12 @@
$repository->getRemoteURI(),
$repository->getLocalPath());
- if ($repository->isWorkingCopyBare()) {
- list($stdout) = $repository->execxLocalCommand(
- 'branch --verbose --no-abbrev');
- $branches = DiffusionGitBranch::parseLocalBranchOutput(
- $stdout);
- } else {
- list($stdout) = $repository->execxLocalCommand(
- 'branch -r --verbose --no-abbrev');
- $branches = DiffusionGitBranch::parseRemoteBranchOutput(
- $stdout,
- $only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
- }
+ $refs = id(new DiffusionLowLevelGitRefQuery())
+ ->setRepository($repository)
+ ->withIsOriginBranch(true)
+ ->execute();
+
+ $branches = mpull($refs, 'getCommitIdentifier', 'getShortName');
if (!$branches) {
// This repository has no branches at all, so we don't need to do

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/2a/lr/bwmmjy2qctjp25l5
Default Alt Text
D7447.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility