Skip to content

Commit

Permalink
Merge utility/support changes from "wilds" to "master"
Browse files Browse the repository at this point in the history
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
  • Loading branch information
epriestley committed Feb 13, 2020
1 parent 0d62a10 commit acf0607
Show file tree
Hide file tree
Showing 80 changed files with 524 additions and 194 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@

# This is an OS X build artifact.
/support/xhpast/xhpast.dSYM

# Generated shell completion rulesets.
/support/shell/rules/
26 changes: 0 additions & 26 deletions resources/shell/bash-completion

This file was deleted.

2 changes: 1 addition & 1 deletion scripts/__init_script__.php
Original file line number Diff line number Diff line change
@@ -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';
18 changes: 13 additions & 5 deletions scripts/breakout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'. "
"It must support at least 30 columns and 15 rows of next-gen "
"full-color 3D graphics.")
'Your computer is not powerful enough to run "arc anoid". '
'It must support at least 32 columns and 15 rows of next-gen '
'full-color 3D graphics.')

status = curses.newwin(1, width, 0, 0)
height -= 1
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 0 additions & 2 deletions src/__phutil_library_map__.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@
'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',
Expand Down Expand Up @@ -1130,7 +1129,6 @@
'ArcanistJSHintLinter' => 'ArcanistExternalLinter',
'ArcanistJSHintLinterTestCase' => 'ArcanistExternalLinterTestCase',
'ArcanistJSONLintLinter' => 'ArcanistExternalLinter',
'ArcanistJSONLintLinterTestCase' => 'ArcanistExternalLinterTestCase',
'ArcanistJSONLintRenderer' => 'ArcanistLintRenderer',
'ArcanistJSONLinter' => 'ArcanistLinter',
'ArcanistJSONLinterTestCase' => 'ArcanistLinterTestCase',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ public function testCloseSocketWriteChannel() {
}

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);
Expand Down
27 changes: 21 additions & 6 deletions src/error/__tests__/PhutilOpaqueEnvelopeTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@ public function testOpaqueEnvelope() {
// 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));
Expand All @@ -24,23 +28,34 @@ public function testOpaqueEnvelope() {
$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();
}

Expand Down
36 changes: 31 additions & 5 deletions src/filesystem/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,8 @@ public static function readRandomBytes($number_of_bytes) {
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".',
__METHOD__.'()',
'php.ini'));
'installed but not enabled by default. Enable it in your "php.ini".',
__METHOD__.'()'));
}

throw new Exception(
Expand Down Expand Up @@ -950,8 +949,23 @@ public static function resolvePath($path, $relative_to = null) {
// 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);
Expand Down Expand Up @@ -1104,6 +1118,18 @@ public static function pathsAreEquivalent($u, $v) {
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 )------------------------------------------------------------- */

Expand Down
11 changes: 11 additions & 0 deletions src/filesystem/__tests__/FileFinderTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ public function testFinderWithNameAndSuffix() {
}

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.
Expand Down Expand Up @@ -211,6 +221,7 @@ private function assertFinder($label, FileFinder $finder, $expect) {
'php',
'shell',
);

foreach ($modes as $mode) {
$actual = id(clone $finder)
->setForceMode($mode)
Expand Down
6 changes: 6 additions & 0 deletions src/filesystem/__tests__/FilesystemTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ public function testWalkToRoot() {
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));
Expand Down
20 changes: 12 additions & 8 deletions src/filesystem/__tests__/PhutilFileLockTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,16 +170,20 @@ private function holdLock($file) {
throw new Exception(pht('Unable to hold lock in external process!'));
}

private function buildLockFuture($flags, $file) {
$root = dirname(phutil_get_library_root('arcanist'));
$bin = $root.'/support/test/lock-file.php';

$flags = (array)$flags;
private function buildLockFuture(/* ... */) {
$argv = func_get_args();
$bin = $this->getSupportExecutable('lock');

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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ public function testExecException() {
}

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();
Expand Down
6 changes: 4 additions & 2 deletions src/future/__tests__/FutureIteratorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
final class FutureIteratorTestCase extends PhutilTestCase {

public function testAddingFuture() {
$future1 = new ExecFuture('cat');
$future2 = new ExecFuture('cat');
$bin = $this->getSupportExecutable('cat');

$future1 = new ExecFuture('php -f %R', $bin);
$future2 = new ExecFuture('php -f %R', $bin);

$iterator = new FutureIterator(array($future1));
$iterator->limit(2);
Expand Down
Loading

0 comments on commit acf0607

Please sign in to comment.