1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-08 16:02:39 +01:00

[Wilds] Shove the logging stuff into a bit of an abstraction before it gets out of control

Summary:
Ref T13098. The logging stuff is starting to get a little sketchy, so wrap it in several layers of class-based indirection before it can escape its cage.

This at least gives us an actual API and structure to work with later instead of scattering a lot of `fprintf(STDERR, ....)` all over the place.

Test Plan: Ran a few commands, got slightly more sensible/consistent logging out of them.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19699
This commit is contained in:
epriestley 2018-09-20 15:28:19 -07:00
parent e6c37bd4b3
commit b6f93a46d7
10 changed files with 186 additions and 46 deletions

View file

@ -311,6 +311,8 @@ phutil_register_library_map(array(
'ArcanistListConfigOption' => 'config/option/ArcanistListConfigOption.php',
'ArcanistListWorkflow' => 'workflow/ArcanistListWorkflow.php',
'ArcanistLocalConfigurationSource' => 'config/source/ArcanistLocalConfigurationSource.php',
'ArcanistLogEngine' => 'log/ArcanistLogEngine.php',
'ArcanistLogMessage' => 'log/ArcanistLogMessage.php',
'ArcanistLogicalOperatorsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLogicalOperatorsXHPASTLinterRule.php',
'ArcanistLogicalOperatorsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistLogicalOperatorsXHPASTLinterRuleTestCase.php',
'ArcanistLowercaseFunctionsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLowercaseFunctionsXHPASTLinterRule.php',
@ -1416,6 +1418,8 @@ phutil_register_library_map(array(
'ArcanistListConfigOption' => 'ArcanistConfigOption',
'ArcanistListWorkflow' => 'ArcanistWorkflow',
'ArcanistLocalConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
'ArcanistLogEngine' => 'Phobject',
'ArcanistLogMessage' => 'Phobject',
'ArcanistLogicalOperatorsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistLogicalOperatorsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistLowercaseFunctionsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',

View file

@ -131,7 +131,7 @@ final class ArcanistConfigurationSourceList
return $this->configOptions;
}
public function validateConfiguration() {
public function validateConfiguration(ArcanistRuntime $runtime) {
$options = $this->getConfigOptions();
$aliases = array();
@ -158,7 +158,7 @@ final class ArcanistConfigurationSourceList
// for config files we emit a warning; for "--config" we fatal.
if (!$option) {
$source->didReadUnknownOption($key);
$source->didReadUnknownOption($runtime, $key);
continue;
}

View file

@ -22,18 +22,13 @@ abstract class ArcanistConfigurationSource
return false;
}
public function didReadUnknownOption($key) {
// TOOLSETS: Standardize this kind of messaging? On ArcanistRuntime?
fprintf(
STDERR,
tsprintf(
"<bg:yellow>** %s **</bg> %s\n",
pht('WARNING'),
pht(
'Ignoring unrecognized configuration option ("%s") from source: %s.',
$key,
$this->getSourceDisplayName())));
public function didReadUnknownOption(ArcanistRuntime $runtime, $key) {
$runtime->getLogEngine()->writeWarning(
pht('UNKNOWN CONFIGURATION'),
pht(
'Ignoring unrecognized configuration option ("%s") from source: %s.',
$key,
$this->getSourceDisplayName()));
}
}

View file

@ -30,7 +30,7 @@ final class ArcanistRuntimeConfigurationSource
parent::__construct($map);
}
public function didReadUnknownOption($key) {
public function didReadUnknownOption(ArcanistRuntime $runtime, $key) {
throw new PhutilArgumentUsageException(
pht(
'Configuration option ("%s") specified with "--config" flag is not '.

View file

@ -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.',
),
);
}

View file

@ -0,0 +1,81 @@
<?php
final class ArcanistLogEngine
extends Phobject {
private $showTraceMessages;
public function setShowTraceMessages($show_trace_messages) {
$this->showTraceMessages = $show_trace_messages;
return $this;
}
public function getShowTraceMessages() {
return $this->showTraceMessages;
}
public function newMessage() {
return new ArcanistLogMessage();
}
public function writeMessage(ArcanistLogMessage $message) {
$color = $message->getColor();
fprintf(
STDERR,
'%s',
tsprintf(
"**<bg:".$color."> %s </bg>** %s\n",
$message->getLabel(),
$message->getMessage()));
return $message;
}
public function writeWarning($label, $message) {
return $this->writeMessage(
$this->newMessage()
->setColor('yellow')
->setLabel($label)
->setMessage($message));
}
public function writeError($label, $message) {
return $this->writeMessage(
$this->newMessage()
->setColor('red')
->setLabel($label)
->setMessage($message));
}
public function writeSuccess($label, $message) {
return $this->writeMessage(
$this->newMessage()
->setColor('green')
->setLabel($label)
->setMessage($message));
}
public function writeStatus($label, $message) {
return $this->writeMessage(
$this->newMessage()
->setColor('blue')
->setLabel($label)
->setMessage($message));
}
public function writeTrace($label, $message) {
$trace = $this->newMessage()
->setColor('magenta')
->setLabel($label)
->setMessage($message);
if ($this->getShowTraceMessages()) {
$this->writeMessage($trace);
}
return $trace;
}
}

View file

@ -0,0 +1,48 @@
<?php
final class ArcanistLogMessage
extends Phobject {
private $label;
private $message;
private $color;
private $channel;
public function setLabel($label) {
$this->label = $label;
return $this;
}
public function getLabel() {
return $this->label;
}
public function setColor($color) {
$this->color = $color;
return $this;
}
public function getColor() {
return $this->color;
}
public function setChannel($channel) {
$this->channel = $channel;
return $this;
}
public function getChannel() {
return $this->channel;
}
public function setMessage($message) {
$this->message = $message;
return $this;
}
public function getMessage() {
return $this->message;
}
}

View file

@ -185,4 +185,8 @@ abstract class ArcanistWorkflow extends Phobject {
return $this->conduitEngine;
}
final protected function getLogEngine() {
return $this->getRuntime()->getLogEngine();
}
}

View file

@ -31,6 +31,8 @@ EOTEXT
}
public function runWorkflow() {
$log = $this->getLogEngine();
$argv = $this->getArgument('argv');
if (count($argv) > 1) {
throw new ArcanistUsageException(
@ -38,6 +40,10 @@ EOTEXT
'Provide only one path to "arc liberate". The path should identify '.
'a directory where you want to create or update a library.'));
} else if (!$argv) {
$log->writeStatus(
pht('SCAN'),
pht('Searching for libraries in the current working directory...'));
$init_files = id(new FileFinder(getcwd()))
->withPath('*/__phutil_library_init__.php')
->find();
@ -60,9 +66,16 @@ EOTEXT
}
foreach ($paths as $path) {
$log->writeStatus(
pht('WORK'),
pht('Updating library: %s', Filesystem::readablePath($path)));
$this->liberatePath($path);
}
$log->writeSuccess(
pht('DONE'),
pht('Updated %s librarie(s).', phutil_count($paths)));
return 0;
}

View file

@ -3,6 +3,7 @@
final class ArcanistRuntime {
private $workflows;
private $logEngine;
public function execute(array $argv) {
@ -19,28 +20,23 @@ final class ArcanistRuntime {
->setLocale(PhutilLocale::loadLocale('en_US'))
->setTranslations(PhutilTranslation::getTranslationMapForLocale('en_US'));
$log = new ArcanistLogEngine();
$this->logEngine = $log;
try {
return $this->executeCore($argv);
} catch (ArcanistConduitException $ex) {
fwrite(
STDERR,
tsprintf(
"**%s:** %s\n",
pht('Conduit Exception'),
$ex->getMessage()));
$log->writeError(pht('CONDUIT'), $ex->getMessage());
} catch (PhutilArgumentUsageException $ex) {
fwrite(
STDERR,
tsprintf(
"**%s:** %s\n",
pht('Usage Exception'),
$ex->getMessage()));
return 77;
$log->writeError(pht('USAGE EXCEPTION'), $ex->getMessage());
}
return 1;
}
private function executeCore(array $argv) {
$log = $this->getLogEngine();
$config_args = array(
array(
'name' => 'library',
@ -68,9 +64,9 @@ final class ArcanistRuntime {
->parseStandardArguments();
$is_trace = $args->getArg('trace');
if ($is_trace) {
$this->logTrace(pht('ARGV'), csprintf('%Ls', $argv));
}
$log->setShowTraceMessages($is_trace);
$log->writeTrace(pht('ARGV'), csprintf('%Ls', $argv));
$args->parsePartial($config_args, true);
@ -83,7 +79,7 @@ final class ArcanistRuntime {
// Do this before continuing since configuration can impact other
// behaviors immediately and we want to catch any issues right away.
$config->setConfigOptions($config_engine->newConfigOptionsMap());
$config->validateConfiguration();
$config->validateConfiguration($this);
$toolset = $this->newToolset($argv);
@ -483,32 +479,26 @@ final class ArcanistRuntime {
return $map;
}
private function logTrace($label, $message) {
echo tsprintf(
"**<bg:magenta> %s </bg>** %s\n",
$label,
$message);
}
public function getWorkflows() {
return $this->workflows;
}
public function getLogEngine() {
return $this->logEngine;
}
private function applyAliasEffects(array $effects, array $argv) {
assert_instances_of($effects, 'ArcanistAliasEffect');
$log = $this->getLogEngine();
$command = null;
$arguments = null;
foreach ($effects as $effect) {
$message = $effect->getMessage();
if ($message !== null) {
fprintf(
STDERR,
tsprintf(
"**<bg:yellow> %s </bg>** %s\n",
pht('ALIAS'),
$message));
$log->writeInfo(pht('ALIAS'), $message);
}
if ($effect->getCommand()) {