mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-10 08:52:39 +01:00
Arcanist - make Patch workflow automagically apply dependencies
Summary: Nice title. Ref T479. Test Plan: Actually, help on that? I want to make sure I properly build up the "depends" on data. Is it as simple as -- observe at some commit hash RAZZMATAZZ -- git checkout -B "foo" -- <work> -- git commit -m "stash" -- arc diff -> yields DX -- git checkout -B "foo_prime" -- <work> -- git commit -m "stash" -- arc diff -> yield DY -- git checkout RAZZMATAZZ -- arc patch DY -- get prompted in workflow, agree -- git log and observe DX and DY applied Reviewers: epriestley Reviewed By: epriestley CC: Korvin, aran, davidressman Maniphest Tasks: T479 Differential Revision: https://secure.phabricator.com/D6790
This commit is contained in:
parent
79be59863e
commit
b2021586d4
3 changed files with 204 additions and 49 deletions
|
@ -57,6 +57,7 @@ phutil_register_library_map(array(
|
||||||
'ArcanistDiffWorkflow' => 'workflow/ArcanistDiffWorkflow.php',
|
'ArcanistDiffWorkflow' => 'workflow/ArcanistDiffWorkflow.php',
|
||||||
'ArcanistDifferentialCommitMessage' => 'differential/ArcanistDifferentialCommitMessage.php',
|
'ArcanistDifferentialCommitMessage' => 'differential/ArcanistDifferentialCommitMessage.php',
|
||||||
'ArcanistDifferentialCommitMessageParserException' => 'differential/ArcanistDifferentialCommitMessageParserException.php',
|
'ArcanistDifferentialCommitMessageParserException' => 'differential/ArcanistDifferentialCommitMessageParserException.php',
|
||||||
|
'ArcanistDifferentialDependencyGraph' => 'differential/ArcanistDifferentialDependencyGraph.php',
|
||||||
'ArcanistDifferentialRevisionHash' => 'differential/constants/ArcanistDifferentialRevisionHash.php',
|
'ArcanistDifferentialRevisionHash' => 'differential/constants/ArcanistDifferentialRevisionHash.php',
|
||||||
'ArcanistDifferentialRevisionStatus' => 'differential/constants/ArcanistDifferentialRevisionStatus.php',
|
'ArcanistDifferentialRevisionStatus' => 'differential/constants/ArcanistDifferentialRevisionStatus.php',
|
||||||
'ArcanistDownloadWorkflow' => 'workflow/ArcanistDownloadWorkflow.php',
|
'ArcanistDownloadWorkflow' => 'workflow/ArcanistDownloadWorkflow.php',
|
||||||
|
@ -222,6 +223,7 @@ phutil_register_library_map(array(
|
||||||
'ArcanistDiffUtilsTestCase' => 'ArcanistTestCase',
|
'ArcanistDiffUtilsTestCase' => 'ArcanistTestCase',
|
||||||
'ArcanistDiffWorkflow' => 'ArcanistBaseWorkflow',
|
'ArcanistDiffWorkflow' => 'ArcanistBaseWorkflow',
|
||||||
'ArcanistDifferentialCommitMessageParserException' => 'Exception',
|
'ArcanistDifferentialCommitMessageParserException' => 'Exception',
|
||||||
|
'ArcanistDifferentialDependencyGraph' => 'AbstractDirectedGraph',
|
||||||
'ArcanistDownloadWorkflow' => 'ArcanistBaseWorkflow',
|
'ArcanistDownloadWorkflow' => 'ArcanistBaseWorkflow',
|
||||||
'ArcanistEventType' => 'PhutilEventType',
|
'ArcanistEventType' => 'PhutilEventType',
|
||||||
'ArcanistExportWorkflow' => 'ArcanistBaseWorkflow',
|
'ArcanistExportWorkflow' => 'ArcanistBaseWorkflow',
|
||||||
|
|
72
src/differential/ArcanistDifferentialDependencyGraph.php
Normal file
72
src/differential/ArcanistDifferentialDependencyGraph.php
Normal file
|
@ -0,0 +1,72 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistDifferentialDependencyGraph extends AbstractDirectedGraph {
|
||||||
|
|
||||||
|
private $repositoryAPI;
|
||||||
|
private $conduit;
|
||||||
|
private $startPHID;
|
||||||
|
|
||||||
|
public function setStartPHID($start_phid) {
|
||||||
|
$this->startPHID = $start_phid;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
public function getStartPHID() {
|
||||||
|
return $this->startPHID;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function setRepositoryAPI(ArcanistRepositoryAPI $repository_api) {
|
||||||
|
$this->repositoryAPI = $repository_api;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
public function getRepositoryAPI() {
|
||||||
|
return $this->repositoryAPI;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function setConduit(ConduitClient $conduit) {
|
||||||
|
$this->conduit = $conduit;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
public function getConduit() {
|
||||||
|
return $this->conduit;
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function loadEdges(array $nodes) {
|
||||||
|
$repository_api = $this->getRepositoryAPI();
|
||||||
|
|
||||||
|
$dependencies = $this->getConduit()->callMethodSynchronous(
|
||||||
|
'differential.query',
|
||||||
|
array(
|
||||||
|
'phids' => $nodes,
|
||||||
|
));
|
||||||
|
|
||||||
|
$edges = array();
|
||||||
|
foreach ($dependencies as $dependency) {
|
||||||
|
$dependency_revision = $this->getCommitHashFromDict($dependency);
|
||||||
|
if ($repository_api->hasLocalCommit($dependency_revision)) {
|
||||||
|
$edges[$dependency['phid']] = array();
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$auxillary = idx($dependency, 'auxiliary', array());
|
||||||
|
$edges[$dependency['phid']] = idx(
|
||||||
|
$auxillary,
|
||||||
|
'phabricator:depends-on',
|
||||||
|
array());
|
||||||
|
}
|
||||||
|
return $edges;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function getCommitHashFromDict($dict) {
|
||||||
|
$api = $this->getRepositoryAPI();
|
||||||
|
$hashes = idx($dict, 'hashes', array());
|
||||||
|
if ($api instanceof ArcanistGitAPI) {
|
||||||
|
$key = ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT;
|
||||||
|
} else if ($api instanceof ArcanistMercurialAPI) {
|
||||||
|
$key = ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT;
|
||||||
|
} else {
|
||||||
|
$key = null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return idx($hashes, $key);
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -93,6 +93,15 @@ EOTEXT
|
||||||
"Normally under git/hg, if the patch is successful, the changes ".
|
"Normally under git/hg, if the patch is successful, the changes ".
|
||||||
"are committed to the working copy. This flag prevents the commit.",
|
"are committed to the working copy. This flag prevents the commit.",
|
||||||
),
|
),
|
||||||
|
'skip-dependencies' => array(
|
||||||
|
'supports' => array(
|
||||||
|
'git', 'hg'
|
||||||
|
),
|
||||||
|
'help' =>
|
||||||
|
'Normally, if a patch has dependencies that are not present in the '.
|
||||||
|
'working copy, arc tries to apply them as well. This flag prevents '.
|
||||||
|
'such work.',
|
||||||
|
),
|
||||||
'nobranch' => array(
|
'nobranch' => array(
|
||||||
'supports' => array(
|
'supports' => array(
|
||||||
'git', 'hg'
|
'git', 'hg'
|
||||||
|
@ -186,12 +195,7 @@ EOTEXT
|
||||||
}
|
}
|
||||||
|
|
||||||
private function shouldCommit() {
|
private function shouldCommit() {
|
||||||
$no_commit = $this->getArgument('nocommit', false);
|
return !$this->getArgument('nocommit', false);
|
||||||
if ($no_commit) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
return true;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private function canBranch() {
|
private function canBranch() {
|
||||||
|
@ -285,29 +289,6 @@ EOTEXT
|
||||||
return $bookmark_name;
|
return $bookmark_name;
|
||||||
}
|
}
|
||||||
|
|
||||||
private function hasBaseRevision(ArcanistBundle $bundle) {
|
|
||||||
$base_revision = $bundle->getBaseRevision();
|
|
||||||
$repository_api = $this->getRepositoryAPI();
|
|
||||||
|
|
||||||
// verify the base revision is valid
|
|
||||||
if ($repository_api instanceof ArcanistGitAPI) {
|
|
||||||
// in a working copy that uses the git-svn bridge, the base revision might
|
|
||||||
// be a svn uri instead of a git ref
|
|
||||||
|
|
||||||
// NOTE: Use 'cat-file', not 'rev-parse --verify', because 'rev-parse'
|
|
||||||
// always "verifies" any properly-formatted commit even if it does not
|
|
||||||
// exist.
|
|
||||||
list($err) = $repository_api->execManualLocal(
|
|
||||||
'cat-file -t %s',
|
|
||||||
$base_revision);
|
|
||||||
return !$err;
|
|
||||||
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
|
||||||
return $repository_api->hasLocalCommit($base_revision);
|
|
||||||
}
|
|
||||||
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
private function createBranch(ArcanistBundle $bundle, $has_base_revision) {
|
private function createBranch(ArcanistBundle $bundle, $has_base_revision) {
|
||||||
$repository_api = $this->getRepositoryAPI();
|
$repository_api = $this->getRepositoryAPI();
|
||||||
if ($repository_api instanceof ArcanistGitAPI) {
|
if ($repository_api instanceof ArcanistGitAPI) {
|
||||||
|
@ -354,6 +335,10 @@ EOTEXT
|
||||||
return $branch_name;
|
return $branch_name;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function shouldApplyDependencies() {
|
||||||
|
return !$this->getArgument('skip-dependencies', false);
|
||||||
|
}
|
||||||
|
|
||||||
private function shouldUpdateWorkingCopy() {
|
private function shouldUpdateWorkingCopy() {
|
||||||
return $this->getArgument('update', false);
|
return $this->getArgument('update', false);
|
||||||
}
|
}
|
||||||
|
@ -423,24 +408,24 @@ EOTEXT
|
||||||
$bundle->setEncoding($try_encoding);
|
$bundle->setEncoding($try_encoding);
|
||||||
}
|
}
|
||||||
|
|
||||||
$force = $this->getArgument('force', false);
|
$sanity_check = !$this->getArgument('force', false);
|
||||||
if ($force) {
|
|
||||||
// force means don't do any sanity checks about the patch
|
|
||||||
} else {
|
|
||||||
$this->sanityCheck($bundle);
|
|
||||||
}
|
|
||||||
|
|
||||||
// we should update the working copy before we do ANYTHING else
|
// we should update the working copy before we do ANYTHING else to
|
||||||
|
// the working copy
|
||||||
if ($this->shouldUpdateWorkingCopy()) {
|
if ($this->shouldUpdateWorkingCopy()) {
|
||||||
$this->updateWorkingCopy();
|
$this->updateWorkingCopy();
|
||||||
}
|
}
|
||||||
|
|
||||||
$repository_api = $this->getRepositoryAPI();
|
if ($sanity_check) {
|
||||||
|
$this->requireCleanWorkingCopy();
|
||||||
|
}
|
||||||
|
|
||||||
$has_base_revision = $this->hasBaseRevision($bundle);
|
$repository_api = $this->getRepositoryAPI();
|
||||||
if ($this->shouldCommit() &&
|
$has_base_revision = $repository_api->hasLocalCommit(
|
||||||
$this->canBranch() &&
|
$bundle->getBaseRevision());
|
||||||
($this->shouldBranch() || $has_base_revision)) {
|
if ($this->canBranch() &&
|
||||||
|
($this->shouldBranch() ||
|
||||||
|
($this->shouldCommit() && $has_base_revision))) {
|
||||||
|
|
||||||
if ($repository_api instanceof ArcanistGitAPI) {
|
if ($repository_api instanceof ArcanistGitAPI) {
|
||||||
$original_branch = $repository_api->getBranchName();
|
$original_branch = $repository_api->getBranchName();
|
||||||
|
@ -460,6 +445,13 @@ EOTEXT
|
||||||
|
|
||||||
$new_branch = $this->createBranch($bundle, $has_base_revision);
|
$new_branch = $this->createBranch($bundle, $has_base_revision);
|
||||||
}
|
}
|
||||||
|
if (!$has_base_revision && $this->shouldApplyDependencies()) {
|
||||||
|
$this->applyDependencies($bundle);
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($sanity_check) {
|
||||||
|
$this->sanityCheck($bundle);
|
||||||
|
}
|
||||||
|
|
||||||
if ($repository_api instanceof ArcanistSubversionAPI) {
|
if ($repository_api instanceof ArcanistSubversionAPI) {
|
||||||
$patch_err = 0;
|
$patch_err = 0;
|
||||||
|
@ -716,13 +708,16 @@ EOTEXT
|
||||||
$verb = 'applied';
|
$verb = 'applied';
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->shouldCommit() && $this->canBranch() &&
|
if ($this->canBranch() &&
|
||||||
!$this->shouldBranch() && $has_base_revision) {
|
!$this->shouldBranch() &&
|
||||||
|
$this->shouldCommit() && $has_base_revision) {
|
||||||
$repository_api->execxLocal('checkout %s', $original_branch);
|
$repository_api->execxLocal('checkout %s', $original_branch);
|
||||||
$ex = null;
|
$ex = null;
|
||||||
try {
|
try {
|
||||||
$repository_api->execxLocal('cherry-pick %s', $new_branch);
|
$repository_api->execxLocal('cherry-pick %s', $new_branch);
|
||||||
} catch (Exception $ex) {}
|
} catch (Exception $ex) {
|
||||||
|
// do nothing
|
||||||
|
}
|
||||||
$repository_api->execxLocal('branch -D %s', $new_branch);
|
$repository_api->execxLocal('branch -D %s', $new_branch);
|
||||||
if ($ex) {
|
if ($ex) {
|
||||||
echo phutil_console_format(
|
echo phutil_console_format(
|
||||||
|
@ -858,15 +853,72 @@ EOTEXT
|
||||||
return array('ARGUMENT');
|
return array('ARGUMENT');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function applyDependencies(ArcanistBundle $bundle) {
|
||||||
|
// check for (and automagically apply on the user's be-hest) any revisions
|
||||||
|
// this patch depends on
|
||||||
|
$graph = $this->buildDependencyGraph($bundle);
|
||||||
|
if ($graph) {
|
||||||
|
$start_phid = $graph->getStartPHID();
|
||||||
|
$cycle_phids = $graph->detectCycles($start_phid);
|
||||||
|
if ($cycle_phids) {
|
||||||
|
$phids = array_keys($graph->getNodes());
|
||||||
|
$issue = 'The dependencies for this patch have a cycle. Applying them '.
|
||||||
|
'is not guaranteed to work. Continue anyway?';
|
||||||
|
$okay = phutil_console_confirm($issue, true);
|
||||||
|
} else {
|
||||||
|
$phids = $graph->getTopographicallySortedNodes();
|
||||||
|
$phids = array_reverse($phids);
|
||||||
|
$okay = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!$okay) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
$dep_on_revs = $this->getConduit()->callMethodSynchronous(
|
||||||
|
'differential.query',
|
||||||
|
array(
|
||||||
|
'phids' => $phids,
|
||||||
|
'arcanistProjects' => array($bundle->getProjectID())
|
||||||
|
));
|
||||||
|
$revs = array();
|
||||||
|
foreach ($dep_on_revs as $dep_on_rev) {
|
||||||
|
$revs[$dep_on_rev['phid']] = 'D'.$dep_on_rev['id'];
|
||||||
|
}
|
||||||
|
// order them in case we got a topological sort earlier
|
||||||
|
$revs = array_select_keys($revs, $phids);
|
||||||
|
if (!empty($revs)) {
|
||||||
|
$base_args = array(
|
||||||
|
'--force',
|
||||||
|
'--skip-dependencies',
|
||||||
|
'--nobranch');
|
||||||
|
if (!$this->shouldCommit()) {
|
||||||
|
$base_args[] = '--nocommit';
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($revs as $phid => $diff_id) {
|
||||||
|
// we'll apply this, the actual patch, later
|
||||||
|
// this should be the last in the list
|
||||||
|
if ($phid == $start_phid) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$args = $base_args;
|
||||||
|
$args[] = $diff_id;
|
||||||
|
$apply_workflow = $this->buildChildWorkflow(
|
||||||
|
'patch',
|
||||||
|
$args);
|
||||||
|
$apply_workflow->run();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Do the best we can to prevent PEBKAC and id10t issues.
|
* Do the best we can to prevent PEBKAC and id10t issues.
|
||||||
*/
|
*/
|
||||||
private function sanityCheck(ArcanistBundle $bundle) {
|
private function sanityCheck(ArcanistBundle $bundle) {
|
||||||
$repository_api = $this->getRepositoryAPI();
|
$repository_api = $this->getRepositoryAPI();
|
||||||
|
|
||||||
// Require clean working copy
|
|
||||||
$this->requireCleanWorkingCopy();
|
|
||||||
|
|
||||||
// Check to see if the bundle's project id matches the working copy
|
// Check to see if the bundle's project id matches the working copy
|
||||||
// project id
|
// project id
|
||||||
$bundle_project_id = $bundle->getProjectID();
|
$bundle_project_id = $bundle->getProjectID();
|
||||||
|
@ -953,8 +1005,6 @@ EOTEXT
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO -- more sanity checks here
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -1007,4 +1057,35 @@ EOTEXT
|
||||||
}
|
}
|
||||||
return $found_revision;
|
return $found_revision;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function buildDependencyGraph(ArcanistBundle $bundle) {
|
||||||
|
$graph = null;
|
||||||
|
if ($this->getRepositoryAPI() instanceof ArcanistSubversionAPI) {
|
||||||
|
return $graph;
|
||||||
|
}
|
||||||
|
$revision_id = $bundle->getRevisionID();
|
||||||
|
if ($revision_id) {
|
||||||
|
$revisions = $this->getConduit()->callMethodSynchronous(
|
||||||
|
'differential.query',
|
||||||
|
array(
|
||||||
|
'ids' => array($revision_id),
|
||||||
|
));
|
||||||
|
if ($revisions) {
|
||||||
|
$revision = head($revisions);
|
||||||
|
$rev_auxiliary = idx($revision, 'auxiliary', array());
|
||||||
|
$phids = idx($rev_auxiliary, 'phabricator:depends-on', array());
|
||||||
|
if ($phids) {
|
||||||
|
$revision_phid = $revision['phid'];
|
||||||
|
$graph = id(new ArcanistDifferentialDependencyGraph())
|
||||||
|
->setConduit($this->getConduit())
|
||||||
|
->setRepositoryAPI($this->getRepositoryAPI())
|
||||||
|
->setStartPHID($revision_phid)
|
||||||
|
->addNodes(array($revision_phid => $phids))
|
||||||
|
->loadGraph();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $graph;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue