From 276b013d8758becf6634049b9eda3d723343c0f3 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 7 Mar 2012 13:02:53 -0800 Subject: [PATCH] Arc patch - upgrade "same base rev" check to "does this commit exist in the working copy?" check Summary: good title Test Plan: ran "arc patch DX" a bunch Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Maniphest Tasks: T892 Differential Revision: https://secure.phabricator.com/D1789 --- .../api/base/ArcanistRepositoryAPI.php | 4 +++ src/repository/api/git/ArcanistGitAPI.php | 7 ++++ .../api/mercurial/ArcanistMercurialAPI.php | 9 ++++-- .../api/subversion/ArcanistSubversionAPI.php | 4 +++ src/workflow/patch/ArcanistPatchWorkflow.php | 32 +++++++++---------- 5 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/repository/api/base/ArcanistRepositoryAPI.php b/src/repository/api/base/ArcanistRepositoryAPI.php index ed7ab1d3..62657bea 100644 --- a/src/repository/api/base/ArcanistRepositoryAPI.php +++ b/src/repository/api/base/ArcanistRepositoryAPI.php @@ -161,6 +161,10 @@ abstract class ArcanistRepositoryAPI { ConduitClient $conduit, array $query); + public function hasLocalCommit($commit) { + throw new ArcanistCapabilityNotSupportedException($this); + } + public function getCommitMessageForRevision($revision) { throw new ArcanistCapabilityNotSupportedException($this); } diff --git a/src/repository/api/git/ArcanistGitAPI.php b/src/repository/api/git/ArcanistGitAPI.php index bb9ac71a..1adcfef3 100644 --- a/src/repository/api/git/ArcanistGitAPI.php +++ b/src/repository/api/git/ArcanistGitAPI.php @@ -551,6 +551,13 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { return true; } + public function hasLocalCommit($commit) { + list($err) = $this->execManualLocal( + 'merge-base %s HEAD', + $commit); + return !$err; + } + public function parseRelativeLocalCommit(array $argv) { if (count($argv) == 0) { return; diff --git a/src/repository/api/mercurial/ArcanistMercurialAPI.php b/src/repository/api/mercurial/ArcanistMercurialAPI.php index 9392299a..04daa133 100644 --- a/src/repository/api/mercurial/ArcanistMercurialAPI.php +++ b/src/repository/api/mercurial/ArcanistMercurialAPI.php @@ -71,12 +71,10 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { } public function setRelativeCommit($commit) { - list($err) = $this->execManualLocal('id -ir %s', $commit); - if ($err) { + if (!$this->hasLocalCommit($commit)) { throw new ArcanistUsageException( "Commit '{$commit}' is not a valid Mercurial commit identifier."); } - $this->relativeCommit = $commit; return $this; } @@ -338,6 +336,11 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { return true; } + public function hasLocalCommit($commit) { + list($err) = $this->execManualLocal('id -ir %s', $commit); + return !$err; + } + public function parseRelativeLocalCommit(array $argv) { if (count($argv) == 0) { return; diff --git a/src/repository/api/subversion/ArcanistSubversionAPI.php b/src/repository/api/subversion/ArcanistSubversionAPI.php index 55ce4379..f476b6f1 100644 --- a/src/repository/api/subversion/ArcanistSubversionAPI.php +++ b/src/repository/api/subversion/ArcanistSubversionAPI.php @@ -502,6 +502,10 @@ EODIFF; return false; } + public function hasLocalCommit($commit) { + return false; + } + public function getWorkingCopyRevision() { return $this->getSourceControlBaseRevision(); } diff --git a/src/workflow/patch/ArcanistPatchWorkflow.php b/src/workflow/patch/ArcanistPatchWorkflow.php index de4d025b..3f9fd0ff 100644 --- a/src/workflow/patch/ArcanistPatchWorkflow.php +++ b/src/workflow/patch/ArcanistPatchWorkflow.php @@ -687,26 +687,26 @@ EOTEXT // Check to see if the bundle's base revision matches the working copy // base revision - $bundle_base_rev = $bundle->getBaseRevision(); - if (empty($bundle_base_rev)) { - // this means $source is SOURCE_PATCH || SOURCE_BUNDLE w/ $version < 2 - // they don't have a base rev so just do nothing - } else { - $repository_api = $this->getRepositoryAPI(); - $source_base_rev = $repository_api->getWorkingCopyRevision(); - - if ($source_base_rev != $bundle_base_rev) { + $repository_api = $this->getRepositoryAPI(); + if ($repository_api->supportsRelativeLocalCommits()) { + $bundle_base_rev = $bundle->getBaseRevision(); + if (empty($bundle_base_rev)) { + // this means $source is SOURCE_PATCH || SOURCE_BUNDLE w/ $version < 2 + // they don't have a base rev so just do nothing + $commit_exists = true; + } else { + $commit_exists = + $repository_api->hasLocalCommit($bundle_base_rev); + } + if (!$commit_exists) { // we have a problem...! lots of work because we need to ask // differential for revision information for these base revisions // to improve our error message. $bundle_base_rev_str = null; + $source_base_rev = $repository_api->getWorkingCopyRevision(); $source_base_rev_str = null; - // SVN doesn't store these hashes, so we're basically done already - // and will have a relatively "lame" error message - if ($repository_api instanceof ArcanistSubversionAPI) { - $hash_type = null; - } else if ($repository_api instanceof ArcanistGitAPI) { + if ($repository_api instanceof ArcanistGitAPI) { $hash_type = ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT; } else if ($repository_api instanceof ArcanistMercurialAPI) { $hash_type = ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT; @@ -738,8 +738,8 @@ EOTEXT $ok = phutil_console_confirm( "This diff is against commit {$bundle_base_rev_str}, but the ". - "working copy is at {$source_base_rev_str}. ". - "Still try to apply it?", + "commit is nowhere in the working copy. Try to apply it against ". + "the current working copy state? ({$source_base_rev_str})", $default_no = false ); if (!$ok) {