Page MenuHomePhabricator
Files F15540461

D17951.id.diff
No OneTemporary
Actions

D17951.id.diff
View Options

diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php
--- a/src/workflow/ArcanistPatchWorkflow.php
+++ b/src/workflow/ArcanistPatchWorkflow.php
@@ -427,6 +427,25 @@
$this->requireCleanWorkingCopy();
}
+ $dependencies = array();
+ if ($this->shouldApplyDependencies()) {
+ $dependencies = $this->getDependencyList($bundle);
+ if (!empty($dependencies)) {
+ // If there are dependencies, determine the "based on" information for
+ // the first of them, and use that as our "based on" commit so we branch
+ // from the appropriate place, below. Values in $dep_data are diff ids;
+ // see `getDependencyList`
+ $dep_data = array_values($dependencies);
+ $diffs = $this->getConduit()->callMethodSynchronous(
+ 'differential.querydiffs',
+ array(
+ 'ids' => array($dep_data[0]),
+ ));
+ $bundle->setBaseRevision(
+ $diffs[$dep_data[0]]['sourceControlBaseRevision']);
+ }
+ }
+
$repository_api = $this->getRepositoryAPI();
$has_base_revision = $repository_api->hasLocalCommit(
$bundle->getBaseRevision());
@@ -442,36 +461,17 @@
}
}
- if ($this->canBranch() &&
- ($this->shouldBranch() ||
- ($this->shouldCommit() && $has_base_revision))) {
-
- if ($repository_api instanceof ArcanistGitAPI) {
- $original_branch = $repository_api->getBranchName();
- } else if ($repository_api instanceof ArcanistMercurialAPI) {
- $original_branch = $repository_api->getActiveBookmark();
- }
-
- // If we weren't on a branch, then record the ref we'll return to
- // instead.
- if ($original_branch === null) {
- if ($repository_api instanceof ArcanistGitAPI) {
- $original_branch = $repository_api->getCanonicalRevisionName('HEAD');
- } else if ($repository_api instanceof ArcanistMercurialAPI) {
- $original_branch = $repository_api->getCanonicalRevisionName('.');
- }
- }
-
- $new_branch = $this->createBranch($bundle, $has_base_revision);
- }
- if (!$has_base_revision && $this->shouldApplyDependencies()) {
- $this->applyDependencies($bundle);
- }
-
if ($sanity_check) {
$this->sanityCheck($bundle);
}
+ if ($this->canBranch() && $this->shouldBranch()) {
+ $new_branch = $this->createBranch($bundle, $has_base_revision);
+ }
+ if ($this->shouldApplyDependencies()) {
+ $this->applyDependencies($dependencies);
+ }
+
if ($repository_api instanceof ArcanistSubversionAPI) {
$patch_err = 0;
@@ -742,25 +742,6 @@
$verb = pht('applied');
}
- if ($this->canBranch() &&
- !$this->shouldBranch() &&
- $this->shouldCommit() && $has_base_revision) {
- $repository_api->execxLocal('checkout %s', $original_branch);
- $ex = null;
- try {
- $repository_api->execxLocal('cherry-pick %s', $new_branch);
- } catch (Exception $ex) {
- // do nothing
- }
- $repository_api->execxLocal('branch -D %s', $new_branch);
- if ($ex) {
- echo phutil_console_format(
- "\n<bg:red>** %s**</bg>\n",
- pht('Cherry Pick Failed!'));
- throw $ex;
- }
- }
-
echo phutil_console_format(
"<bg:green>** %s **</bg> %s\n",
pht('OKAY'),
@@ -807,30 +788,6 @@
$future->write($commit_message);
$future->resolvex();
- if (!$this->shouldBranch() && $has_base_revision) {
- $original_rev = $repository_api->getCanonicalRevisionName(
- $original_branch);
- $current_parent = $repository_api->getCanonicalRevisionName(
- hgsprintf('%s^', $new_branch));
-
- $err = 0;
- if ($original_rev != $current_parent) {
- list($err) = $repository_api->execManualLocal(
- 'rebase --dest %s --rev %s',
- hgsprintf('%s', $original_branch),
- hgsprintf('%s', $new_branch));
- }
-
- $repository_api->execxLocal('bookmark --delete %s', $new_branch);
- if ($err) {
- $repository_api->execManualLocal('rebase --abort');
- throw new ArcanistUsageException(
- phutil_console_format(
- "\n<bg:red>** %s**</bg>\n",
- pht('Rebase onto %s failed!', $original_branch)));
- }
- }
-
$verb = pht('committed');
} else {
$verb = pht('applied');
@@ -899,64 +856,69 @@
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
+ private function getDependencyList(ArcanistBundle $bundle) {
+ // Returns a (possibly-empty) list of revisions, in the proper order, which
+ // must be applied before this one can be. Keys are revision PHIDs, values
+ // are their most recent diff IDs.
$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 = pht(
- '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 (!$graph) {
+ return array();
+ }
+ $start_phid = $graph->getStartPHID();
+ $cycle_phids = $graph->detectCycles($start_phid);
+ if ($cycle_phids) {
+ $phids = array_keys($graph->getNodes());
+ $issue = pht(
+ 'The dependencies for this patch have a cycle. Applying them '.
+ 'is not guaranteed to work. Continue anyway?');
+ $okay = phutil_console_confirm($issue, true);
if (!$okay) {
- return;
+ throw new ArcanistUserAbortException();
}
+ } else {
+ $phids = $graph->getTopographicallySortedNodes();
+ $phids = array_reverse($phids);
+ }
- $dep_on_revs = $this->getConduit()->callMethodSynchronous(
- 'differential.query',
- array(
- 'phids' => $phids,
- ));
- $revs = array();
- foreach ($dep_on_revs as $dep_on_rev) {
- $revs[$dep_on_rev['phid']] = 'D'.$dep_on_rev['id'];
+ $dep_on_revs = $this->getConduit()->callMethodSynchronous(
+ 'differential.query',
+ array(
+ 'phids' => $phids,
+ ));
+ $revs = array();
+ foreach ($dep_on_revs as $dep_on_rev) {
+ // Omit the actual diff we're looking for
+ if ($dep_on_rev['phid'] == $start_phid) {
+ continue;
}
- // 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';
- }
+ // Store diff ids, which we can both pass to `arc patch`, as well as use
+ // to look up "based on" data (unlike revision ids)
+ $revs[$dep_on_rev['phid']] = $dep_on_rev['diffs'][0];
+ }
+ // order them in case we got a topological sort earlier
+ $revs = array_select_keys($revs, $phids);
+ return $revs;
+ }
- 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();
- }
- }
+ private function applyDependencies($revs) {
+ $base_args = array(
+ '--force',
+ '--skip-dependencies',
+ '--nobranch',
+ );
+ if (!$this->shouldCommit()) {
+ $base_args[] = '--nocommit';
+ }
+ $base_args[] = '--diff';
+
+ foreach ($revs as $phid => $diff_id) {
+ $args = $base_args;
+ $args[] = $diff_id;
+ $apply_workflow = $this->buildChildWorkflow(
+ 'patch',
+ $args);
+ $apply_workflow->run();
}
}

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 26, 7:43 AM (8 h, 2 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7228595
Default Alt Text
D17951.id.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility