From c04f141ab0231e593a513356b3832a30f9404627 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 9 Jun 2017 06:52:18 -0700 Subject: [PATCH] Remove obsolete "arc land" flags: --update-with-merge/rebase, --delete-remote Summary: Fixes T12815. During the last update to "arc land", some flags were disabled but remained in place in case we needed to retain them. It now seems reasonably clear that we do not. The "rebase" and "merge" strategies for landing were replaced by a better "headless" strategy which seems to avoid the original issues, so these flags no longer do anything or reasonably could do anything. `--delete-remote` is slightly more ambiguous (e.g., see T12650 and maybe others) but the only real use case is "git push = save changes". Test Plan: Ran `arc land --update-with-rebase`, was told the flag does not exist. Grepped for affected flags/symbols. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12815 Differential Revision: https://secure.phabricator.com/D18108 --- src/configuration/ArcanistSettings.php | 7 -- src/workflow/ArcanistLandWorkflow.php | 125 +------------------------ 2 files changed, 3 insertions(+), 129 deletions(-) diff --git a/src/configuration/ArcanistSettings.php b/src/configuration/ArcanistSettings.php index f87fb356..fdf68259 100644 --- a/src/configuration/ArcanistSettings.php +++ b/src/configuration/ArcanistSettings.php @@ -80,13 +80,6 @@ final class ArcanistSettings extends Phobject { 'arc land'), 'example' => '"develop"', ), - 'arc.land.update.default' => array( - 'type' => 'string', - 'help' => pht( - 'The default strategy to use when arc land updates the feature '. - 'branch. Supports "rebase" and "merge" strategies.'), - 'example' => '"rebase"', - ), 'arc.lint.cache' => array( 'type' => 'bool', 'help' => pht( diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index ee59a8d2..66ce42d5 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -17,7 +17,6 @@ final class ArcanistLandWorkflow extends ArcanistWorkflow { private $remote; private $useSquash; private $keepBranch; - private $shouldUpdateWithRebase; private $branchType; private $ontoType; private $preview; @@ -197,43 +196,8 @@ EOTEXT 'conflicts' => array( 'keep-branch' => true, ), - ), - 'update-with-rebase' => array( - 'help' => pht( - "When updating the feature branch, use rebase instead of merge. ". - "This might make things work better in some cases. Set ". - "%s to '%s' to make this the default.", - 'arc.land.update.default', - 'rebase'), - 'conflicts' => array( - 'merge' => pht( - 'The %s strategy does not update the feature branch.', - '--merge'), - 'update-with-merge' => pht( - 'Cannot be used with %s.', - '--update-with-merge'), - ), 'supports' => array( - 'git', - ), - ), - 'update-with-merge' => array( - 'help' => pht( - "When updating the feature branch, use merge instead of rebase. ". - "This is the default behavior. Setting %s to '%s' can also be ". - "used to make this the default.", - 'arc.land.update.default', - 'merge'), - 'conflicts' => array( - 'merge' => pht( - 'The %s strategy does not update the feature branch.', - '--merge'), - 'update-with-rebase' => pht( - 'Cannot be used with %s.', - '--update-with-rebase'), - ), - 'supports' => array( - 'git', + 'hg', ), ), 'revision' => array( @@ -261,22 +225,6 @@ EOTEXT if ($engine) { $this->readEngineArguments(); - - $obsolete = array( - 'delete-remote', - 'update-with-merge', - 'update-with-rebase', - ); - - foreach ($obsolete as $flag) { - if ($this->getArgument($flag)) { - throw new ArcanistUsageException( - pht( - 'Flag "%s" is no longer supported under Git.', - '--'.$flag)); - } - } - $this->requireCleanWorkingCopy(); $should_hold = $this->getArgument('hold'); @@ -514,14 +462,6 @@ EOTEXT $this->branch = head($branch); $this->keepBranch = $this->getArgument('keep-branch'); - $update_strategy = $this->getConfigFromAnySource('arc.land.update.default'); - $this->shouldUpdateWithRebase = $update_strategy == 'rebase'; - if ($this->getArgument('update-with-rebase')) { - $this->shouldUpdateWithRebase = true; - } else if ($this->getArgument('update-with-merge')) { - $this->shouldUpdateWithRebase = false; - } - $this->preview = $this->getArgument('preview'); if (!$this->branchType) { @@ -951,42 +891,7 @@ EOTEXT $repository_api = $this->getRepositoryAPI(); chdir($repository_api->getPath()); - if ($this->isGit) { - if ($this->shouldUpdateWithRebase) { - echo phutil_console_format(pht( - 'Rebasing **%s** onto **%s**', - $this->branch, - $this->onto)."\n"); - $err = phutil_passthru('git rebase %s', $this->onto); - if ($err) { - throw new ArcanistUsageException(pht( - "'%s' failed. You can abort with '%s', or resolve conflicts ". - "and use '%s' to continue forward. After resolving the rebase, ". - "run '%s' again.", - sprintf('git rebase %s', $this->onto), - 'git rebase --abort', - 'git rebase --continue', - 'arc land')); - } - } else { - echo phutil_console_format(pht( - 'Merging **%s** into **%s**', - $this->branch, - $this->onto)."\n"); - $err = phutil_passthru( - 'git merge --no-stat %s -m %s', - $this->onto, - pht("Automatic merge by '%s'", 'arc land')); - if ($err) { - throw new ArcanistUsageException(pht( - "'%s' failed. To continue: resolve the conflicts, commit ". - "the changes, then run '%s' again. To abort: run '%s'.", - sprintf('git merge %s', $this->onto), - 'arc land', - 'git merge --abort')); - } - } - } else if ($this->isHg) { + if ($this->isHg) { $onto_tip = $repository_api->getCanonicalRevisionName($this->onto); $common_ancestor = $repository_api->getCanonicalRevisionName( hgsprintf('ancestor(%s, %s)', $this->onto, $this->branch)); @@ -1367,31 +1272,7 @@ EOTEXT } if ($this->getArgument('delete-remote')) { - if ($this->isGit) { - list($err, $ref) = $repository_api->execManualLocal( - 'rev-parse --verify %s/%s', - $this->remote, - $this->branch); - - if ($err) { - echo pht( - 'No remote feature %s to clean up.', - $this->branchType); - echo "\n"; - } else { - - // NOTE: In Git, you delete a remote branch by pushing it with a - // colon in front of its name: - // - // git push : - - echo pht('Cleaning up remote feature %s...', $this->branchType), "\n"; - $repository_api->execxLocal( - 'push %s :%s', - $this->remote, - $this->branch); - } - } else if ($this->isHg) { + if ($this->isHg) { // named branches were closed as part of the earlier commit // so only worry about bookmarks if ($repository_api->isBookmark($this->branch)) {