Page MenuHomePhabricator
Files F13194692

D14357.diff
No OneTemporary
Actions

D14357.diff
View Options

diff --git a/src/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php
--- a/src/land/ArcanistGitLandEngine.php
+++ b/src/land/ArcanistGitLandEngine.php
@@ -167,6 +167,8 @@
$original_date,
$this->getCommitMessageFile());
+ $this->getWorkflow()->didCommitMerge();
+
list($stdout) = $api->execxLocal(
'rev-parse --verify %s',
'HEAD');
diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -1333,4 +1333,71 @@
$this->resolvedHeadCommit = null;
}
+ /**
+ * Follow the chain of tracking branches upstream until we reach a remote
+ * or cycle locally.
+ *
+ * @param string Ref to start from.
+ * @return list<wild> Path to an upstream.
+ */
+ public function getPathToUpstream($start) {
+ $cursor = $start;
+ $path = array();
+ while (true) {
+ list($err, $upstream) = $this->execManualLocal(
+ 'rev-parse --symbolic-full-name %s@{upstream}',
+ $cursor);
+
+ if ($err) {
+ // We ended up somewhere with no tracking branch, so we're done.
+ break;
+ }
+
+ $upstream = trim($upstream);
+
+ if (preg_match('(^refs/heads/)', $upstream)) {
+ $upstream = preg_replace('(^refs/heads/)', '', $upstream);
+
+ $is_cycle = isset($path[$upstream]);
+
+ $path[$cursor] = array(
+ 'type' => 'local',
+ 'name' => $upstream,
+ 'cycle' => $is_cycle,
+ );
+
+ if ($is_cycle) {
+ // We ran into a local cycle, so we're done.
+ break;
+ }
+
+ // We found another local branch, so follow that one upriver.
+ $cursor = $upstream;
+ continue;
+ }
+
+ if (preg_match('(^refs/remotes/)', $upstream)) {
+ $upstream = preg_replace('(^refs/remotes/)', '', $upstream);
+ list($remote, $branch) = explode('/', $upstream, 2);
+
+ $path[$cursor] = array(
+ 'type' => 'remote',
+ 'name' => $branch,
+ 'remote' => $remote,
+ );
+
+ // We found a remote, so we're done.
+ break;
+ }
+
+ throw new Exception(
+ pht(
+ 'Got unrecognized upstream format ("%s") from Git, expected '.
+ '"refs/heads/..." or "refs/remotes/...".',
+ $upstream));
+ }
+
+ return $path;
+ }
+
}
diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php
--- a/src/workflow/ArcanistLandWorkflow.php
+++ b/src/workflow/ArcanistLandWorkflow.php
@@ -38,7 +38,7 @@
public function getCommandSynopses() {
return phutil_console_format(<<<EOTEXT
- **land** [__options__] [__branch__] [--onto __master__]
+ **land** [__options__] [__ref__]
EOTEXT
);
}
@@ -47,18 +47,71 @@
return phutil_console_format(<<<EOTEXT
Supports: git, hg
- Land an accepted change (currently sitting in local feature branch
- __branch__) onto __master__ and push it to the remote. Then, delete
- the feature branch. If you omit __branch__, the current branch will
- be used.
+ Publish an accepted revision after review. This command is the last
+ step in the standard Differential pre-publish code review workflow.
- In mutable repositories, this will perform a --squash merge (the
- entire branch will be represented by one commit on __master__). In
- immutable repositories (or when --merge is provided), it will perform
- a --no-ff merge (the branch will always be merged into __master__ with
- a merge commit).
+ This command merges and pushes changes associated with an accepted
+ revision that are currently sitting in __ref__, which is usually the
+ name of a local branch. Without __ref__, the current working copy
+ state will be used.
+
+ Under Git: branches, tags, and arbitrary commits (detached HEADs)
+ may be landed.
+
+ Under Mercurial: branches and bookmarks may be landed, but only
+ onto a target of the same type. See T3855.
+
+ The workflow selects a target branch to land onto and a remote where
+ the change will be pushed to.
+
+ A target branch is selected by examining these sources in order:
+
+ - the **--onto** flag;
+ - the upstream of the current branch, recursively (Git only);
+ - the __arc.land.onto.default__ configuration setting;
+ - or by falling back to a standard default:
+ - "master" in Git;
+ - "default" in Mercurial.
+
+ A remote is selected by examining these sources in order:
+
+ - the **--remote** flag;
+ - the upstream of the current branch, recursively (Git only);
+ - or by falling back to a standard default:
+ - "origin" in Git;
+ - the default remote in Mercurial.
+
+ After selecting a target branch and a remote, the commits which will
+ be landed are printed.
+
+ With **--preview**, execution stops here, before the change is
+ merged.
+
+ The change is merged into the target branch, following these rules:
+
+ In mutable repositories or with **--squash**, this will perform a
+ squash merge (the entire branch will be represented as one commit on
+ the target branch).
+
+ In immutable repositories or with **--merge**, this will perform a
+ strict merge (a merge commit will always be created, and local
+ commits will be preserved).
+
+ The resulting commit will be given an up-to-date commit message
+ describing the final state of the revision in Differential.
+
+ With **--hold**, execution stops here, before the change is pushed.
+
+ The change is pushed into the remote.
+
+ Consulting mystical sources of power, the workflow makes a guess
+ about what state you wanted to end up in after the process finishes
+ and the working copy is put into that state.
+
+ The branch which was landed is deleted, unless the **--keep-branch**
+ flag was passed or the landing branch is the same as the target
+ branch.
- Under hg, bookmarks can be landed the same way as branches.
EOTEXT
);
}
@@ -203,6 +256,8 @@
}
if ($engine) {
+ $this->readEngineArguments();
+
$obsolete = array(
'delete-remote',
'update-with-merge',
@@ -236,7 +291,7 @@
$engine->execute();
- if (!$should_hold) {
+ if (!$should_hold && !$this->preview) {
$this->didPush();
}
@@ -303,6 +358,137 @@
return null;
}
+ private function readEngineArguments() {
+ // NOTE: This is hard-coded for Git right now.
+ // TODO: Clean this up and move it into LandEngines.
+
+ $onto = $this->getEngineOnto();
+ $remote = $this->getEngineRemote();
+
+ // This just overwrites work we did earlier, but it has to be up in this
+ // class for now because other parts of the workflow still depend on it.
+ $this->onto = $onto;
+ $this->remote = $remote;
+ $this->ontoRemoteBranch = $this->remote.'/'.$onto;
+ }
+
+ private function getEngineOnto() {
+ $onto = $this->getArgument('onto');
+ if ($onto !== null) {
+ $this->writeInfo(
+ pht('TARGET'),
+ pht(
+ 'Landing onto "%s", selected by the --onto flag.',
+ $onto));
+ return $onto;
+ }
+
+ $api = $this->getRepositoryAPI();
+ $path = $api->getPathToUpstream($this->branch);
+
+ if ($path) {
+ $last = last($path);
+ if (isset($last['cycle'])) {
+ $this->writeWarn(
+ pht('LOCAL CYCLE'),
+ pht(
+ 'Local branch tracks an upstream, but following it leads to a '.
+ 'local cycle; ignoring branch upstream.'));
+
+ echo tsprintf(
+ "\n %s\n\n",
+ $this->formatUpstreamPathCycle($path));
+
+ } else {
+ if ($last['type'] == 'remote') {
+ $onto = $last['name'];
+ $this->writeInfo(
+ pht('TARGET'),
+ pht(
+ 'Landing onto "%s", selected by following tracking branches '.
+ 'upstream to the closest remote.',
+ $onto));
+ return $onto;
+ } else {
+ $this->writeInfo(
+ pht('NO PATH TO UPSTREAM'),
+ pht(
+ 'Local branch tracks an upstream, but there is no path '.
+ 'to a remote; ignoring branch upstream.'));
+ }
+ }
+ }
+
+ $config_key = 'arc.land.onto.default';
+ $onto = $this->getConfigFromAnySource($config_key);
+ if ($onto !== null) {
+ $this->writeInfo(
+ pht('TARGET'),
+ pht(
+ 'Landing onto "%s", selected by "%s" configuration.',
+ $onto,
+ $config_key));
+ return $onto;
+ }
+
+ $onto = 'master';
+ $this->writeInfo(
+ pht('TARGET'),
+ pht(
+ 'Landing onto "%s", the default target under git.',
+ $onto));
+ return $onto;
+ }
+
+ private function getEngineRemote() {
+ $remote = $this->getArgument('remote');
+ if ($remote !== null) {
+ $this->writeInfo(
+ pht('REMOTE'),
+ pht(
+ 'Using remote "%s", selected by the --remote flag.',
+ $remote));
+ return $remote;
+ }
+
+ $api = $this->getRepositoryAPI();
+ $path = $api->getPathToUpstream($this->branch);
+
+ if ($path) {
+ $last = last($path);
+ if ($last['type'] == 'remote') {
+ $remote = $last['remote'];
+ $this->writeInfo(
+ pht('REMOTE'),
+ pht(
+ 'Using remote "%s", selected by following tracking branches '.
+ 'upstream to the closest remote.',
+ $remote));
+ return $remote;
+ }
+ }
+
+ $remote = 'origin';
+ $this->writeInfo(
+ pht('REMOTE'),
+ pht(
+ 'Using remote "%s", the default remote under git.',
+ $remote));
+ return $remote;
+ }
+
+ private function formatUpstreamPathCycle(array $cycle) {
+ $parts = array();
+ foreach ($cycle as $key => $value) {
+ $parts[] = $key;
+ }
+ $parts[] = idx(last($cycle), 'name');
+ $parts[] = pht('...');
+
+ return implode(' -> ', $parts);
+ }
+
+
private function readArguments() {
$repository_api = $this->getRepositoryAPI();
$this->isGit = $repository_api instanceof ArcanistGitAPI;
@@ -320,9 +506,12 @@
$branch = $this->getArgument('branch');
if (empty($branch)) {
$branch = $this->getBranchOrBookmark();
-
if ($branch) {
$this->branchType = $this->getBranchType($branch);
+
+ // TODO: This message is misleading when landing a detached head or
+ // a tag in Git.
+
echo pht("Landing current %s '%s'.", $this->branchType, $branch), "\n";
$branch = array($branch);
}
@@ -1079,9 +1268,7 @@
// We dispatch this event so we can run checks on the merged revision,
// right before it gets pushed out. It's easier to do this in arc land
// than to try to hook into git/hg.
- $this->dispatchEvent(
- ArcanistEventType::TYPE_LAND_WILLPUSHREVISION,
- array());
+ $this->didCommitMerge();
} catch (Exception $ex) {
$this->executeCleanupAfterFailedPush();
throw $ex;
@@ -1366,6 +1553,12 @@
$engine->setCommitMessageFile($this->messageFile);
}
+ public function didCommitMerge() {
+ $this->dispatchEvent(
+ ArcanistEventType::TYPE_LAND_WILLPUSHREVISION,
+ array());
+ }
+
public function didPush() {
$this->askForRepositoryUpdate();

File Metadata

Mime Type
text/plain
Expires
Mon, May 13, 9:47 PM (2 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6275858
Default Alt Text
D14357.diff (11 KB)

Event Timeline

Phabricator · Built by Phacility