Page MenuHomePhabricator

D14361.diff
No OneTemporary

D14361.diff

diff --git a/src/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php
--- a/src/land/ArcanistGitLandEngine.php
+++ b/src/land/ArcanistGitLandEngine.php
@@ -158,10 +158,25 @@
}
} catch (Exception $ex) {
$api->execManualLocal('merge --abort');
+ $api->execManualLocal('reset --hard HEAD --');
- // TODO: Maybe throw a better or more helpful exception here?
+ throw new Exception(
+ pht(
+ 'Local "%s" does not merge cleanly into "%s". Merge or rebase '.
+ 'local changes so they can merge cleanly.',
+ $this->getSourceRef(),
+ $this->getTargetFullRef()));
+ }
- throw $ex;
+ list($changes) = $api->execxLocal('diff HEAD --');
+ $changes = trim($changes);
+ if (!strlen($changes)) {
+ throw new Exception(
+ pht(
+ 'Merging local "%s" into "%s" produces an empty diff. '.
+ 'This usually means these changes have already landed.',
+ $this->getSourceRef(),
+ $this->getTargetFullRef()));
}
$api->execxLocal(
@@ -210,100 +225,221 @@
if ($this->localRef != $this->getSourceRef()) {
// The user ran `arc land X` but was on a different branch, so just put
// them back wherever they were before.
- echo tsprintf(
- "%s\n",
+ $this->writeInfo(
+ pht('RESTORE'),
pht('Switching back to "%s".', $this->localRef));
$this->restoreLocalState();
return;
}
- list($err) = $api->execManualLocal(
- 'rev-parse --verify %s',
- $this->getTargetOnto());
- if ($err) {
- echo tsprintf(
- "%s\n",
+ // We're going to try to find a path to the upstream target branch. We
+ // try in two different ways:
+ //
+ // - follow the source branch directly along tracking branches until
+ // we reach the upstream; or
+ // - follow a local branch with the same name as the target branch until
+ // we reach the upstream.
+
+ // First, get the path from whatever we landed to wherever it goes.
+ $local_branch = $this->getSourceRef();
+
+ $path = $api->getPathToUpstream($local_branch);
+ if ($path->getLength()) {
+ // We may want to discard the thing we landed from the path, if we're
+ // going to delete it. In this case, we don't want to update it or worry
+ // if it's dirty.
+ if ($this->getSourceRef() == $this->getTargetOnto()) {
+ // In this case, we've done something like land "master" onto itself,
+ // so we do want to update the actual branch. We're going to use the
+ // entire path.
+ } else {
+ // Otherwise, we're going to delete the branch at the end of the
+ // workflow, so throw it away the most-local branch that isn't long
+ // for this world.
+ $path->removeUpstream($local_branch);
+
+ if (!$path->getLength()) {
+ $this->writeInfo(
+ pht('UPDATE'),
+ pht(
+ 'Local branch "%s" directly tracks remote, staying on '.
+ 'detached HEAD.',
+ $local_branch));
+ return;
+ }
+
+ $local_branch = head($path->getLocalBranches());
+ }
+ } else {
+ // The source branch has no upstream, so look for a local branch with
+ // the same name as the target branch. This corresponds to the common
+ // case where you have "master" and checkout local branches from it
+ // with "git checkout -b feature", then land onto "master".
+
+ $local_branch = $this->getTargetOnto();
+
+ list($err) = $api->execManualLocal(
+ 'rev-parse --verify %s',
+ $local_branch);
+ if ($err) {
+ $this->writeInfo(
+ pht('UPDATE'),
+ pht(
+ 'Local branch "%s" does not exist, staying on detached HEAD.',
+ $local_branch));
+ return;
+ }
+
+ $path = $api->getPathToUpstream($local_branch);
+ }
+
+ if ($path->getCycle()) {
+ $this->writeWarn(
+ pht('LOCAL CYCLE'),
pht(
- 'Local branch "%s" does not exist, staying on detached HEAD.',
- $this->getTargetOnto()));
+ 'Local branch "%s" tracks an upstream but following it leads to '.
+ 'a local cycle, staying on detached HEAD.',
+ $local_branch));
return;
}
- list($err, $upstream) = $api->execManualLocal(
- 'rev-parse --verify --symbolic-full-name %s',
- $this->getTargetOnto().'@{upstream}');
- if ($err) {
- echo tsprintf(
- "%s\n",
+ if (!$path->isConnectedToRemote()) {
+ $this->writeInfo(
+ pht('UPDATE'),
pht(
- 'Local branch "%s" has no upstream, staying on detached HEAD.',
- $this->getTargetOnto()));
+ 'Local branch "%s" is not connected to a remote, staying on '.
+ 'detached HEAD.',
+ $local_branch));
return;
}
- $upstream = trim($upstream);
- $expect_upstream = 'refs/remotes/'.$this->getTargetFullRef();
- if ($upstream != $expect_upstream) {
- echo tsprintf(
- "%s\n",
+ $remote_remote = $path->getRemoteRemoteName();
+ $remote_branch = $path->getRemoteBranchName();
+
+ $remote_actual = $remote_remote.'/'.$remote_branch;
+ $remote_expect = $this->getTargetFullRef();
+ if ($remote_actual != $remote_expect) {
+ $this->writeInfo(
+ pht('UPDATE'),
pht(
- 'Local branch "%s" tracks remote "%s" (not target remote "%s"), '.
- 'staying on detached HEAD.',
- $this->getTargetOnto(),
- $upstream,
- $expect_upstream));
+ 'Local branch "%s" is connected to a remote ("%s") other than '.
+ 'the target remote ("%s"), staying on detached HEAD.',
+ $local_branch,
+ $remote_actual,
+ $remote_expect));
return;
}
- list($stdout) = $api->execxLocal(
- 'log %s..%s --',
- $this->mergedRef,
- $this->getTargetOnto());
- $stdout = trim($stdout);
+ // If we get this far, we have a sequence of branches which ultimately
+ // connect to the remote. We're going to try to update them all in reverse
+ // order, from most-upstream to most-local.
+
+ $cascade_branches = $path->getLocalBranches();
+ $cascade_branches = array_reverse($cascade_branches);
+
+ // First, check if any of them are ahead of the remote.
- if (!strlen($stdout)) {
- echo tsprintf(
- "%s\n",
+ $ahead_of_remote = array();
+ foreach ($cascade_branches as $cascade_branch) {
+ list($stdout) = $api->execxLocal(
+ 'log %s..%s --',
+ $this->mergedRef,
+ $cascade_branch);
+ $stdout = trim($stdout);
+
+ if (strlen($stdout)) {
+ $ahead_of_remote[$cascade_branch] = $cascade_branch;
+ }
+ }
+
+ // We're going to handle the last branch (the thing we ultimately intend
+ // to check out) differently. It's OK if it's ahead of the remote, as long
+ // as we just landed it.
+
+ $local_ahead = isset($ahead_of_remote[$local_branch]);
+ unset($ahead_of_remote[$local_branch]);
+ $land_self = ($this->getTargetOnto() === $this->getSourceRef());
+
+ // We aren't going to pull anything if anything upstream from us is ahead
+ // of the remote, or the local is ahead of the remote and we didn't land
+ // it onto itself.
+ $skip_pull = ($ahead_of_remote || ($local_ahead && !$land_self));
+
+ if ($skip_pull) {
+ $this->writeInfo(
+ pht('UPDATE'),
pht(
- 'Local "%s" tracks target remote "%s", checking out and '.
- 'pulling changes.',
- $this->getTargetOnto(),
- $this->getTargetFullRef()));
+ 'Local "%s" is ahead of remote "%s". Checking out "%s" but '.
+ 'not pulling changes.',
+ nonempty(head($ahead_of_remote), $local_branch),
+ $this->getTargetFullRef(),
+ $local_branch));
- $api->execxLocal('checkout %s --', $this->getTargetOnto());
- $api->execxLocal('pull --');
+ $this->writeInfo(
+ pht('CHECKOUT'),
+ pht(
+ 'Checking out "%s".',
+ $local_branch));
+
+ $api->execxLocal('checkout %s --', $local_branch);
return;
}
- if ($this->getTargetOnto() !== $this->getSourceRef()) {
- echo tsprintf(
- "%s\n",
+ // If nothing upstream from our nearest branch is ahead of the remote,
+ // pull it all.
+
+ $cascade_targets = array();
+ if (!$ahead_of_remote) {
+ foreach ($cascade_branches as $cascade_branch) {
+ if ($local_ahead && ($local_branch == $cascade_branch)) {
+ continue;
+ }
+ $cascade_targets[] = $cascade_branch;
+ }
+ }
+
+ if ($cascade_targets) {
+ $this->writeInfo(
+ pht('UPDATE'),
pht(
- 'Local "%s" is ahead of remote "%s". Checking out but '.
- 'not pulling changes.',
- $this->getTargetOnto(),
+ 'Local "%s" tracks target remote "%s", checking out and '.
+ 'pulling changes.',
+ $local_branch,
$this->getTargetFullRef()));
- $api->execxLocal('checkout %s --', $this->getTargetOnto());
+ foreach ($cascade_targets as $cascade_branch) {
+ $this->writeInfo(
+ pht('PULL'),
+ pht(
+ 'Checking out and pulling "%s".',
+ $cascade_branch));
- return;
+ $api->execxLocal('checkout %s --', $cascade_branch);
+ $api->execxLocal('pull --');
+ }
+
+ if (!$local_ahead) {
+ return;
+ }
}
// In this case, the user did something like land a branch onto itself,
// and the branch is tracking the correct remote. We're going to discard
// the local state and reset it to the state we just pushed.
- echo tsprintf(
- "%s\n",
+ $this->writeInfo(
+ pht('RESET'),
pht(
'Local "%s" landed into remote "%s", resetting local branch to '.
'remote state.',
$this->getTargetOnto(),
$this->getTargetFullRef()));
- $api->execxLocal('checkout %s --', $this->getTargetOnto());
+ $api->execxLocal('checkout %s --', $local_branch);
$api->execxLocal('reset --hard %s --', $this->getTargetFullRef());
+
+ return;
}
private function destroyLocalBranch() {
diff --git a/src/repository/api/ArcanistGitUpstreamPath.php b/src/repository/api/ArcanistGitUpstreamPath.php
--- a/src/repository/api/ArcanistGitUpstreamPath.php
+++ b/src/repository/api/ArcanistGitUpstreamPath.php
@@ -13,6 +13,11 @@
return $this;
}
+ public function removeUpstream($key) {
+ unset($this->path[$key]);
+ return $this;
+ }
+
public function getUpstream($key) {
return idx($this->path, $key);
}
@@ -36,6 +41,9 @@
return ($last['type'] == self::TYPE_REMOTE);
}
+ public function getLocalBranches() {
+ return array_keys($this->path);
+ }
public function getRemoteBranchName() {
if (!$this->isConnectedToRemote()) {

File Metadata

Mime Type
text/plain
Expires
Sun, Jan 12, 6:43 AM (20 h, 50 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6986491
Default Alt Text
D14361.diff (10 KB)

Event Timeline