Merge utility/support changes from "wilds" to "master"

Summary:
Ref T13395. Merge a lot of stuff which doesn't break existing workflows:

    - Merge a UTF8 fix for "cmd.exe" on Windows.
    - Merge minor changes to JSON linters.
    - Merge some shell completion stuff.
    - Merge some "arc anoid" fixes.
    - Merge various Windows improvements to unit tests which interact with processes / the filesystem.
    - Merge some other Windows path fixes.
    - Merge a UTF8 character class fix.
    - Merge script initialization.
    - Merge unit test support scripts.
    - Merge some initialization code.
    - Merge Windows stdout/stderr-as-files code.
    - Merge a bunch of code for making exec tests work on Windows.
    - Merge more Windows unit test fixes.
    - Merge "continue on failure" mode when loading symbols.
    - Merge "GPC" order CLI fixes.

Test Plan: Ran `arc unit --everything`; created this change. There's likely some less-than-perfect code here.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20988

.gitignore

@@ -30,3 +30,6 @@
 
 # This is an OS X build artifact.
 /support/xhpast/xhpast.dSYM
+
+# Generated shell completion rulesets.
+/support/shell/rules/

resources/shell/bash-completion

@@ -1,26 +0,0 @@
-if [[ -n ${ZSH_VERSION-} ]]; then
-  autoload -U +X bashcompinit && bashcompinit
-fi
-
-_arc ()
-{
-  CUR="${COMP_WORDS[COMP_CWORD]}"
-  COMPREPLY=()
-  OPTS=$(echo | arc shell-complete --current ${COMP_CWORD} -- ${COMP_WORDS[@]})
-
-  if [ $? -ne 0 ]; then
-    return $?
-  fi
-
-  if [ "$OPTS" = "FILE" ]; then
-    COMPREPLY=( $(compgen -f -- ${CUR}) )
-    return 0
-  fi
-
-  if [ "$OPTS" = "ARGUMENT" ]; then
-    return 0
-  fi
-
-  COMPREPLY=( $(compgen -W "${OPTS}" -- ${CUR}) )
-}
-complete -F _arc -o filenames arc

scripts/__init_script__.php

@@ -1,3 +1,3 @@
 <?php
 
-require_once dirname(dirname(__FILE__)).'/scripts/init/init-script.php';
+require_once dirname(dirname(__FILE__)).'/support/init/init-script.php';

scripts/breakout.py

@@ -141,11 +141,11 @@ def main(stdscr):
 
     height, width = stdscr.getmaxyx()
 
-    if height < 15 or width < 30:
+    if height < 15 or width < 32:
         raise PowerOverwhelmingException(
-            "Your computer is not powerful enough to run 'arc anoid'. "
+            'Your computer is not powerful enough to run "arc anoid". '
-            "It must support at least 30 columns and 15 rows of next-gen "
+            'It must support at least 32 columns and 15 rows of next-gen '
-            "full-color 3D graphics.")
+            'full-color 3D graphics.')
 
     status = curses.newwin(1, width, 0, 0)
     height -= 1
@@ -194,7 +194,15 @@ def main(stdscr):
         status.addstr('%s/%s ' % (Block.killed, Block.total), curses.A_BOLD)
         status.addch(curses.ACS_VLINE)
         status.addstr(' DEATHS: ', curses.A_BOLD | curses.color_pair(4))
-        status.addstr('%s ' % Ball.killed, curses.A_BOLD)
+
+        # See T8693. At the minimum display size, we only have room to render
+        # two characters for the death count, so just display "99" if the
+        # player has more than 99 deaths.
+        display_deaths = Ball.killed
+        if (display_deaths > 99):
+            display_deaths = 99
+
+        status.addstr('%s ' % display_deaths, curses.A_BOLD)
         status.addch(curses.ACS_LTEE)
 
         if Block.killed == Block.total:

src/__phutil_library_map__.php

@@ -232,7 +232,6 @@ phutil_register_library_map(array(
     'ArcanistJSHintLinter' => 'lint/linter/ArcanistJSHintLinter.php',
     'ArcanistJSHintLinterTestCase' => 'lint/linter/__tests__/ArcanistJSHintLinterTestCase.php',
     'ArcanistJSONLintLinter' => 'lint/linter/ArcanistJSONLintLinter.php',
-    'ArcanistJSONLintLinterTestCase' => 'lint/linter/__tests__/ArcanistJSONLintLinterTestCase.php',
     'ArcanistJSONLintRenderer' => 'lint/renderer/ArcanistJSONLintRenderer.php',
     'ArcanistJSONLinter' => 'lint/linter/ArcanistJSONLinter.php',
     'ArcanistJSONLinterTestCase' => 'lint/linter/__tests__/ArcanistJSONLinterTestCase.php',
@@ -1130,7 +1129,6 @@ phutil_register_library_map(array(
     'ArcanistJSHintLinter' => 'ArcanistExternalLinter',
     'ArcanistJSHintLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistJSONLintLinter' => 'ArcanistExternalLinter',
-    'ArcanistJSONLintLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistJSONLintRenderer' => 'ArcanistLintRenderer',
     'ArcanistJSONLinter' => 'ArcanistLinter',
     'ArcanistJSONLinterTestCase' => 'ArcanistLinterTestCase',

src/channel/__tests__/PhutilPHPObjectProtocolChannelTestCase.php

@@ -45,7 +45,8 @@ final class PhutilPHPObjectProtocolChannelTestCase extends PhutilTestCase {
   }
 
   public function testCloseExecWriteChannel() {
-    $future = new ExecFuture('cat');
+    $bin = $this->getSupportExecutable('cat');
+    $future = new ExecFuture('php -f %R', $bin);
 
     // If this test breaks, we want to explode, not hang forever.
     $future->setTimeout(5);

src/error/__tests__/PhutilOpaqueEnvelopeTestCase.php

@@ -8,9 +8,13 @@ final class PhutilOpaqueEnvelopeTestCase extends PhutilTestCase {
     // the diff itself, and thus this source code. Since we look for the secret
     // in traces later on, split it apart here so that invocation via
     // "arc diff" doesn't create a false test failure.
-
     $secret = 'hunter'.'2';
 
+    // Also split apart this "signpost" value which we are not going to put in
+    // an envelope. We expect to be able to find it in the argument lists in
+    // stack traces, and don't want a false positive.
+    $signpost = 'shaman'.'3';
+
     $envelope = new PhutilOpaqueEnvelope($secret);
 
     $this->assertFalse(strpos(var_export($envelope, true), $secret));
@@ -24,23 +28,34 @@ final class PhutilOpaqueEnvelopeTestCase extends PhutilTestCase {
     $this->assertFalse(strpos($dump, $secret));
 
     try {
-      $this->throwTrace($envelope);
+      $this->throwTrace($envelope, $signpost);
     } catch (Exception $ex) {
       $trace = $ex->getTrace();
-      $this->assertFalse(strpos(print_r($trace, true), $secret));
+
+      // NOTE: The entire trace may be very large and contain complex
+      // recursive datastructures. Look at only the last few frames: we expect
+      // to see the signpost value but not the secret.
+      $trace = array_slice($trace, 0, 2);
+      $trace = print_r($trace, true);
+
+      $this->assertTrue(strpos($trace, $signpost) !== false);
+      $this->assertFalse(strpos($trace, $secret));
     }
 
-    $backtrace = $this->getBacktrace($envelope);
+    $backtrace = $this->getBacktrace($envelope, $signpost);
+    $backtrace = array_slice($backtrace, 0, 2);
+
+    $this->assertTrue(strpos($trace, $signpost) !== false);
     $this->assertFalse(strpos(print_r($backtrace, true), $secret));
 
     $this->assertEqual($secret, $envelope->openEnvelope());
   }
 
-  private function throwTrace($v) {
+  private function throwTrace($v, $w) {
     throw new Exception('!');
   }
 
-  private function getBacktrace($v) {
+  private function getBacktrace($v, $w) {
     return debug_backtrace();
   }
 

src/filesystem/Filesystem.php

@@ -488,9 +488,8 @@ final class Filesystem extends Phobject {
         pht(
           '%s requires the PHP OpenSSL extension to be installed and enabled '.
           'to access an entropy source. On Windows, this extension is usually '.
-          'installed but not enabled by default. Enable it in your "s".',
+          'installed but not enabled by default. Enable it in your "php.ini".',
-          __METHOD__.'()',
+          __METHOD__.'()'));
-          'php.ini'));
     }
 
     throw new Exception(
@@ -950,8 +949,23 @@ final class Filesystem extends Phobject {
     // This won't work if the file doesn't exist or is on an unreadable mount
     // or something crazy like that. Try to resolve a parent so we at least
     // cover the nonexistent file case.
-    $parts = explode(DIRECTORY_SEPARATOR, trim($path, DIRECTORY_SEPARATOR));
+
-    while (end($parts) !== false) {
+    // We're also normalizing path separators to whatever is normal for the
+    // environment.
+
+    if (phutil_is_windows()) {
+      $parts = trim($path, '/\\');
+      $parts = preg_split('([/\\\\])', $parts);
+
+      // Normalize the directory separators in the path. If we find a parent
+      // below, we'll overwrite this with a better resolved path.
+      $path = str_replace('/', '\\', $path);
+    } else {
+      $parts = trim($path, '/');
+      $parts = explode('/', $parts);
+    }
+
+    while ($parts) {
       array_pop($parts);
       if (phutil_is_windows()) {
         $attempt = implode(DIRECTORY_SEPARATOR, $parts);
@@ -1104,6 +1118,18 @@ final class Filesystem extends Phobject {
     return ($u == $v);
   }
 
+  public static function concatenatePaths(array $components) {
+    $components = implode($components, DIRECTORY_SEPARATOR);
+
+    // Replace any extra sequences of directory separators with a single
+    // separator, so we don't end up with "path//to///thing.c".
+    $components = preg_replace(
+      '('.preg_quote(DIRECTORY_SEPARATOR).'{2,})',
+      DIRECTORY_SEPARATOR,
+      $components);
+
+    return $components;
+  }
 
 /* -(  Assert  )------------------------------------------------------------- */
 

src/filesystem/__tests__/FileFinderTestCase.php

@@ -125,6 +125,16 @@ final class FileFinderTestCase extends PhutilTestCase {
   }
 
   public function testFinderWithGlobMagic() {
+    if (phutil_is_windows()) {
+      // We can't write files with "\" since this is the path separator.
+      // We can't write files with "*" since Windows rejects them.
+      // This doesn't leave us too many interesting paths to test, so just
+      // skip this test case under Windows.
+      $this->assertSkipped(
+        pht(
+          'Windows can not write files with sufficiently absurd names.'));
+    }
+
     // Fill a temporary directory with all this magic garbage so we don't have
     // to check a bunch of files with backslashes in their names into version
     // control.
@@ -211,6 +221,7 @@ final class FileFinderTestCase extends PhutilTestCase {
       'php',
       'shell',
     );
+
     foreach ($modes as $mode) {
       $actual = id(clone $finder)
         ->setForceMode($mode)

src/filesystem/__tests__/FilesystemTestCase.php

@@ -127,6 +127,12 @@ final class FilesystemTestCase extends PhutilTestCase {
     foreach ($test_cases as $test_case) {
       list($path, $root, $expected) = $test_case;
 
+      // On Windows, paths will have backslashes rather than forward slashes.
+      // Normalize our expectations to the path format for the environment.
+      foreach ($expected as $key => $epath) {
+        $expected[$key] = str_replace('/', DIRECTORY_SEPARATOR, $epath);
+      }
+
       $this->assertEqual(
         $expected,
         Filesystem::walkToRoot($path, $root));

src/filesystem/__tests__/PhutilFileLockTestCase.php

@@ -170,16 +170,20 @@ final class PhutilFileLockTestCase extends PhutilTestCase {
     throw new Exception(pht('Unable to hold lock in external process!'));
   }
 
-  private function buildLockFuture($flags, $file) {
+  private function buildLockFuture(/* ... */) {
-    $root = dirname(phutil_get_library_root('arcanist'));
+    $argv = func_get_args();
-    $bin = $root.'/support/test/lock-file.php';
+    $bin = $this->getSupportExecutable('lock');
 
-    $flags = (array)$flags;
+    if (phutil_is_windows()) {
+      $future = new ExecFuture('php -f %R -- %Ls', $bin, $argv);
+    } else {
+      // NOTE: Use `exec` so this passes on Ubuntu, where the default `dash`
+      // shell will eat any kills we send during the tests.
+      $future = new ExecFuture('exec php -f %R -- %Ls', $bin, $argv);
+    }
 
-    // NOTE: Use `exec` so this passes on Ubuntu, where the default `dash` shell
-    // will eat any kills we send during the tests.
-    $future = new ExecFuture('exec php -f %R -- %Ls %R', $bin, $flags, $file);
     $future->start();
+
     return $future;
   }
 

src/filesystem/linesofalarge/__tests__/LinesOfALargeExecFutureTestCase.php

@@ -43,7 +43,9 @@ final class LinesOfALargeExecFutureTestCase extends PhutilTestCase {
   }
 
   private function writeAndRead($write, $read) {
-    $future = new ExecFuture('cat');
+    $bin = $this->getSupportExecutable('cat');
+    $future = new ExecFuture('php -f %R', $bin);
+
     $future->write($write);
 
     $lines = array();

src/future/__tests__/FutureIteratorTestCase.php

@@ -3,8 +3,10 @@
 final class FutureIteratorTestCase extends PhutilTestCase {
 
   public function testAddingFuture() {
-    $future1 = new ExecFuture('cat');
+    $bin = $this->getSupportExecutable('cat');
-    $future2 = new ExecFuture('cat');
+
+    $future1 = new ExecFuture('php -f %R', $bin);
+    $future2 = new ExecFuture('php -f %R', $bin);
 
     $iterator = new FutureIterator(array($future1));
     $iterator->limit(2);

src/future/exec/ExecFuture.php

@@ -42,7 +42,6 @@ final class ExecFuture extends PhutilExecutableFuture {
   private $profilerCallID;
   private $killedByTimeout;
 
-  private $useWindowsFileStreams = false;
   private $windowsStdoutTempFile = null;
   private $windowsStderrTempFile = null;
 
@@ -181,21 +180,6 @@ final class ExecFuture extends PhutilExecutableFuture {
   }
 
 
-  /**
-   * Set whether to use non-blocking streams on Windows.
-   *
-   * @param bool Whether to use non-blocking streams.
-   * @return this
-   * @task config
-   */
-  public function setUseWindowsFileStreams($use_streams) {
-    if (phutil_is_windows()) {
-      $this->useWindowsFileStreams = $use_streams;
-    }
-    return $this;
-  }
-
-
 /* -(  Interacting With Commands  )------------------------------------------ */
 
 
@@ -587,6 +571,7 @@ final class ExecFuture extends PhutilExecutableFuture {
     // classes are always available.
 
     if (!$this->pipes) {
+      $is_windows = phutil_is_windows();
 
       // NOTE: See note above about Phage.
       if (class_exists('PhutilServiceProfiler')) {
@@ -610,18 +595,6 @@ final class ExecFuture extends PhutilExecutableFuture {
 
       $pipes = array();
 
-      if (phutil_is_windows()) {
-        // See T4395. proc_open under Windows uses "cmd /C [cmd]", which will
-        // strip the first and last quote when there aren't exactly two quotes
-        // (and some other conditions as well). This results in a command that
-        // looks like `command" "path to my file" "something something` which is
-        // clearly wrong. By surrounding the command string with quotes we can
-        // be sure this process is harmless.
-        if (strpos($unmasked_command, '"') !== false) {
-          $unmasked_command = '"'.$unmasked_command.'"';
-        }
-      }
-
       if ($this->hasEnv()) {
         $env = $this->getEnv();
       } else {
@@ -638,21 +611,31 @@ final class ExecFuture extends PhutilExecutableFuture {
       }
 
       $spec = self::$descriptorSpec;
-      if ($this->useWindowsFileStreams) {
+      if ($is_windows) {
-        $this->windowsStdoutTempFile = new TempFile();
+        $stdout_file = new TempFile();
-        $this->windowsStderrTempFile = new TempFile();
+        $stderr_file = new TempFile();
+
+        $stdout_handle = fopen($stdout_file, 'wb');
+        if (!$stdout_handle) {
+          throw new Exception(
+            pht(
+              'Unable to open stdout temporary file ("%s") for writing.',
+              $stdout_file));
+        }
+
+        $stderr_handle = fopen($stderr_file, 'wb');
+        if (!$stderr_handle) {
+          throw new Exception(
+            pht(
+              'Unable to open stderr temporary file ("%s") for writing.',
+              $stderr_file));
+        }
 
         $spec = array(
-          0 => self::$descriptorSpec[0],  // stdin
+          0 => self::$descriptorSpec[0],
-          1 => fopen($this->windowsStdoutTempFile, 'wb'),  // stdout
+          1 => $stdout_handle,
-          2 => fopen($this->windowsStderrTempFile, 'wb'),  // stderr
+          2 => $stderr_handle,
         );
-
-        if (!$spec[1] || !$spec[2]) {
-          throw new Exception(pht(
-            'Unable to create temporary files for '.
-            'Windows stdout / stderr streams'));
-        }
       }
 
       $proc = @proc_open(
@@ -660,23 +643,10 @@ final class ExecFuture extends PhutilExecutableFuture {
         $spec,
         $pipes,
         $cwd,
-        $env);
+        $env,
-
+        array(
-      if ($this->useWindowsFileStreams) {
+          'bypass_shell' => true,
-        fclose($spec[1]);
+        ));
-        fclose($spec[2]);
-        $pipes = array(
-          0 => head($pipes),  // stdin
-          1 => fopen($this->windowsStdoutTempFile, 'rb'),  // stdout
-          2 => fopen($this->windowsStderrTempFile, 'rb'),  // stderr
-        );
-
-        if (!$pipes[1] || !$pipes[2]) {
-          throw new Exception(pht(
-            'Unable to open temporary files for '.
-            'reading Windows stdout / stderr streams'));
-        }
-      }
 
       if ($trap) {
         $err = $trap->getErrorsAsString();
@@ -685,12 +655,56 @@ final class ExecFuture extends PhutilExecutableFuture {
         $err = error_get_last();
       }
 
+      if ($is_windows) {
+        fclose($stdout_handle);
+        fclose($stderr_handle);
+      }
+
       if (!is_resource($proc)) {
-        throw new Exception(
+        // When you run an invalid command on a Linux system, the "proc_open()"
+        // works and then the process (really a "/bin/sh -c ...") exits after
+        // it fails to resolve the command.
+
+        // When you run an invalid command on a Windows system, we bypass the
+        // shell and the "proc_open()" itself fails. Throw a "CommandException"
+        // here for consistency with the Linux behavior in this common failure
+        // case.
+
+        throw new CommandException(
           pht(
-            'Failed to `%s`: %s',
+            'Call to "proc_open()" to open a subprocess failed: %s',
-            'proc_open()',
+            $err),
-            $err));
+          $this->command,
+          1,
+          '',
+          '');
+      }
+
+      if ($is_windows) {
+        $stdout_handle = fopen($stdout_file, 'rb');
+        if (!$stdout_handle) {
+          throw new Exception(
+            pht(
+              'Unable to open stdout temporary file ("%s") for reading.',
+              $stdout_file));
+        }
+
+        $stderr_handle = fopen($stderr_file, 'rb');
+        if (!$stderr_handle) {
+          throw new Exception(
+            pht(
+              'Unable to open stderr temporary file ("%s") for reading.',
+              $stderr_file));
+        }
+
+        $pipes = array(
+          0 => $pipes[0],
+          1 => $stdout_handle,
+          2 => $stderr_handle,
+        );
+
+        $this->windowsStdoutTempFile = $stdout_file;
+        $this->windowsStderrTempFile = $stderr_file;
       }
 
       $this->pipes = $pipes;
@@ -698,11 +712,11 @@ final class ExecFuture extends PhutilExecutableFuture {
 
       list($stdin, $stdout, $stderr) = $pipes;
 
-      if (!phutil_is_windows()) {
+      if (!$is_windows) {
 
         // On Windows, we redirect process standard output and standard error
-        // through temporary files, and then use stream_select to determine
+        // through temporary files. Files don't block, so we don't need to make
-        // if there's more data to read.
+        // these streams nonblocking.
 
         if ((!stream_set_blocking($stdout, false)) ||
             (!stream_set_blocking($stderr, false)) ||
@@ -780,11 +794,6 @@ final class ExecFuture extends PhutilExecutableFuture {
     }
 
     if ($is_done) {
-      if ($this->useWindowsFileStreams) {
-        fclose($stdout);
-        fclose($stderr);
-      }
-
       // If the subprocess got nuked with `kill -9`, we get a -1 exitcode.
       // Upgrade this to a slightly more informative value by examining the
       // terminating signal code.
@@ -864,7 +873,10 @@ final class ExecFuture extends PhutilExecutableFuture {
       @proc_close($this->proc);
       $this->proc = null;
     }
-    $this->stdin  = null;
+    $this->stdin = null;
+
+    unset($this->windowsStdoutTempFile);
+    unset($this->windowsStderrTempFile);
 
     if ($this->profilerCallID !== null) {
       $profiler = PhutilServiceProfiler::getInstance();

src/future/exec/__tests__/ExecFutureTestCase.php

@@ -6,15 +6,27 @@ final class ExecFutureTestCase extends PhutilTestCase {
     // NOTE: This is mostly testing that we don't hang while doing an empty
     // write.
 
-    list($stdout) = id(new ExecFuture('cat'))->write('')->resolvex();
+    list($stdout) = $this->newCat()
+      ->write('')
+      ->resolvex();
 
     $this->assertEqual('', $stdout);
   }
 
+  private function newCat() {
+    $bin = $this->getSupportExecutable('cat');
+    return new ExecFuture('php -f %R', $bin);
+  }
+
+  private function newSleep($duration) {
+    $bin = $this->getSupportExecutable('sleep');
+    return new ExecFuture('php -f %R -- %s', $bin, $duration);
+  }
+
   public function testKeepPipe() {
     // NOTE: This is mostly testing the semantics of $keep_pipe in write().
 
-    list($stdout) = id(new ExecFuture('cat'))
+    list($stdout) = $this->newCat()
       ->write('', true)
       ->start()
       ->write('x', true)
@@ -30,14 +42,14 @@ final class ExecFutureTestCase extends PhutilTestCase {
     // flushing a buffer.
 
     $data = str_repeat('x', 1024 * 1024 * 4);
-    list($stdout) = id(new ExecFuture('cat'))->write($data)->resolvex();
+    list($stdout) = $this->newCat()->write($data)->resolvex();
 
     $this->assertEqual($data, $stdout);
   }
 
   public function testBufferLimit() {
     $data = str_repeat('x', 1024 * 1024);
-    list($stdout) = id(new ExecFuture('cat'))
+    list($stdout) = $this->newCat()
       ->setStdoutSizeLimit(1024)
       ->write($data)
       ->resolvex();
@@ -49,7 +61,7 @@ final class ExecFutureTestCase extends PhutilTestCase {
     // NOTE: This tests interactions between the resolve() timeout and the
     // ExecFuture timeout, which are similar but not identical.
 
-    $future = id(new ExecFuture('sleep 32000'))->start();
+    $future = $this->newSleep(32000)->start();
     $future->setTimeout(32000);
 
     // We expect this to return in 0.01s.
@@ -66,7 +78,7 @@ final class ExecFutureTestCase extends PhutilTestCase {
   public function testTerminateWithoutStart() {
     // We never start this future, but it should be fine to kill a future from
     // any state.
-    $future = new ExecFuture('sleep 1');
+    $future = $this->newSleep(1);
     $future->resolveKill();
 
     $this->assertTrue(true);
@@ -76,7 +88,7 @@ final class ExecFutureTestCase extends PhutilTestCase {
     // NOTE: This is partly testing that we choose appropriate select wait
     // times; this test should run for significantly less than 1 second.
 
-    $future = new ExecFuture('sleep 32000');
+    $future = $this->newSleep(32000);
     list($err) = $future->setTimeout(0.01)->resolve();
 
     $this->assertTrue($err > 0);
@@ -86,7 +98,7 @@ final class ExecFutureTestCase extends PhutilTestCase {
   public function testMultipleTimeoutsTestShouldRunLessThan1Sec() {
     $futures = array();
     for ($ii = 0; $ii < 4; $ii++) {
-      $futures[] = id(new ExecFuture('sleep 32000'))->setTimeout(0.01);
+      $futures[] = $this->newSleep(32000)->setTimeout(0.01);
     }
 
     foreach (new FutureIterator($futures) as $future) {
@@ -100,8 +112,9 @@ final class ExecFutureTestCase extends PhutilTestCase {
   public function testMultipleResolves() {
     // It should be safe to call resolve(), resolvex(), resolveKill(), etc.,
     // as many times as you want on the same process.
+    $bin = $this->getSupportExecutable('echo');
 
-    $future = new ExecFuture('echo quack');
+    $future = new ExecFuture('php -f %R -- quack', $bin);
     $future->resolve();
     $future->resolvex();
     list($err) = $future->resolveKill();
@@ -114,7 +127,7 @@ final class ExecFutureTestCase extends PhutilTestCase {
     $str_len_4 = 'abcd';
 
     // This is a write/read with no read buffer.
-    $future = new ExecFuture('cat');
+    $future = $this->newCat();
     $future->write($str_len_8);
 
     do {
@@ -131,7 +144,7 @@ final class ExecFutureTestCase extends PhutilTestCase {
 
 
     // This is a write/read with a read buffer.
-    $future = new ExecFuture('cat');
+    $future = $this->newCat();
     $future->write($str_len_8);
 
     // Set the read buffer size.

src/future/exec/__tests__/ExecPassthruTestCase.php

@@ -8,7 +8,9 @@ final class ExecPassthruTestCase extends PhutilTestCase {
     // the terminal, which is undesirable). This makes crafting effective unit
     // tests a fairly involved process.
 
-    $exec = new PhutilExecPassthru('exit');
+    $bin = $this->getSupportExecutable('exit');
+
+    $exec = new PhutilExecPassthru('php -f %R', $bin);
     $err = $exec->execute();
     $this->assertEqual(0, $err);
   }

src/future/oauth/__tests__/PhutilOAuth1FutureTestCase.php

@@ -63,6 +63,10 @@ final class PhutilOAuth1FutureTestCase extends PhutilTestCase {
   }
 
   public function testOAuth1SigningWithJIRAExamples() {
+    if (!function_exists('openssl_pkey_get_private')) {
+      $this->assertSkipped(
+        pht('Required "openssl" extension is not installed.'));
+    }
 
     // NOTE: This is an emprically example against JIRA v6.0.6, in that the
     // code seems to work when actually authing. It primarily serves as a check

src/internationalization/ArcanistUSEnglishTranslation.php

@@ -81,6 +81,11 @@ final class ArcanistUSEnglishTranslation extends PhutilTranslation {
         'This commit will be landed:',
         'These commits will be landed:',
       ),
+
+      'Updated %s librarie(s).' => array(
+        'Updated library.',
+        'Updated %s libraries.',
+      ),
     );
   }
 

src/lint/linter/__tests__/ArcanistJSONLintLinterTestCase.php

@@ -1,10 +0,0 @@
-<?php
-
-final class ArcanistJSONLintLinterTestCase
-  extends ArcanistExternalLinterTestCase {
-
-  public function testLinter() {
-    $this->executeTestsInDirectory(dirname(__FILE__).'/jsonlint/');
-  }
-
-}

src/lint/linter/__tests__/ArcanistJSONLinterTestCase.php

@@ -3,7 +3,7 @@
 final class ArcanistJSONLinterTestCase extends ArcanistLinterTestCase {
 
   public function testLinter() {
-    $this->executeTestsInDirectory(dirname(__FILE__).'/jsonlint/');
+    $this->executeTestsInDirectory(dirname(__FILE__).'/json/');
   }
 
 }

src/lint/linter/xhpast/rules/__tests__/ArcanistXHPASTLinterRuleTestCase.php

@@ -29,6 +29,8 @@ abstract class ArcanistXHPASTLinterRuleTestCase
    * @return ArcanistXHPASTLinterRule
    */
   protected function getLinterRule() {
+    $this->assertExecutable('xhpast');
+
     $class = get_class($this);
     $matches = null;
 

src/moduleutils/PhutilLibraryMapBuilder.php

@@ -183,7 +183,7 @@ final class PhutilLibraryMapBuilder extends Phobject {
    * Load the library symbol cache, if it exists and is readable and valid.
    *
    * @return dict  Map of content hashes to cache of output from
-   *               `phutil_symbols.php`.
+   *               `extract-symbols.php`.
    *
    * @task symbol
    */
@@ -256,7 +256,7 @@ final class PhutilLibraryMapBuilder extends Phobject {
   }
 
   /**
-   * Build a future which returns a `phutil_symbols.php` analysis of a source
+   * Build a future which returns a `extract-symbols.php` analysis of a source
    * file.
    *
    * @param  string  Relative path to the source file to analyze.
@@ -442,7 +442,7 @@ EOPHP;
     $symbol_cache = $this->loadSymbolCache();
 
     // If the XHPAST binary is not up-to-date, build it now. Otherwise,
-    // `phutil_symbols.php` will attempt to build the binary and will fail
+    // `extract-symbols.php` will attempt to build the binary and will fail
     // miserably because it will be trying to build the same file multiple
     // times in parallel.
     if (!PhutilXHPASTBinary::isAvailable()) {

src/parser/PhutilEditorConfig.php

@@ -107,9 +107,16 @@ final class PhutilEditorConfig extends Phobject {
     $configs = $this->getEditorConfigs($path);
     $matches = array();
 
+    // Normalize directory separators to "/". The ".editorconfig" standard
+    // uses only "/" as a directory separator, not "\".
+    $path = str_replace(DIRECTORY_SEPARATOR, '/', $path);
+
     foreach ($configs as $config) {
       list($path_prefix, $editorconfig) = $config;
 
+      // Normalize path separators, as above.
+      $path_prefix = str_replace(DIRECTORY_SEPARATOR, '/', $path_prefix);
+
       foreach ($editorconfig as $glob => $properties) {
         if (!$glob) {
           continue;
@@ -163,12 +170,11 @@ final class PhutilEditorConfig extends Phobject {
    * return list<pair<string, map>>
    */
   private function getEditorConfigs($path) {
-    $configs    = array();
+    $configs = array();
-    $found_root = false;
-    $root       = $this->root;
 
-    do {
+    $found_root = false;
-      $path = dirname($path);
+    $paths = Filesystem::walkToRoot($path, $this->root);
+    foreach ($paths as $path) {
       $file = $path.'/.editorconfig';
 
       if (!Filesystem::pathExists($file)) {
@@ -187,7 +193,7 @@ final class PhutilEditorConfig extends Phobject {
       if ($found_root) {
         break;
       }
-    } while ($path != $root && Filesystem::isDescendant($path, $root));
+    }
 
     return $configs;
   }

src/parser/PhutilJSONParser.php

@@ -16,8 +16,8 @@ final class PhutilJSONParser extends Phobject {
   }
 
   public function parse($json) {
-    $jsonlint_root = phutil_get_library_root('arcanist');
+    $arcanist_root = phutil_get_library_root('arcanist');
-    $jsonlint_root = $jsonlint_root.'/../externals/jsonlint';
+    $jsonlint_root = $arcanist_root.'/../externals/jsonlint';
 
     require_once $jsonlint_root.'/src/Seld/JsonLint/JsonParser.php';
     require_once $jsonlint_root.'/src/Seld/JsonLint/Lexer.php';

src/phage/__tests__/PhageAgentTestCase.php

@@ -3,6 +3,10 @@
 final class PhageAgentTestCase extends PhutilTestCase {
 
   public function testPhagePHPAgent() {
+    if (phutil_is_windows()) {
+      $this->assertSkipped(pht('Phage does not target Windows.'));
+    }
+
     return $this->runBootloaderTests(new PhagePHPAgentBootloader());
   }
 

src/symbols/PhutilClassMapQuery.php

@@ -45,6 +45,7 @@ final class PhutilClassMapQuery extends Phobject {
   private $filterNull = false;
   private $uniqueMethod;
   private $sortMethod;
+  private $continueOnFailure;
 
   // NOTE: If you add more configurable properties here, make sure that
   // cache key construction in getCacheKey() is updated properly.
@@ -162,6 +163,10 @@ final class PhutilClassMapQuery extends Phobject {
     return $this;
   }
 
+  public function setContinueOnFailure($continue) {
+    $this->continueOnFailure = $continue;
+    return $this;
+  }
 
 /* -(  Executing the Query  )------------------------------------------------ */
 
@@ -236,6 +241,7 @@ final class PhutilClassMapQuery extends Phobject {
 
     $objects = id(new PhutilSymbolLoader())
       ->setAncestorClass($ancestor)
+      ->setContinueOnFailure($this->continueOnFailure)
       ->loadObjects();
 
     // Apply the "expand" mechanism, if it is configured.

src/symbols/PhutilSymbolLoader.php

@@ -49,6 +49,7 @@ final class PhutilSymbolLoader {
   private $pathPrefix;
 
   private $suppressLoad;
+  private $continueOnFailure;
 
 
   /**
@@ -148,6 +149,10 @@ final class PhutilSymbolLoader {
     return $this;
   }
 
+  public function setContinueOnFailure($continue) {
+    $this->continueOnFailure = $continue;
+    return $this;
+  }
 
 /* -(  Load  )--------------------------------------------------------------- */
 
@@ -250,19 +255,55 @@ final class PhutilSymbolLoader {
     }
 
     if (!$this->suppressLoad) {
+      // Loading a class may trigger the autoloader to load more classes
+      // (usually, the parent class), so we need to keep track of whether we
+      // are currently loading in "continue on failure" mode. Otherwise, we'll
+      // fail anyway if we fail to load a parent class.
+
+      // The driving use case for the "continue on failure" mode is to let
+      // "arc liberate" run so it can rebuild the library map, even if you have
+      // made changes to Workflow or Config classes which it must load before
+      // it can operate. If we don't let it continue on failure, it is very
+      // difficult to remove or move Workflows.
+
+      static $continue_depth = 0;
+      if ($this->continueOnFailure) {
+        $continue_depth++;
+      }
+
       $caught = null;
-      foreach ($symbols as $symbol) {
+      foreach ($symbols as $key => $symbol) {
         try {
           $this->loadSymbol($symbol);
         } catch (Exception $ex) {
+          // If we failed to load this symbol, remove it from the results.
+          // Otherwise, we may fatal below when trying to reflect it.
+          unset($symbols[$key]);
+
           $caught = $ex;
         }
       }
+
+      $should_continue = ($continue_depth > 0);
+
+      if ($this->continueOnFailure) {
+        $continue_depth--;
+      }
+
       if ($caught) {
         // NOTE: We try to load everything even if we fail to load something,
         // primarily to make it possible to remove functions from a libphutil
         // library without breaking library startup.
-        throw $caught;
+        if ($should_continue) {
+          // We may not have `pht()` yet.
+          fprintf(
+            STDERR,
+            "%s: %s\n",
+            'IGNORING CLASS LOAD FAILURE',
+            $caught->getMessage());
+        } else {
+          throw $caught;
+        }
       }
     }
 
@@ -386,11 +427,11 @@ final class PhutilSymbolLoader {
     $load_failed = null;
     if ($is_function) {
       if (!function_exists($name)) {
-        $load_failed = 'function';
+        $load_failed = pht('function');
       }
     } else {
       if (!class_exists($name, false) && !interface_exists($name, false)) {
-        $load_failed = 'class/interface';
+        $load_failed = pht('class or interface');
       }
     }
 
@@ -400,13 +441,14 @@ final class PhutilSymbolLoader {
         $name,
         $load_failed,
         pht(
-          'The symbol map for library "%s" (at "%s") claims this symbol '.
+          "The symbol map for library '%s' (at '%s') claims this %s is ".
-          '(of type "%s") is defined in "%s", but loading that source file '.
+          "defined in '%s', but loading that source file did not cause the ".
-          'did not cause the symbol to become defined.',
+          "%s to become defined.",
           $lib_name,
           $lib_path,
           $load_failed,
-          $where));
+          $where,
+          $load_failed));
     }
   }
 

src/unit/engine/phutil/PhutilTestCase.php

@@ -20,6 +20,7 @@ abstract class PhutilTestCase extends Phobject {
   private $paths;
   private $renderer;
 
+  private static $executables = array();
 
 /* -(  Making Test Assertions  )--------------------------------------------- */
 
@@ -748,4 +749,37 @@ abstract class PhutilTestCase extends Phobject {
     throw new PhutilTestTerminatedException($output);
   }
 
+  final protected function assertExecutable($binary) {
+    if (!isset(self::$executables[$binary])) {
+      switch ($binary) {
+        case 'xhpast':
+          $ok = true;
+          if (!PhutilXHPASTBinary::isAvailable()) {
+            try {
+              PhutilXHPASTBinary::build();
+            } catch (Exception $ex) {
+              $ok = false;
+            }
+          }
+          break;
+        default:
+          $ok = Filesystem::binaryExists($binary);
+          break;
+      }
+
+      self::$executables[$binary] = $ok;
+    }
+
+    if (!self::$executables[$binary]) {
+      $this->assertSkipped(
+        pht('Required executable "%s" is not available.', $binary));
+    }
+  }
+
+  final protected function getSupportExecutable($executable) {
+    $root = dirname(phutil_get_library_root('arcanist'));
+    return $root.'/support/unit/'.$executable.'.php';
+  }
+
+
 }

src/utils/PhutilExecutionEnvironment.php

@@ -13,4 +13,34 @@ final class PhutilExecutionEnvironment extends Phobject {
     return php_uname('r');
   }
 
+  /**
+   * If the PHP configuration setting "variables_order" does not include "E",
+   * the `$_ENV` superglobal is not populated with the containing environment.
+   * For details, see T12071.
+   *
+   * This can be fixed by adding "E" to the configuration, but we can also
+   * repair it ourselves by re-executing a subprocess with the configuration
+   * option defined to include "E". This is clumsy, but saves users from
+   * needing to go find and edit their PHP files.
+   *
+   * @return void
+   */
+  public static function repairMissingVariablesOrder() {
+    $variables_order = ini_get('variables_order');
+    $variables_order = strtoupper($variables_order);
+
+    if (strpos($variables_order, 'E') !== false) {
+      // The "variables_order" option already has "E", so we don't need to
+      // repair $_ENV.
+      return;
+    }
+
+    list($env) = execx(
+      'php -d variables_order=E -r %s',
+      'echo json_encode($_ENV);');
+    $env = phutil_json_decode($env);
+
+    $_ENV = $_ENV + $env;
+  }
+
 }

src/utils/__tests__/PhutilUTF8TestCase.php

@@ -61,6 +61,13 @@ final class PhutilUTF8TestCase extends PhutilTestCase {
     );
 
     foreach ($map as $input => $expect) {
+      if ($input !== $expect) {
+        $this->assertEqual(
+          false,
+          phutil_is_utf8_slowly($input),
+          pht('Slowly reject overlong form of: %s', $input));
+      }
+
       $actual = phutil_utf8ize($input);
       $this->assertEqual(
         $expect,
@@ -77,6 +84,13 @@ final class PhutilUTF8TestCase extends PhutilTestCase {
     );
 
     foreach ($map as $input => $expect) {
+      if ($input !== $expect) {
+        $this->assertEqual(
+          false,
+          phutil_is_utf8_slowly($input),
+          pht('Slowly reject surrogate: %s', $input));
+      }
+
       $actual = phutil_utf8ize($input);
       $this->assertEqual(
         $expect,

src/utils/__tests__/PhutilUtilsTestCase.php

@@ -570,6 +570,7 @@ final class PhutilUtilsTestCase extends PhutilTestCase {
       } catch (Exception $ex) {
         $caught = $ex;
       }
+
       $this->assertTrue($caught instanceof PhutilJSONParserException);
     }
   }
@@ -965,5 +966,4 @@ final class PhutilUtilsTestCase extends PhutilTestCase {
     }
   }
 
-
 }

src/utils/utf8.php

@@ -149,6 +149,34 @@ function phutil_is_utf8_slowly($string, $only_bmp = false) {
         continue;
       }
       return false;
+    } else if ($chr == 0xED) {
+      // See T11525. Some sequences in this block are surrogate codepoints
+      // that are reserved for use in UTF16. We should reject them.
+      $codepoint = ($chr & 0x0F) << 12;
+      ++$ii;
+      if ($ii >= $len) {
+        return false;
+      }
+      $chr = ord($string[$ii]);
+      $codepoint += ($chr & 0x3F) << 6;
+      if ($chr >= 0x80 && $chr <= 0xBF) {
+        ++$ii;
+        if ($ii >= $len) {
+          return false;
+        }
+        $chr = ord($string[$ii]);
+        $codepoint += ($chr & 0x3F);
+
+        if ($codepoint >= 0xD800 && $codepoint <= 0xDFFF) {
+          // Reject these surrogate codepoints.
+          return false;
+        }
+
+        if ($chr >= 0x80 && $chr <= 0xBF) {
+          continue;
+        }
+      }
+      return false;
     } else if ($chr > 0xE0 && $chr <= 0xEF) {
       ++$ii;
       if ($ii >= $len) {

src/utils/utils.php

@@ -1065,8 +1065,8 @@ function phutil_fwrite_nonblocking_stream($stream, $bytes) {
   // the stream, write to it again if PHP claims that it's writable, and
   // consider the pipe broken if the write fails.
 
-  // (Signals received signals during the "fwrite()" do not appear to affect
+  // (Signals received during the "fwrite()" do not appear to affect anything,
-  // anything, see D20083.)
+  // see D20083.)
 
   $read = array();
   $write = array($stream);

src/xsprintf/PhutilTerminalString.php

@@ -70,6 +70,13 @@ final class PhutilTerminalString extends Phobject {
       $value = preg_replace('/\r(?!\n)/', '<CR>', $value);
     }
 
+    // See T13209. If we print certain invalid unicode byte sequences to the
+    // terminal under "cmd.exe", the entire string is silently dropped. Avoid
+    // printing invalid sequences.
+    if (phutil_is_windows()) {
+      $value = phutil_utf8ize($value);
+    }
+
     return $value;
   }
 }

src/xsprintf/__tests__/PhutilCsprintfTestCase.php

@@ -39,25 +39,33 @@ final class PhutilCsprintfTestCase extends PhutilTestCase {
   }
 
   public function testNoPowershell() {
-    if (!phutil_is_windows()) {
+    if (phutil_is_windows()) {
-      $cmd = csprintf('%s', '#');
+      // TOOLSETS: Restructure this. We must skip because tests fail if they
-      $cmd->setEscapingMode(PhutilCommandString::MODE_DEFAULT);
+      // do not make any assertions.
-
+      $this->assertSkipped(
-      $this->assertEqual(
+        pht(
-        '\'#\'',
+          'This test can not currently run under Windows.'));
-        (string)$cmd);
     }
+
+    $cmd = csprintf('%s', '#');
+    $cmd->setEscapingMode(PhutilCommandString::MODE_DEFAULT);
+
+    $this->assertEqual(
+      '\'#\'',
+      (string)$cmd);
   }
 
   public function testPasswords() {
+    $bin = $this->getSupportExecutable('echo');
+
     // Normal "%s" doesn't do anything special.
-    $command = csprintf('echo %s', 'hunter2trustno1');
+    $command = csprintf('php -f %R -- %s', $bin, 'hunter2trustno1');
     $this->assertTrue(strpos($command, 'hunter2trustno1') !== false);
 
     // "%P" takes a PhutilOpaqueEnvelope.
     $caught = null;
     try {
-      csprintf('echo %P', 'hunter2trustno1');
+      csprintf('php -f %R -- %P', $bin, 'hunter2trustno1');
     } catch (Exception $ex) {
       $caught = $ex;
     }
@@ -65,7 +73,10 @@ final class PhutilCsprintfTestCase extends PhutilTestCase {
 
 
     // "%P" masks the provided value.
-    $command = csprintf('echo %P', new PhutilOpaqueEnvelope('hunter2trustno1'));
+    $command = csprintf(
+      'php -f %R -- %P',
+      $bin,
+      new PhutilOpaqueEnvelope('hunter2trustno1'));
     $this->assertFalse(strpos($command, 'hunter2trustno1'));
 
 

support/init/init-script.php

@@ -1,12 +1,6 @@
 <?php
 
-if (function_exists('pcntl_async_signals')) {
+function __arcanist_init_script__() {
-  pcntl_async_signals(true);
-} else {
-  declare(ticks = 1);
-}
-
-function __phutil_init_script__() {
   // Adjust the runtime language configuration to be reasonable and inline with
   // expectations. We do this first, then load libraries.
 
@@ -88,6 +82,13 @@ function __phutil_init_script__() {
   require_once $root.'/src/init/init-library.php';
 
   PhutilErrorHandler::initialize();
+
+  PhutilErrorHandler::initialize();
+
+  // If "variables_order" excludes "E", silently repair it so that $_ENV has
+  // the values we expect.
+  PhutilExecutionEnvironment::repairMissingVariablesOrder();
+
   $router = PhutilSignalRouter::initialize();
 
   $handler = new PhutilBacktraceSignalHandler();
@@ -97,4 +98,4 @@ function __phutil_init_script__() {
   $router->installHandler('phutil.winch', $handler);
 }
 
-__phutil_init_script__();
+__arcanist_init_script__();

support/lib/extract-symbols.php

@@ -6,7 +6,7 @@
 $builtins = phutil_symbols_get_builtins();
 
 $root = dirname(dirname(dirname(__FILE__)));
-require_once $root.'/scripts/init/init-script.php';
+require_once $root.'/support/init/init-script.php';
 
 $args = new PhutilArgumentParser($argv);
 $args->setTagline(pht('identify symbols in a PHP source file'));

support/lib/rebuild-map.php

@@ -2,7 +2,7 @@
 <?php
 
 $root = dirname(dirname(dirname(__FILE__)));
-require_once $root.'/scripts/init/init-script.php';
+require_once $root.'/support/init/init-script.php';
 
 $args = new PhutilArgumentParser($argv);
 $args->setTagline(pht('rebuild the library map file'));

support/shell/hooks/bash-completion.sh

@@ -0,0 +1,9 @@
+SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" > /dev/null && pwd )"
+
+# Try to generate the shell completion rules if they do not yet exist.
+if [ ! -f "${SCRIPTDIR}/bash-rules.sh" ]; then
+  arc shell-complete --generate >/dev/null 2>/dev/null
+fi;
+
+# Source the shell completion rules.
+source "${SCRIPTDIR}/../rules/bash-rules.sh"

support/shell/templates/bash-template.sh

@@ -0,0 +1,22 @@
+_arcanist_complete_{{{BIN}}} ()
+{
+  COMPREPLY=()
+
+  RESULT=$(echo | {{{BIN}}} shell-complete \
+    --current ${COMP_CWORD} \
+    -- \
+    "${COMP_WORDS[@]}" \
+    2>/dev/null)
+
+  if [ $? -ne 0 ]; then
+    return $?
+  fi
+
+  if [ "$RESULT" == "<compgen:file>" ]; then
+    RESULT=$( compgen -A file -- ${COMP_WORDS[COMP_CWORD]} )
+  fi
+
+  local IFS=$'\n'
+  COMPREPLY=( $RESULT )
+}
+complete -F _arcanist_complete_{{{BIN}}} -o filenames {{{BIN}}}

support/unit/cat.php

@@ -0,0 +1,4 @@
+#!/usr/bin/env php
+<?php
+
+echo file_get_contents('php://stdin');

support/unit/echo.php

@@ -0,0 +1,9 @@
+#!/usr/bin/env php
+<?php
+
+$args = array_slice($argv, 1);
+foreach ($args as $key => $arg) {
+  $args[$key] = addcslashes($arg, "\\\n");
+}
+$args = implode("\n", $args);
+echo $args;

support/unit/exit.php

@@ -0,0 +1,4 @@
+#!/usr/bin/env php
+<?php
+
+exit(0);

support/unit/lock.php

@@ -1,7 +1,8 @@
 #!/usr/bin/env php
 <?php
 
-require_once dirname(__FILE__).'/../../scripts/init/init-script.php';
+$arcanist_root = dirname(dirname(dirname(__FILE__)));
+require_once $arcanist_root.'/support/init/init-script.php';
 
 $args = new PhutilArgumentParser($argv);
 $args->setTagline(pht('acquire and hold a lockfile'));

support/unit/sleep.php

@@ -0,0 +1,20 @@
+<?php
+
+if ($argc != 2) {
+  echo "usage: sleep <duration>\n";
+  exit(1);
+}
+
+// NOTE: Sleep for the requested duration even if our actual sleep() call is
+// interrupted by a signal.
+
+$then = microtime(true) + (double)$argv[1];
+while (true) {
+  $now = microtime(true);
+  if ($now >= $then) {
+    break;
+  }
+
+  $sleep = max(1, ($then - $now));
+  usleep((int)($sleep * 1000000));
+}

support/xhpast/build-xhpast.php

@@ -2,7 +2,7 @@
 <?php
 
 $root = dirname(dirname(dirname(__FILE__)));
-require_once $root.'/scripts/init/init-script.php';
+require_once $root.'/support/init/init-script.php';
 
 PhutilXHPASTBinary::build();