diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e5a92876..54701ee5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -21,6 +21,7 @@ phutil_register_library_map(array( 'ArcanistCapabilityNotSupportedException' => 'workflow/exception/notsupported', 'ArcanistChooseInvalidRevisionException' => 'exception', 'ArcanistChooseNoRevisionsException' => 'exception', + 'ArcanistCloseRevisionWorkflow' => 'workflow/close-revision', 'ArcanistCloseWorkflow' => 'workflow/close', 'ArcanistCommitWorkflow' => 'workflow/commit', 'ArcanistConduitLinter' => 'lint/linter/conduit', @@ -70,7 +71,6 @@ phutil_register_library_map(array( 'ArcanistMercurialAPI' => 'repository/api/mercurial', 'ArcanistMercurialParser' => 'repository/parser/mercurial', 'ArcanistMercurialParserTestCase' => 'repository/parser/mercurial/__tests__', - 'ArcanistMergeWorkflow' => 'workflow/merge', 'ArcanistNoEffectException' => 'exception/usage/noeffect', 'ArcanistNoEngineException' => 'exception/usage/noengine', 'ArcanistNoLintLinter' => 'lint/linter/nolint', @@ -127,6 +127,7 @@ phutil_register_library_map(array( 'ArcanistBranchWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistBundleTestCase' => 'ArcanistPhutilTestCase', 'ArcanistCallConduitWorkflow' => 'ArcanistBaseWorkflow', + 'ArcanistCloseRevisionWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistCloseWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistCommitWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistConduitLinter' => 'ArcanistLinter', @@ -154,7 +155,6 @@ phutil_register_library_map(array( 'ArcanistMarkCommittedWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistMercurialAPI' => 'ArcanistRepositoryAPI', 'ArcanistMercurialParserTestCase' => 'ArcanistPhutilTestCase', - 'ArcanistMergeWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistNoEffectException' => 'ArcanistUsageException', 'ArcanistNoEngineException' => 'ArcanistUsageException', 'ArcanistNoLintLinter' => 'ArcanistLinter', diff --git a/src/differential/constants/revisionstatus/ArcanistDifferentialRevisionStatus.php b/src/differential/constants/revisionstatus/ArcanistDifferentialRevisionStatus.php index 5b9ec4c6..e587a9d9 100644 --- a/src/differential/constants/revisionstatus/ArcanistDifferentialRevisionStatus.php +++ b/src/differential/constants/revisionstatus/ArcanistDifferentialRevisionStatus.php @@ -21,7 +21,8 @@ final class ArcanistDifferentialRevisionStatus { const NEEDS_REVIEW = 0; const NEEDS_REVISION = 1; const ACCEPTED = 2; - const COMMITTED = 3; + const COMMITTED = 3; // TODO: Remove. + const CLOSED = 3; // NOTE: Duplicate of "COMMITTED". const ABANDONED = 4; public static function getNameForRevisionStatus($status) { @@ -29,7 +30,7 @@ final class ArcanistDifferentialRevisionStatus { self::NEEDS_REVIEW => 'Needs Review', self::NEEDS_REVISION => 'Needs Revision', self::ACCEPTED => 'Accepted', - self::COMMITTED => 'Committed', + self::CLOSED => 'Closed', self::ABANDONED => 'Abandoned', ); diff --git a/src/workflow/amend/ArcanistAmendWorkflow.php b/src/workflow/amend/ArcanistAmendWorkflow.php index b0e46092..b779bac5 100644 --- a/src/workflow/amend/ArcanistAmendWorkflow.php +++ b/src/workflow/amend/ArcanistAmendWorkflow.php @@ -176,7 +176,7 @@ EOTEXT $repository_api->amendGitHeadCommit($message); $mark_workflow = $this->buildChildWorkflow( - 'mark-committed', + 'close-revision', array( '--finalize', $revision_id, diff --git a/src/workflow/close-revision/ArcanistCloseRevisionWorkflow.php b/src/workflow/close-revision/ArcanistCloseRevisionWorkflow.php new file mode 100644 index 00000000..32152f1f --- /dev/null +++ b/src/workflow/close-revision/ArcanistCloseRevisionWorkflow.php @@ -0,0 +1,174 @@ + array( + 'help' => + "Close only if the repository is untracked and the revision is ". + "accepted. Continue even if the close can't happen. This is a soft ". + "version of 'close-revision' used by other workflows.", + ), + 'quiet' => array( + 'help' => 'Do not print a success message.', + ), + '*' => 'revision', + ); + } + + public function requiresConduit() { + return true; + } + + public function requiresAuthentication() { + return true; + } + + public function requiresRepositoryAPI() { + // NOTE: Technically we only use this to generate the right message at + // the end, and you can even get the wrong message (e.g., if you run + // "arc close-revision D123" from a git repository, but D123 is an SVN + // revision). We could be smarter about this, but it's just display fluff. + return true; + } + + public function run() { + $is_finalize = $this->getArgument('finalize'); + + $conduit = $this->getConduit(); + + $revision_list = $this->getArgument('revision', array()); + if (!$revision_list) { + throw new ArcanistUsageException( + "close-revision requires a revision number."); + } + if (count($revision_list) != 1) { + throw new ArcanistUsageException( + "close-revision requires exactly one revision."); + } + $revision_id = reset($revision_list); + $revision_id = $this->normalizeRevisionID($revision_id); + + $revision = null; + try { + $revision = $conduit->callMethodSynchronous( + 'differential.getrevision', + array( + 'revision_id' => $revision_id, + ) + ); + } catch (Exception $ex) { + if (!$is_finalize) { + throw new ArcanistUsageException( + "Revision D{$revision_id} does not exist." + ); + } + } + + $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; + + if (!$is_finalize && $revision['status'] != $status_accepted) { + throw new ArcanistUsageException( + "Revision D{$revision_id} can not be closed. You can only close ". + "revisions which have been 'accepted'."); + } + + if ($revision) { + if (!$is_finalize && $revision['authorPHID'] != $this->getUserPHID()) { + $prompt = "You are not the author of revision D{$revision_id}, ". + 'are you sure you want to close it?'; + if (!phutil_console_confirm($prompt)) { + throw new ArcanistUserAbortException(); + } + } + + $actually_close = true; + if ($is_finalize) { + $project_info = $conduit->callMethodSynchronous( + 'arcanist.projectinfo', + array( + 'name' => $this->getWorkingCopy()->getProjectID(), + )); + if ($project_info['tracked'] || + $revision['status'] != $status_accepted) { + $actually_close = false; + } + } + if ($actually_close) { + $revision_name = $revision['title']; + + echo "Closing revision D{$revision_id} '{$revision_name}'...\n"; + + $conduit->callMethodSynchronous( + 'differential.markcommitted', + array( + 'revision_id' => $revision_id, + )); + } + } + + $status = $revision['status']; + if ($status == $status_accepted || $status == $status_closed) { + // If this has already been attached to commits, don't show the + // "you can push this commit" message since we know it's been pushed + // already. + $is_finalized = empty($revision['commits']); + } else { + $is_finalized = false; + } + + if (!$this->getArgument('quiet')) { + if ($is_finalized) { + $message = $this->getRepositoryAPI()->getFinalizedRevisionMessage(); + echo phutil_console_wrap($message)."\n"; + } else { + echo "Done.\n"; + } + } + + return 0; + } +} diff --git a/src/workflow/merge/__init__.php b/src/workflow/close-revision/__init__.php similarity index 56% rename from src/workflow/merge/__init__.php rename to src/workflow/close-revision/__init__.php index e28bc909..f837b0e5 100644 --- a/src/workflow/merge/__init__.php +++ b/src/workflow/close-revision/__init__.php @@ -6,10 +6,12 @@ +phutil_require_module('arcanist', 'differential/constants/revisionstatus'); phutil_require_module('arcanist', 'exception/usage'); +phutil_require_module('arcanist', 'exception/usage/userabort'); phutil_require_module('arcanist', 'workflow/base'); phutil_require_module('phutil', 'console'); -phutil_require_source('ArcanistMergeWorkflow.php'); +phutil_require_source('ArcanistCloseRevisionWorkflow.php'); diff --git a/src/workflow/commit/ArcanistCommitWorkflow.php b/src/workflow/commit/ArcanistCommitWorkflow.php index 04e8acdd..d6eeec83 100644 --- a/src/workflow/commit/ArcanistCommitWorkflow.php +++ b/src/workflow/commit/ArcanistCommitWorkflow.php @@ -169,7 +169,7 @@ EOTEXT } $mark_workflow = $this->buildChildWorkflow( - 'mark-committed', + 'close-revision', array( '--finalize', $revision_id, diff --git a/src/workflow/land/ArcanistLandWorkflow.php b/src/workflow/land/ArcanistLandWorkflow.php index 9f5e1924..928f6276 100644 --- a/src/workflow/land/ArcanistLandWorkflow.php +++ b/src/workflow/land/ArcanistLandWorkflow.php @@ -290,7 +290,7 @@ EOTEXT } $mark_workflow = $this->buildChildWorkflow( - 'mark-committed', + 'close-revision', array( '--finalize', '--quiet', diff --git a/src/workflow/mark-committed/ArcanistMarkCommittedWorkflow.php b/src/workflow/mark-committed/ArcanistMarkCommittedWorkflow.php index ff56bf4f..093855ae 100644 --- a/src/workflow/mark-committed/ArcanistMarkCommittedWorkflow.php +++ b/src/workflow/mark-committed/ArcanistMarkCommittedWorkflow.php @@ -17,44 +17,27 @@ */ /** - * Explicitly marks Differential revisions as "Committed". - * * @group workflow */ final class ArcanistMarkCommittedWorkflow extends ArcanistBaseWorkflow { public function getCommandSynopses() { return phutil_console_format(<< array( - 'help' => - "Mark committed only if the repository is untracked and the ". - "revision is accepted. Continue even if the mark can't happen. This ". - "is a soft version of 'mark-committed' used by other workflows.", - ), - 'quiet' => array( - 'help' => 'Do not print a success message.', - ), - '*' => 'revision', + '*' => 'deprecated', ); } @@ -67,106 +50,12 @@ EOTEXT } public function requiresRepositoryAPI() { - // NOTE: Technically we only use this to generate the right message at - // the end, and you can even get the wrong message (e.g., if you run - // "arc mark-committed D123" from a git repository, but D123 is an SVN - // revision). We could be smarter about this, but it's just display fluff. return true; } public function run() { - $is_finalize = $this->getArgument('finalize'); - - $conduit = $this->getConduit(); - - $revision_list = $this->getArgument('revision', array()); - if (!$revision_list) { - throw new ArcanistUsageException( - "mark-committed requires a revision number."); - } - if (count($revision_list) != 1) { - throw new ArcanistUsageException( - "mark-committed requires exactly one revision."); - } - $revision_id = reset($revision_list); - $revision_id = $this->normalizeRevisionID($revision_id); - - $revision = null; - try { - $revision = $conduit->callMethodSynchronous( - 'differential.getrevision', - array( - 'revision_id' => $revision_id, - ) - ); - } catch (Exception $ex) { - if (!$is_finalize) { - throw new ArcanistUsageException( - "Revision D{$revision_id} does not exist." - ); - } - } - - if (!$is_finalize && $revision['statusName'] != 'Accepted') { - throw new ArcanistUsageException( - "Revision D{$revision_id} is not committable. You can only mark ". - "revisions which have been 'accepted' as committed." - ); - } - - if ($revision) { - if (!$is_finalize && $revision['authorPHID'] != $this->getUserPHID()) { - $prompt = "You are not the author of revision D{$revision_id}, ". - 'are you sure you want to mark it committed?'; - if (!phutil_console_confirm($prompt)) { - throw new ArcanistUserAbortException(); - } - } - - $actually_mark = true; - if ($is_finalize) { - $project_info = $conduit->callMethodSynchronous( - 'arcanist.projectinfo', - array( - 'name' => $this->getWorkingCopy()->getProjectID(), - )); - if ($project_info['tracked'] || $revision['statusName'] != 'Accepted') { - $actually_mark = false; - } - } - if ($actually_mark) { - $revision_name = $revision['title']; - - echo "Marking revision D{$revision_id} '{$revision_name}' ". - "committed...\n"; - - $conduit->callMethodSynchronous( - 'differential.markcommitted', - array( - 'revision_id' => $revision_id, - )); - } - } - - $status = $revision['statusName']; - if ($status == 'Accepted' || $status == 'Committed') { - // If this has already been attached to commits, don't show the - // "you can push this commit" message since we know it's been committed - // already. - $is_finalized = empty($revision['commits']); - } else { - $is_finalized = false; - } - - if (!$this->getArgument('quiet')) { - if ($is_finalized) { - $message = $this->getRepositoryAPI()->getFinalizedRevisionMessage(); - echo phutil_console_wrap($message)."\n"; - } else { - echo "Done.\n"; - } - } - - return 0; + throw new ArcanistUsageException( + "'arc mark-committed' is now 'arc close-revision' (because ". + "'mark-committed' only really made sense under SVN)."); } } diff --git a/src/workflow/mark-committed/__init__.php b/src/workflow/mark-committed/__init__.php index cfeea091..83bdcd8d 100644 --- a/src/workflow/mark-committed/__init__.php +++ b/src/workflow/mark-committed/__init__.php @@ -7,7 +7,6 @@ phutil_require_module('arcanist', 'exception/usage'); -phutil_require_module('arcanist', 'exception/usage/userabort'); phutil_require_module('arcanist', 'workflow/base'); phutil_require_module('phutil', 'console'); diff --git a/src/workflow/merge/ArcanistMergeWorkflow.php b/src/workflow/merge/ArcanistMergeWorkflow.php deleted file mode 100644 index 22c9f32d..00000000 --- a/src/workflow/merge/ArcanistMergeWorkflow.php +++ /dev/null @@ -1,65 +0,0 @@ - 'ignored', - ); - } - - public function run() { - $repository_api = $this->getRepositoryAPI(); - - if ($repository_api instanceof ArcanistGitAPI) { - throw new ArcanistUsageException( - "'arc merge' no longer supports git. Use ". - "'arc land --keep-branch --hold --merge ' instead."); - } - - throw new ArcanistUsageException('arc merge is no longer supported.'); - } - -}