From 8f3ae81143f0f6c96163762fa0dbb33315abf4af Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 4 Nov 2013 12:16:53 -0800 Subject: [PATCH] Fix tag content display in Git Summary: Fixes the junk I broke in D7484. Before that, tag content was a side effect of resolving the ref name. Now, fetch it explicitly in `diffusion.tagsquery`. Test Plan: Looked at a tag, saw the annotation/message. Reviewers: btrahan Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D7485 --- .../ConduitAPI_diffusion_tagsquery_Method.php | 133 ++++++++++++------ .../controller/DiffusionBrowseController.php | 24 +++- .../diffusion/data/DiffusionRepositoryTag.php | 33 ++++- .../diffusion/request/DiffusionRequest.php | 4 + 4 files changed, 140 insertions(+), 54 deletions(-) diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php index da40a3b4a8..1ffe98433a 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php @@ -7,8 +7,7 @@ final class ConduitAPI_diffusion_tagsquery_Method extends ConduitAPI_diffusion_abstractquery_Method { public function getMethodDescription() { - return - 'Find tags for a given commit or list tags in the repository.'; + return pht('Retrieve information about tags in a repository.'); } public function defineReturnType() { @@ -17,7 +16,9 @@ final class ConduitAPI_diffusion_tagsquery_Method protected function defineCustomParamTypes() { return array( + 'names' => 'optional list', 'commit' => 'optional string', + 'needMessages' => 'optional bool', 'offset' => 'optional int', 'limit' => 'optional int', ); @@ -28,50 +29,41 @@ final class ConduitAPI_diffusion_tagsquery_Method $repository = $drequest->getRepository(); $commit = $drequest->getRawCommit(); + $commit_filter = null; + if ($commit) { + $commit_filter = $this->loadTagNamesForCommit($commit); + } + + $name_filter = $request->getValue('names', null); + + $all_tags = $this->loadGitTagList(); + $all_tags = mpull($all_tags, null, 'getName'); + if ($name_filter !== null) { + $all_tags = array_intersect_key($all_tags, array_fuse($name_filter)); + } + if ($commit_filter !== null) { + $all_tags = array_intersect_key($all_tags, array_fuse($commit_filter)); + } + $tags = array_values($all_tags); + $offset = $request->getValue('offset'); $limit = $request->getValue('limit'); - - if (!$commit) { - return $this->loadGitTagList($offset, $limit); - } - - list($err, $stdout) = $repository->execLocalCommand( - 'tag -l --contains %s', - $commit); - - if ($err) { - // Git exits with an error code if the commit is bogus. - return array(); - } - - $stdout = trim($stdout); - if (!strlen($stdout)) { - return array(); - } - - $tag_names = explode("\n", $stdout); - $tag_names = array_fill_keys($tag_names, true); - - $tags = $this->loadGitTagList($offset = 0, $limit = 0, $serialize = false); - - $result = array(); - foreach ($tags as $tag) { - if (isset($tag_names[$tag->getName()])) { - $result[] = $tag->toDictionary(); - } - } - if ($offset) { - $result = array_slice($result, $offset); - } - if ($limit) { - $result = array_slice($result, 0, $limit); + $tags = array_slice($tags, $offset); } - return $result; + if ($limit) { + $tags = array_slice($tags, 0, $limit); + } + + if ($request->getValue('needMessages')) { + $this->loadMessagesForTags($all_tags); + } + + return mpull($tags, 'toDictionary'); } - private function loadGitTagList($offset, $limit, $serialize=true) { + private function loadGitTagList() { $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); @@ -94,17 +86,68 @@ final class ConduitAPI_diffusion_tagsquery_Method $tags[] = $tag; } - if ($offset) { - $tags = array_slice($tags, $offset); + return $tags; + } + + private function loadTagNamesForCommit($commit) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + list($err, $stdout) = $repository->execLocalCommand( + 'tag -l --contains %s', + $commit); + + if ($err) { + // Git exits with an error code if the commit is bogus. + return array(); } - if ($limit) { - $tags = array_slice($tags, 0, $limit); + $stdout = rtrim($stdout, "\n"); + if (!strlen($stdout)) { + return array(); } - if ($serialize) { - $tags = mpull($tags, 'toDictionary'); + $tag_names = explode("\n", $stdout); + $tag_names = array_fill_keys($tag_names, true); + + return $tag_names; + } + + private function loadMessagesForTags(array $tags) { + assert_instances_of($tags, 'DiffusionRepositoryTag'); + + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $futures = array(); + foreach ($tags as $key => $tag) { + $futures[$key] = $repository->getLocalCommandFuture( + 'cat-file tag %s', + $tag->getName()); } + + Futures($futures)->resolveAll(); + + foreach ($tags as $key => $tag) { + $future = $futures[$key]; + list($err, $stdout) = $future->resolve(); + + $message = null; + if ($err) { + // Not all tags are actually "tag" objects: a "tag" object is only + // created if you provide a message or sign the tag. Tags created with + // `git tag x [commit]` are "lightweight tags" and `git cat-file tag` + // will fail on them. This is fine: they don't have messages. + } else { + $parts = explode("\n\n", $stdout, 2); + if (count($parts) == 2) { + $message = last($parts); + } + } + + $tag->attachMessage($message); + } + return $tags; } diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 84b8b9f2dd..33fc2e2d15 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -146,13 +146,25 @@ abstract class DiffusionBrowseController extends DiffusionController { ), $drequest->getRepository()->formatCommitName($stable_commit))); - if ($drequest->getTagContent()) { - $view->addProperty( - pht('Tag'), - $drequest->getSymbolicCommit()); + if ($drequest->getCommitType() == 'tag') { + $symbolic = $drequest->getSymbolicCommit(); + $view->addProperty(pht('Tag'), $symbolic); - $view->addSectionHeader(pht('Tag Content')); - $view->addTextContent($this->markupText($drequest->getTagContent())); + $tags = $this->callConduitWithDiffusionRequest( + 'diffusion.tagsquery', + array( + 'names' => array($symbolic), + 'needMessages' => true, + )); + $tags = DiffusionRepositoryTag::newFromConduit($tags); + + $tags = mpull($tags, null, 'getName'); + $tag = idx($tags, $symbolic); + + if ($tag && strlen($tag->getMessage())) { + $view->addSectionHeader(pht('Tag Content')); + $view->addTextContent($this->markupText($tag->getMessage())); + } } return $view; diff --git a/src/applications/diffusion/data/DiffusionRepositoryTag.php b/src/applications/diffusion/data/DiffusionRepositoryTag.php index 47e60eedd3..a8e3696b46 100644 --- a/src/applications/diffusion/data/DiffusionRepositoryTag.php +++ b/src/applications/diffusion/data/DiffusionRepositoryTag.php @@ -9,6 +9,8 @@ final class DiffusionRepositoryTag { private $description; private $type; + private $message = false; + public function setType($type) { $this->type = $type; return $this; @@ -63,26 +65,51 @@ final class DiffusionRepositoryTag { return $this->author; } + public function attachMessage($message) { + $this->message = $message; + return $this; + } + + public function getMessage() { + if ($this->message === false) { + throw new Exception("Message is not attached!"); + } + return $this->message; + } + public function toDictionary() { - return array( + $dict = array( 'author' => $this->getAuthor(), 'epoch' => $this->getEpoch(), 'commitIdentifier' => $this->getCommitIdentifier(), 'name' => $this->getName(), 'description' => $this->getDescription(), - 'type' => $this->getType()); + 'type' => $this->getType(), + ); + + if ($this->message !== false) { + $dict['message'] = $this->message; + } + + return $dict; } public static function newFromConduit(array $dicts) { $tags = array(); foreach ($dicts as $dict) { - $tags[] = id(new DiffusionRepositoryTag()) + $tag = id(new DiffusionRepositoryTag()) ->setAuthor($dict['author']) ->setEpoch($dict['epoch']) ->setCommitIdentifier($dict['commitIdentifier']) ->setName($dict['name']) ->setDescription($dict['description']) ->setType($dict['type']); + + if (array_key_exists('message', $dict)) { + $tag->attachMessage($dict['message']); + } + + $tags[] = $tag; } return $tags; } diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index d61c5b63bf..035ed245c0 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -646,6 +646,10 @@ abstract class DiffusionRequest { $this->tagContent = $commit_data['tagContent']; } + public function getCommitType() { + return $this->commitType; + } + private function queryStableCommitName() { if ($this->commit) { $this->stableCommitName = $this->commit;