1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2025-04-10 03:18:40 +02:00

Use execxLocal() for ArcanistGitAPI

Summary:
  - This is simpler and reuses more code than doing "(cd %s && ...)" every time.
  - If we have an issue like HGPLAIN, we have one place to fix it now.
  - On Windows, the construct "(cd %s && ...)" does not mean what we'd like it to.

Test Plan: Ran "arc diff" in a git repo with this change.

Reviewers: btrahan, Makinde

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1761
This commit is contained in:
epriestley 2012-03-02 16:47:34 -08:00
parent 77262c0cff
commit 075c4f84d4
5 changed files with 98 additions and 102 deletions

View file

@ -52,7 +52,7 @@ final class BranchInfo {
$name_sha1_map = mpull($branches, 'getSha1', 'getName'); $name_sha1_map = mpull($branches, 'getSha1', 'getName');
$commits_list = $api->multigetCommitMessages( $commits_list = $api->multigetCommitMessages(
array_unique(array_values($name_sha1_map)), array_unique(array_values($name_sha1_map)),
"%%ct%%n%%an%%n%%s%%n%%b"); //don't ask "%ct%n%an%n%s%n%b");
foreach ($branches as $branch) { foreach ($branches as $branch) {
$sha1 = $name_sha1_map[$branch->getName()]; $sha1 = $name_sha1_map[$branch->getName()];
$branch->setSha1($sha1); $branch->setSha1($sha1);

View file

@ -180,4 +180,16 @@ abstract class ArcanistRepositoryAPI {
throw new ArcanistCapabilityNotSupportedException($this); throw new ArcanistCapabilityNotSupportedException($this);
} }
public function execxLocal($pattern /*, ... */) {
$args = func_get_args();
return $this->buildLocalFuture($args)->resolvex();
}
public function execManualLocal($pattern /*, ... */) {
$args = func_get_args();
return $this->buildLocalFuture($args)->resolve();
}
abstract protected function buildLocalFuture(array $argv);
} }

View file

@ -38,6 +38,16 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return new ArcanistGitAPI($root); return new ArcanistGitAPI($root);
} }
protected function buildLocalFuture(array $argv) {
$argv[0] = 'git '.$argv[0];
$future = newv('ExecFuture', $argv);
$future->setCWD($this->getPath());
return $future;
}
public function getSourceControlSystemName() { public function getSourceControlSystemName() {
return 'git'; return 'git';
} }
@ -65,9 +75,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
$against = $this->getRelativeCommit().'..HEAD'; $against = $this->getRelativeCommit().'..HEAD';
} }
list($info) = execx( list($info) = $this->execxLocal(
'(cd %s && git log %s --format=%s --)', 'log %s --format=%s --',
$this->getPath(),
$against, $against,
'%H%x00%T%x00%P%x00%at%x00%an%x00%s'); '%H%x00%T%x00%P%x00%at%x00%an%x00%s');
@ -94,13 +103,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
public function getRelativeCommit() { public function getRelativeCommit() {
if ($this->relativeCommit === null) { if ($this->relativeCommit === null) {
list($err) = exec_manual( list($err) = $this->execManualLocal('rev-parse --verify HEAD^');
'(cd %s; git rev-parse --verify HEAD^)',
$this->getPath());
if ($err) { if ($err) {
list($err) = exec_manual( list($err) = $this->execManualLocal('rev-parse --verify HEAD');
'(cd %s; git rev-parse --verify HEAD)',
$this->getPath());
if ($err) { if ($err) {
$this->repositoryHasNoCommits = true; $this->repositoryHasNoCommits = true;
} }
@ -142,18 +147,16 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
public function getFullGitDiff() { public function getFullGitDiff() {
$options = $this->getDiffFullOptions(); $options = $this->getDiffFullOptions();
list($stdout) = execx( list($stdout) = $this->execxLocal(
"(cd %s; git diff {$options} %s --)", "diff {$options} %s --",
$this->getPath(),
$this->getRelativeCommit()); $this->getRelativeCommit());
return $stdout; return $stdout;
} }
public function getRawDiffText($path) { public function getRawDiffText($path) {
$options = $this->getDiffFullOptions(); $options = $this->getDiffFullOptions();
list($stdout) = execx( list($stdout) = $this->execxLocal(
"(cd %s; git diff {$options} %s -- %s)", "diff {$options} %s -- %s",
$this->getPath(),
$this->getRelativeCommit(), $this->getRelativeCommit(),
$path); $path);
return $stdout; return $stdout;
@ -165,9 +168,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
// $ git rev-parse --abbrev-ref `git symbolic-ref HEAD` // $ git rev-parse --abbrev-ref `git symbolic-ref HEAD`
// //
// But that may fail if you're not on a branch. // But that may fail if you're not on a branch.
list($stdout) = execx( list($stdout) = $this->execxLocal('branch');
'(cd %s; git branch)',
$this->getPath());
$matches = null; $matches = null;
if (preg_match('/^\* (.+)$/m', $stdout, $matches)) { if (preg_match('/^\* (.+)$/m', $stdout, $matches)) {
@ -188,32 +189,27 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return ''; return '';
} else if ($relative == self::GIT_MAGIC_ROOT_COMMIT) { } else if ($relative == self::GIT_MAGIC_ROOT_COMMIT) {
// First commit. // First commit.
list($stdout) = execx( list($stdout) = $this->execxLocal('log --format=medium HEAD');
'(cd %s; git log --format=medium HEAD)',
$this->getPath());
} else { } else {
// 2..N commits. // 2..N commits.
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s; git log --first-parent --format=medium %s..HEAD)', 'log --first-parent --format=medium %s..HEAD',
$this->getPath(),
$this->getRelativeCommit()); $this->getRelativeCommit());
} }
return $stdout; return $stdout;
} }
public function getGitHistoryLog() { public function getGitHistoryLog() {
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s; git log --format=medium -n%d %s)', 'log --format=medium -n%d %s',
$this->getPath(),
self::SEARCH_LENGTH_FOR_PARENT_REVISIONS, self::SEARCH_LENGTH_FOR_PARENT_REVISIONS,
$this->getRelativeCommit()); $this->getRelativeCommit());
return $stdout; return $stdout;
} }
public function getSourceControlBaseRevision() { public function getSourceControlBaseRevision() {
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s; git rev-parse %s)', 'rev-parse %s',
$this->getPath(),
$this->getRelativeCommit()); $this->getRelativeCommit());
return rtrim($stdout, "\n"); return rtrim($stdout, "\n");
} }
@ -228,9 +224,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
} else { } else {
$flags = ''; $flags = '';
} }
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s; git rev-parse %s HEAD)', 'rev-parse %s HEAD',
$this->getPath(),
$flags); $flags);
return rtrim($stdout, "\n"); return rtrim($stdout, "\n");
} }
@ -243,31 +238,35 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
// -- parallelize these slow cpu bound git calls. // -- parallelize these slow cpu bound git calls.
// Find committed changes. // Find committed changes.
$committed_future = new ExecFuture( $committed_future = $this->buildLocalFuture(
"(cd %s; git diff {$options} --raw %s --)", array(
$this->getPath(), "diff {$options} --raw %s --",
$this->getRelativeCommit()); $this->getRelativeCommit(),
));
// Find uncommitted changes. // Find uncommitted changes.
$uncommitted_future = new ExecFuture( $uncommitted_future = $this->buildLocalFuture(
"(cd %s; git diff {$options} --raw %s --)", array(
$this->getPath(), "diff {$options} --raw %s --",
$this->repositoryHasNoCommits $this->repositoryHasNoCommits
? self::GIT_MAGIC_ROOT_COMMIT ? self::GIT_MAGIC_ROOT_COMMIT
: 'HEAD'); : 'HEAD',
));
// Untracked files // Untracked files
$untracked_future = new ExecFuture( $untracked_future = $this->buildLocalFuture(
'(cd %s; git ls-files --others --exclude-standard)', array(
$this->getPath()); 'ls-files --others --exclude-standard',
));
// TODO: This doesn't list unstaged adds. It's not clear how to get that // TODO: This doesn't list unstaged adds. It's not clear how to get that
// list other than "git status --porcelain" and then parsing it. :/ // list other than "git status --porcelain" and then parsing it. :/
// Unstaged changes // Unstaged changes
$unstaged_future = new ExecFuture( $unstaged_future = $this->buildLocalFuture(
'(cd %s; git ls-files -m)', array(
$this->getPath()); 'ls-files -m',
));
$futures = array( $futures = array(
$committed_future, $committed_future,
@ -320,17 +319,15 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
} }
public function amendGitHeadCommit($message) { public function amendGitHeadCommit($message) {
execx( $this->execxLocal(
'(cd %s; git commit --amend --allow-empty --message %s)', 'commit --amend --allow-empty --message %s',
$this->getPath(),
$message); $message);
} }
public function getPreReceiveHookStatus($old_ref, $new_ref) { public function getPreReceiveHookStatus($old_ref, $new_ref) {
$options = $this->getDiffBaseOptions(); $options = $this->getDiffBaseOptions();
list($stdout) = execx( list($stdout) = $this->execxLocal(
"(cd %s && git diff {$options} --raw %s %s --)", "diff {$options} --raw %s %s --",
$this->getPath(),
$old_ref, $old_ref,
$new_ref); $new_ref);
return $this->parseGitStatus($stdout, $full = true); return $this->parseGitStatus($stdout, $full = true);
@ -376,9 +373,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
public function getBlame($path) { public function getBlame($path) {
// TODO: 'git blame' supports --porcelain and we should probably use it. // TODO: 'git blame' supports --porcelain and we should probably use it.
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s; git blame --date=iso -w -M %s -- %s)', 'blame --date=iso -w -M %s -- %s',
$this->getPath(),
$this->getRelativeCommit(), $this->getRelativeCommit(),
$path); $path);
@ -454,9 +450,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
// and treat it as though it as a file containing a list of other files, // and treat it as though it as a file containing a list of other files,
// which is silly. // which is silly.
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s && git ls-tree %s -- %s)', 'ls-tree %s -- %s',
$this->getPath(),
$revision, $revision,
$path); $path);
@ -472,9 +467,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return null; return null;
} }
list($stdout) = execx( list($stdout) = $this->execxLocal(
'(cd %s && git cat-file blob %s)', 'cat-file blob %s',
$this->getPath(),
$info[$path]['ref']); $info[$path]['ref']);
return $stdout; return $stdout;
} }
@ -485,8 +479,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
* @return array where each element is a triple ('name', 'sha1', 'current') * @return array where each element is a triple ('name', 'sha1', 'current')
*/ */
public function getAllBranches() { public function getAllBranches() {
list($branch_info) = execx( list($branch_info) = $this->execxLocal('branch --no-color');
'cd %s && git branch --no-color', $this->getPath());
$lines = explode("\n", trim($branch_info)); $lines = explode("\n", trim($branch_info));
$result = array(); $result = array();
foreach ($lines as $line) { foreach ($lines as $line) {
@ -506,10 +499,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
$all_names = ipull($result, 'name'); $all_names = ipull($result, 'name');
// Calling 'git branch' first and then 'git rev-parse' is way faster than // Calling 'git branch' first and then 'git rev-parse' is way faster than
// 'git branch -v' for some reason. // 'git branch -v' for some reason.
list($sha1s_string) = execx( list($sha1s_string) = $this->execxLocal('rev-parse %Ls', $all_names);
"cd %s && git rev-parse %Ls",
$this->path,
$all_names);
$sha1_map = array_combine($all_names, explode("\n", trim($sha1s_string))); $sha1_map = array_combine($all_names, explode("\n", trim($sha1s_string)));
foreach ($result as &$branch) { foreach ($result as &$branch) {
$branch['sha1'] = $sha1_map[$branch['name']]; $branch['sha1'] = $sha1_map[$branch['name']];
@ -525,30 +516,25 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
* @param string $format the format to show messages in * @param string $format the format to show messages in
*/ */
public function multigetCommitMessages($revs, $format) { public function multigetCommitMessages($revs, $format) {
$delimiter = "%%x00";
$revs_list = implode(' ', $revs); list($commits_string) = $this->execxLocal(
$show_command = "show -s --pretty='format:'%s%s %Ls",
"git show -s --pretty=\"format:$format$delimiter\" $revs_list"; $format,
list($commits_string) = execx( '%x00',
"cd %s && $show_command", $revs);
$this->getPath());
$commits_list = array_slice(explode("\0", $commits_string), 0, -1); $commits_list = array_slice(explode("\0", $commits_string), 0, -1);
$commits_list = array_combine($revs, $commits_list); $commits_list = array_combine($revs, $commits_list);
return $commits_list; return $commits_list;
} }
public function getRepositoryOwner() { public function getRepositoryOwner() {
list($owner) = execx( list($owner) = $this->execxLocal('config --get user.name');
'cd %s && git config --get user.name',
$this->getPath());
return trim($owner); return trim($owner);
} }
public function getWorkingCopyRevision() { public function getWorkingCopyRevision() {
list($stdout) = execx( list($stdout) = $this->execxLocal('rev-parse HEAD');
'(cd %s; git rev-parse %s)',
$this->getPath(),
'HEAD');
return rtrim($stdout, "\n"); return rtrim($stdout, "\n");
} }
@ -567,9 +553,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
if ($base == ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT) { if ($base == ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT) {
$merge_base = $base; $merge_base = $base;
} else { } else {
list($err, $merge_base) = exec_manual( list($err, $merge_base) = $this->execManualLocal(
'(cd %s; git merge-base %s HEAD)', 'merge-base %s HEAD',
$this->getPath(),
$base); $base);
if ($err) { if ($err) {
throw new ArcanistUsageException( throw new ArcanistUsageException(
@ -614,9 +599,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
} }
public function getCommitMessageForRevision($rev) { public function getCommitMessageForRevision($rev) {
list($message) = execx( list($message) = $this->execxLocal(
'(cd %s && git log -n1 %s)', 'log -n1 %s',
$this->getPath(),
$rev); $rev);
$parser = new ArcanistDiffParser(); $parser = new ArcanistDiffParser();
return head($parser->parseDiff($message)); return head($parser->parseDiff($message));

View file

@ -29,17 +29,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
private $workingCopyRevision; private $workingCopyRevision;
private $localCommitInfo; private $localCommitInfo;
public function execxLocal($pattern /*, ... */) { protected function buildLocalFuture(array $argv) {
$args = func_get_args();
return $this->buildLocalFuture($args)->resolvex();
}
public function execManualLocal($pattern /*, ... */) {
$args = func_get_args();
return $this->buildLocalFuture($args)->resolve();
}
private function buildLocalFuture(array $argv) {
// Mercurial has a "defaults" feature which basically breaks automation by // Mercurial has a "defaults" feature which basically breaks automation by
// allowing the user to add random flags to any command. This feature is // allowing the user to add random flags to any command. This feature is

View file

@ -36,6 +36,16 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
return 'svn'; return 'svn';
} }
protected function buildLocalFuture(array $argv) {
$argv[0] = 'svn '.$argv[0];
$future = newv('ExecFuture', $argv);
$future->setCWD($this->getPath());
return $future;
}
public function hasMergeConflicts() { public function hasMergeConflicts() {
foreach ($this->getSVNStatus() as $path => $mask) { foreach ($this->getSVNStatus() as $path => $mask) {
if ($mask & self::FLAG_CONFLICT) { if ($mask & self::FLAG_CONFLICT) {