From 1ea5e7e9cffeb8226a7bfb2a765692f31b1b39d4 Mon Sep 17 00:00:00 2001 From: Jamie Wong Date: Mon, 4 Jun 2012 15:30:50 -0400 Subject: [PATCH] Support mutable history in mercurial for arc diff and arc amend Summary: This adds basic support for mutable history for arc diff and arc amend for mercurial. This is purely opt-in (so it shouldn't affect anyone who doesn't want this feature) by explicitly setting "immutable_history" : false in arc configuration. This also fixes another instance of weird behaviour for multiple heads - the first instance was fixed here: https://github.com/facebook/arcanist/pull/28 Test Plan: without "immutable_history" turned on)> When ##arc diff## produces an update diff, it should list only commits that are ancestors of the current revision - not ones from other heads. Reviewers: epriestley CC: csilvers, aran, Koolvin Differential Revision: https://secure.phabricator.com/D2654 --- src/repository/api/ArcanistGitAPI.php | 10 +++- src/repository/api/ArcanistMercurialAPI.php | 56 +++++++++++++++++++- src/repository/api/ArcanistRepositoryAPI.php | 6 +++ src/repository/api/ArcanistSubversionAPI.php | 8 +++ src/workflow/ArcanistAmendWorkflow.php | 20 ++++--- src/workflow/ArcanistBaseWorkflow.php | 9 +++- src/workflow/ArcanistDiffWorkflow.php | 11 ++-- 7 files changed, 104 insertions(+), 16 deletions(-) diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php index 70ce4649..2ccbed0b 100644 --- a/src/repository/api/ArcanistGitAPI.php +++ b/src/repository/api/ArcanistGitAPI.php @@ -459,7 +459,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { return $this->status; } - public function amendGitHeadCommit($message) { + public function amendCommit($message) { $tmp_file = new TempFile(); Filesystem::writeFile($tmp_file, $message); $this->execxLocal( @@ -681,6 +681,14 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { return rtrim($stdout, "\n"); } + public function isHistoryDefaultImmutable() { + return false; + } + + public function supportsAmend() { + return true; + } + public function supportsRelativeLocalCommits() { return true; } diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php index 2acb0de1..fa5c74c6 100644 --- a/src/repository/api/ArcanistMercurialAPI.php +++ b/src/repository/api/ArcanistMercurialAPI.php @@ -406,6 +406,19 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { return $this->workingCopyRevision; } + public function isHistoryDefaultImmutable() { + return true; + } + + public function supportsAmend() { + list ($err, $stdout) = $this->execManualLocal('help commit'); + if ($err) { + return false; + } else { + return (strstr($stdout, "amend") !== false); + } + } + public function supportsRelativeLocalCommits() { return true; } @@ -473,9 +486,9 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { public function getCommitMessageLog() { list($stdout) = $this->execxLocal( - "log --template '{node}\\1{desc}\\0' --rev %s..%s --", + "log --template '{node}\\1{desc}\\0' --prune %s --rev %s --", $this->getRelativeCommit(), - $this->getWorkingCopyRevision()); + "ancestors({$this->getWorkingCopyRevision()})"); $map = array(); @@ -492,6 +505,37 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { ConduitClient $conduit, array $query) { + $messages = $this->getCommitMessageLog(); + $parser = new ArcanistDiffParser(); + + // First, try to find revisions by explicit revision IDs in commit messages. + $reason_map = array(); + $revision_ids = array(); + foreach ($messages as $node_id => $message) { + $object = ArcanistDifferentialCommitMessage::newFromRawCorpus($message); + + if ($object->getRevisionID()) { + $revision_ids[] = $object->getRevisionID(); + $reason_map[$object->getRevisionID()] = $node_id; + } + } + + if ($revision_ids) { + $results = $conduit->callMethodSynchronous( + 'differential.query', + $query + array( + 'ids' => $revision_ids, + )); + + foreach ($results as $key => $result) { + $hash = substr($reason_map[$result['id']], 0, 16); + $results[$key]['why'] = + "Commit message for '{$hash}' has explicit 'Differential Revision'."; + } + + return $results; + } + // Try to find revisions by hash. $hashes = array(); foreach ($this->getLocalCommitInformation() as $commit) { @@ -517,6 +561,14 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { $this->execxLocal('up'); } + public function amendCommit($message) { + $tmp_file = new TempFile(); + Filesystem::writeFile($tmp_file, $message); + $this->execxLocal( + 'commit --amend -l %s', + $tmp_file); + } + public function setIncludeDirectoryStateInDiffs($include) { $this->includeDirectoryStateInDiffs = $include; return $this; diff --git a/src/repository/api/ArcanistRepositoryAPI.php b/src/repository/api/ArcanistRepositoryAPI.php index 30f77db7..44feef77 100644 --- a/src/repository/api/ArcanistRepositoryAPI.php +++ b/src/repository/api/ArcanistRepositoryAPI.php @@ -170,6 +170,8 @@ abstract class ArcanistRepositoryAPI { abstract public function getLocalCommitInformation(); abstract public function getSourceControlBaseRevision(); abstract public function getCanonicalRevisionName($string); + abstract public function isHistoryDefaultImmutable(); + abstract public function supportsAmend(); abstract public function supportsRelativeLocalCommits(); abstract public function getWorkingCopyRevision(); abstract public function updateWorkingCopy(); @@ -177,6 +179,10 @@ abstract class ArcanistRepositoryAPI { ConduitClient $conduit, array $query); + public function amendCommit() { + throw new ArcanistCapabilityNotSupportedException($this); + } + public function getAllBranches() { // TODO: Implement for Mercurial/SVN and make abstract. return array(); diff --git a/src/repository/api/ArcanistSubversionAPI.php b/src/repository/api/ArcanistSubversionAPI.php index fa8e15b5..ee27665a 100644 --- a/src/repository/api/ArcanistSubversionAPI.php +++ b/src/repository/api/ArcanistSubversionAPI.php @@ -507,6 +507,14 @@ EODIFF; return null; } + public function isHistoryDefaultImmutable() { + return true; + } + + public function supportsAmend() { + return false; + } + public function supportsRelativeLocalCommits() { return false; } diff --git a/src/workflow/ArcanistAmendWorkflow.php b/src/workflow/ArcanistAmendWorkflow.php index db7e2220..d4eb3972 100644 --- a/src/workflow/ArcanistAmendWorkflow.php +++ b/src/workflow/ArcanistAmendWorkflow.php @@ -32,9 +32,11 @@ EOTEXT public function getCommandHelp() { return phutil_console_format(<<getArgument('show'); $repository_api = $this->getRepositoryAPI(); - if (!($repository_api instanceof ArcanistGitAPI)) { - throw new ArcanistUsageException( - "You may only run 'arc amend' in a git working copy."); - } - if (!$is_show) { + if (!$repository_api->supportsAmend()) { + throw new ArcanistUsageException( + "You may only run 'arc amend' in a git or hg (version ". + "2.2 or newer) working copy."); + } + if ($this->isHistoryImmutable()) { throw new ArcanistUsageException( "This project is marked as adhering to a conservative history ". @@ -175,7 +178,8 @@ EOTEXT echo phutil_console_format( "Amending commit message to reflect revision **%s**.\n", "D{$revision_id}: {$revision_title}"); - $repository_api->amendGitHeadCommit($message); + + $repository_api->amendCommit($message); $mark_workflow = $this->buildChildWorkflow( 'close-revision', @@ -191,7 +195,7 @@ EOTEXT protected function getSupportedRevisionControlSystems() { - return array('git'); + return array('git', 'hg'); } } diff --git a/src/workflow/ArcanistBaseWorkflow.php b/src/workflow/ArcanistBaseWorkflow.php index d97ffaaf..19841126 100644 --- a/src/workflow/ArcanistBaseWorkflow.php +++ b/src/workflow/ArcanistBaseWorkflow.php @@ -1027,8 +1027,15 @@ abstract class ArcanistBaseWorkflow { } protected function isHistoryImmutable() { + $repository_api = $this->getRepositoryAPI(); $working_copy = $this->getWorkingCopy(); - return ($working_copy->getConfig('immutable_history') === true); + + $project_config = $working_copy->getConfig('immutable_history'); + if ($project_config !== null) { + return $project_config; + } + + return $repository_api->isHistoryDefaultImmutable(); } /** diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index fa0054a6..ea135461 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -473,12 +473,15 @@ EOTEXT 'revision_id' => $result['revisionid'], )); - if (!$this->isRawDiffSource()) { + if (!$this->isRawDiffSource() && $this->shouldAmend()) { $repository_api = $this->getRepositoryAPI(); - if (($repository_api instanceof ArcanistGitAPI) && - $this->shouldAmend()) { + + if ($repository_api->supportsAmend()) { echo "Updating commit message...\n"; - $repository_api->amendGitHeadCommit($revised_message); + $repository_api->amendCommit($revised_message); + } else { + echo "Commit message was not amended. Amending commit message is ". + "only supported in git and hg (version 2.2 or newer)"; } }