diff --git a/resources/sql/patches/093.gitremotes.php b/resources/sql/patches/093.gitremotes.php new file mode 100644 index 0000000000..d483cdc972 --- /dev/null +++ b/resources/sql/patches/093.gitremotes.php @@ -0,0 +1,58 @@ +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"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a3313624e9..17b1efed9b 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/diffusion/data/branch/DiffusionBranchInformation.php b/src/applications/diffusion/data/branch/DiffusionBranchInformation.php index 3418b52e9a..4c8121f3f1 100644 --- a/src/applications/diffusion/data/branch/DiffusionBranchInformation.php +++ b/src/applications/diffusion/data/branch/DiffusionBranchInformation.php @@ -18,6 +18,8 @@ final class DiffusionBranchInformation { + const DEFAULT_GIT_REMOTE = 'origin'; + private $name; private $headCommitIdentifier; diff --git a/src/applications/diffusion/query/branch/git/DiffusionGitBranchQuery.php b/src/applications/diffusion/query/branch/git/DiffusionGitBranchQuery.php index 4dfb46ad8f..a420cbd8bb 100644 --- a/src/applications/diffusion/query/branch/git/DiffusionGitBranchQuery.php +++ b/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; diff --git a/src/applications/diffusion/query/branch/git/__tests__/DiffusionGitBranchQueryTestCase.php b/src/applications/diffusion/query/branch/git/__tests__/DiffusionGitBranchQueryTestCase.php new file mode 100644 index 0000000000..1111679c5b --- /dev/null +++ b/src/applications/diffusion/query/branch/git/__tests__/DiffusionGitBranchQueryTestCase.php @@ -0,0 +1,51 @@ + 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')); + } + +} diff --git a/src/applications/diffusion/query/branch/git/__tests__/__init__.php b/src/applications/diffusion/query/branch/git/__tests__/__init__.php new file mode 100644 index 0000000000..31566e49ce --- /dev/null +++ b/src/applications/diffusion/query/branch/git/__tests__/__init__.php @@ -0,0 +1,13 @@ +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) { diff --git a/src/applications/diffusion/request/git/__init__.php b/src/applications/diffusion/request/git/__init__.php index f63f9da1b5..35ebc5a2a9 100644 --- a/src/applications/diffusion/request/git/__init__.php +++ b/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'); diff --git a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php index d29a08dcce..25c95ee1ab 100644 --- a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php +++ b/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.'. - ' NOTE: This feature is not yet fully supported.'. ''); $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.'. - ' NOTE: This feature is not yet fully supported.'. '') ->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 remote branch to show in Diffusion.')); } diff --git a/src/applications/repository/daemon/commitdiscovery/git/PhabricatorRepositoryGitCommitDiscoveryDaemon.php b/src/applications/repository/daemon/commitdiscovery/git/PhabricatorRepositoryGitCommitDiscoveryDaemon.php index 359443b568..9f59c2550c 100644 --- a/src/applications/repository/daemon/commitdiscovery/git/PhabricatorRepositoryGitCommitDiscoveryDaemon.php +++ b/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) { diff --git a/src/applications/repository/daemon/commitdiscovery/git/__init__.php b/src/applications/repository/daemon/commitdiscovery/git/__init__.php index 5900f05893..f06c780f94 100644 --- a/src/applications/repository/daemon/commitdiscovery/git/__init__.php +++ b/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'); diff --git a/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php b/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php index 1fabb8c06f..881eb4a560 100644 --- a/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php +++ b/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, '/')); }