From b26ecb189e486998acf469db0cba87993ef6ad1b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Jun 2016 05:22:00 -0700 Subject: [PATCH] (stable) Remove all uses of PhutilGitURI in Phabricator Summary: Ref T11137. This class is removed in D16099. Depends on D16099. `PhutilURI` now attempts to "just work" with Git-style URIs, so at least in theory we can just delete all of this code and pretend it does not exist. (I've left "Display URI" and "Effective URI" as distinct, at least for now, because I think the distinction may be relevant in the future even though it isn't right now, and to keep this diff small, although I may go remove one after I think about this for a bit.) Test Plan: - Created a new Git repository with a Git URI. - Pulled/updated it, which now works correctly and should resolve the original issue in T11137. - Verified that daemons now align the origin to a Git-style URI with a relative path, which should resolve the original issue in T11004. - Grepped for `PhutilGitURI`. - Also grepped in `arcanist/`, but found no matches, so no patch for that. - Checked display/conduit URIs. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11137 Differential Revision: https://secure.phabricator.com/D16100 --- ...rmasterCircleCIBuildStepImplementation.php | 5 -- .../PhabricatorRepositoryURINormalizer.php | 20 +----- .../storage/PhabricatorRepository.php | 45 +++---------- .../storage/PhabricatorRepositoryURI.php | 63 ++++++------------- 4 files changed, 29 insertions(+), 104 deletions(-) diff --git a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php index 365cf657de..e74e25a395 100644 --- a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php @@ -69,11 +69,6 @@ EOTEXT $uri_object = new PhutilURI($uri); $domain = $uri_object->getDomain(); - if (!strlen($domain)) { - $uri_object = new PhutilGitURI($uri); - $domain = $uri_object->getDomain(); - } - $domain = phutil_utf8_strtolower($domain); switch ($domain) { case 'github.com': diff --git a/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php b/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php index 42224c9553..27e93535ab 100644 --- a/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php +++ b/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php @@ -78,16 +78,7 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { switch ($this->type) { case self::TYPE_GIT: $uri = new PhutilURI($this->uri); - if ($uri->getProtocol()) { - return $uri->getPath(); - } - - $uri = new PhutilGitURI($this->uri); - if ($uri->getDomain()) { - return $uri->getPath(); - } - - return $this->uri; + return $uri->getPath(); case self::TYPE_SVN: case self::TYPE_MERCURIAL: $uri = new PhutilURI($this->uri); @@ -136,14 +127,7 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { $domain = null; $uri = new PhutilURI($this->uri); - if ($uri->getProtocol()) { - $domain = $uri->getDomain(); - } - - if (!strlen($domain)) { - $uri = new PhutilGitURI($this->uri); - $domain = $uri->getDomain(); - } + $domain = $uri->getDomain(); if (!strlen($domain)) { $domain = ''; diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 84c7cfd055..b771066ed6 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1201,27 +1201,19 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO */ public function getRemoteProtocol() { $uri = $this->getRemoteURIObject(); - - if ($uri instanceof PhutilGitURI) { - return 'ssh'; - } else { - return $uri->getProtocol(); - } + return $uri->getProtocol(); } /** - * Get a parsed object representation of the repository's remote URI. This - * may be a normal URI (returned as a @{class@libphutil:PhutilURI}) or a git - * URI (returned as a @{class@libphutil:PhutilGitURI}). + * Get a parsed object representation of the repository's remote URI.. * - * @return wild A @{class@libphutil:PhutilURI} or - * @{class@libphutil:PhutilGitURI}. + * @return wild A @{class@libphutil:PhutilURI}. * @task uri */ public function getRemoteURIObject() { $raw_uri = $this->getDetail('remote-uri'); - if (!$raw_uri) { + if (!strlen($raw_uri)) { return new PhutilURI(''); } @@ -1229,17 +1221,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return new PhutilURI('file://'.$raw_uri); } - $uri = new PhutilURI($raw_uri); - if ($uri->getProtocol()) { - return $uri; - } - - $uri = new PhutilGitURI($raw_uri); - if ($uri->getDomain()) { - return $uri; - } - - throw new Exception(pht("Remote URI '%s' could not be parsed!", $raw_uri)); + return new PhutilURI($raw_uri); } @@ -1666,27 +1648,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $this; } - public static function getRemoteURIProtocol($raw_uri) { - $uri = new PhutilURI($raw_uri); - if ($uri->getProtocol()) { - return strtolower($uri->getProtocol()); - } - - $git_uri = new PhutilGitURI($raw_uri); - if (strlen($git_uri->getDomain()) && strlen($git_uri->getPath())) { - return 'ssh'; - } - - return null; - } - public static function assertValidRemoteURI($uri) { if (trim($uri) != $uri) { throw new Exception( pht('The remote URI has leading or trailing whitespace.')); } - $protocol = self::getRemoteURIProtocol($uri); + $uri_object = new PhutilURI($uri); + $protocol = $uri_object->getProtocol(); // Catch confusion between Git/SCP-style URIs and normal URIs. See T3619 // for discussion. This is usually a user adding "ssh://" to an implicit diff --git a/src/applications/repository/storage/PhabricatorRepositoryURI.php b/src/applications/repository/storage/PhabricatorRepositoryURI.php index 853dcfa1b3..15e25205c8 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryURI.php +++ b/src/applications/repository/storage/PhabricatorRepositoryURI.php @@ -191,11 +191,6 @@ final class PhabricatorRepositoryURI return self::IO_NONE; } - - public function getDisplayURI() { - return $this->getURIObject(false); - } - public function getNormalizedURI() { $vcs = $this->getRepository()->getVersionControlSystem(); @@ -216,8 +211,12 @@ final class PhabricatorRepositoryURI return $normal_uri->getNormalizedURI(); } + public function getDisplayURI() { + return $this->getURIObject(); + } + public function getEffectiveURI() { - return $this->getURIObject(true); + return $this->getURIObject(); } public function getURIEnvelope() { @@ -243,11 +242,10 @@ final class PhabricatorRepositoryURI return new PhutilOpaqueEnvelope((string)$uri); } - private function getURIObject($normalize) { + private function getURIObject() { // Users can provide Git/SCP-style URIs in the form "user@host:path". - // These are equivalent to "ssh://user@host/path". We use the more standard - // form internally, and convert to it if we need to specify a port number, - // but try to preserve what the user typed when displaying the URI. + // In the general case, these are not equivalent to any "ssh://..." form + // because the path is relative. if ($this->isBuiltin()) { $builtin_protocol = $this->getForcedProtocol(); @@ -271,43 +269,22 @@ final class PhabricatorRepositoryURI // with it; this should always be provided by the associated credential. $uri->setPass(null); - if (!$uri->getProtocol()) { - $git_uri = new PhutilGitURI($raw_uri); - - // We must normalize this Git-style URI into a normal URI - $must_normalize = ($port && ($port != $default_ports['ssh'])); - if ($must_normalize || $normalize) { - $domain = $git_uri->getDomain(); - - - $uri = id(new PhutilURI("ssh://{$domain}")) - ->setUser($git_uri->getUser()) - ->setPath($git_uri->getPath()); - } else { - $uri = $git_uri; - } + $protocol = $this->getForcedProtocol(); + if ($protocol) { + $uri->setProtocol($protocol); } - $is_normal = ($uri instanceof PhutilURI); + if ($port) { + $uri->setPort($port); + } - if ($is_normal) { - $protocol = $this->getForcedProtocol(); - if ($protocol) { - $uri->setProtocol($protocol); - } + // Remove any explicitly set default ports. + $uri_port = $uri->getPort(); + $uri_protocol = $uri->getProtocol(); - if ($port) { - $uri->setPort($port); - } - - // Remove any explicitly set default ports. - $uri_port = $uri->getPort(); - $uri_protocol = $uri->getProtocol(); - - $uri_default = idx($default_ports, $uri_protocol); - if ($uri_default && ($uri_default == $uri_port)) { - $uri->setPort(null); - } + $uri_default = idx($default_ports, $uri_protocol); + if ($uri_default && ($uri_default == $uri_port)) { + $uri->setPort(null); } $user = $this->getForcedUser();