From c3c4f6ed4c8d68430003319b636b60af416a5fc3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 30 Nov 2011 09:15:37 -0800 Subject: [PATCH] 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 --- src/repository/api/git/ArcanistGitAPI.php | 46 ++++++++++++++++++---- src/workflow/diff/ArcanistDiffWorkflow.php | 28 +++++++------ 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/repository/api/git/ArcanistGitAPI.php b/src/repository/api/git/ArcanistGitAPI.php index 6df4e49c..ee58e80f 100644 --- a/src/repository/api/git/ArcanistGitAPI.php +++ b/src/repository/api/git/ArcanistGitAPI.php @@ -25,6 +25,7 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI { private $status; private $relativeCommit = null; + private $repositoryHasNoCommits = false; const SEARCH_LENGTH_FOR_PARENT_REVISIONS = 16; /** @@ -41,17 +42,33 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI { return 'git'; } + public function getHasCommits() { + return !$this->repositoryHasNoCommits; + } + public function setRelativeCommit($relative_commit) { $this->relativeCommit = $relative_commit; return $this; } 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( - '(cd %s && git log %s..%s --format=%s --)', + '(cd %s && git log %s --format=%s --)', $this->getPath(), - $this->getRelativeCommit(), - 'HEAD', + $against, '%H%x00%T%x00%P%x00%at%x00%an%x00%s'); $commits = array(); @@ -81,7 +98,14 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI { '(cd %s; git rev-parse --verify HEAD^)', $this->getPath()); 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; + } else { $this->relativeCommit = 'HEAD^'; } @@ -159,11 +183,16 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI { public function getGitCommitLog() { $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( '(cd %s; git log --format=medium HEAD)', $this->getPath()); } else { + // 2..N commits. list($stdout) = execx( '(cd %s; git log --first-parent --format=medium %s..HEAD)', $this->getPath(), @@ -221,8 +250,11 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI { // Find uncommitted changes. $uncommitted_future = new ExecFuture( - "(cd %s; git diff {$options} --raw HEAD --)", - $this->getPath()); + "(cd %s; git diff {$options} --raw %s --)", + $this->getPath(), + $this->repositoryHasNoCommits + ? self::GIT_MAGIC_ROOT_COMMIT + : 'HEAD'); // Untracked files $untracked_future = new ExecFuture( @@ -533,7 +565,7 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI { $base); if ($err) { 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)); diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index fc573492..63ac69ec 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -678,23 +678,12 @@ EOTEXT $repository_api, $paths); } else if ($repository_api instanceof ArcanistGitAPI) { - $diff = $repository_api->getFullGitDiff(); if (!strlen($diff)) { - list($base, $tip) = $repository_api->getCommitRange(); - if ($tip == 'HEAD') { - 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})"); + throw new ArcanistUsageException( + "No changes found. (Did you specify the wrong commit range?)"); } $changes = $parser->parseDiff($diff); - } else if ($repository_api instanceof ArcanistMercurialAPI) { $diff = $repository_api->getFullMercurialDiff(); $changes = $parser->parseDiff($diff); @@ -937,6 +926,19 @@ EOTEXT $parser = new ArcanistDiffParser(); $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); $problems = array();