1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-10 00:42:40 +01:00

Refactor how Mercurial runs commands that require extensions

Summary:
Currently there are a number of `--config extensions.blah=` sprinkled throughout the Mercurial APIs which aren't pleasant to look at. Additionally it turns out the `rebase` command requires the `rebase` extension which is not turned on by default. Uses of `rebase` should also be including this flag to enable the extension when it's in use.

This adds `ArcanistMercurialAPI::buildMercurialExtensionCommand()` which allows running a Mercurial command that requires an extension just by naming the extension instead of providing the full `--config..` argument.

It can be used e.g.
```lang=php
$api->execxLocalWithExtension(
  'strip',
  'strip --rev %s --',
  $rev);

$api->execxLocalWithExtension(
  'arc-hg',
  'arc-amend --logfile %s',
  $tmp_file);
```
Which produces
```lang=console
$ hg --encoding utf-8 --config 'extensions.strip=' strip --rev 82567759e2d703e1e0497f5f41363de3a1500188 --

$ hg --encoding utf-8 --config 'extensions.arg-hg=/Users/cspeckrun/Source/phacility/arcanist/support/hg/arc-hg.py' arc-amend --logfile /private/var/folders/cg/364w44254v5767ydf_x1tg_80000gn/T/6bwck66ccx0kwskw/89989-5F8JaL
```

Refs T13659

Test Plan: I ran through several scenarios of running `arc diff` and `arc land` with uncommitted changes on non-head commits, while not having the `evolve` extension enabled. Using the `--trace` argument I verified that `rebase`, `strip`, `arc-amend`, and `arc-ls-markers` were all invoked and the corresponding `arc diff` and `arc land` commands succeeded. I also ran `arc land --onto-remote default` while `arc-hg.py` raised an exception for the "remote" case and verified that the error was caught by Arcanist and displayed the error and prevented further execution. I removed the error from `arc-hg.py` and re-ran the command and verified it succeeded.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T13659

Differential Revision: https://secure.phabricator.com/D21697
This commit is contained in:
Christopher Speck 2021-07-20 00:27:29 -04:00
parent 35c1b9bf02
commit ac365c3ee5
4 changed files with 165 additions and 63 deletions

View file

@ -1160,8 +1160,9 @@ final class ArcanistMercurialLandEngine
'prune --rev %s', 'prune --rev %s',
$rev_set); $rev_set);
} else { } else {
$api->execxLocal( $api->execxLocalWithExtension(
'--config extensions.strip= strip --rev %s', 'strip',
'strip --rev %s',
$rev_set); $rev_set);
} }
} }

View file

@ -5,6 +5,13 @@
*/ */
final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
/**
* Mercurial deceptively indicates that the default encoding is UTF-8 however
* however the actual default appears to be "something else", at least on
* Windows systems. Force all mercurial commands to use UTF-8 encoding.
*/
const ROOT_HG_COMMAND = 'hg --encoding utf-8 ';
private $branch; private $branch;
private $localCommitInfo; private $localCommitInfo;
private $rawDiffCache = array(); private $rawDiffCache = array();
@ -13,28 +20,24 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
private $featureFutures = array(); private $featureFutures = array();
protected function buildLocalFuture(array $argv) { protected function buildLocalFuture(array $argv) {
$env = $this->getMercurialEnvironmentVariables(); $argv[0] = self::ROOT_HG_COMMAND.$argv[0];
// Mercurial deceptively indicates that the default encoding is UTF-8 return $this->newConfiguredFuture(newv('ExecFuture', $argv));
// however the actual default appears to be "something else", at least on
// Windows systems. Force all mercurial commands to use UTF-8 encoding.
$argv[0] = 'hg --encoding utf-8 '.$argv[0];
$future = newv('ExecFuture', $argv)
->setEnv($env)
->setCWD($this->getPath());
return $future;
} }
public function newPassthru($pattern /* , ... */) { public function newPassthru($pattern /* , ... */) {
$args = func_get_args(); $args = func_get_args();
$args[0] = self::ROOT_HG_COMMAND.$args[0];
return $this->newConfiguredFuture(newv('PhutilExecPassthru', $args));
}
private function newConfiguredFuture(PhutilExecutableFuture $future) {
$args = func_get_args();
$env = $this->getMercurialEnvironmentVariables(); $env = $this->getMercurialEnvironmentVariables();
$args[0] = 'hg '.$args[0]; return $future
return newv('PhutilExecPassthru', $args)
->setEnv($env) ->setEnv($env)
->setCWD($this->getPath()); ->setCWD($this->getPath());
} }
@ -730,14 +733,10 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
'log --template %s --rev . --', 'log --template %s --rev . --',
'{node}'); '{node}');
$argv = array(); $this->execxLocalWithExtension(
foreach ($this->getMercurialExtensionArguments() as $arg) { 'arc-hg',
$argv[] = $arg; 'arc-amend --logfile %s',
} $tmp_file);
$argv[] = 'arc-amend';
$argv[] = '--logfile';
$argv[] = $tmp_file;
$this->execxLocal('%Ls', $argv);
list($new_commit) = $this->execxLocal( list($new_commit) = $this->execxLocal(
'log --rev tip --template %s --', 'log --rev tip --template %s --',
@ -753,13 +752,20 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
$rebase_args[] = $child; $rebase_args[] = $child;
} }
$this->execxLocal('rebase %Ls --', $rebase_args); $this->execxLocalWithExtension(
'rebase',
'rebase %Ls --',
$rebase_args);
} catch (CommandException $ex) { } catch (CommandException $ex) {
$this->execxLocal('rebase --abort --'); $this->execxLocalWithExtension(
'rebase',
'rebase --abort --');
throw $ex; throw $ex;
} }
$this->execxLocal('--config extensions.strip= strip --rev %s --', $this->execxLocalWithExtension(
'strip',
'strip --rev %s --',
$current); $current);
} }
@ -1034,6 +1040,115 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
return $this->executeMercurialFeatureTest($feature, true); return $this->executeMercurialFeatureTest($feature, true);
} }
/**
* Returns the necessary flag for using a Mercurial extension. This will
* enable Mercurial built-in extensions and the "arc-hg" extension that is
* included with Arcanist. This will not enable other extensions, e.g.
* "evolve".
*
* @param string The name of the extension to enable.
* @return string A new command pattern that includes the necessary flags to
* enable the specified extension.
*/
private function getMercurialExtensionFlag($extension) {
switch ($extension) {
case 'arc-hg':
$path = phutil_get_library_root('arcanist');
$path = dirname($path);
$path = $path.'/support/hg/arc-hg.py';
$ext_config = 'extensions.arg-hg='.$path;
break;
case 'rebase':
$ext_config = 'extensions.rebase=';
break;
case 'shelve':
$ext_config = 'extensions.shelve=';
break;
case 'strip':
$ext_config = 'extensions.strip=';
break;
default:
throw new Exception(
pht('Unknown Mercurial Extension: "%s".', $extension));
}
return csprintf('--config %s', $ext_config);
}
/**
* Produces the arguments that should be passed to Mercurial command
* execution that enables a desired extension.
*
* @param string The name of the extension to enable.
* @param string The command pattern that will be run with the extension
* enabled.
* @param array Parameters for the command pattern argument.
* @return array An array where the first item is a Mercurial command
* pattern that includes the necessary flag for enabling the
* desired extension, and all remaining items are parameters
* to that command pattern.
*/
private function buildMercurialExtensionCommand(
$extension,
$pattern /* , ... */) {
$args = func_get_args();
$pattern_args = array_slice($args, 2);
$ext_flag = $this->getMercurialExtensionFlag($extension);
$full_cmd = $ext_flag.' '.$pattern;
$args = array_merge(
array($full_cmd),
$pattern_args);
return $args;
}
public function execxLocalWithExtension(
$extension,
$pattern /* , ... */) {
$args = func_get_args();
$extended_args = call_user_func_array(
array($this, 'buildMercurialExtensionCommand'),
$args);
return call_user_func_array(
array($this, 'execxLocal'),
$extended_args);
}
public function execFutureLocalWithExtension(
$extension,
$pattern /* , ... */) {
$args = func_get_args();
$extended_args = call_user_func_array(
array($this, 'buildMercurialExtensionCommand'),
$args);
return call_user_func_array(
array($this, 'execFutureLocal'),
$extended_args);
}
public function execPassthruWithExtension(
$extension,
$pattern /* , ... */) {
$args = func_get_args();
$extended_args = call_user_func_array(
array($this, 'buildMercurialExtensionCommand'),
$args);
return call_user_func_array(
array($this, 'execPassthru'),
$extended_args);
}
private function executeMercurialFeatureTest($feature, $resolve) { private function executeMercurialFeatureTest($feature, $resolve) {
if (array_key_exists($feature, $this->featureResults)) { if (array_key_exists($feature, $this->featureResults)) {
return $this->featureResults[$feature]; return $this->featureResults[$feature];
@ -1059,8 +1174,9 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
private function newMercurialFeatureFuture($feature) { private function newMercurialFeatureFuture($feature) {
switch ($feature) { switch ($feature) {
case 'shelve': case 'shelve':
return $this->execFutureLocal( return $this->execFutureLocalWithExtension(
'--config extensions.shelve= shelve --help --'); 'shelve',
'shelve --help --');
case 'evolve': case 'evolve':
return $this->execFutureLocal('prune --help --'); return $this->execFutureLocal('prune --help --');
default: default:
@ -1094,17 +1210,6 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
return new ArcanistMercurialRepositoryRemoteQuery(); return new ArcanistMercurialRepositoryRemoteQuery();
} }
public function getMercurialExtensionArguments() {
$path = phutil_get_library_root('arcanist');
$path = dirname($path);
$path = $path.'/support/hg/arc-hg.py';
return array(
'--config',
'extensions.arc-hg='.$path,
);
}
protected function newNormalizedURI($uri) { protected function newNormalizedURI($uri) {
return new ArcanistRepositoryURINormalizer( return new ArcanistRepositoryURINormalizer(
ArcanistRepositoryURINormalizer::TYPE_MERCURIAL, ArcanistRepositoryURINormalizer::TYPE_MERCURIAL,

View file

@ -19,12 +19,6 @@ final class ArcanistMercurialRepositoryMarkerQuery
// to provide a command which works like "git for-each-ref" locally and // to provide a command which works like "git for-each-ref" locally and
// "git ls-remote" when given a remote. // "git ls-remote" when given a remote.
$argv = array();
foreach ($api->getMercurialExtensionArguments() as $arg) {
$argv[] = $arg;
}
$argv[] = 'arc-ls-markers';
// NOTE: In remote mode, we're using passthru and a tempfile on this // NOTE: In remote mode, we're using passthru and a tempfile on this
// because it's a remote command and may prompt the user to provide // because it's a remote command and may prompt the user to provide
// credentials interactively. In local mode, we can just read stdout. // credentials interactively. In local mode, we can just read stdout.
@ -33,20 +27,17 @@ final class ArcanistMercurialRepositoryMarkerQuery
$tmpfile = new TempFile(); $tmpfile = new TempFile();
Filesystem::remove($tmpfile); Filesystem::remove($tmpfile);
$argv = array();
$argv[] = '--output'; $argv[] = '--output';
$argv[] = phutil_string_cast($tmpfile); $argv[] = phutil_string_cast($tmpfile);
} $argv[] = '--';
$argv[] = '--';
if ($remote !== null) {
$argv[] = $remote->getRemoteName(); $argv[] = $remote->getRemoteName();
}
if ($remote !== null) { $err = $api->execPassthruWithExtension(
$passthru = $api->newPassthru('%Ls', $argv); 'arc-hg',
'arc-ls-markers %Ls',
$argv);
$err = $passthru->execute();
if ($err) { if ($err) {
throw new Exception( throw new Exception(
pht( pht(
@ -57,8 +48,10 @@ final class ArcanistMercurialRepositoryMarkerQuery
$raw_data = Filesystem::readFile($tmpfile); $raw_data = Filesystem::readFile($tmpfile);
unset($tmpfile); unset($tmpfile);
} else { } else {
$future = $api->newFuture('%Ls', $argv); $future = $api->execFutureLocalWithExtension(
list($raw_data) = $future->resolve(); 'arc-hg',
'arc-ls-markers --');
list($err, $raw_data) = $future->resolve();
} }
$items = phutil_json_decode($raw_data); $items = phutil_json_decode($raw_data);

View file

@ -160,8 +160,9 @@ final class ArcanistMercurialLocalState
'arc-%s', 'arc-%s',
Filesystem::readRandomCharacters(12)); Filesystem::readRandomCharacters(12));
$api->execxLocal( $api->execxLocalWithExtension(
'--config extensions.shelve= shelve --unknown --name %s --', 'shelve',
'shelve --unknown --name %s --',
$stash_ref); $stash_ref);
$log->writeStatus( $log->writeStatus(
@ -179,16 +180,18 @@ final class ArcanistMercurialLocalState
pht('UNSHELVE'), pht('UNSHELVE'),
pht('Restoring uncommitted changes to working copy.')); pht('Restoring uncommitted changes to working copy.'));
$api->execxLocal( $api->execxLocalWithExtension(
'--config extensions.shelve= unshelve --keep --name %s --', 'shelve',
'unshelve --keep --name %s --',
$stash_ref); $stash_ref);
} }
protected function discardStash($stash_ref) { protected function discardStash($stash_ref) {
$api = $this->getRepositoryAPI(); $api = $this->getRepositoryAPI();
$api->execxLocal( $api->execxLocalWithExtension(
'--config extensions.shelve= shelve --delete %s --', 'shelve',
'shelve --delete %s --',
$stash_ref); $stash_ref);
} }