Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14042241
D16460.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
21 KB
Referenced Files
None
Subscribers
None
D16460.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
@@ -5260,7 +5260,7 @@
'DiffusionQueryCommitsConduitAPIMethod' => 'DiffusionConduitAPIMethod',
'DiffusionQueryConduitAPIMethod' => 'DiffusionConduitAPIMethod',
'DiffusionQueryPathsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
- 'DiffusionRawDiffQuery' => 'DiffusionQuery',
+ 'DiffusionRawDiffQuery' => 'DiffusionFileFutureQuery',
'DiffusionRawDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
'DiffusionReadmeView' => 'DiffusionView',
'DiffusionRefDatasource' => 'PhabricatorTypeaheadDatasource',
diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
--- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
+++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
@@ -36,7 +36,7 @@
'repository' => $repository,
));
- $raw_diff = DiffusionQuery::callConduitWithDiffusionRequest(
+ $diff_info = DiffusionQuery::callConduitWithDiffusionRequest(
$viewer,
$drequest,
'diffusion.rawdiffquery',
@@ -44,6 +44,21 @@
'commit' => $identifier,
));
+ $file_phid = $diff_info['filePHID'];
+ $diff_file = id(new PhabricatorFileQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($file_phid))
+ ->executeOne();
+ if (!$diff_file) {
+ throw new Exception(
+ pht(
+ 'Failed to load file ("%s") returned by "%s".',
+ $file_phid,
+ 'diffusion.rawdiffquery'));
+ }
+
+ $raw_diff = $diff_file->loadFileData();
+
// TODO: Support adds, deletes and moves under SVN.
if (strlen($raw_diff)) {
$changes = id(new ArcanistDiffParser())->parseDiff($raw_diff);
diff --git a/src/applications/diffusion/DiffusionLintSaveRunner.php b/src/applications/diffusion/DiffusionLintSaveRunner.php
--- a/src/applications/diffusion/DiffusionLintSaveRunner.php
+++ b/src/applications/diffusion/DiffusionLintSaveRunner.php
@@ -262,7 +262,7 @@
$query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest);
$queries[$path] = $query;
- $futures[$path] = $query->getFileContentFuture();
+ $futures[$path] = new ImmediateFuture($query->executeInline());
}
$authors = array();
diff --git a/src/applications/diffusion/conduit/DiffusionDiffQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionDiffQueryConduitAPIMethod.php
--- a/src/applications/diffusion/conduit/DiffusionDiffQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionDiffQueryConduitAPIMethod.php
@@ -207,7 +207,7 @@
$raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest)
->setAnchorCommit($effective_commit);
- $raw_diff = $raw_query->loadRawDiff();
+ $raw_diff = $raw_query->executeInline();
if (!$raw_diff) {
return $this->getEmptyResult(2);
}
diff --git a/src/applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php
--- a/src/applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php
@@ -21,39 +21,27 @@
return array(
'commit' => 'required string',
'path' => 'optional string',
- 'timeout' => 'optional int',
- 'byteLimit' => 'optional int',
'linesOfContext' => 'optional int',
'againstCommit' => 'optional string',
- );
+ ) + DiffusionFileFutureQuery::getConduitParameters();
}
protected function getResult(ConduitAPIRequest $request) {
$drequest = $this->getDiffusionRequest();
- $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest);
-
- $timeout = $request->getValue('timeout');
- if ($timeout !== null) {
- $raw_query->setTimeout($timeout);
- }
+ $query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest);
$lines_of_context = $request->getValue('linesOfContext');
if ($lines_of_context !== null) {
- $raw_query->setLinesOfContext($lines_of_context);
+ $query->setLinesOfContext($lines_of_context);
}
$against_commit = $request->getValue('againstCommit');
if ($against_commit !== null) {
- $raw_query->setAgainstCommit($against_commit);
- }
-
- $byte_limit = $request->getValue('byteLimit');
- if ($byte_limit !== null) {
- $raw_query->setByteLimit($byte_limit);
+ $query->setAgainstCommit($against_commit);
}
- return $raw_query->loadRawDiff();
+ return $query->respondToConduitRequest($request);
}
}
diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php
--- a/src/applications/diffusion/controller/DiffusionBrowseController.php
+++ b/src/applications/diffusion/controller/DiffusionBrowseController.php
@@ -1527,19 +1527,36 @@
private function getBeforeLineNumber($target_commit) {
$drequest = $this->getDiffusionRequest();
+ $viewer = $this->getViewer();
$line = $drequest->getLine();
if (!$line) {
return null;
}
- $raw_diff = $this->callConduitWithDiffusionRequest(
+ $diff_info = $this->callConduitWithDiffusionRequest(
'diffusion.rawdiffquery',
array(
'commit' => $drequest->getCommit(),
'path' => $drequest->getPath(),
'againstCommit' => $target_commit,
));
+
+ $file_phid = $diff_info['filePHID'];
+ $file = id(new PhabricatorFileQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($file_phid))
+ ->executeOne();
+ if (!$file) {
+ throw new Exception(
+ pht(
+ 'Failed to load file ("%s") returned by "%s".',
+ $file_phid,
+ 'diffusion.rawdiffquery.'));
+ }
+
+ $raw_diff = $file->loadFileData();
+
$old_line = 0;
$new_line = 0;
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -1027,24 +1027,26 @@
}
private function buildRawDiffResponse(DiffusionRequest $drequest) {
- $raw_diff = $this->callConduitWithDiffusionRequest(
+ $diff_info = $this->callConduitWithDiffusionRequest(
'diffusion.rawdiffquery',
array(
'commit' => $drequest->getCommit(),
'path' => $drequest->getPath(),
));
- $file = PhabricatorFile::buildFromFileDataOrHash(
- $raw_diff,
- array(
- 'name' => $drequest->getCommit().'.diff',
- 'ttl' => (60 * 60 * 24),
- 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
- ));
+ $file_phid = $diff_info['filePHID'];
- $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
- $file->attachToObject($drequest->getRepository()->getPHID());
- unset($unguarded);
+ $file = id(new PhabricatorFileQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs(array($file_phid))
+ ->executeOne();
+ if (!$file) {
+ throw new Exception(
+ pht(
+ 'Failed to load file ("%s") returned by "%s".',
+ $file_phid,
+ 'diffusion.rawdiffquery'));
+ }
return $file->getRedirectResponse();
}
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
@@ -1095,7 +1095,7 @@
->setTimeout($time_limit)
->setByteLimit($byte_limit)
->setLinesOfContext(0)
- ->loadRawDiff();
+ ->executeInline();
break;
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
// TODO: This diff has 3 lines of context, which produces slightly
diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php
--- a/src/applications/diffusion/herald/HeraldCommitAdapter.php
+++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php
@@ -204,25 +204,50 @@
}
private function loadCommitDiff() {
+ $viewer = PhabricatorUser::getOmnipotentUser();
+
$byte_limit = self::getEnormousByteLimit();
+ $time_limit = self::getEnormousTimeLimit();
- $raw = $this->callConduit(
+ $diff_info = $this->callConduit(
'diffusion.rawdiffquery',
array(
'commit' => $this->commit->getCommitIdentifier(),
- 'timeout' => self::getEnormousTimeLimit(),
+ 'timeout' => $time_limit,
'byteLimit' => $byte_limit,
'linesOfContext' => 0,
));
- if (strlen($raw) >= $byte_limit) {
+ if ($diff_info['tooHuge']) {
throw new Exception(
pht(
- 'The raw text of this change is enormous (larger than %d bytes). '.
+ 'The raw text of this change is enormous (larger than %s byte(s)). '.
'Herald can not process it.',
- $byte_limit));
+ new PhutilNumber($byte_limit)));
}
+ if ($diff_info['tooSlow']) {
+ throw new Exception(
+ pht(
+ 'The raw text of this change took too long to process (longer '.
+ 'than %s second(s)). Herald can not process it.',
+ new PhutilNumber($time_limit)));
+ }
+
+ $file_phid = $diff_info['filePHID'];
+ $diff_file = id(new PhabricatorFileQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($file_phid))
+ ->executeOne();
+ if (!$diff_file) {
+ throw new Exception(
+ pht(
+ 'Failed to load diff ("%s") for this change.',
+ $file_phid));
+ }
+
+ $raw = $diff_file->loadFileData();
+
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw);
diff --git a/src/applications/diffusion/query/DiffusionFileFutureQuery.php b/src/applications/diffusion/query/DiffusionFileFutureQuery.php
--- a/src/applications/diffusion/query/DiffusionFileFutureQuery.php
+++ b/src/applications/diffusion/query/DiffusionFileFutureQuery.php
@@ -42,7 +42,7 @@
return $this->didHitTimeLimit;
}
- abstract protected function getFileContentFuture();
+ abstract protected function newQueryFuture();
final public function respondToConduitRequest(ConduitAPIRequest $request) {
$drequest = $this->getRequest();
@@ -82,13 +82,13 @@
}
final public function executeInline() {
- $future = $this->getFileContentFuture();
+ $future = $this->newConfiguredQueryFuture();
list($stdout) = $future->resolvex();
- return $future;
+ return $stdout;
}
final protected function executeQuery() {
- $future = $this->newConfiguredFileContentFuture();
+ $future = $this->newQueryFuture();
$drequest = $this->getRequest();
@@ -134,8 +134,8 @@
return $file;
}
- private function newConfiguredFileContentFuture() {
- $future = $this->getFileContentFuture();
+ private function newConfiguredQueryFuture() {
+ $future = $this->newQueryFuture();
if ($this->getTimeout()) {
$future->setTimeout($this->getTimeout());
@@ -149,5 +149,4 @@
return $future;
}
-
}
diff --git a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
--- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
+++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
@@ -2,7 +2,7 @@
final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery {
- protected function getFileContentFuture() {
+ protected function newQueryFuture() {
$drequest = $this->getRequest();
$repository = $drequest->getRepository();
diff --git a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php
--- a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php
+++ b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php
@@ -3,7 +3,7 @@
final class DiffusionMercurialFileContentQuery
extends DiffusionFileContentQuery {
- protected function getFileContentFuture() {
+ protected function newQueryFuture() {
$drequest = $this->getRequest();
$repository = $drequest->getRepository();
diff --git a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php
--- a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php
+++ b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php
@@ -2,7 +2,7 @@
final class DiffusionSvnFileContentQuery extends DiffusionFileContentQuery {
- protected function getFileContentFuture() {
+ protected function newQueryFuture() {
$drequest = $this->getRequest();
$repository = $drequest->getRepository();
diff --git a/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php
--- a/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php
+++ b/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php
@@ -2,7 +2,7 @@
final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery {
- protected function executeQuery() {
+ protected function newQueryFuture() {
$drequest = $this->getRequest();
$repository = $drequest->getRepository();
@@ -17,54 +17,34 @@
'--dst-prefix=b/',
'-U'.(int)$this->getLinesOfContext(),
);
- $options = implode(' ', $options);
$against = $this->getAgainstCommit();
if ($against === null) {
- $against = $commit.'^';
- }
-
- // If there's no path, get the entire raw diff.
- $path = nonempty($drequest->getPath(), '.');
-
- $future = $repository->getLocalCommandFuture(
- 'diff %C %s %s -- %s',
- $options,
- $against,
- $commit,
- $path);
-
- $this->configureFuture($future);
-
- try {
- list($raw_diff) = $future->resolvex();
- } catch (CommandException $ex) {
- // Check if this is the root commit by seeing if it has parents.
+ // Check if this is the root commit by seeing if it has parents, since
+ // `git diff X^ X` does not work if "X" is the initial commit.
list($parents) = $repository->execxLocalCommand(
'log --format=%s %s --',
- '%P', // "parents"
+ '%P',
$commit);
if (strlen(trim($parents))) {
- throw $ex;
+ $against = $commit.'^';
+ } else {
+ $against = ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT;
}
+ }
- // No parents means we're looking at the root revision. Diff against
- // the empty tree hash instead, since there is no parent so "^" does
- // not work. See ArcanistGitAPI for more discussion.
- $future = $repository->getLocalCommandFuture(
- 'diff %C %s %s -- %s',
- $options,
- ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT,
- $commit,
- $drequest->getPath());
-
- $this->configureFuture($future);
-
- list($raw_diff) = $future->resolvex();
+ $path = $drequest->getPath();
+ if (!strlen($path)) {
+ $path = '.';
}
- return $raw_diff;
+ return $repository->getLocalCommandFuture(
+ 'diff %Ls %s %s -- %s',
+ $options,
+ $against,
+ $commit,
+ $path);
}
}
diff --git a/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php
--- a/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php
+++ b/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php
@@ -2,11 +2,7 @@
final class DiffusionMercurialRawDiffQuery extends DiffusionRawDiffQuery {
- protected function executeQuery() {
- return $this->executeRawDiffCommand();
- }
-
- protected function executeRawDiffCommand() {
+ protected function newQueryFuture() {
$drequest = $this->getRequest();
$repository = $drequest->getRepository();
@@ -30,11 +26,7 @@
$commit,
$path);
- $this->configureFuture($future);
-
- list($raw_diff) = $future->resolvex();
-
- return $raw_diff;
+ return $future;
}
}
diff --git a/src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php
--- a/src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php
+++ b/src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php
@@ -1,40 +1,17 @@
<?php
-abstract class DiffusionRawDiffQuery extends DiffusionQuery {
+abstract class DiffusionRawDiffQuery
+ extends DiffusionFileFutureQuery {
- private $timeout;
private $linesOfContext = 65535;
private $anchorCommit;
private $againstCommit;
- private $byteLimit;
final public static function newFromDiffusionRequest(
DiffusionRequest $request) {
return parent::newQueryObject(__CLASS__, $request);
}
- final public function loadRawDiff() {
- return $this->executeQuery();
- }
-
- final public function setTimeout($timeout) {
- $this->timeout = $timeout;
- return $this;
- }
-
- final public function getTimeout() {
- return $this->timeout;
- }
-
- public function setByteLimit($byte_limit) {
- $this->byteLimit = $byte_limit;
- return $this;
- }
-
- public function getByteLimit() {
- return $this->byteLimit;
- }
-
final public function setLinesOfContext($lines_of_context) {
$this->linesOfContext = $lines_of_context;
return $this;
@@ -66,15 +43,4 @@
return $this->getRequest()->getStableCommit();
}
- protected function configureFuture(ExecFuture $future) {
- if ($this->getTimeout()) {
- $future->setTimeout($this->getTimeout());
- }
-
- if ($this->getByteLimit()) {
- $future->setStdoutSizeLimit($this->getByteLimit());
- $future->setStderrSizeLimit($this->getByteLimit());
- }
- }
-
}
diff --git a/src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php
--- a/src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php
+++ b/src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php
@@ -2,7 +2,7 @@
final class DiffusionSvnRawDiffQuery extends DiffusionRawDiffQuery {
- protected function executeQuery() {
+ protected function newQueryFuture() {
$drequest = $this->getRequest();
$repository = $drequest->getRepository();
@@ -22,10 +22,7 @@
$commit,
$repository->getSubversionPathURI($drequest->getPath()));
- $this->configureFuture($future);
-
- list($raw_diff) = $future->resolvex();
- return $raw_diff;
+ return $future;
}
}
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
@@ -105,40 +105,64 @@
private function loadRawPatchText(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
+ $viewer = PhabricatorUser::getOmnipotentUser();
+
+ $identifier = $commit->getCommitIdentifier();
$drequest = DiffusionRequest::newFromDictionary(
array(
- 'user' => PhabricatorUser::getOmnipotentUser(),
+ 'user' => $viewer,
'repository' => $repository,
- 'commit' => $commit->getCommitIdentifier(),
));
- $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest);
- $raw_query->setLinesOfContext(3);
-
$time_key = 'metamta.diffusion.time-limit';
$byte_key = 'metamta.diffusion.byte-limit';
$time_limit = PhabricatorEnv::getEnvConfig($time_key);
$byte_limit = PhabricatorEnv::getEnvConfig($byte_key);
- if ($time_limit) {
- $raw_query->setTimeout($time_limit);
- }
+ $diff_info = DiffusionQuery::callConduitWithDiffusionRequest(
+ $viewer,
+ $drequest,
+ 'diffusion.rawdiffquery',
+ array(
+ 'commit' => $identifier,
+ 'linesOfContext' => 3,
+ 'timeout' => $time_limit,
+ 'byteLimit' => $byte_limit,
+ ));
- $raw_diff = $raw_query->loadRawDiff();
+ if ($diff_info['tooSlow']) {
+ throw new Exception(
+ pht(
+ 'Patch generation took longer than configured limit ("%s") of '.
+ '%s second(s).',
+ $time_key,
+ new PhutilNumber($time_limit)));
+ }
- $size = strlen($raw_diff);
- if ($byte_limit && $size > $byte_limit) {
- $pretty_size = phutil_format_bytes($size);
+ if ($diff_info['tooHuge']) {
$pretty_limit = phutil_format_bytes($byte_limit);
- throw new Exception(pht(
- 'Patch size of %s exceeds configured byte size limit (%s) of %s.',
- $pretty_size,
- $byte_key,
- $pretty_limit));
+ throw new Exception(
+ pht(
+ 'Patch size exceeds configured byte size limit ("%s") of %s.',
+ $byte_key,
+ $pretty_limit));
+ }
+
+ $file_phid = $diff_info['filePHID'];
+ $file = id(new PhabricatorFileQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($file_phid))
+ ->executeOne();
+ if (!$file) {
+ throw new Exception(
+ pht(
+ 'Failed to load file ("%s") returned by "%s".',
+ $file_phid,
+ 'diffusion.rawdiffquery'));
}
- return $raw_diff;
+ return $file->loadFileData();
}
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Nov 13, 1:39 AM (5 d, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6722065
Default Alt Text
D16460.diff (21 KB)
Attached To
Mode
D16460: Return Diffusion diffs through Files, not directly over Conduit
Attached
Detach File
Event Timeline
Log In to Comment