mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-12 00:26:13 +01:00
(stable) Fix several error handling issues with Subversion commits in Diffusion
Summary: Ref T9513. I checked this briefly but didn't do a very thorough job of it. - Don't try to query merges for Subversion, since it doesn't support them. - Fix up "existsquery" to work properly (and efficiently) for both hosted and imported repositories. - Fix up "parentsquery" to have similar behavior on invalid commits to other VCSes (throw an exception). Test Plan: - No more merges warning on SVN. - Hosted SVN gets the right exists result now. - Visiting "r23980283789287" now 404's instead of "not parsed yet". Reviewers: chad Reviewed By: chad Maniphest Tasks: T9513 Differential Revision: https://secure.phabricator.com/D14239
This commit is contained in:
parent
f760b3ec98
commit
5194ff3db5
3 changed files with 33 additions and 19 deletions
|
@ -33,16 +33,13 @@ final class DiffusionExistsQueryConduitAPIMethod
|
||||||
protected function getSVNResult(ConduitAPIRequest $request) {
|
protected function getSVNResult(ConduitAPIRequest $request) {
|
||||||
$repository = $this->getDiffusionRequest()->getRepository();
|
$repository = $this->getDiffusionRequest()->getRepository();
|
||||||
$commit = $request->getValue('commit');
|
$commit = $request->getValue('commit');
|
||||||
list($info) = $repository->execxRemoteCommand(
|
|
||||||
'info %s',
|
$refs = id(new DiffusionCachedResolveRefsQuery())
|
||||||
$repository->getRemoteURI());
|
->setRepository($repository)
|
||||||
$exists = false;
|
->withRefs(array($commit))
|
||||||
$matches = null;
|
->execute();
|
||||||
if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
|
|
||||||
$base_revision = $matches[1];
|
return (bool)$refs;
|
||||||
$exists = $base_revision >= $commit;
|
|
||||||
}
|
|
||||||
return $exists;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function getMercurialResult(ConduitAPIRequest $request) {
|
protected function getMercurialResult(ConduitAPIRequest $request) {
|
||||||
|
|
|
@ -1136,14 +1136,17 @@ final class DiffusionCommitController extends DiffusionController {
|
||||||
$merge_limit = $this->getMergeDisplayLimit();
|
$merge_limit = $this->getMergeDisplayLimit();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
if ($repository->isSVN()) {
|
||||||
|
$this->commitMerges = array();
|
||||||
|
} else {
|
||||||
$merges = $this->callConduitWithDiffusionRequest(
|
$merges = $this->callConduitWithDiffusionRequest(
|
||||||
'diffusion.mergedcommitsquery',
|
'diffusion.mergedcommitsquery',
|
||||||
array(
|
array(
|
||||||
'commit' => $commit,
|
'commit' => $commit,
|
||||||
'limit' => $merge_limit + 1,
|
'limit' => $merge_limit + 1,
|
||||||
));
|
));
|
||||||
|
|
||||||
$this->commitMerges = DiffusionPathChange::newFromConduit($merges);
|
$this->commitMerges = DiffusionPathChange::newFromConduit($merges);
|
||||||
|
}
|
||||||
} catch (Exception $ex) {
|
} catch (Exception $ex) {
|
||||||
$this->commitMerges = false;
|
$this->commitMerges = false;
|
||||||
$exceptions[] = $ex;
|
$exceptions[] = $ex;
|
||||||
|
|
|
@ -70,7 +70,21 @@ final class DiffusionLowLevelParentsQuery
|
||||||
}
|
}
|
||||||
|
|
||||||
private function loadSubversionParents() {
|
private function loadSubversionParents() {
|
||||||
$n = (int)$this->identifier;
|
$repository = $this->getRepository();
|
||||||
|
$identifier = $this->identifier;
|
||||||
|
|
||||||
|
$refs = id(new DiffusionCachedResolveRefsQuery())
|
||||||
|
->setRepository($repository)
|
||||||
|
->withRefs(array($identifier))
|
||||||
|
->execute();
|
||||||
|
if (!$refs) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'No commit "%s" in this repository.',
|
||||||
|
$identifier));
|
||||||
|
}
|
||||||
|
|
||||||
|
$n = (int)$identifier;
|
||||||
if ($n > 1) {
|
if ($n > 1) {
|
||||||
$ids = array($n - 1);
|
$ids = array($n - 1);
|
||||||
} else {
|
} else {
|
||||||
|
|
Loading…
Reference in a new issue