@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

+33 -19
+7 -10
src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
··· 33 33 protected function getSVNResult(ConduitAPIRequest $request) { 34 34 $repository = $this->getDiffusionRequest()->getRepository(); 35 35 $commit = $request->getValue('commit'); 36 - list($info) = $repository->execxRemoteCommand( 37 - 'info %s', 38 - $repository->getRemoteURI()); 39 - $exists = false; 40 - $matches = null; 41 - if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) { 42 - $base_revision = $matches[1]; 43 - $exists = $base_revision >= $commit; 44 - } 45 - return $exists; 36 + 37 + $refs = id(new DiffusionCachedResolveRefsQuery()) 38 + ->setRepository($repository) 39 + ->withRefs(array($commit)) 40 + ->execute(); 41 + 42 + return (bool)$refs; 46 43 } 47 44 48 45 protected function getMercurialResult(ConduitAPIRequest $request) {
+11 -8
src/applications/diffusion/controller/DiffusionCommitController.php
··· 1136 1136 $merge_limit = $this->getMergeDisplayLimit(); 1137 1137 1138 1138 try { 1139 - $merges = $this->callConduitWithDiffusionRequest( 1140 - 'diffusion.mergedcommitsquery', 1141 - array( 1142 - 'commit' => $commit, 1143 - 'limit' => $merge_limit + 1, 1144 - )); 1145 - 1146 - $this->commitMerges = DiffusionPathChange::newFromConduit($merges); 1139 + if ($repository->isSVN()) { 1140 + $this->commitMerges = array(); 1141 + } else { 1142 + $merges = $this->callConduitWithDiffusionRequest( 1143 + 'diffusion.mergedcommitsquery', 1144 + array( 1145 + 'commit' => $commit, 1146 + 'limit' => $merge_limit + 1, 1147 + )); 1148 + $this->commitMerges = DiffusionPathChange::newFromConduit($merges); 1149 + } 1147 1150 } catch (Exception $ex) { 1148 1151 $this->commitMerges = false; 1149 1152 $exceptions[] = $ex;
+15 -1
src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
··· 70 70 } 71 71 72 72 private function loadSubversionParents() { 73 - $n = (int)$this->identifier; 73 + $repository = $this->getRepository(); 74 + $identifier = $this->identifier; 75 + 76 + $refs = id(new DiffusionCachedResolveRefsQuery()) 77 + ->setRepository($repository) 78 + ->withRefs(array($identifier)) 79 + ->execute(); 80 + if (!$refs) { 81 + throw new Exception( 82 + pht( 83 + 'No commit "%s" in this repository.', 84 + $identifier)); 85 + } 86 + 87 + $n = (int)$identifier; 74 88 if ($n > 1) { 75 89 $ids = array($n - 1); 76 90 } else {