1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-26 08:42:40 +01:00

Adding arc revert command[]

Summary:
Arc Revert does the following:
1. Git revert
2. Go to the differential of the rev you are reverting and either repoen it or set it to a reverted state
3. File a hipri task to orig author

[Preview] Creating Arc Revert workflow

Porting arc revert from FB4A to phabricator for general usage. This is my first stab,
so totally appreciate feedback and assistance. I'm currently focused
on making this work for git. However, I built out the functions through the GitAPI so this
could be easily extendable to Mercurial later on.

Stuck on the following (help):
1. Creating a task for FB internal. I tried building on top of existing arc listeners
but getting errors on failures to load the TaskCreator (and other) tasks.

2. I'm using a hacky way to grab the diff revision id from the newly created
revert diff. (see line 204) I'm looking for a way to just fetch the diff ID
from arc after the diff is created; is this possible?

Test Plan:
 -

1. Ran arc revert on a www and fbcode diff
2. Confirmed that revert was run on the diffs and a proper diff filed
 -

Reviewers: royw, sdwilsh, nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin, pti, keir

Maniphest Tasks: T1751

Differential Revision: https://secure.phabricator.com/D5553
This commit is contained in:
Howard Chan 2013-05-14 11:00:56 -07:00
parent 0476bf8f4a
commit 3116d3656a
5 changed files with 315 additions and 6 deletions

View file

@ -15,6 +15,7 @@ phutil_register_library_map(array(
'ArcanistAnoidWorkflow' => 'workflow/ArcanistAnoidWorkflow.php', 'ArcanistAnoidWorkflow' => 'workflow/ArcanistAnoidWorkflow.php',
'ArcanistApacheLicenseLinter' => 'lint/linter/ArcanistApacheLicenseLinter.php', 'ArcanistApacheLicenseLinter' => 'lint/linter/ArcanistApacheLicenseLinter.php',
'ArcanistArcanistLinterTestCase' => 'lint/linter/__tests__/ArcanistArcanistLinterTestCase.php', 'ArcanistArcanistLinterTestCase' => 'lint/linter/__tests__/ArcanistArcanistLinterTestCase.php',
'ArcanistBackoutWorkflow' => 'workflow/ArcanistBackoutWorkflow.php',
'ArcanistBaseCommitParser' => 'parser/ArcanistBaseCommitParser.php', 'ArcanistBaseCommitParser' => 'parser/ArcanistBaseCommitParser.php',
'ArcanistBaseCommitParserTestCase' => 'parser/__tests__/ArcanistBaseCommitParserTestCase.php', 'ArcanistBaseCommitParserTestCase' => 'parser/__tests__/ArcanistBaseCommitParserTestCase.php',
'ArcanistBaseTestResultParser' => 'unit/engine/ArcanistBaseTestResultParser.php', 'ArcanistBaseTestResultParser' => 'unit/engine/ArcanistBaseTestResultParser.php',
@ -119,6 +120,7 @@ phutil_register_library_map(array(
'ArcanistRepositoryAPI' => 'repository/api/ArcanistRepositoryAPI.php', 'ArcanistRepositoryAPI' => 'repository/api/ArcanistRepositoryAPI.php',
'ArcanistRepositoryAPIMiscTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIMiscTestCase.php', 'ArcanistRepositoryAPIMiscTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIMiscTestCase.php',
'ArcanistRepositoryAPIStateTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php', 'ArcanistRepositoryAPIStateTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php',
'ArcanistRevertWorkflow' => 'workflow/ArcanistRevertWorkflow.php',
'ArcanistRubyLinter' => 'lint/linter/ArcanistRubyLinter.php', 'ArcanistRubyLinter' => 'lint/linter/ArcanistRubyLinter.php',
'ArcanistRubyLinterTestCase' => 'lint/linter/__tests__/ArcanistRubyLinterTestCase.php', 'ArcanistRubyLinterTestCase' => 'lint/linter/__tests__/ArcanistRubyLinterTestCase.php',
'ArcanistScalaSBTLinter' => 'lint/linter/ArcanistScalaSBTLinter.php', 'ArcanistScalaSBTLinter' => 'lint/linter/ArcanistScalaSBTLinter.php',
@ -178,6 +180,7 @@ phutil_register_library_map(array(
'ArcanistAnoidWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistAnoidWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistApacheLicenseLinter' => 'ArcanistLicenseLinter', 'ArcanistApacheLicenseLinter' => 'ArcanistLicenseLinter',
'ArcanistArcanistLinterTestCase' => 'ArcanistLinterTestCase', 'ArcanistArcanistLinterTestCase' => 'ArcanistLinterTestCase',
'ArcanistBackoutWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistBaseCommitParserTestCase' => 'ArcanistTestCase', 'ArcanistBaseCommitParserTestCase' => 'ArcanistTestCase',
'ArcanistBaseWorkflow' => 'Phobject', 'ArcanistBaseWorkflow' => 'Phobject',
'ArcanistBaseXHPASTLinter' => 'ArcanistFutureLinter', 'ArcanistBaseXHPASTLinter' => 'ArcanistFutureLinter',
@ -255,6 +258,7 @@ phutil_register_library_map(array(
'ArcanistPyLintLinter' => 'ArcanistLinter', 'ArcanistPyLintLinter' => 'ArcanistLinter',
'ArcanistRepositoryAPIMiscTestCase' => 'ArcanistTestCase', 'ArcanistRepositoryAPIMiscTestCase' => 'ArcanistTestCase',
'ArcanistRepositoryAPIStateTestCase' => 'ArcanistTestCase', 'ArcanistRepositoryAPIStateTestCase' => 'ArcanistTestCase',
'ArcanistRevertWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistRubyLinter' => 'ArcanistLinter', 'ArcanistRubyLinter' => 'ArcanistLinter',
'ArcanistRubyLinterTestCase' => 'ArcanistArcanistLinterTestCase', 'ArcanistRubyLinterTestCase' => 'ArcanistArcanistLinterTestCase',
'ArcanistScalaSBTLinter' => 'ArcanistLinter', 'ArcanistScalaSBTLinter' => 'ArcanistLinter',

View file

@ -392,9 +392,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
public function getCanonicalRevisionName($string) { public function getCanonicalRevisionName($string) {
$match = null; $match = null;
if (preg_match('/@([0-9]+)$/', $string, $match)) { if (preg_match('/@([0-9]+)$/', $string, $match)) {
list($stdout) = $this->execxLocal( $stdout = $this->getHashFromFromSVNRevisionNumber($match[1]);
'svn find-rev r%d',
$match[1]);
} else { } else {
list($stdout) = $this->execxLocal( list($stdout) = $this->execxLocal(
'show -s --format=%C %s', 'show -s --format=%C %s',
@ -404,6 +402,34 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return rtrim($stdout); return rtrim($stdout);
} }
private function executeSVNFindRev($input, $vcs) {
$match = array();
list($stdout) = $this->execxLocal(
'svn find-rev %s',
$input);
if (!$stdout) {
throw new ArcanistUsageException("Cannot find the {$vcs} equivalent "
."of {$input}.");
}
// When git performs a partial-rebuild during svn
// look-up, we need to parse the final line
$lines = explode("\n", $stdout);
$stdout = $lines[count($lines) - 2];
return rtrim($stdout);
}
// Convert svn revision number to git hash
public function getHashFromFromSVNRevisionNumber($revision_id) {
return $this->executeSVNFindRev("r".$revision_id, "Git");
}
// Convert a git hash to svn revision number
public function getSVNRevisionNumberFromHash($hash) {
return $this->executeSVNFindRev($hash, "SVN");
}
protected function buildUncommittedStatus() { protected function buildUncommittedStatus() {
$diff_options = $this->getDiffBaseOptions(); $diff_options = $this->getDiffBaseOptions();
@ -882,6 +908,24 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return trim($summary); return trim($summary);
} }
public function backoutCommit($commit_hash) {
$this->execxLocal(
'revert %s -n --no-edit', $commit_hash);
$this->reloadWorkingCopy();
if (!$this->getUncommittedStatus()) {
throw new ArcanistUsageException(
"{$commit_hash} has already been reverted.");
}
}
public function getBackoutMessage($commit_hash) {
return "This reverts commit ".$commit_hash.".";
}
public function isGitSubversionRepo() {
return Filesystem::pathExists($this->getPath('.git/svn'));
}
public function resolveBaseCommitRule($rule, $source) { public function resolveBaseCommitRule($rule, $source) {
list($type, $name) = explode(':', $rule, 2); list($type, $name) = explode(':', $rule, 2);

View file

@ -74,6 +74,32 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
return $stdout; return $stdout;
} }
public function getHashFromFromSVNRevisionNumber($revision_id) {
$matches = array();
$string = hgsprintf('svnrev(%s)', $revision_id);
list($stdout) = $this->execxLocal(
'log -l 1 --template %s -r %s --',
'{node}',
$string);
if (!$stdout) {
throw new ArcanistUsageException("Cannot find the HG equivalent "
."of {$revision_id} given.");
}
return $stdout;
}
public function getSVNRevisionNumberFromHash($hash) {
$matches = array();
list($stdout) = $this->execxLocal(
'log -r %s --template {svnrev}', $hash);
if (!$stdout) {
throw new ArcanistUsageException("Cannot find the SVN equivalent "
."of {$hash} given.");
}
return $stdout;
}
public function getSourceControlPath() { public function getSourceControlPath() {
return '/'; return '/';
} }
@ -742,6 +768,20 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
return trim($summary); return trim($summary);
} }
public function backoutCommit($commit_hash) {
$this->execxLocal(
'backout -r %s', $commit_hash);
$this->reloadWorkingCopy();
if (!$this->getUncommittedStatus()) {
throw new ArcanistUsageException(
"{$commit_hash} has already been reverted.");
}
}
public function getBackoutMessage($commit_hash) {
return "Backed out changeset ".$commit_hash.".";
}
public function resolveBaseCommitRule($rule, $source) { public function resolveBaseCommitRule($rule, $source) {
list($type, $name) = explode(':', $rule, 2); list($type, $name) = explode(':', $rule, 2);
@ -774,7 +814,6 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
'log --template {node} --rev %s', 'log --template {node} --rev %s',
$name); $name);
} }
if (!$err) { if (!$err) {
$this->setBaseCommitExplanation( $this->setBaseCommitExplanation(
"it is specified by '{$rule}' in your {$source} 'base' ". "it is specified by '{$rule}' in your {$source} 'base' ".

View file

@ -0,0 +1,184 @@
<?php
/**
* Runs git revert and assigns hi pri task to original author
* @group workflow
*/
final class ArcanistBackoutWorkflow extends ArcanistBaseWorkflow {
private $console;
private $conduit;
private $revision;
public function getWorkflowName() {
return 'backout';
}
public function getCommandSynopses() {
return phutil_console_format(<<<EOTEXT
**backout**
EOTEXT
);
}
public function getCommandHelp() {
return phutil_console_format(<<<EOTEXT
Reverts/backouts on a previous commit. Supports: git
Command is used like this: arc backout <commithash> | <diff revision>
Entering a differential revision will only work if there is only one commit
associated with the revision. This requires your working copy is up to date
and that the commit exists in the working copy.
EOTEXT
);
}
public function getArguments() {
return array(
'*' => 'input',
);
}
public function requiresWorkingCopy() {
return true;
}
public function requiresRepositoryAPI() {
return true;
}
public function requiresAuthentication() {
return true;
}
// Given a differential revision ID, fetches the commit ID
private function getCommitIDFromRevisionID($revision_id) {
$conduit = $this->getConduit();
$revisions = $conduit->callMethodSynchronous(
'differential.query',
array(
'ids' => array($revision_id),
));
if (!$revisions) {
throw new ArcanistUsageException(
'The revision you provided does not exist!');
}
$revision = $revisions[0];
$commits = $revision["commits"];
if (!$commits) {
throw new ArcanistUsageException(
'This revision has not been committed yet!');
}
elseif (count($commits) > 1) {
throw new ArcanistUsageException(
'The revision you provided has multiple commits!');
}
$commit_phid = $commits[0];
$commit = $conduit->callMethodSynchronous(
'phid.query',
array(
'phids' => array($commit_phid),
));
$commit_id = $commit[$commit_phid]["name"];
return $commit_id;
}
// Fetches an array of commit info provided a Commit_id
// in the form of rE123456 (not local commit hash)
private function getDiffusionCommit($commit_id) {
$result = $this->getConduit()->callMethodSynchronous(
'diffusion.getcommits',
array(
'commits' => array($commit_id),
));
$commit = $result[$commit_id];
// This commit was not found in Diffusion
if (array_key_exists("error", $commit)) {
return null;
}
return $commit;
}
// Retrieves default template from differential and prefills info
private function buildCommitMessage($commit_hash) {
$conduit = $this->getConduit();
$repository_api = $this->getRepositoryAPI();
$summary = $repository_api->getBackoutMessage($commit_hash);
$fields = array('summary' => $summary,
'testPlan' => 'revert-hammer',
);
$template = $conduit->callMethodSynchronous(
'differential.getcommitmessage',
array(
'revision_id' => null,
'edit' => 'create',
'fields' => $fields
));
$template = $this->newInteractiveEditor($template)
->setName('new-commit')
->editInteractively();
return $template;
}
// Performs the backout/revert of a revision and creates a commit
public function run() {
$console = PhutilConsole::getConsole();
$conduit = $this->getConduit();
$repository_api = $this->getRepositoryAPI();
$repository = $this->loadProjectRepository();
$callsign = $repository["callsign"];
$is_git_svn = $repository_api instanceof ArcanistGitAPI &&
$repository_api->isGitSubversionRepo();
$is_hg_svn = $repository_api instanceof ArcanistMercurialAPI &&
$repository_api->isHgSubversionRepo();
$revision_id = null;
if (!($repository_api instanceof ArcanistGitAPI) &&
!($repository_api instanceof ArcanistMercurialAPI)) {
throw new ArcanistUsageException(
'Backout currently only supports Git and Mercurial'
);
}
$console->writeOut("Starting backout\n");
$input = $this->getArgument('input');
if (!$input || count($input) != 1) {
throw new ArcanistUsageException(
'You must specify one commit to backout!');
}
// Input looks like a Differential revision, so
// we try to find the commit attached to it
$matches = array();
if (preg_match('/^D(\d+)$/i', $input[0], $matches)) {
$revision_id = $matches[1];
$commit_id = $this->getCommitIDFromRevisionID($revision_id);
$commit = $this->getDiffusionCommit($commit_id);
$commit_hash = $commit['commitIdentifier'];
// Convert commit hash from SVN to Git/HG (for FB case)
if ($is_git_svn || $is_hg_svn) {
$commit_hash = $repository_api->
getHashFromFromSVNRevisionNumber($commit_hash);
}
} else {
// Assume input is a commit hash
$commit_hash = $input[0];
}
if (!$repository_api->hasLocalCommit($commit_hash)) {
throw new ArcanistUsageException('Invalid commit provided or does not'.
'exist in the working copy!');
}
// Run 'backout'.
$subject = $repository_api->getCommitSummary($commit_hash);
$console->writeOut("Backing out commit {$commit_hash} {$subject} \n");
$repository_api->backoutCommit($commit_hash);
// Create commit message and execute the commit
$message = $this->buildCommitMessage($commit_hash);
$repository_api->doCommit($message);
$console->writeOut("Double-check the commit and push when ready\n");
}
}

View file

@ -0,0 +1,38 @@
<?php
/**
* Redirects to arc backout workflow
* @group workflow
*/
final class ArcanistRevertWorkflow extends ArcanistBaseWorkflow {
public function getWorkflowName() {
return 'revert';
}
public function getCommandSynopses() {
return phutil_console_format(<<<EOTEXT
**revert**
EOTEXT
);
}
public function getCommandHelp() {
return phutil_console_format(<<<EOTEXT
Please use arc backout instead
EOTEXT
);
}
public function getArguments() {
return array(
'*' => 'input',
);
}
public function run() {
$console = PhutilConsole::getConsole();
$console->writeOut("Please use arc backout instead.\n");
}
}