Page MenuHomePhabricator
Files F15437048

D19431.diff
No OneTemporary
Actions

D19431.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
@@ -816,6 +816,7 @@
'DiffusionHovercardEngineExtension' => 'applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php',
'DiffusionInlineCommentController' => 'applications/diffusion/controller/DiffusionInlineCommentController.php',
'DiffusionInlineCommentPreviewController' => 'applications/diffusion/controller/DiffusionInlineCommentPreviewController.php',
+ 'DiffusionInternalAncestorsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionInternalAncestorsConduitAPIMethod.php',
'DiffusionInternalGitRawDiffQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php',
'DiffusionLastModifiedController' => 'applications/diffusion/controller/DiffusionLastModifiedController.php',
'DiffusionLastModifiedQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php',
@@ -6149,6 +6150,7 @@
'DiffusionHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController',
'DiffusionInlineCommentPreviewController' => 'PhabricatorInlineCommentPreviewController',
+ 'DiffusionInternalAncestorsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
'DiffusionInternalGitRawDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
'DiffusionLastModifiedController' => 'DiffusionController',
'DiffusionLastModifiedQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
diff --git a/src/applications/audit/query/PhabricatorCommitSearchEngine.php b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
--- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php
+++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
@@ -53,6 +53,10 @@
$query->withUnreachable($map['unreachable']);
}
+ if ($map['ancestorsOf']) {
+ $query->withAncestorsOf($map['ancestorsOf']);
+ }
+
return $query;
}
@@ -103,6 +107,13 @@
pht(
'Find or exclude unreachable commits which are not ancestors of '.
'any branch, tag, or ref.')),
+ id(new PhabricatorSearchStringListField())
+ ->setLabel(pht('Ancestors Of'))
+ ->setKey('ancestorsOf')
+ ->setDescription(
+ pht(
+ 'Find commits which are ancestors of a particular ref, '.
+ 'like "master".')),
);
}
diff --git a/src/applications/diffusion/conduit/DiffusionInternalAncestorsConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionInternalAncestorsConduitAPIMethod.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/conduit/DiffusionInternalAncestorsConduitAPIMethod.php
@@ -0,0 +1,51 @@
+<?php
+
+final class DiffusionInternalAncestorsConduitAPIMethod
+ extends DiffusionQueryConduitAPIMethod {
+
+ public function isInternalAPI() {
+ return true;
+ }
+
+ public function getAPIMethodName() {
+ return 'diffusion.internal.ancestors';
+ }
+
+ public function getMethodDescription() {
+ return pht('Internal method for filtering ref ancestors.');
+ }
+
+ protected function defineReturnType() {
+ return 'list<string>';
+ }
+
+ protected function defineCustomParamTypes() {
+ return array(
+ 'ref' => 'required string',
+ 'commits' => 'required list<string>',
+ );
+ }
+
+ protected function getResult(ConduitAPIRequest $request) {
+ $drequest = $this->getDiffusionRequest();
+ $repository = $drequest->getRepository();
+
+ $commits = $request->getValue('commits');
+ $ref = $request->getValue('ref');
+
+ $graph = new PhabricatorGitGraphStream($repository, $ref);
+
+ $keep = array();
+ foreach ($commits as $identifier) {
+ try {
+ $graph->getCommitDate($identifier);
+ $keep[] = $identifier;
+ } catch (Exception $ex) {
+ // Not an ancestor.
+ }
+ }
+
+ return $keep;
+ }
+
+}
diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php
--- a/src/applications/diffusion/query/DiffusionCommitQuery.php
+++ b/src/applications/diffusion/query/DiffusionCommitQuery.php
@@ -22,10 +22,14 @@
private $epochMin;
private $epochMax;
private $importing;
+ private $ancestorsOf;
private $needCommitData;
private $needDrafts;
+ private $mustFilterRefs = false;
+ private $refRepository;
+
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
@@ -92,7 +96,7 @@
}
public function withRepositoryIDs(array $repository_ids) {
- $this->repositoryIDs = $repository_ids;
+ $this->repositoryIDs = array_unique($repository_ids);
return $this;
}
@@ -152,6 +156,11 @@
return $this;
}
+ public function withAncestorsOf(array $refs) {
+ $this->ancestorsOf = $refs;
+ return $this;
+ }
+
public function getIdentifierMap() {
if ($this->identifierMap === null) {
throw new Exception(
@@ -307,6 +316,54 @@
protected function didFilterPage(array $commits) {
$viewer = $this->getViewer();
+ if ($this->mustFilterRefs) {
+ // If this flag is set, the query has an "Ancestors Of" constraint and
+ // at least one of the constraining refs had too many ancestors for us
+ // to apply the constraint with a big "commitIdentifier IN (%Ls)" clause.
+ // We're going to filter each page and hope we get a full result set
+ // before the query overheats.
+
+ $ancestor_list = mpull($commits, 'getCommitIdentifier');
+ $ancestor_list = array_values($ancestor_list);
+
+ foreach ($this->ancestorsOf as $ref) {
+ try {
+ $ancestor_list = DiffusionQuery::callConduitWithDiffusionRequest(
+ $viewer,
+ DiffusionRequest::newFromDictionary(
+ array(
+ 'repository' => $this->refRepository,
+ 'user' => $viewer,
+ )),
+ 'diffusion.internal.ancestors',
+ array(
+ 'ref' => $ref,
+ 'commits' => $ancestor_list,
+ ));
+ } catch (ConduitClientException $ex) {
+ throw new PhabricatorSearchConstraintException(
+ $ex->getMessage());
+ }
+
+ if (!$ancestor_list) {
+ break;
+ }
+ }
+
+ $ancestor_list = array_fuse($ancestor_list);
+ foreach ($commits as $key => $commit) {
+ $identifier = $commit->getCommitIdentifier();
+ if (!isset($ancestor_list[$identifier])) {
+ $this->didRejectResult($commit);
+ unset($commits[$key]);
+ }
+ }
+
+ if (!$commits) {
+ return $commits;
+ }
+ }
+
if ($this->needCommitData) {
$data = id(new PhabricatorRepositoryCommitData())->loadAllWhere(
'commitID in (%Ld)',
@@ -364,6 +421,95 @@
$this->withRepositoryIDs($repository_ids);
}
+ if ($this->ancestorsOf !== null) {
+ if (count($this->repositoryIDs) !== 1) {
+ throw new PhabricatorSearchConstraintException(
+ pht(
+ 'To search for commits which are ancestors of particular refs, '.
+ 'you must constrain the search to exactly one repository.'));
+ }
+
+ $repository_id = head($this->repositoryIDs);
+ $history_limit = $this->getRawResultLimit() * 32;
+ $viewer = $this->getViewer();
+
+ $repository = id(new PhabricatorRepositoryQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($repository_id))
+ ->executeOne();
+
+ if (!$repository) {
+ throw new PhabricatorEmptyQueryException();
+ }
+
+ if ($repository->isSVN()) {
+ throw new PhabricatorSearchConstraintException(
+ pht(
+ 'Subversion does not support searching for ancestors of '.
+ 'a particular ref. This operation is not meaningful in '.
+ 'Subversion.'));
+ }
+
+ if ($repository->isHg()) {
+ throw new PhabricatorSearchConstraintException(
+ pht(
+ 'Mercurial does not currently support searching for ancestors of '.
+ 'a particular ref.'));
+ }
+
+ $can_constrain = true;
+ $history_identifiers = array();
+ foreach ($this->ancestorsOf as $key => $ref) {
+ try {
+ $raw_history = DiffusionQuery::callConduitWithDiffusionRequest(
+ $viewer,
+ DiffusionRequest::newFromDictionary(
+ array(
+ 'repository' => $repository,
+ 'user' => $viewer,
+ )),
+ 'diffusion.historyquery',
+ array(
+ 'commit' => $ref,
+ 'limit' => $history_limit,
+ ));
+ } catch (ConduitClientException $ex) {
+ throw new PhabricatorSearchConstraintException(
+ $ex->getMessage());
+ }
+
+ $ref_identifiers = array();
+ foreach ($raw_history['pathChanges'] as $change) {
+ $ref_identifiers[] = $change['commitIdentifier'];
+ }
+
+ // If this ref had fewer total commits than the limit, we're safe to
+ // apply the constraint as a large `IN (...)` query for a list of
+ // commit identifiers. This is efficient.
+ if ($history_limit) {
+ if (count($ref_identifiers) >= $history_limit) {
+ $can_constrain = false;
+ break;
+ }
+ }
+
+ $history_identifiers += array_fuse($ref_identifiers);
+ }
+
+ // If all refs had a small number of ancestors, we can just put the
+ // constraint into the query here and we're done. Otherwise, we need
+ // to filter each page after it comes out of the MySQL layer.
+ if ($can_constrain) {
+ $where[] = qsprintf(
+ $conn,
+ 'commit.commitIdentifier IN (%Ls)',
+ $history_identifiers);
+ } else {
+ $this->mustFilterRefs = true;
+ $this->refRepository = $repository;
+ }
+ }
+
if ($this->ids !== null) {
$where[] = qsprintf(
$conn,

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 5:10 PM (4 d, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7723851
Default Alt Text
D19431.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility