diff --git a/conf/aphlict/aphlict.default.json b/conf/aphlict/aphlict.default.json --- a/conf/aphlict/aphlict.default.json +++ b/conf/aphlict/aphlict.default.json @@ -14,5 +14,11 @@ "ssl.key": null, "ssl.cert": null } - ] + ], + "logs": [ + { + "path": "/var/log/aphlict.log" + } + ], + "pidfile": "/var/tmp/aphlict/pid/aphlict.pid" } diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php --- a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php @@ -4,6 +4,7 @@ extends PhabricatorManagementWorkflow { private $debug = false; + private $configData; private $configPath; final protected function setDebug($debug) { @@ -74,6 +75,8 @@ $data, array( 'servers' => 'list', + 'logs' => 'optional list', + 'pidfile' => 'string', )); } catch (Exception $ex) { throw new PhutilArgumentUsageException( @@ -174,43 +177,54 @@ 'admin')); } - $this->configPath = $full_path; - } + $logs = $data['logs']; + foreach ($logs as $index => $log) { + PhutilTypeSpec::checkMap( + $log, + array( + 'path' => 'string', + )); - final public function getPIDPath() { - $path = PhabricatorEnv::getEnvConfig('notification.pidfile'); + $path = $log['path']; - try { - $dir = dirname($path); - if (!Filesystem::pathExists($dir)) { - Filesystem::createDirectory($dir, 0755, true); + try { + $dir = dirname($path); + if (!Filesystem::pathExists($dir)) { + Filesystem::createDirectory($dir, 0755, true); + } + } catch (FilesystemException $ex) { + throw new PhutilArgumentUsageException( + pht( + 'Failed to create directory "%s" for specified log file (with '. + 'index "%s"). You should manually create this directory or '. + 'choose a different logfile location. %s', + $dir, + $ex->getMessage())); } - } catch (FilesystemException $ex) { - throw new Exception( - pht( - "Failed to create '%s'. You should manually create this directory.", - $dir)); } - return $path; - } - - final public function getLogPath() { - $path = PhabricatorEnv::getEnvConfig('notification.log'); + $this->configData = $data; + $this->configPath = $full_path; + $pid_path = $this->getPIDPath(); try { $dir = dirname($path); if (!Filesystem::pathExists($dir)) { Filesystem::createDirectory($dir, 0755, true); } } catch (FilesystemException $ex) { - throw new Exception( + throw new PhutilArgumentUsageException( pht( - "Failed to create '%s'. You should manually create this directory.", - $dir)); + 'Failed to create directory "%s" for specified PID file. You '. + 'should manually create this directory or choose a different '. + 'PID file location. %s', + $dir, + $ex->getMessage())); } + } - return $path; + final public function getPIDPath() { + return $this->configData['pidfile']; } final public function getPID() { @@ -293,12 +307,8 @@ } private function getServerArgv() { - $log = $this->getLogPath(); - $server_argv = array(); $server_argv[] = '--config='.$this->configPath; - $server_argv[] = '--log='.$log; - return $server_argv; } diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -305,6 +305,8 @@ 'notification.ssl-cert' => $aphlict_reason, 'notification.ssl-key' => $aphlict_reason, + 'notification.pidfile' => $aphlict_reason, + 'notification.log' => $aphlict_reason, ); return $ancient_config; diff --git a/src/applications/config/option/PhabricatorNotificationConfigOptions.php b/src/applications/config/option/PhabricatorNotificationConfigOptions.php --- a/src/applications/config/option/PhabricatorNotificationConfigOptions.php +++ b/src/applications/config/option/PhabricatorNotificationConfigOptions.php @@ -44,13 +44,6 @@ 'string', 'http://localhost:22281/') ->setDescription(pht('Location of the notification receiver server.')), - $this->newOption('notification.log', 'string', '/var/log/aphlict.log') - ->setDescription(pht('Location of the server log file.')), - $this->newOption( - 'notification.pidfile', - 'string', - '/var/tmp/aphlict/pid/aphlict.pid') - ->setDescription(pht('Location of the server PID file.')), ); } diff --git a/src/docs/user/configuration/notifications.diviner b/src/docs/user/configuration/notifications.diviner --- a/src/docs/user/configuration/notifications.diviner +++ b/src/docs/user/configuration/notifications.diviner @@ -75,7 +75,9 @@ The configuration file has these settings: - - `servers`: A list of servers to start. + - `servers`: //Required list.// A list of servers to start. + - `logs`: //Optional list.// A list of logs to write to. + - `pidfile`: //Required string.// Path to a PID file. Each server in the `servers` list should be an object with these keys: @@ -91,6 +93,10 @@ - `ssl.cert`: //Optional string.// If you want to use SSL on this port, the path to an SSL certificate. +Each log in the `logs` list should be an object with these keys: + + - `path`: //Required string.// Path to the log file. + The defaults are appropriate for simple cases, but you may need to adjust them if you are running a more complex configuration. @@ -104,9 +110,6 @@ connect to in order to listen for notifications. - `notification.server-uri` Internally-facing host and port that Phabricator will connect to in order to publish notifications. - - `notification.log` Log file location for the server. - - `notification.pidfile` Pidfile location used to stop any running server when - aphlict is restarted. Verifying Server Status diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js --- a/support/aphlict/server/aphlict_server.js +++ b/support/aphlict/server/aphlict_server.js @@ -8,7 +8,6 @@ function parse_command_line_arguments(argv) { var args = { - log: '/var/log/aphlict.log', test: false, config: null }; @@ -49,9 +48,9 @@ process.on('uncaughtException', function(err) { var context = null; - if (err.code == 'EACCES' && err.path == args.log) { + if (err.code == 'EACCES') { context = util.format( - 'Unable to open logfile ("%s"). Check that permissions are set ' + + 'Unable to open file ("%s"). Check that permissions are set ' + 'correctly.', err.path); } @@ -68,11 +67,6 @@ set_exit_code(1); }); -// Add the logfile so we'll fail if we can't write to it. -if (args.log) { - debug.addLog(args.log); -} - try { require('ws'); } catch (ex) { @@ -89,6 +83,12 @@ require('./lib/AphlictClientServer'); var ii; + +var logs = config.logs || []; +for (ii = 0; ii < logs.length; ii++) { + debug.addLog(logs[ii].path); +} + var servers = []; for (ii = 0; ii < config.servers.length; ii++) { var spec = config.servers[ii]; @@ -116,6 +116,10 @@ debug.log('Starting servers (service PID %d).', process.pid); +for (ii = 0; ii < logs.length; ii++) { + debug.log('Logging to "%s".', logs[ii].path); +} + var aphlict_servers = []; var aphlict_clients = []; var aphlict_admins = [];