1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 14:52:40 +01:00

Minimize reliance on 'git branch' output format

Summary:
Ref T5554. Both the current branch name (if on a branch), as well as the
list of all local branches, can be retrieved without having to parse the
output from "git branch".

Unfortunately, there seems to be no git plumbing for "get list of
branches containing this commit" yet.
(see http://marc.info/?l=git&m=141408477614635&w=2)

For that case, this commit whitelists the output from "git branch" using
the known valid branch names from "git for-each-ref".

Test Plan:
Set up a test repo with this structure:
```
|   *  Commit B1, on branch "subfeature"
|  /
| *    Commit A1, on branch "feature"
|/
*      Commit M1, on branch "master"
|
```

In `subfeature`, I tried:
* `arc which --base 'git:branch-unique(master)'`
* `arc feature`
After that, I detached my HEAD (don't worry, I got better) and tried again.

Nothing looked broken.

(Tested with git 1.7.2.5 and 2.5.0.)

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T5554

Differential Revision: https://secure.phabricator.com/D13989
This commit is contained in:
Javier Arteaga 2015-08-24 17:57:41 -07:00 committed by epriestley
parent 6ecb3fb87d
commit 46009145f7

View file

@ -482,22 +482,37 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return $stdout; return $stdout;
} }
public function getBranchName() { private function getBranchNameFromRef($ref) {
// TODO: consider: $count = 0;
// $branch = preg_replace('/^refs\/heads\//', '', $ref, 1, $count);
// $ git rev-parse --abbrev-ref `git symbolic-ref HEAD` if ($count !== 1) {
// return null;
// But that may fail if you're not on a branch.
list($stdout) = $this->execxLocal('branch --no-color');
// Assume that any branch beginning with '(' means 'no branch', or whatever
// 'no branch' is in the current locale.
$matches = null;
if (preg_match('/^\* ([^\(].*)$/m', $stdout, $matches)) {
return $matches[1];
} }
return null; return $branch;
}
public function getBranchName() {
list($err, $stdout, $stderr) = $this->execManualLocal(
'symbolic-ref --quiet HEAD');
if ($err === 0) {
// We expect the branch name to come qualified with a refs/heads/ prefix.
// Verify this, and strip it.
$ref = rtrim($stdout);
$branch = $this->getBranchNameFromRef($ref);
if (!$branch) {
throw new Exception(
pht('Failed to parse %s output!', 'git symbolic-ref'));
}
return $branch;
} else if ($err === 1) {
// Exit status 1 with --quiet indicates that HEAD is detached.
return null;
} else {
throw new Exception(
pht('Command %s failed: %s', 'git symbolic-ref', $stderr));
}
} }
public function getRemoteURI() { public function getRemoteURI() {
@ -886,24 +901,21 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
* @return list<dict<string, string>> Dictionary of branch information. * @return list<dict<string, string>> Dictionary of branch information.
*/ */
public function getAllBranches() { public function getAllBranches() {
list($branch_info) = $this->execxLocal( list($ref_list) = $this->execxLocal(
'branch --no-color'); 'for-each-ref --format=%s refs/heads',
$lines = explode("\n", rtrim($branch_info)); '%(refname)');
$refs = explode("\n", rtrim($ref_list));
$current = $this->getBranchName();
$result = array(); $result = array();
foreach ($lines as $line) { foreach ($refs as $ref) {
$branch = $this->getBranchNameFromRef($ref);
if (preg_match('@^[* ]+\(no branch|detached from \w+/\w+\)@', $line)) { if ($branch) {
// This is indicating that the working copy is in a detached state; $result[] = array(
// just ignore it. 'current' => ($branch === $current),
continue; 'name' => $branch,
);
} }
list($current, $name) = preg_split('/\s+/', $line, 2);
$result[] = array(
'current' => !empty($current),
'name' => $name,
);
} }
return $result; return $result;
@ -1134,11 +1146,28 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
$commits[] = $merge_base; $commits[] = $merge_base;
$head_branch_count = null; $head_branch_count = null;
$all_branch_names = ipull($this->getAllBranches(), 'name');
foreach ($commits as $commit) { foreach ($commits as $commit) {
// Ideally, we would use something like "for-each-ref --contains"
// to get a filtered list of branches ready for script consumption.
// Instead, try to get predictable output from "branch --contains".
list($branches) = $this->execxLocal( list($branches) = $this->execxLocal(
'branch --contains %s', '-c column.ui=never -c color.ui=never branch --contains %s',
$commit); $commit);
$branches = array_filter(explode("\n", $branches)); $branches = array_filter(explode("\n", $branches));
// Filter the list, removing the "current" marker (*) and ignoring
// anything other than known branch names (mainly, any possible
// "detached HEAD" or "no branch" line).
foreach ($branches as $key => $branch) {
$branch = trim($branch, ' *');
if (in_array($branch, $all_branch_names)) {
$branches[$key] = $branch;
} else {
unset($branches[$key]);
}
}
if ($head_branch_count === null) { if ($head_branch_count === null) {
// If this is the first commit, it's HEAD. Count how many // If this is the first commit, it's HEAD. Count how many
// branches it is on; we want to include commits on the same // branches it is on; we want to include commits on the same
@ -1147,9 +1176,6 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
// for whatever reason. // for whatever reason.
$head_branch_count = count($branches); $head_branch_count = count($branches);
} else if (count($branches) > $head_branch_count) { } else if (count($branches) > $head_branch_count) {
foreach ($branches as $key => $branch) {
$branches[$key] = trim($branch, ' *');
}
$branches = implode(', ', $branches); $branches = implode(', ', $branches);
$this->setBaseCommitExplanation( $this->setBaseCommitExplanation(
pht( pht(