From f3037bf216e52e14f01e5c93158c6c767b94696d Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Wed, 8 Feb 2017 16:24:20 -0800 Subject: [PATCH 1/5] [git] Override `diff.submodule` so `git diff` output is always parseable Test Plan: Removed a submodule with `diff.submodule` set to `log`, saw `arc diff` error; with this change, it no longer does. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Maniphest Tasks: T10881 Differential Revision: https://secure.phabricator.com/D17327 --- src/repository/api/ArcanistGitAPI.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php index abe0a220..8efe1bce 100644 --- a/src/repository/api/ArcanistGitAPI.php +++ b/src/repository/api/ArcanistGitAPI.php @@ -446,6 +446,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { // would ship up the binaries for 'arc patch' but display the textconv // output in the visual diff. '--no-textconv', + // Provide a standard view of submodule changes; the 'log' and 'diff' + // values do not parse by the diff parser. + '--submodule=short', ); return implode(' ', $options); } From bc9b70508edf224a15a3bdb45998f7fd75847765 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Wed, 15 Feb 2017 13:38:57 +0000 Subject: [PATCH 2/5] arc set-config: warn about unknown keys Summary: See T12266. Warn when the user tries to set a value we will probably not read. Test Plan: `arc set-config foo bar`, see fancy warning. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D17357 --- src/workflow/ArcanistSetConfigWorkflow.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/workflow/ArcanistSetConfigWorkflow.php b/src/workflow/ArcanistSetConfigWorkflow.php index 9d9b099d..7bad38c1 100644 --- a/src/workflow/ArcanistSetConfigWorkflow.php +++ b/src/workflow/ArcanistSetConfigWorkflow.php @@ -69,6 +69,16 @@ EOTEXT $settings = new ArcanistSettings(); + $console = PhutilConsole::getConsole(); + + if (!$settings->getHelp($key)) { + $warn = pht( + 'The configuration key \'%s\' is not recognized by arc. It may '. + 'be misspelled or out of date.', + $key); + $console->writeErr("**%s:** %s\n", pht('Warning'), $warn); + } + $old = null; if (array_key_exists($key, $config)) { $old = $config[$key]; @@ -85,12 +95,12 @@ EOTEXT $old = $settings->formatConfigValueForDisplay($key, $old); if ($old === null) { - echo pht( + $console->writeOut( "Deleted key '%s' from %s config.\n", $key, $which); } else { - echo pht( + $console->writeOut( "Deleted key '%s' from %s config (was %s).\n", $key, $which, @@ -110,13 +120,13 @@ EOTEXT $old = $settings->formatConfigValueForDisplay($key, $old); if ($old === null) { - echo pht( + $console->writeOut( "Set key '%s' = %s in %s config.\n", $key, $val, $which); } else { - echo pht( + $console->writeOut( "Set key '%s' = %s in %s config (was %s).\n", $key, $val, From 13596cd10fbe8bc98956ce4cb47c15a0936cbc78 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Thu, 16 Feb 2017 11:08:45 +0000 Subject: [PATCH 3/5] Add back pht()s and tsprintf()s to arg set-config Summary: See D17357 Test Plan: invoke and still see output in English? Reviewers: #blessed_reviewers, joshuaspence, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D17362 --- src/workflow/ArcanistSetConfigWorkflow.php | 31 +++++++++++++--------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/workflow/ArcanistSetConfigWorkflow.php b/src/workflow/ArcanistSetConfigWorkflow.php index 7bad38c1..75b4334f 100644 --- a/src/workflow/ArcanistSetConfigWorkflow.php +++ b/src/workflow/ArcanistSetConfigWorkflow.php @@ -72,11 +72,14 @@ EOTEXT $console = PhutilConsole::getConsole(); if (!$settings->getHelp($key)) { - $warn = pht( - 'The configuration key \'%s\' is not recognized by arc. It may '. - 'be misspelled or out of date.', - $key); - $console->writeErr("**%s:** %s\n", pht('Warning'), $warn); + $warning = tsprintf( + "**%s:** %s\n", + pht('Warning'), + pht( + 'The configuration key "%s" is not recognized by arc. It may '. + 'be misspelled or out of date.', + $key)); + $console->writeErr('%s', $warning); } $old = null; @@ -95,17 +98,18 @@ EOTEXT $old = $settings->formatConfigValueForDisplay($key, $old); if ($old === null) { - $console->writeOut( - "Deleted key '%s' from %s config.\n", + $message = pht( + 'Deleted key "%s" from %s config.', $key, $which); } else { - $console->writeOut( - "Deleted key '%s' from %s config (was %s).\n", + $message = pht( + 'Deleted key "%s" from %s config (was %s).', $key, $which, $old); } + $console->writeOut('%s', tsprintf("%s\n", $message)); } else { $val = $settings->willWriteValue($key, $val); @@ -120,19 +124,20 @@ EOTEXT $old = $settings->formatConfigValueForDisplay($key, $old); if ($old === null) { - $console->writeOut( - "Set key '%s' = %s in %s config.\n", + $message = pht( + 'Set key "%s" = %s in %s config.', $key, $val, $which); } else { - $console->writeOut( - "Set key '%s' = %s in %s config (was %s).\n", + $message = pht( + 'Set key "%s" = %s in %s config (was %s).', $key, $val, $which, $old); } + $console->writeOut('%s', tsprintf("%s\n", $message)); } return 0; From 460b0e46eea0b380dcb04154ead43f0adc7a710d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 16 Feb 2017 05:08:51 -0800 Subject: [PATCH 4/5] Fix a property name collision in ArcanistHgServerChannel Summary: See D2665. Here, `channel` is the Mercurial wire protocol channel name ("o" = "output", "e" = "error", etc), not the underlying channel from `PhutilProxyChannel`. Rename the property so there's no collision. Test Plan: ``` $ ../../core/lib/arcanist/scripts/hgdaemon/hgdaemon_client.php . --trace --skip-hello array(3) { [0]=> int(0) [1]=> string(103) "capabilities: getencoding runcommand encoding: UTF-8 pid: 73263e20cf21273d50f1f66cab6e0f7c74f4864e67f0f" [2]=> string(0) "" } Executed in 30647 us. ``` Reviewers: vrana, chad Reviewed By: vrana Differential Revision: https://secure.phabricator.com/D17366 --- src/hgdaemon/ArcanistHgServerChannel.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hgdaemon/ArcanistHgServerChannel.php b/src/hgdaemon/ArcanistHgServerChannel.php index 1850c9af..22f6b4ab 100644 --- a/src/hgdaemon/ArcanistHgServerChannel.php +++ b/src/hgdaemon/ArcanistHgServerChannel.php @@ -54,6 +54,7 @@ final class ArcanistHgServerChannel extends PhutilProtocolChannel { private $mode = self::MODE_CHANNEL; private $byteLengthOfNextChunk = 1; private $buf = ''; + private $outputChannel; /* -( Protocol Implementation )-------------------------------------------- */ @@ -137,7 +138,7 @@ final class ArcanistHgServerChannel extends PhutilProtocolChannel { // 'output', 'error', 'result' or 'debug' respectively. This is a // single byte long. Next, we'll expect a length. - $this->channel = $chunk; + $this->outputChannel = $chunk; $this->byteLengthOfNextChunk = 4; $this->mode = self::MODE_LENGTH; break; @@ -153,11 +154,11 @@ final class ArcanistHgServerChannel extends PhutilProtocolChannel { // given length. We produce a message from the channel and the data // and return it. Next, we expect another channel name. - $message = array($this->channel, $chunk); + $message = array($this->outputChannel, $chunk); $this->byteLengthOfNextChunk = 1; $this->mode = self::MODE_CHANNEL; - $this->channel = null; + $this->outputChannel = null; $messages[] = $message; break; From d0353d238113bfc1c1a3341288e29a7d26128a52 Mon Sep 17 00:00:00 2001 From: Jakub Vrana Date: Thu, 16 Feb 2017 13:54:43 +0000 Subject: [PATCH 5/5] Fix errors found by PHPStan Test Plan: Ran `phpstan analyze -a autoload.php arcanist/src` with `autoload.php` containing: restoreWhenDestroyed) { - $this->writeWARN( + $this->writeWarn( pht('INTERRUPTED!'), pht('Restoring working copy to its original state.')); diff --git a/src/lint/linter/ArcanistBaseXHPASTLinter.php b/src/lint/linter/ArcanistBaseXHPASTLinter.php index 72a9bd7e..f0f44827 100644 --- a/src/lint/linter/ArcanistBaseXHPASTLinter.php +++ b/src/lint/linter/ArcanistBaseXHPASTLinter.php @@ -189,7 +189,7 @@ abstract class ArcanistBaseXHPASTLinter extends ArcanistFutureLinter { * Get a path's parse exception from the responsible linter. * * @param string Path to retrieve exception for. - * @return Exeption|null Parse exception, if available. + * @return Exception|null Parse exception, if available. * @task sharing */ final protected function getXHPASTExceptionForPath($path) { diff --git a/src/lint/linter/__tests__/ArcanistClosureLinterTestCase.php b/src/lint/linter/__tests__/ArcanistClosureLinterTestCase.php index 50c01ca5..f02dc21b 100644 --- a/src/lint/linter/__tests__/ArcanistClosureLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistClosureLinterTestCase.php @@ -3,11 +3,14 @@ final class ArcanistClosureLinterTestCase extends ArcanistExternalLinterTestCase { - public function testLinter() { + protected function getLinter() { $linter = new ArcanistClosureLinter(); $linter->setFlags(array('--additional_extensions=lint-test')); + return $linter; + } - $this->executeTestsInDirectory(dirname(__FILE__).'/gjslint/', $linter); + public function testLinter() { + $this->executeTestsInDirectory(dirname(__FILE__).'/gjslint/'); } } diff --git a/src/unit/parser/__tests__/XUnitTestResultParserTestCase.php b/src/unit/parser/__tests__/XUnitTestResultParserTestCase.php index c26356d0..960bb46c 100644 --- a/src/unit/parser/__tests__/XUnitTestResultParserTestCase.php +++ b/src/unit/parser/__tests__/XUnitTestResultParserTestCase.php @@ -28,7 +28,7 @@ final class XUnitTestResultParserTestCase extends PhutilTestCase { $parsed_results = id(new ArcanistXUnitTestResultParser()) ->parseTestResults(''); - $this->failTest(pht('Should throw on empty input')); + $this->assertFailure(pht('Should throw on empty input')); } catch (Exception $e) { // OK } @@ -43,7 +43,7 @@ final class XUnitTestResultParserTestCase extends PhutilTestCase { $parsed_results = id(new ArcanistXUnitTestResultParser()) ->parseTestResults($stubbed_results); - $this->failTest(pht('Should throw on non-xml input')); + $this->assertFailure(pht('Should throw on non-xml input')); } catch (Exception $e) { // OK } diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index 884f3a54..8323ce24 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -514,9 +514,7 @@ EOTEXT $this->branch = head($branch); $this->keepBranch = $this->getArgument('keep-branch'); - $update_strategy = $this->getConfigFromAnySource( - 'arc.land.update.default', - 'merge'); + $update_strategy = $this->getConfigFromAnySource('arc.land.update.default'); $this->shouldUpdateWithRebase = $update_strategy == 'rebase'; if ($this->getArgument('update-with-rebase')) { $this->shouldUpdateWithRebase = true; diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index bb53cde7..36126c5d 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/src/workflow/ArcanistLintWorkflow.php @@ -227,7 +227,7 @@ EOTEXT } if ($everything) { - $paths = iterator_to_array($this->getRepositoryApi()->getAllFiles()); + $paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles()); $this->shouldLintAll = true; } else { $paths = $this->selectPathsForWorkflow($paths, $rev); diff --git a/src/workflow/ArcanistStartWorkflow.php b/src/workflow/ArcanistStartWorkflow.php index b940265e..b39cea5a 100644 --- a/src/workflow/ArcanistStartWorkflow.php +++ b/src/workflow/ArcanistStartWorkflow.php @@ -76,7 +76,7 @@ EOTEXT "%s: %s\n\n", pht('Started'), implode(', ', ipull($phid_query, 'fullName'))); - $this->printCurrentTracking(true); + $this->printCurrentTracking(); } } diff --git a/src/workflow/ArcanistStopWorkflow.php b/src/workflow/ArcanistStopWorkflow.php index 1e9f2518..2af72edb 100644 --- a/src/workflow/ArcanistStopWorkflow.php +++ b/src/workflow/ArcanistStopWorkflow.php @@ -105,7 +105,7 @@ EOTEXT "%s %s\n\n", pht('Stopped:'), implode(', ', ipull($phid_query, 'fullName'))); - $this->printCurrentTracking(true); + $this->printCurrentTracking(); } } diff --git a/src/workflow/ArcanistTasksWorkflow.php b/src/workflow/ArcanistTasksWorkflow.php index 95cc7de4..3b9a0789 100644 --- a/src/workflow/ArcanistTasksWorkflow.php +++ b/src/workflow/ArcanistTasksWorkflow.php @@ -81,7 +81,7 @@ EOTEXT $unassigned = $this->getArgument('unassigned'); if ($owner) { - $owner_phid = $this->findOwnerPhid($owner); + $owner_phid = $this->findOwnerPHID($owner); } else if ($unassigned) { $owner_phid = null; } else { diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php index a1aadd3b..00ef8a59 100644 --- a/src/workflow/ArcanistUnitWorkflow.php +++ b/src/workflow/ArcanistUnitWorkflow.php @@ -140,7 +140,7 @@ EOTEXT } if ($everything) { - $paths = iterator_to_array($this->getRepositoryApi()->getAllFiles()); + $paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles()); } else { $paths = $this->selectPathsForWorkflow($paths, $rev); } diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index 2365083a..192cf243 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -1008,9 +1008,7 @@ abstract class ArcanistWorkflow extends Phobject { } $should_commit = true; } else { - $permit_autostash = $this->getConfigFromAnySource( - 'arc.autostash', - false); + $permit_autostash = $this->getConfigFromAnySource('arc.autostash'); if ($permit_autostash && $api->canStashChanges()) { echo pht( 'Stashing uncommitted changes. (You can restore them with `%s`).',