When observing a repository in Git, just "fetch <url>" without worrying about the "origin" remote

Summary:
Depends on D20420. Ref T13277. We currently spend substantial effort trying to detect and correct the URL of the "origin" remote in Git repositories.

I believe this is unnecessary, and we can always `git fetch <url> ...` to get the desired result instead of `git muck-with-origin + git fetch origin ...`. We already do this in the more recent parts of the codebase (e.g., intracluster sync) and it works correctly in every case I'm aware of.

Test Plan:

  - Grepped for `origin`, ` origin `.
  - Ran `bin/repository update ...` to fetch a mirrored repository.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13277

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

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -128,10 +128,6 @@ final class PhabricatorRepositoryDiscoveryEngine
   private function discoverGitCommits() {
     $repository = $this->getRepository();
 
-    if (!$repository->isHosted()) {
-      $this->verifyGitOrigin($repository);
-    }
-
     $heads = id(new DiffusionLowLevelGitRefQuery())
       ->setRepository($repository)
       ->execute();

src/applications/repository/engine/PhabricatorRepositoryEngine.php

@@ -70,123 +70,6 @@ abstract class PhabricatorRepositoryEngine extends Phobject {
     return PhabricatorGlobalLock::newLock($lock_key, $lock_parts);
   }
 
-
-  /**
-   * Verify that the "origin" remote exists, and points at the correct URI.
-   *
-   * This catches or corrects some types of misconfiguration, and also repairs
-   * an issue where Git 1.7.1 does not create an "origin" for `--bare` clones.
-   * See T4041.
-   *
-   * @param   PhabricatorRepository Repository to verify.
-   * @return  void
-   */
-  protected function verifyGitOrigin(PhabricatorRepository $repository) {
-    try {
-      list($remotes) = $repository->execxLocalCommand(
-        'remote show -n origin');
-    } catch (CommandException $ex) {
-      throw new PhutilProxyException(
-        pht(
-          'Expected to find a Git working copy at path "%s", but the '.
-          'path exists and is not a valid working copy. If you remove '.
-          'this directory, the daemons will automatically recreate it '.
-          'correctly. Phabricator will not destroy the directory for you '.
-          'because it can not be sure that it does not contain important '.
-          'data.',
-          $repository->getLocalPath()),
-        $ex);
-    }
-
-    $matches = null;
-    if (!preg_match('/^\s*Fetch URL:\s*(.*?)\s*$/m', $remotes, $matches)) {
-      throw new Exception(
-        pht(
-          "Expected '%s' in '%s'.",
-          'Fetch URL',
-          'git remote show -n origin'));
-    }
-
-    $remote_uri = $matches[1];
-    $expect_remote = $repository->getRemoteURI();
-
-    if ($remote_uri == 'origin') {
-      // If a remote does not exist, git pretends it does and prints out a
-      // made up remote where the URI is the same as the remote name. This is
-      // definitely not correct.
-
-      // Possibly, we should use `git remote --verbose` instead, which does not
-      // suffer from this problem (but is a little more complicated to parse).
-      $valid = false;
-      $exists = false;
-    } else {
-      $normal_type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
-
-      $remote_normal = id(new PhabricatorRepositoryURINormalizer(
-        $normal_type_git,
-        $remote_uri))->getNormalizedPath();
-
-      $expect_normal = id(new PhabricatorRepositoryURINormalizer(
-        $normal_type_git,
-        $expect_remote))->getNormalizedPath();
-
-      $valid = ($remote_normal == $expect_normal);
-      $exists = true;
-    }
-
-    // These URIs may have plaintext HTTP credentials. If they do, censor
-    // them for display. See T12945.
-    $display_remote = phutil_censor_credentials($remote_uri);
-    $display_expect = phutil_censor_credentials($expect_remote);
-
-    if (!$valid) {
-      if (!$exists) {
-        // If there's no "origin" remote, just create it regardless of how
-        // strongly we own the working copy. There is almost no conceivable
-        // scenario in which this could do damage.
-        $this->log(
-          pht(
-            'Remote "origin" does not exist. Creating "origin", with '.
-            'URI "%s".',
-            $expect_remote));
-        $repository->execxLocalCommand(
-          'remote add origin %P',
-          $repository->getRemoteURIEnvelope());
-
-        // NOTE: This doesn't fetch the origin (it just creates it), so we won't
-        // know about origin branches until the next "pull" happens. That's fine
-        // for our purposes, but might impact things in the future.
-      } else {
-        if ($repository->canDestroyWorkingCopy()) {
-          // Bad remote, but we can try to repair it.
-          $this->log(
-            pht(
-              'Remote "origin" exists, but is pointed at the wrong URI, "%s". '.
-              'Resetting origin URI to "%s.',
-              $remote_uri,
-              $expect_remote));
-          $repository->execxLocalCommand(
-            'remote set-url origin %P',
-            $repository->getRemoteURIEnvelope());
-        } else {
-          // Bad remote and we aren't comfortable repairing it.
-          $message = pht(
-            'Working copy at "%s" has a mismatched origin URI, "%s". '.
-            'The expected origin URI is "%s". Fix your configuration, or '.
-            'set the remote URI correctly. To avoid breaking anything, '.
-            'Phabricator will not automatically fix this.',
-            $repository->getLocalPath(),
-            $display_remote,
-            $display_expect);
-          throw new Exception($message);
-        }
-      }
-    }
-  }
-
-
-
-
   /**
    * @task internal
    */

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -122,7 +122,6 @@ final class PhabricatorRepositoryPullEngine
             $repository->getDisplayName()));
 
         if ($is_git) {
-          $this->verifyGitOrigin($repository);
           $this->executeGitUpdate();
         } else if ($is_hg) {
           $this->executeMercurialUpdate();
@@ -352,20 +351,10 @@ final class PhabricatorRepositoryPullEngine
 
     $this->logRefDifferences($remote_refs, $local_refs);
 
-    // Force the "origin" URI to the configured value.
-    $repository->execxLocalCommand(
-      'remote set-url origin -- %P',
-      $repository->getRemoteURIEnvelope());
-
-    if ($repository->isWorkingCopyBare()) {
-      // For bare working copies, we need this magic incantation.
     $future = $repository->getRemoteCommandFuture(
-        'fetch origin %s --prune',
+      'fetch %P %s --prune',
+      $repository->getRemoteURIEnvelope(),
       '+refs/*:refs/*');
-    } else {
-      $future = $repository->getRemoteCommandFuture(
-        'fetch --all --prune');
-    }
 
     $future
       ->setCWD($path)