From 58c09ab36da6f9028a69350916b4c3bd9384c6dc Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 9 Aug 2011 18:54:10 -0700 Subject: [PATCH] Improve Arcanist Mercurial support Summary: - Build the manifest of file changes so unit and lint workflows work. - Default to creating a diff between the parent of the first outgoing change and the tip. Test Plan: - Ran "arc diff" in a dirty mercurial repo, got warned about untracked/uncommitted changes. - Ran "arc diff" in a clean mercurial repo, got a diff of everything I'd done locally. Reviewed By: aran Reviewers: Makinde, aran, jungejason, tuomaspelkonen CC: aran, epriestley Differential Revision: 796 --- .../api/mercurial/ArcanistMercurialAPI.php | 163 +++++++++++++++++- src/repository/api/mercurial/__init__.php | 4 + src/workflow/base/ArcanistBaseWorkflow.php | 4 + 3 files changed, 167 insertions(+), 4 deletions(-) diff --git a/src/repository/api/mercurial/ArcanistMercurialAPI.php b/src/repository/api/mercurial/ArcanistMercurialAPI.php index 16bcdffa..0571cced 100644 --- a/src/repository/api/mercurial/ArcanistMercurialAPI.php +++ b/src/repository/api/mercurial/ArcanistMercurialAPI.php @@ -25,6 +25,7 @@ class ArcanistMercurialAPI extends ArcanistRepositoryAPI { private $status; private $base; + private $relativeCommit; public function getSourceControlSystemName() { return 'hg'; @@ -50,9 +51,35 @@ class ArcanistMercurialAPI extends ArcanistRepositoryAPI { return $stdout; } + public function setRelativeCommit($commit) { + list($err) = exec_manual( + '(cd %s && hg id -ir %s)', + $this->getPath(), + $commit); + + if ($err) { + throw new ArcanistUsageException( + "Commit '{$commit}' is not a valid Mercurial commit identifier."); + } + + $this->relativeCommit = $commit; + return $this; + } + public function getRelativeCommit() { - // TODO: This is hardcoded. - return 'tip~1'; + if (empty($this->relativeCommit)) { + list($stdout) = execx( + '(cd %s && hg outgoing --limit 1)', + $this->getPath()); + $logs = $this->parseMercurialLog($stdout); + if (!count($logs)) { + throw new ArcanistUsageException("You have no outgoing changes!"); + } + $oldest_log = head($logs); + + $this->relativeCommit = $oldest_log['rev'].'~1'; + } + return $this->relativeCommit; } public function getBlame($path) { @@ -85,9 +112,52 @@ class ArcanistMercurialAPI extends ArcanistRepositoryAPI { public function getWorkingCopyStatus() { - // TODO: This is critical and not yet implemented. + // A reviewable revision spans multiple local commits in Mercurial, but + // there is no way to get file change status across multiple commits, so + // just take the entire diff and parse it to figure out what's changed. - return array(); + $diff = $this->getFullMercurialDiff(); + $parser = new ArcanistDiffParser(); + $changes = $parser->parseDiff($diff); + + $status_map = array(); + + foreach ($changes as $change) { + $flags = 0; + switch ($change->getType()) { + case ArcanistDiffChangeType::TYPE_ADD: + case ArcanistDiffChangeType::TYPE_MOVE_HERE: + case ArcanistDiffChangeType::TYPE_COPY_HERE: + $flags |= self::FLAG_ADDED; + break; + case ArcanistDiffChangeType::TYPE_CHANGE: + case ArcanistDiffChangeType::TYPE_COPY_AWAY: // Check for changes? + $flags |= self::FLAG_MODIFIED; + break; + case ArcanistDiffChangeType::TYPE_DELETE: + case ArcanistDiffChangeType::TYPE_MOVE_AWAY: + case ArcanistDiffChangeType::TYPE_MULTICOPY: + $flags |= self::FLAG_DELETED; + break; + } + $status_map[$change->getCurrentPath()] = $flags; + } + + list($stdout) = execx( + '(cd %s && hg status)', + $this->getPath()); + + $working_status = $this->parseMercurialStatus($stdout); + foreach ($working_status as $path => $status) { + $status |= self::FLAG_UNCOMMITTED; + if (!empty($status_map[$path])) { + $status_map[$path] |= $status; + } else { + $status_map[$path] = $status; + } + } + + return $status_map; } private function getDiffOptions() { @@ -139,4 +209,89 @@ class ArcanistMercurialAPI extends ArcanistRepositoryAPI { return $stdout; } + private function parseMercurialStatus($status) { + $result = array(); + + $status = trim($status); + if (!strlen($status)) { + return $result; + } + + $lines = explode("\n", $status); + foreach ($lines as $line) { + $flags = 0; + list($code, $path) = explode(' ', $line, 2); + switch ($code) { + case 'A': + $flags |= self::FLAG_ADDED; + break; + case 'R': + $flags |= self::FLAG_REMOVED; + break; + case 'M': + $flags |= self::FLAG_MODIFIED; + break; + case 'C': + // This is "clean" and included only for completeness, these files + // have not been changed. + break; + case '!': + $flags |= self::FLAG_MISSING; + break; + case '?': + $flags |= self::FLAG_UNTRACKED; + break; + case 'I': + // This is "ignored" and included only for completeness. + break; + default: + throw new Exception("Unknown Mercurial status '{$code}'."); + } + + $result[$path] = $flags; + } + + return $result; + } + + private function parseMercurialLog($log) { + $result = array(); + + $chunks = explode("\n\n", trim($log)); + foreach ($chunks as $chunk) { + $commit = array(); + $lines = explode("\n", $chunk); + foreach ($lines as $line) { + if (preg_match('/^(comparing with|searching for changes)/', $line)) { + // These are sent to stdout when you run "hg outgoing" although the + // format is otherwise identical to "hg log". + continue; + } + list($name, $value) = explode(':', $line, 2); + $value = trim($value); + switch ($name) { + case 'user': + $commit['user'] = $value; + break; + case 'date': + $commit['date'] = strtotime($value); + break; + case 'summary': + $commit['summary'] = $value; + break; + case 'changeset': + list($local, $rev) = explode(':', $value, 2); + $commit['local'] = $local; + $commit['rev'] = $rev; + break; + default: + throw new Exception("Unknown Mercurial log field '{$name}'!"); + } + } + $result[] = $commit; + } + + return $result; + } + } diff --git a/src/repository/api/mercurial/__init__.php b/src/repository/api/mercurial/__init__.php index 480bf00d..f4ae19bc 100644 --- a/src/repository/api/mercurial/__init__.php +++ b/src/repository/api/mercurial/__init__.php @@ -6,9 +6,13 @@ +phutil_require_module('arcanist', 'exception/usage'); +phutil_require_module('arcanist', 'parser/diff'); +phutil_require_module('arcanist', 'parser/diff/changetype'); phutil_require_module('arcanist', 'repository/api/base'); phutil_require_module('phutil', 'future/exec'); +phutil_require_module('phutil', 'utils'); phutil_require_source('ArcanistMercurialAPI.php'); diff --git a/src/workflow/base/ArcanistBaseWorkflow.php b/src/workflow/base/ArcanistBaseWorkflow.php index 4eb876e6..0d622ac4 100644 --- a/src/workflow/base/ArcanistBaseWorkflow.php +++ b/src/workflow/base/ArcanistBaseWorkflow.php @@ -598,6 +598,10 @@ class ArcanistBaseWorkflow { echo phutil_console_wrap( "Since you don't have 'svn:ignore' rules for these files, you may ". "have forgotten to 'svn add' them."); + } else if ($api instanceof ArcanistMercurialAPI) { + echo phutil_console_wrap( + "Since you don't have '.hgignore' rules for these files, you ". + "may have forgotten to 'hg add' them to your commit."); } $prompt = "Do you want to continue without adding these files?";