Make tracked git repositories use an implicit 'origin' remote

Summary:
See T624. I originally wrote this to require an explicit remote, but this
creates an ugly "origin:" in all the URIs and makes T270 more difficult.

Treat all branch names as implying 'origin/'.

Test Plan:
  - Pulled and imported a fresh copy of libphutil without issues.
  - Browsed various git repositories.
  - Browsed Javelin's various branches.
  - Ran upgrade script, got a bunch of clean 'origin/master' -> 'master'
conversions.
  - Tried to specify an explicit remote in a default branch name.
  - Unit tests.

Reviewers: nh, jungejason, btrahan

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T624

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

resources/sql/patches/093.gitremotes.php

@@ -0,0 +1,58 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+echo "Stripping remotes from repository default branches...\n";
+
+$table = new PhabricatorRepository();
+$conn_w = $table->establishConnection('w');
+
+$repos = queryfx_all(
+  $conn_w,
+  'SELECT id, name, details FROM %T WHERE versionControlSystem = %s',
+  $table->getTableName(),
+  'git');
+
+foreach ($repos as $repo) {
+  $details = json_decode($repo['details'], true);
+
+  $old = idx($details, 'default-branch', '');
+  if (strpos($old, '/') === false) {
+    continue;
+  }
+
+  $parts = explode('/', $old);
+  $parts = array_filter($parts);
+  $new = end($parts);
+
+  $details['default-branch'] = $new;
+  $new_details = json_encode($details);
+
+  $id = $repo['id'];
+  $name = $repo['name'];
+
+  echo "Updating default branch for repository #{$id} '{$name}' from ".
+       "'{$old}' to '{$new}' to remove the explicit remote.\n";
+  queryfx(
+    $conn_w,
+    'UPDATE %T SET details = %s WHERE id = %d',
+    $table->getTableName(),
+    $new_details,
+    $id);
+}
+
+echo "Done.\n";

src/__phutil_library_map__.php

@@ -260,6 +260,7 @@ phutil_register_library_map(array(
     'DiffusionFileContent' => 'applications/diffusion/data/filecontent',
     'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/base',
     'DiffusionGitBranchQuery' => 'applications/diffusion/query/branch/git',
+    'DiffusionGitBranchQueryTestCase' => 'applications/diffusion/query/branch/git/__tests__',
     'DiffusionGitBrowseQuery' => 'applications/diffusion/query/browse/git',
     'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/git',
     'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/git',
@@ -977,6 +978,7 @@ phutil_register_library_map(array(
     'DiffusionController' => 'PhabricatorController',
     'DiffusionDiffController' => 'DiffusionController',
     'DiffusionGitBranchQuery' => 'DiffusionBranchQuery',
+    'DiffusionGitBranchQueryTestCase' => 'PhabricatorTestCase',
     'DiffusionGitBrowseQuery' => 'DiffusionBrowseQuery',
     'DiffusionGitDiffQuery' => 'DiffusionDiffQuery',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',

src/applications/diffusion/data/branch/DiffusionBranchInformation.php

@@ -18,6 +18,8 @@
 
 final class DiffusionBranchInformation {
 
+  const DEFAULT_GIT_REMOTE = 'origin';
+
   private $name;
   private $headCommitIdentifier;
 

src/applications/diffusion/query/branch/git/DiffusionGitBranchQuery.php

@@ -27,8 +27,12 @@ final class DiffusionGitBranchQuery extends DiffusionBranchQuery {
     list($stdout) = $repository->execxLocalCommand(
       'branch -r --verbose --no-abbrev');
 
+    $branch_list = self::parseGitRemoteBranchOutput(
+      $stdout,
+      $only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
+
     $branches = array();
-    foreach (self::parseGitRemoteBranchOutput($stdout) as $name => $head) {
+    foreach ($branch_list as $name => $head) {
       $branch = new DiffusionBranchInformation();
       $branch->setName($name);
       $branch->setHeadCommitIdentifier($head);
@@ -38,7 +42,29 @@ final class DiffusionGitBranchQuery extends DiffusionBranchQuery {
     return $branches;
   }
 
-  public static function parseGitRemoteBranchOutput($stdout) {
+
+  /**
+   * Parse the output of 'git branch -r --verbose --no-abbrev' or similar into
+   * a map. For instance:
+   *
+   *   array(
+   *     'origin/master' => '99a9c082f9a1b68c7264e26b9e552484a5ae5f25',
+   *   );
+   *
+   * If you specify $only_this_remote, branches will be filtered to only those
+   * on the given remote, **and the remote name will be stripped**. For example:
+   *
+   *   array(
+   *     'master' => '99a9c082f9a1b68c7264e26b9e552484a5ae5f25',
+   *   );
+   *
+   * @param string stdout of git branch command.
+   * @param string Filter branches to those on a specific remote.
+   * @return map Map of 'branch' or 'remote/branch' to hash at HEAD.
+   */
+  public static function parseGitRemoteBranchOutput(
+    $stdout,
+    $only_this_remote = null) {
     $map = array();
 
     $lines = array_filter(explode("\n", $stdout));
@@ -57,7 +83,25 @@ final class DiffusionGitBranchQuery extends DiffusionBranchQuery {
       if (!preg_match('/^[ *] (\S+)\s+([a-z0-9]{40}) /', $line, $matches)) {
         throw new Exception("Failed to parse {$line}!");
       }
-      $map[$matches[1]] = $matches[2];
+
+      $remote_branch = $matches[1];
+      $branch_head = $matches[2];
+
+      if ($only_this_remote) {
+        $matches = null;
+        if (!preg_match('#^([^/]+)/(.*)$#', $remote_branch, $matches)) {
+          throw new Exception(
+            "Failed to parse remote branch '{$remote_branch}'!");
+        }
+        $remote_name = $matches[1];
+        $branch_name = $matches[2];
+        if ($remote_name != $only_this_remote) {
+          continue;
+        }
+        $map[$branch_name] = $branch_head;
+      } else {
+        $map[$remote_branch] = $branch_head;
+      }
     }
 
     return $map;

src/applications/diffusion/query/branch/git/__tests__/DiffusionGitBranchQueryTestCase.php

@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DiffusionGitBranchQueryTestCase
+  extends PhabricatorTestCase {
+
+  public function testRemoteBranchParser() {
+
+    $output = <<<EOTXT
+  origin/HEAD           -> origin/master
+  origin/accent-folding bfaea2e72197506e028c604cd1a294b6e37aa17d Add...
+  origin/eventordering  185a90a3c1b0556015e5f318fb86ccf8f7a6f3e3 RFC: Order...
+  origin/master         713f1fc54f9cfc830acbf6bbdb46a2883f772896 Automat...
+  alternate/stuff       4444444444444444444444444444444444444444 Hmm...
+
+EOTXT;
+
+    $this->assertEqual(
+      array(
+        'origin/accent-folding' => 'bfaea2e72197506e028c604cd1a294b6e37aa17d',
+        'origin/eventordering'  => '185a90a3c1b0556015e5f318fb86ccf8f7a6f3e3',
+        'origin/master'         => '713f1fc54f9cfc830acbf6bbdb46a2883f772896',
+        'alternate/stuff'       => '4444444444444444444444444444444444444444',
+      ),
+      DiffusionGitBranchQuery::parseGitRemoteBranchOutput($output));
+
+    $this->assertEqual(
+      array(
+        'accent-folding' => 'bfaea2e72197506e028c604cd1a294b6e37aa17d',
+        'eventordering'  => '185a90a3c1b0556015e5f318fb86ccf8f7a6f3e3',
+        'master'         => '713f1fc54f9cfc830acbf6bbdb46a2883f772896',
+      ),
+      DiffusionGitBranchQuery::parseGitRemoteBranchOutput($output, 'origin'));
+  }
+
+}

src/applications/diffusion/query/branch/git/__tests__/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/diffusion/query/branch/git');
+phutil_require_module('phabricator', 'infrastructure/testing/testcase');
+
+
+phutil_require_source('DiffusionGitBranchQueryTestCase.php');

src/applications/diffusion/request/git/DiffusionGitRequest.php

@@ -53,7 +53,8 @@ class DiffusionGitRequest extends DiffusionRequest {
       // followed a bad link, or misconfigured the default branch in the
       // Repository tool.
       list($this->stableCommitName) = $repository->execxLocalCommand(
-        'rev-parse --verify %s',
+        'rev-parse --verify %s/%s',
+        DiffusionBranchInformation::DEFAULT_GIT_REMOTE,
         $branch);
 
       if ($this->commit) {
@@ -103,7 +104,7 @@ class DiffusionGitRequest extends DiffusionRequest {
       return $this->branch;
     }
     if ($this->repository) {
-      return $this->repository->getDetail('default-branch', 'origin/master');
+      return $this->repository->getDetail('default-branch', 'master');
     }
     throw new Exception("Unable to determine branch!");
   }
@@ -117,7 +118,8 @@ class DiffusionGitRequest extends DiffusionRequest {
     if ($this->commit) {
       return $this->commit;
     }
-    return $this->getBranch();
+    $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE;
+    return $remote.'/'.$this->getBranch();
   }
 
   public function getStableCommitName() {
@@ -129,7 +131,17 @@ class DiffusionGitRequest extends DiffusionRequest {
   }
 
   private function decodeBranchName($branch) {
-    return str_replace(':', '/', $branch);
+    $branch = str_replace(':', '/', $branch);
+
+    // Backward compatibility for older-style URIs which had an explicit
+    // "origin" remote in the branch name. If a remote is specified, strip it
+    // away.
+    if (strpos($branch, '/') !== false) {
+      $parts = explode('/', $branch);
+      $branch = end($parts);
+    }
+
+    return $branch;
   }
 
   private function encodeBranchName($branch) {

src/applications/diffusion/request/git/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'applications/diffusion/data/branch');
 phutil_require_module('phabricator', 'applications/diffusion/request/base');
 
 

src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php

@@ -211,6 +211,7 @@ class PhabricatorRepositoryEditController
 
     $e_ssh_key = null;
     $e_ssh_keyfile = null;
+    $e_branch = null;
 
     switch ($repository->getVersionControlSystem()) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
@@ -245,6 +246,15 @@ class PhabricatorRepositoryEditController
         $repository->setDetail(
           'default-branch',
           $request->getStr('default-branch'));
+        if ($is_git) {
+          $branch_name = $repository->getDetail('default-branch');
+          if (strpos($branch_name, '/') !== false) {
+            $e_branch = 'Invalid';
+            $errors[] = "Your branch name should not specify an explicit ".
+                        "remote. For instance, use 'master', not ".
+                        "'origin/master'.";
+          }
+        }
       }
 
       $repository->setDetail(
@@ -444,7 +454,6 @@ class PhabricatorRepositoryEditController
         'If you want to connect to this repository over SSH, enter the '.
         'username and private key to use. You can leave these fields blank if '.
         'the repository does not use SSH.'.
-        ' <strong>NOTE: This feature is not yet fully supported.</strong>'.
       '</div>');
 
     $form
@@ -478,7 +487,6 @@ class PhabricatorRepositoryEditController
             'If you want to connect to this repository over HTTP Basic Auth, '.
             'enter the username and password to use. You can leave these '.
             'fields blank if the repository does not use HTTP Basic Auth.'.
-            ' <strong>NOTE: This feature is not yet fully supported.</strong>'.
           '</div>')
         ->appendChild(
           id(new AphrontFormTextControl())
@@ -562,7 +570,7 @@ class PhabricatorRepositoryEditController
       if ($is_mercurial) {
         $default_branch_name = 'default';
       } else if ($is_git) {
-        $default_branch_name = 'origin/master';
+        $default_branch_name = 'master';
       }
 
       $form
@@ -574,6 +582,7 @@ class PhabricatorRepositoryEditController
               $repository->getDetail(
                 'default-branch',
                 $default_branch_name))
+            ->setError($e_branch)
             ->setCaption(
               'Default <strong>remote</strong> branch to show in Diffusion.'));
     }

src/applications/repository/daemon/commitdiscovery/git/PhabricatorRepositoryGitCommitDiscoveryDaemon.php

@@ -47,7 +47,9 @@ class PhabricatorRepositoryGitCommitDiscoveryDaemon
     list($stdout) = $repository->execxLocalCommand(
       'branch -r --verbose --no-abbrev');
 
-    $branches = DiffusionGitBranchQuery::parseGitRemoteBranchOutput($stdout);
+    $branches = DiffusionGitBranchQuery::parseGitRemoteBranchOutput(
+      $stdout,
+      $only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
 
     $got_something = false;
     foreach ($branches as $name => $commit) {

src/applications/repository/daemon/commitdiscovery/git/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'applications/diffusion/data/branch');
 phutil_require_module('phabricator', 'applications/diffusion/query/branch/git');
 phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
 phutil_require_module('phabricator', 'applications/repository/daemon/commitdiscovery/base');

src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php

@@ -28,7 +28,7 @@ final class PhabricatorRepositoryGitFetchDaemon
     $local_path) {
 
     $repository->execxRemoteCommand(
-      'clone %s %s',
+      'clone --origin origin %s %s',
       $repository->getRemoteURI(),
       rtrim($local_path, '/'));
   }