recaptime.dev / phorge
@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.

When users browse to a submodule path in Diffusion explicitly, don't fatal

Summary: Ref T13030. See PHI254. This behavior could be cleaner than I've made it, but it fixes the "this is totally broken" issue, replacing a fatal/exception with an informative (just not terribly useful) page.

Test Plan:
- Added a submodule to a repository.
- In Diffusion, clicked some other file next to the submodule, then edited the URI to the submodule path instead.
- Before patch: fatal.
- After patch: relatively useful message about this being a submodule.

Note that it's normally hard to hit this URI directly. In the browse view, submodules are marked up as directories and linked to a separate submodule resolution flow.

{F5321524}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13030

Differential Revision: https://secure.phabricator.com/D18831

epriestley 7cbbe2cc a1ad184d

+38
+30
src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php
··· 52 52 $commit, 53 53 $path); 54 54 } catch (CommandException $e) { 55 + // The "cat-file" command may fail if the path legitimately does not 56 + // exist, but it may also fail if the path is a submodule. This can 57 + // produce either "Not a valid object name" or "could not get object 58 + // info". 59 + 60 + // To detect if we have a submodule, use `git ls-tree`. If the path 61 + // is a submodule, we'll get a "160000" mode mask with type "commit". 62 + 63 + list($sub_err, $sub_stdout) = $repository->execLocalCommand( 64 + 'ls-tree %s -- %s', 65 + $commit, 66 + $path); 67 + if (!$sub_err) { 68 + // If the path failed "cat-file" but "ls-tree" worked, we assume it 69 + // must be a submodule. If it is, the output will look something 70 + // like this: 71 + // 72 + // 160000 commit <hash> <path> 73 + // 74 + // We make sure it has the 160000 mode mask to confirm that it's 75 + // definitely a submodule. 76 + $mode = (int)$sub_stdout; 77 + if ($mode & 160000) { 78 + $submodule_reason = DiffusionBrowseResultSet::REASON_IS_SUBMODULE; 79 + $result 80 + ->setReasonForEmptyResultSet($submodule_reason); 81 + return $result; 82 + } 83 + } 84 + 55 85 $stderr = $e->getStderr(); 56 86 if (preg_match('/^fatal: Not a valid object name/', $stderr)) { 57 87 // Grab two logs, since the first one is when the object was deleted.
+1
src/applications/diffusion/data/DiffusionBrowseResultSet.php
··· 3 3 final class DiffusionBrowseResultSet extends Phobject { 4 4 5 5 const REASON_IS_FILE = 'is-file'; 6 + const REASON_IS_SUBMODULE = 'is-submodule'; 6 7 const REASON_IS_DELETED = 'is-deleted'; 7 8 const REASON_IS_NONEXISTENT = 'nonexistent'; 8 9 const REASON_BAD_COMMIT = 'bad-commit';
+7
src/applications/diffusion/view/DiffusionEmptyResultView.php
··· 40 40 $body = pht('This path was an empty directory at %s.', $commit); 41 41 $severity = PHUIInfoView::SEVERITY_NOTICE; 42 42 break; 43 + case DiffusionBrowseResultSet::REASON_IS_SUBMODULE: 44 + $title = pht('Submodule'); 45 + // TODO: We could improve this, but it is normally difficult to 46 + // reach this page for a submodule. 47 + $body = pht('This path was a submodule at %s.', $commit); 48 + $severity = PHUIInfoView::SEVERITY_NOTICE; 49 + break; 43 50 case DiffusionBrowseResultSet::REASON_IS_DELETED: 44 51 $deleted = $this->browseResultSet->getDeletedAtCommit(); 45 52 $existed = $this->browseResultSet->getExistedAtCommit();