Survive pull/discover for hosted repositories in all VCSes

Summary:
Hosted repositories only sometimes survive the pull/discover phases right now, due to issues like:

  - Pull tries to `git clone`, but should `git init`.
  - Mercurial doesn't handle empty repositories with on branches.
  - SVN tries to connect to an invalid remote.
  - None of them set the INIT repo flag correctly, so status doesn't get updated properly in the UI.

Fix all this stuff.

Test Plan:
  - For each of Git, SVN and Mercurial:
    - Created a new repository from the web UI in a deactivated state.
    - Made it hosted.
    - Manually ran pull/discover.
    - Verified we end up with initialized, empty repositories in consistent states.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

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

src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php

@@ -536,20 +536,22 @@ final class PhabricatorRepositoryPullLocalDaemon
   private function executeGitDiscover(
     PhabricatorRepository $repository) {
 
-    list($remotes) = $repository->execxLocalCommand(
+    if (!$repository->isHosted()) {
-      'remote show -n origin');
+      list($remotes) = $repository->execxLocalCommand(
+        'remote show -n origin');
 
-    $matches = null;
+      $matches = null;
-    if (!preg_match('/^\s*Fetch URL:\s*(.*?)\s*$/m', $remotes, $matches)) {
+      if (!preg_match('/^\s*Fetch URL:\s*(.*?)\s*$/m', $remotes, $matches)) {
-      throw new Exception(
+        throw new Exception(
-        "Expected 'Fetch URL' in 'git remote show -n origin'.");
+          "Expected 'Fetch URL' in 'git remote show -n origin'.");
+      }
+
+      self::executeGitVerifySameOrigin(
+        $matches[1],
+        $repository->getRemoteURI(),
+        $repository->getLocalPath());
     }
 
-    self::executeGitVerifySameOrigin(
-      $matches[1],
-      $repository->getRemoteURI(),
-      $repository->getLocalPath());
-
     $refs = id(new DiffusionLowLevelGitRefQuery())
       ->setRepository($repository)
       ->withIsOriginBranch(true)
@@ -744,6 +746,11 @@ final class PhabricatorRepositoryPullLocalDaemon
     // NOTE: "--debug" gives us 40-character hashes.
     list($stdout) = $repository->execxLocalCommand('--debug branches');
 
+    if (!trim($stdout)) {
+      // No branches; likely a newly initialized repository.
+      return false;
+    }
+
     $branches = ArcanistMercurialParser::parseMercurialBranches($stdout);
     $got_something = false;
     foreach ($branches as $name => $branch) {

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -5,9 +5,13 @@
  * @{class:PhabricatorRepository} objects. Used by
  * @{class:PhabricatorRepositoryPullLocalDaemon}.
  *
+ * This class also covers initial working copy setup through `git clone`,
+ * `git init`, `hg clone`, `hg init`, or `svnadmin create`.
+ *
  * @task pull     Pulling Working Copies
  * @task git      Pulling Git Working Copies
  * @task hg       Pulling Mercurial Working Copies
+ * @task svn      Pulling Subversion Working Copies
  * @task internal Internals
  */
 final class PhabricatorRepositoryPullEngine
@@ -22,28 +26,24 @@ final class PhabricatorRepositoryPullEngine
 
     $is_hg = false;
     $is_git = false;
+    $is_svn = true;
 
     $vcs = $repository->getVersionControlSystem();
     $callsign = $repository->getCallsign();
 
-    if ($repository->isHosted()) {
-      $this->skipPull(
-        pht(
-          'Repository "%s" is hosted, so Phabricator does not pull updates '.
-          'for it.',
-          $callsign));
-      return;
-    }
-
     switch ($vcs) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-        // We never pull a local copy of Subversion repositories.
+        // We never pull a local copy of non-hosted Subversion repositories.
-        $this->skipPull(
+        if (!$repository->isHosted()) {
-          pht(
+          $this->skipPull(
-            "Repository '%s' is a Subversion repository, which does not ".
+            pht(
-            "require a local working copy to be pulled.",
+              "Repository '%s' is a non-hosted Subversion repository, which ".
-            $callsign));
+              "does not require a local working copy to be pulled.",
-        return;
+              $callsign));
+          return;
+        }
+        $is_svn = true;
+        break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
         $is_git = true;
         break;
@@ -76,18 +76,28 @@ final class PhabricatorRepositoryPullEngine
             $callsign));
         if ($is_git) {
           $this->executeGitCreate();
-        } else {
+        } else if ($is_hg) {
           $this->executeMercurialCreate();
+        } else {
+          $this->executeSubversionCreate();
         }
       } else {
-        $this->logPull(
+        if ($repository->isHosted()) {
-          pht(
+          $this->logPull(
-            "Updating the working copy for repository '%s'.",
+            pht(
-            $callsign));
+              "Repository '%s' is hosted, so Phabricator does not pull ".
-        if ($is_git) {
+              "updates for it.",
-          $this->executeGitUpdate();
+              $callsign));
         } else {
-          $this->executeMercurialUpdate();
+          $this->logPull(
+            pht(
+              "Updating the working copy for repository '%s'.",
+              $callsign));
+          if ($is_git) {
+            $this->executeGitUpdate();
+          } else {
+            $this->executeMercurialUpdate();
+          }
         }
       }
     } catch (Exception $ex) {
@@ -102,8 +112,8 @@ final class PhabricatorRepositoryPullEngine
   }
 
   private function skipPull($message) {
-    $this->updateRepositoryInitStatus(null);
     $this->log('%s', $message);
+    $this->donePull();
   }
 
   private function abortPull($message, Exception $ex = null) {
@@ -146,10 +156,18 @@ final class PhabricatorRepositoryPullEngine
   private function executeGitCreate() {
     $repository = $this->getRepository();
 
-    $repository->execxRemoteCommand(
+    $path = rtrim($repository->getLocalPath(), '/');
-      'clone --bare %s %s',
+
-      $repository->getRemoteURI(),
+    if ($repository->isHosted()) {
-      rtrim($repository->getLocalPath(), '/'));
+      $repository->execxRemoteCommand(
+        'init --bare -- %s',
+        $path);
+    } else {
+      $repository->execxRemoteCommand(
+        'clone --bare -- %s %s',
+        $repository->getRemoteURI(),
+        $path);
+    }
   }
 
 
@@ -270,10 +288,18 @@ final class PhabricatorRepositoryPullEngine
   private function executeMercurialCreate() {
     $repository = $this->getRepository();
 
-    $repository->execxRemoteCommand(
+    $path = rtrim($repository->getLocalPath(), '/');
-      'clone %s %s',
+
-      $repository->getRemoteURI(),
+    if ($repository->isHosted()) {
-      rtrim($repository->getLocalPath(), '/'));
+      $repository->execxRemoteCommand(
+        'init -- %s',
+        $path);
+    } else {
+      $repository->execxRemoteCommand(
+        'clone -- %s %s',
+        $repository->getRemoteURI(),
+        $path);
+    }
   }
 
 
@@ -318,6 +344,20 @@ final class PhabricatorRepositoryPullEngine
   }
 
 
+/* -(  Pulling Subversion Working Copies  )---------------------------------- */
+
+
+  /**
+   * @task svn
+   */
+  private function executeSubversionCreate() {
+    $repository = $this->getRepository();
+
+    $path = rtrim($repository->getLocalPath(), '/');
+    execx('svnadmin create -- %s', $path);
+  }
+
+
 /* -(  Internals  )---------------------------------------------------------- */
 
 

src/applications/repository/storage/PhabricatorRepository.php

@@ -161,8 +161,16 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
       throw new Exception("Not a subversion repository!");
     }
 
-    $uri = $this->getDetail('remote-uri');
+    if ($this->isHosted()) {
+      $uri = 'file://'.$this->getLocalPath();
+    } else {
+      $uri = $this->getDetail('remote-uri');
+    }
+
     $subpath = $this->getDetail('svn-subpath');
+    if ($subpath) {
+      $subpath = '/'.ltrim($subpath, '/');
+    }
 
     return $uri.$subpath;
   }
@@ -609,6 +617,10 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    * @task uri
    */
   private function shouldUseSSH() {
+    if ($this->isHosted()) {
+      return false;
+    }
+
     $protocol = $this->getRemoteProtocol();
     if ($this->isSSHProtocol($protocol)) {
       return (bool)$this->getSSHKeyfile();
@@ -626,6 +638,10 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    * @task uri
    */
   private function shouldUseHTTP() {
+    if ($this->isHosted()) {
+      return false;
+    }
+
     $protocol = $this->getRemoteProtocol();
     if ($protocol == 'http' || $protocol == 'https') {
       return (bool)$this->getDetail('http-login');
@@ -643,6 +659,10 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    * @task uri
    */
   private function shouldUseSVNProtocol() {
+    if ($this->isHosted()) {
+      return false;
+    }
+
     $protocol = $this->getRemoteProtocol();
     if ($protocol == 'svn') {
       return (bool)$this->getDetail('http-login');
@@ -788,14 +808,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    * Raise more useful errors when there are basic filesystem problems.
    */
   private function assertLocalExists() {
-    switch ($this->getVersionControlSystem()) {
+    if (!$this->usesLocalWorkingCopy()) {
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+      return;
-        if (!$this->isHosted()) {
-          // For non-hosted SVN repositories, we don't expect a local directory
-          // to exist.
-          return;
-        }
-        break;
     }
 
     $local = $this->getLocalPath();