mirror of
https://we.phorge.it/source/arcanist.git
synced 2025-01-11 15:21:03 +01:00
Improve git behavior in the zero- and one- commit case
Summary: Git works completely differently for commits zero and one than for 2..N so add more special casing to handle them. See: - {T206} - {T596} The getCommitRange() block is also fatal land, although I wasn't able to reach it. I'll follow up with @s on T596. Test Plan: - Created a new, empty repository ("mkdir x; cd x; git init"). - Ran "arc lint", "arc unit", "arc diff" against it with no commits (the first two work, the third fails helpfully). - Made an initial commit. - Ran "arc lint", "arc unit", "arc diff" against it (all work correctly). Reviewers: btrahan, jungejason, aran Reviewed By: aran CC: s, aran Differential Revision: 1142
This commit is contained in:
parent
3629308f2f
commit
c3c4f6ed4c
2 changed files with 54 additions and 20 deletions
|
@ -25,6 +25,7 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
|
|
||||||
private $status;
|
private $status;
|
||||||
private $relativeCommit = null;
|
private $relativeCommit = null;
|
||||||
|
private $repositoryHasNoCommits = false;
|
||||||
const SEARCH_LENGTH_FOR_PARENT_REVISIONS = 16;
|
const SEARCH_LENGTH_FOR_PARENT_REVISIONS = 16;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -41,17 +42,33 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
return 'git';
|
return 'git';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getHasCommits() {
|
||||||
|
return !$this->repositoryHasNoCommits;
|
||||||
|
}
|
||||||
|
|
||||||
public function setRelativeCommit($relative_commit) {
|
public function setRelativeCommit($relative_commit) {
|
||||||
$this->relativeCommit = $relative_commit;
|
$this->relativeCommit = $relative_commit;
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getLocalCommitInformation() {
|
public function getLocalCommitInformation() {
|
||||||
|
if ($this->repositoryHasNoCommits) {
|
||||||
|
// Zero commits.
|
||||||
|
throw new Exception(
|
||||||
|
"You can't get local commit information for a repository with no ".
|
||||||
|
"commits.");
|
||||||
|
} else if ($this->relativeCommit == self::GIT_MAGIC_ROOT_COMMIT) {
|
||||||
|
// One commit.
|
||||||
|
$against = 'HEAD';
|
||||||
|
} else {
|
||||||
|
// 2..N commits.
|
||||||
|
$against = $this->getRelativeCommit().'..HEAD';
|
||||||
|
}
|
||||||
|
|
||||||
list($info) = execx(
|
list($info) = execx(
|
||||||
'(cd %s && git log %s..%s --format=%s --)',
|
'(cd %s && git log %s --format=%s --)',
|
||||||
$this->getPath(),
|
$this->getPath(),
|
||||||
$this->getRelativeCommit(),
|
$against,
|
||||||
'HEAD',
|
|
||||||
'%H%x00%T%x00%P%x00%at%x00%an%x00%s');
|
'%H%x00%T%x00%P%x00%at%x00%an%x00%s');
|
||||||
|
|
||||||
$commits = array();
|
$commits = array();
|
||||||
|
@ -81,7 +98,14 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
'(cd %s; git rev-parse --verify HEAD^)',
|
'(cd %s; git rev-parse --verify HEAD^)',
|
||||||
$this->getPath());
|
$this->getPath());
|
||||||
if ($err) {
|
if ($err) {
|
||||||
|
list($err) = exec_manual(
|
||||||
|
'(cd %s; git rev-parse --verify HEAD)',
|
||||||
|
$this->getPath());
|
||||||
|
if ($err) {
|
||||||
|
$this->repositoryHasNoCommits = true;
|
||||||
|
}
|
||||||
$this->relativeCommit = self::GIT_MAGIC_ROOT_COMMIT;
|
$this->relativeCommit = self::GIT_MAGIC_ROOT_COMMIT;
|
||||||
|
|
||||||
} else {
|
} else {
|
||||||
$this->relativeCommit = 'HEAD^';
|
$this->relativeCommit = 'HEAD^';
|
||||||
}
|
}
|
||||||
|
@ -159,11 +183,16 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
|
|
||||||
public function getGitCommitLog() {
|
public function getGitCommitLog() {
|
||||||
$relative = $this->getRelativeCommit();
|
$relative = $this->getRelativeCommit();
|
||||||
if ($relative == self::GIT_MAGIC_ROOT_COMMIT) {
|
if ($this->repositoryHasNoCommits) {
|
||||||
|
// No commits yet.
|
||||||
|
return '';
|
||||||
|
} else if ($relative == self::GIT_MAGIC_ROOT_COMMIT) {
|
||||||
|
// First commit.
|
||||||
list($stdout) = execx(
|
list($stdout) = execx(
|
||||||
'(cd %s; git log --format=medium HEAD)',
|
'(cd %s; git log --format=medium HEAD)',
|
||||||
$this->getPath());
|
$this->getPath());
|
||||||
} else {
|
} else {
|
||||||
|
// 2..N commits.
|
||||||
list($stdout) = execx(
|
list($stdout) = execx(
|
||||||
'(cd %s; git log --first-parent --format=medium %s..HEAD)',
|
'(cd %s; git log --first-parent --format=medium %s..HEAD)',
|
||||||
$this->getPath(),
|
$this->getPath(),
|
||||||
|
@ -221,8 +250,11 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
|
|
||||||
// Find uncommitted changes.
|
// Find uncommitted changes.
|
||||||
$uncommitted_future = new ExecFuture(
|
$uncommitted_future = new ExecFuture(
|
||||||
"(cd %s; git diff {$options} --raw HEAD --)",
|
"(cd %s; git diff {$options} --raw %s --)",
|
||||||
$this->getPath());
|
$this->getPath(),
|
||||||
|
$this->repositoryHasNoCommits
|
||||||
|
? self::GIT_MAGIC_ROOT_COMMIT
|
||||||
|
: 'HEAD');
|
||||||
|
|
||||||
// Untracked files
|
// Untracked files
|
||||||
$untracked_future = new ExecFuture(
|
$untracked_future = new ExecFuture(
|
||||||
|
@ -533,7 +565,7 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
$base);
|
$base);
|
||||||
if ($err) {
|
if ($err) {
|
||||||
throw new ArcanistUsageException(
|
throw new ArcanistUsageException(
|
||||||
"Unable to parse git commit name '{$base}'.");
|
"Unable to find any git commit named '{$base}' in this repository.");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
$this->setRelativeCommit(trim($merge_base));
|
$this->setRelativeCommit(trim($merge_base));
|
||||||
|
|
|
@ -678,23 +678,12 @@ EOTEXT
|
||||||
$repository_api,
|
$repository_api,
|
||||||
$paths);
|
$paths);
|
||||||
} else if ($repository_api instanceof ArcanistGitAPI) {
|
} else if ($repository_api instanceof ArcanistGitAPI) {
|
||||||
|
|
||||||
$diff = $repository_api->getFullGitDiff();
|
$diff = $repository_api->getFullGitDiff();
|
||||||
if (!strlen($diff)) {
|
if (!strlen($diff)) {
|
||||||
list($base, $tip) = $repository_api->getCommitRange();
|
throw new ArcanistUsageException(
|
||||||
if ($tip == 'HEAD') {
|
"No changes found. (Did you specify the wrong commit range?)");
|
||||||
if (preg_match('/\^+HEAD/', $base)) {
|
|
||||||
$more = 'Did you mean HEAD^ instead of ^HEAD?';
|
|
||||||
} else {
|
|
||||||
$more = 'Did you specify the wrong relative commit?';
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
$more = 'Did you specify the wrong commit range?';
|
|
||||||
}
|
|
||||||
throw new ArcanistUsageException("No changes found. ({$more})");
|
|
||||||
}
|
}
|
||||||
$changes = $parser->parseDiff($diff);
|
$changes = $parser->parseDiff($diff);
|
||||||
|
|
||||||
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
||||||
$diff = $repository_api->getFullMercurialDiff();
|
$diff = $repository_api->getFullMercurialDiff();
|
||||||
$changes = $parser->parseDiff($diff);
|
$changes = $parser->parseDiff($diff);
|
||||||
|
@ -937,6 +926,19 @@ EOTEXT
|
||||||
|
|
||||||
$parser = new ArcanistDiffParser();
|
$parser = new ArcanistDiffParser();
|
||||||
$commit_messages = $repository_api->getGitCommitLog();
|
$commit_messages = $repository_api->getGitCommitLog();
|
||||||
|
|
||||||
|
if (!strlen($commit_messages)) {
|
||||||
|
if (!$repository_api->getHasCommits()) {
|
||||||
|
throw new ArcanistUsageException(
|
||||||
|
"This git repository doesn't have any commits yet. You need to ".
|
||||||
|
"commit something before you can diff against it.");
|
||||||
|
} else {
|
||||||
|
throw new ArcanistUsageException(
|
||||||
|
"The commit range doesn't include any commits. (Did you diff ".
|
||||||
|
"against the wrong commit?)");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$commit_messages = $parser->parseDiff($commit_messages);
|
$commit_messages = $parser->parseDiff($commit_messages);
|
||||||
|
|
||||||
$problems = array();
|
$problems = array();
|
||||||
|
|
Loading…
Reference in a new issue