Page MenuHomePhabricator

D9922.id23839.diff
No OneTemporary

D9922.id23839.diff

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
@@ -21,36 +21,21 @@
return new ArcanistGitAPI($root);
}
- protected function buildLocalFuture(array $argv) {
- $argv[0] = 'git '.$argv[0];
-
- $future = newv('ExecFuture', $argv);
- $future->setCWD($this->getPath());
- return $future;
- }
-
- public function execPassthru($pattern /* , ... */) {
- $args = func_get_args();
-
- static $git = null;
- if ($git === null) {
- if (phutil_is_windows()) {
- // NOTE: On Windows, phutil_passthru() uses 'bypass_shell' because
- // everything goes to hell if we don't. We must provide an absolute
- // path to Git for this to work properly.
+ protected function getBinaryName() {
+ if (phutil_is_windows()) {
+ // NOTE: On Windows, passthru uses 'bypass_shell' because everything
+ // goes to hell if we don't. We must provide an absolute path to Git for
+ // this to work properly.
+ static $git;
+ if ($git === null) {
$git = Filesystem::resolveBinary('git');
- $git = csprintf('%s', $git);
- } else {
- $git = 'git';
}
+ return $git;
+ } else {
+ return 'git';
}
-
- $args[0] = $git.' '.$args[0];
-
- return call_user_func_array('phutil_passthru', $args);
}
-
public function getSourceControlSystemName() {
return 'git';
}
diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php
--- a/src/repository/api/ArcanistMercurialAPI.php
+++ b/src/repository/api/ArcanistMercurialAPI.php
@@ -12,7 +12,11 @@
private $supportsRebase;
private $supportsPhases;
- protected function buildLocalFuture(array $argv) {
+ protected function getBinaryName() {
+ return 'hg';
+ }
+
+ protected function getExecutionEnvironment() {
// Mercurial has a "defaults" feature which basically breaks automation by
// allowing the user to add random flags to any command. This feature is
// "deprecated" and "a bad idea" that you should "forget ... existed"
@@ -23,26 +27,9 @@
// There is an HGPLAIN environmental variable which enables "plain mode"
// and hopefully disables this stuff.
- if (phutil_is_windows()) {
- $argv[0] = 'set HGPLAIN=1 & hg '.$argv[0];
- } else {
- $argv[0] = 'HGPLAIN=1 hg '.$argv[0];
- }
-
- $future = newv('ExecFuture', $argv);
- $future->setCWD($this->getPath());
- return $future;
- }
-
- public function execPassthru($pattern /* , ... */) {
- $args = func_get_args();
- if (phutil_is_windows()) {
- $args[0] = 'hg '.$args[0];
- } else {
- $args[0] = 'HGPLAIN=1 hg '.$args[0];
- }
-
- return call_user_func_array('phutil_passthru', $args);
+ return array(
+ 'HGPLAIN' => 1,
+ );
}
public function getSourceControlSystemName() {
diff --git a/src/repository/api/ArcanistRepositoryAPI.php b/src/repository/api/ArcanistRepositoryAPI.php
--- a/src/repository/api/ArcanistRepositoryAPI.php
+++ b/src/repository/api/ArcanistRepositoryAPI.php
@@ -404,7 +404,98 @@
return $this->buildLocalFuture($args);
}
- abstract protected function buildLocalFuture(array $argv);
+ /**
+ * Provide the name of the VCS binary, like `'git'`, `'hg'` or `'svn'`.
+ *
+ * @return string Name of the VCS binary.
+ */
+ abstract protected function getBinaryName();
+
+ /**
+ * Provide environmental variables specific to this VCS.
+ *
+ * These variables will be passed to VCS subprocesses. For example, Mercurial
+ * needs to execute with HGPLAIN.
+ *
+ * @return map<string, string> Map of environmental variables to pass to
+ * VCS subprocesses.
+ */
+ protected function getExecutionEnvironment() {
+ return array();
+ }
+
+ final protected function getCommonExecutionEnvironment() {
+
+ // The name of the "English, UTF-8" locale varies across systems. Try to
+ // identify the correct name on this system.
+ static $locale = null;
+ if ($locale === null) {
+ list($err, $locales) = exec_manual('locale -a');
+ if ($err) {
+ // If we don't have `locale`, we're probably on Windows? This might
+ // not be the right thing to do?
+ $locale = 'en_US.UTF-8';
+ } else {
+ $locales = phutil_split_lines($locales, $retain_endings = false);
+ $locales = array_fuse($locales);
+
+ $try = array(
+ 'en_US.utf8',
+ 'en_US.UTF-8',
+ );
+
+ foreach ($try as $try_locale) {
+ if (isset($locales[$try_locale])) {
+ $locale = $try_locale;
+ break;
+ }
+ }
+
+ if ($locale === null) {
+ throw new Exception(
+ pht(
+ 'Unable to find a valid English UTF-8 locale on this system. '.
+ 'One of these locales is required: %s. These locales were '.
+ 'found: %s.',
+ implode(', ', $try),
+ implode(', ', $locales)));
+ }
+ }
+ }
+
+ return array(
+ 'LANG' => $locale,
+ );
+ }
+
+ protected function buildLocalFuture(array $argv) {
+ list($command, $env) = $this->formatVCSCommand($argv);
+
+ return id(new ExecFuture('%C', $command))
+ ->setEnv($env)
+ ->setCWD($this->getPath());
+ }
+
+ public function execPassthru($pattern /* , ... */) {
+ $argv = func_get_args();
+ list($command, $env) = $this->formatVCSCommand($argv);
+
+ $passthru = new PhutilExecPassthru('%C', $command);
+ $passthru->setEnv($env);
+ $passthru->setCWD($this->getPath());
+
+ return $passthru->execute();
+ }
+
+ private function formatVCSCommand(array $argv) {
+ $argv = call_user_func_array('csprintf', $argv);
+ $command = csprintf('%s %C', $this->getBinaryName(), $argv);
+
+ $env = $this->getExecutionEnvironment() +
+ $this->getCommonExecutionEnvironment();
+
+ return array($command, $env);
+ }
public function canStashChanges() {
return false;
diff --git a/src/repository/api/ArcanistSubversionAPI.php b/src/repository/api/ArcanistSubversionAPI.php
--- a/src/repository/api/ArcanistSubversionAPI.php
+++ b/src/repository/api/ArcanistSubversionAPI.php
@@ -36,13 +36,8 @@
return $svn_dir;
}
- protected function buildLocalFuture(array $argv) {
-
- $argv[0] = 'svn '.$argv[0];
-
- $future = newv('ExecFuture', $argv);
- $future->setCWD($this->getPath());
- return $future;
+ protected function getBinaryName() {
+ return 'svn';
}
protected function buildCommitRangeStatus() {
diff --git a/src/workflow/ArcanistCommitWorkflow.php b/src/workflow/ArcanistCommitWorkflow.php
--- a/src/workflow/ArcanistCommitWorkflow.php
+++ b/src/workflow/ArcanistCommitWorkflow.php
@@ -142,23 +142,10 @@
$tmp_file = new TempFile();
Filesystem::writeFile($tmp_file, $message);
- $command = csprintf(
- 'svn commit %Ls --encoding utf-8 -F %s',
- $files,
- $tmp_file);
-
- // make sure to specify LANG on non-windows systems to suppress any fancy
- // warnings; see @{method:getSVNLangEnvVar}.
- if (!phutil_is_windows()) {
- $command = csprintf('LANG=%C %C', $this->getSVNLangEnvVar(), $command);
- }
-
- chdir($repository_api->getPath());
-
- $err = phutil_passthru('%C', $command);
- if ($err) {
- throw new Exception("Executing 'svn commit' failed!");
- }
+ $err = $repository_api->execPassthru(
+ 'commit --encoding utf-8 -F %s -- %Ls',
+ $tmp_file,
+ $files);
$mark_workflow = $this->buildChildWorkflow(
'close-revision',
@@ -273,32 +260,6 @@
return array('svn');
}
- /**
- * On some systems, we need to specify "en_US.UTF-8" instead of "en_US.utf8",
- * and SVN spews some bewildering warnings if we don't:
- *
- * svn: warning: cannot set LC_CTYPE locale
- * svn: warning: environment variable LANG is en_US.utf8
- * svn: warning: please check that your locale name is correct
- *
- * For example, it happens on epriestley's Mac (10.6.7) with
- * Subversion 1.6.15.
- */
- private function getSVNLangEnvVar() {
- $locale = 'en_US.utf8';
- try {
- list($locales) = execx('locale -a');
- $locales = explode("\n", trim($locales));
- $locales = array_fill_keys($locales, true);
- if (isset($locales['en_US.UTF-8'])) {
- $locale = 'en_US.UTF-8';
- }
- } catch (Exception $ex) {
- // Ignore.
- }
- return $locale;
- }
-
private function runSanityChecks(array $revision) {
$repository_api = $this->getRepositoryAPI();
$revision_id = $revision['id'];

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 23, 5:25 AM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6743633
Default Alt Text
D9922.id23839.diff (8 KB)

Event Timeline