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 @@ -4350,6 +4350,7 @@ 'PhabricatorRepositoryEngine' => 'applications/repository/engine/PhabricatorRepositoryEngine.php', 'PhabricatorRepositoryEnormousTransaction' => 'applications/repository/xaction/PhabricatorRepositoryEnormousTransaction.php', 'PhabricatorRepositoryFerretEngine' => 'applications/repository/search/PhabricatorRepositoryFerretEngine.php', + 'PhabricatorRepositoryFetchRefsTransaction' => 'applications/repository/xaction/PhabricatorRepositoryFetchRefsTransaction.php', 'PhabricatorRepositoryFilesizeLimitTransaction' => 'applications/repository/xaction/PhabricatorRepositoryFilesizeLimitTransaction.php', 'PhabricatorRepositoryFulltextEngine' => 'applications/repository/search/PhabricatorRepositoryFulltextEngine.php', 'PhabricatorRepositoryGitCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryGitCommitChangeParserWorker.php', @@ -10603,6 +10604,7 @@ 'PhabricatorRepositoryEngine' => 'Phobject', 'PhabricatorRepositoryEnormousTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryFerretEngine' => 'PhabricatorFerretEngine', + 'PhabricatorRepositoryFetchRefsTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryFilesizeLimitTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryFulltextEngine' => 'PhabricatorFulltextEngine', 'PhabricatorRepositoryGitCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker', diff --git a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php --- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php @@ -212,6 +212,7 @@ ->setObject($object) ->execute(); + $fetch_value = $object->getFetchRules(); $track_value = $object->getTrackOnlyRules(); $autoclose_value = $object->getAutocloseOnlyRules(); @@ -364,6 +365,17 @@ ->setConduitDescription(pht('Set the default branch name.')) ->setConduitTypeDescription(pht('New default branch name.')) ->setValue($object->getDetail('default-branch')), + id(new PhabricatorTextAreaEditField()) + ->setIsStringList(true) + ->setKey('fetchRefs') + ->setLabel(pht('Fetch Refs')) + ->setTransactionType( + PhabricatorRepositoryFetchRefsTransaction::TRANSACTIONTYPE) + ->setIsCopyable(true) + ->setDescription(pht('Fetch only these refs.')) + ->setConduitDescription(pht('Set the fetched refs.')) + ->setConduitTypeDescription(pht('New fetched refs.')) + ->setValue($fetch_value), id(new PhabricatorTextAreaEditField()) ->setIsStringList(true) ->setKey('trackOnly') diff --git a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php --- a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php @@ -36,6 +36,7 @@ protected function getEditEngineFieldKeys() { return array( 'defaultBranch', + 'fetchRefs', 'trackOnly', 'autocloseOnly', ); @@ -78,6 +79,16 @@ phutil_tag('em', array(), $repository->getDefaultBranch())); $view->addProperty(pht('Default Branch'), $default_branch); + if ($repository->supportsFetchRules()) { + $fetch_only = $repository->getFetchRules(); + if ($fetch_only) { + $fetch_display = implode(', ', $fetch_only); + } else { + $fetch_display = phutil_tag('em', array(), pht('Fetch All Refs')); + } + $view->addProperty(pht('Fetch Refs'), $fetch_display); + } + $track_only_rules = $repository->getTrackOnlyRules(); $track_only_rules = implode(', ', $track_only_rules); $track_only = nonempty( diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php --- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -339,8 +339,17 @@ throw new Exception($message); } - $remote_refs = $this->loadGitRemoteRefs($repository); - $local_refs = $this->loadGitLocalRefs($repository); + // Load the refs we're planning to fetch from the remote repository. + $remote_refs = $this->loadGitRemoteRefs( + $repository, + $repository->getRemoteURIEnvelope()); + + // Load the refs we're planning to fetch from the local repository, by + // using the local working copy path as the "remote" repository URI. + $local_refs = $this->loadGitRemoteRefs( + $repository, + new PhutilOpaqueEnvelope($path)); + if ($remote_refs === $local_refs) { $this->log( pht( @@ -351,16 +360,49 @@ $this->logRefDifferences($remote_refs, $local_refs); + $fetch_rules = $this->getGitFetchRules($repository); + $future = $repository->getRemoteCommandFuture( - 'fetch %P %s --prune', + 'fetch --prune -- %P %Ls', $repository->getRemoteURIEnvelope(), - '+refs/*:refs/*'); + $fetch_rules); $future ->setCWD($path) ->resolvex(); } + private function getGitRefRules(PhabricatorRepository $repository) { + $ref_rules = $repository->getFetchRules($repository); + + if (!$ref_rules) { + $ref_rules = array( + 'refs/*', + ); + } + + return $ref_rules; + } + + private function getGitFetchRules(PhabricatorRepository $repository) { + $ref_rules = $this->getGitRefRules($repository); + + // Rewrite each ref rule "X" into "+X:X". + + // The "X" means "fetch ref X". + // The "...:X" means "...and copy it into local ref X". + // The "+..." means "...and overwrite the local ref if it already exists". + + $fetch_rules = array(); + foreach ($ref_rules as $key => $ref_rule) { + $fetch_rules[] = sprintf( + '+%s:%s', + $ref_rule, + $ref_rule); + } + + return $fetch_rules; + } /** * @task git @@ -378,15 +420,30 @@ $this->installHook($root.$path); } - private function loadGitRemoteRefs(PhabricatorRepository $repository) { - $remote_envelope = $repository->getRemoteURIEnvelope(); + private function loadGitRemoteRefs( + PhabricatorRepository $repository, + PhutilOpaqueEnvelope $remote_envelope) { + + $ref_rules = $this->getGitRefRules($repository); // NOTE: "git ls-remote" does not support "--" until circa January 2016. - // See T12416. None of the flags to "ls-remote" appear dangerous, and - // other checks make it difficult to configure a suspicious remote URI. + // See T12416. None of the flags to "ls-remote" appear dangerous, but + // refuse to list any refs beginning with "-" just in case. + + foreach ($ref_rules as $ref_rule) { + if (preg_match('/^-/', $ref_rule)) { + throw new Exception( + pht( + 'Refusing to list potentially dangerous ref ("%s") beginning '. + 'with "-".', + $ref_rule)); + } + } + list($stdout) = $repository->execxRemoteCommand( - 'ls-remote %P', - $remote_envelope); + 'ls-remote %P %Ls', + $remote_envelope, + $ref_rules); // Empty repositories don't have any refs. if (!strlen(rtrim($stdout))) { diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1213,6 +1213,22 @@ return $this; } + public function supportsFetchRules() { + if ($this->isGit()) { + return true; + } + + return false; + } + + public function getFetchRules() { + return $this->getDetail('fetch-rules', array()); + } + + public function setFetchRules(array $rules) { + return $this->setDetail('fetch-rules', $rules); + } + /* -( Repository URI Management )------------------------------------------ */ diff --git a/src/applications/repository/xaction/PhabricatorRepositoryFetchRefsTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryFetchRefsTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/repository/xaction/PhabricatorRepositoryFetchRefsTransaction.php @@ -0,0 +1,94 @@ +getFetchRules(); + } + + public function applyInternalEffects($object, $value) { + $object->setFetchRules($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if (!$new) { + return pht( + '%s set this repository to fetch all refs.', + $this->renderAuthor()); + } else if (!$old) { + return pht( + '%s set this repository to fetch refs: %s.', + $this->renderAuthor(), + $this->renderValue(implode(', ', $new))); + } else { + return pht( + '%s changed fetched refs from %s to %s.', + $this->renderAuthor(), + $this->renderValue(implode(', ', $old)), + $this->renderValue(implode(', ', $new))); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + if (!is_array($new_value) || !phutil_is_natural_list($new_value)) { + $errors[] = $this->newInvalidError( + pht( + 'Fetch rules must be a list of strings, got "%s".', + phutil_describe_type($new_value)), + $xaction); + continue; + } + + foreach ($new_value as $idx => $rule) { + if (!is_string($rule)) { + $errors[] = $this->newInvalidError( + pht( + 'Fetch rule (at index "%s") must be a string, got "%s".', + $idx, + phutil_describe_type($rule)), + $xaction); + continue; + } + + if (!strlen($rule)) { + $errors[] = $this->newInvalidError( + pht( + 'Fetch rule (at index "%s") is empty. Fetch rules must '. + 'contain text.', + $idx), + $xaction); + continue; + } + + // Since we fetch ref "X" as "+X:X", don't allow rules to include + // colons. This is specific to Git and may not be relevant if + // Mercurial repositories eventually get fetch rules. + if (preg_match('/:/', $rule)) { + $errors[] = $this->newInvalidError( + pht( + 'Fetch rule ("%s", at index "%s") is invalid: fetch rules '. + 'must not contain colons.', + $rule, + $idx), + $xaction); + continue; + } + + } + } + + return $errors; + } + +}