From 2c1687721e57b1917dc3f3bed62aa01b59078f5c Mon Sep 17 00:00:00 2001 From: David Steele Date: Mon, 19 Mar 2018 13:08:42 -0400 Subject: [PATCH] Fix issue where specifying log-level-stderr > warn would cause a local/remote process to error on exit due to output found on stderr when none was expected. The max value for a local/remote process is now error since there's no reason for these processes to emit warnings. Reported by Clinton Adams. --- build/lib/pgBackRestBuild/Config/Build.pm | 2 ++ build/lib/pgBackRestBuild/Config/Data.pm | 13 +++++++ doc/xml/release.xml | 8 +++++ src/config/config.auto.c | 16 +++++++++ src/config/config.c | 13 +++++++ src/config/config.h | 1 + src/config/load.c | 41 ++++++++++++----------- test/src/module/config/configTest.c | 6 +++- test/src/module/config/loadTest.c | 7 ++-- 9 files changed, 84 insertions(+), 23 deletions(-) diff --git a/build/lib/pgBackRestBuild/Config/Build.pm b/build/lib/pgBackRestBuild/Config/Build.pm index 94fb679dc..e1e4baed8 100644 --- a/build/lib/pgBackRestBuild/Config/Build.pm +++ b/build/lib/pgBackRestBuild/Config/Build.pm @@ -145,6 +145,8 @@ sub buildConfig "\n" . " CONFIG_COMMAND_LOG_FILE(" . ($rhCommand->{&CFGDEF_LOG_FILE} ? 'true' : 'false') . ")\n" . " CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevel" . ucfirst(lc($rhCommand->{&CFGDEF_LOG_LEVEL_DEFAULT})) . ")\n" . + " CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevel" . + ucfirst(lc($rhCommand->{&CFGDEF_LOG_LEVEL_STDERR_MAX})) . ")\n" . " )\n"; $iCommandTotal++; diff --git a/build/lib/pgBackRestBuild/Config/Data.pm b/build/lib/pgBackRestBuild/Config/Data.pm index d0ea2d5a6..38c3e0da3 100644 --- a/build/lib/pgBackRestBuild/Config/Data.pm +++ b/build/lib/pgBackRestBuild/Config/Data.pm @@ -448,6 +448,11 @@ use constant CFGDEF_LOG_FILE => 'log-file use constant CFGDEF_LOG_LEVEL_DEFAULT => 'log-level-default'; push @EXPORT, qw(CFGDEF_LOG_LEVEL_DEFAULT); +# Defines the max log level that may be assigned for the output type. For instance, it's not OK for the local and remote processes +# to log anything but errors to stderr because it will cause the process to error on exit. +use constant CFGDEF_LOG_LEVEL_STDERR_MAX => 'log-level-stderr-max'; + push @EXPORT, qw(CFGDEF_LOG_LEVEL_STDERR_MAX); + # Option defines #----------------------------------------------------------------------------------------------------------------------------------- use constant CFGDEF_ALLOW_LIST => 'allow-list'; @@ -554,11 +559,13 @@ my $rhCommandDefine = &CFGCMD_LOCAL => { &CFGDEF_LOG_FILE => false, + &CFGDEF_LOG_LEVEL_STDERR_MAX => ERROR, }, &CFGCMD_REMOTE => { &CFGDEF_LOG_FILE => false, + &CFGDEF_LOG_LEVEL_STDERR_MAX => ERROR, }, &CFGCMD_RESTORE => @@ -2194,6 +2201,12 @@ foreach my $strCommand (sort(keys(%{$rhCommandDefine}))) { $rhCommandDefine->{$strCommand}{&CFGDEF_LOG_LEVEL_DEFAULT} = INFO; } + + # Default max stderr log level is TRACE + if (!defined($rhCommandDefine->{$strCommand}{&CFGDEF_LOG_LEVEL_STDERR_MAX})) + { + $rhCommandDefine->{$strCommand}{&CFGDEF_LOG_LEVEL_STDERR_MAX} = TRACE; + } } #################################################################################################################################### diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 56cd660bc..c0e73e5c1 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -38,6 +38,14 @@

Fix issue passing --no-config to embedded Perl.

+ + + + + + +

Fix issue where specifying log-level-stderr > warn would cause a local/remote process to error on exit due to output found on stderr when none was expected. The max value for a local/remote process is now error since there's no reason for these processes to emit warnings.

+ diff --git a/src/config/config.auto.c b/src/config/config.auto.c index 635a5c621..4bffa3acd 100644 --- a/src/config/config.auto.c +++ b/src/config/config.auto.c @@ -15,6 +15,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -23,6 +24,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -31,6 +33,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -39,6 +42,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -47,6 +51,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -55,6 +60,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelDebug) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -63,6 +69,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelDebug) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -71,6 +78,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelError) ) CONFIG_COMMAND @@ -79,6 +87,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelError) ) CONFIG_COMMAND @@ -87,6 +96,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -95,6 +105,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -103,6 +114,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -111,6 +123,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -119,6 +132,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -127,6 +141,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(true) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelInfo) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) CONFIG_COMMAND @@ -135,6 +150,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L CONFIG_COMMAND_LOG_FILE(false) CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelDebug) + CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace) ) ) diff --git a/src/config/config.c b/src/config/config.c index 43120545d..361c56b76 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -16,6 +16,7 @@ typedef struct ConfigCommandData const char *name; bool logFile:1; unsigned int logLevelDefault:4; + unsigned int logLevelStdErrMax:4; } ConfigCommandData; #define CONFIG_COMMAND_LIST(...) \ @@ -28,6 +29,8 @@ typedef struct ConfigCommandData .logFile = logFileParam, #define CONFIG_COMMAND_LOG_LEVEL_DEFAULT(logLevelDefaultParam) \ .logLevelDefault = logLevelDefaultParam, +#define CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelStdErrMaxParam) \ + .logLevelStdErrMax = logLevelStdErrMaxParam, #define CONFIG_COMMAND_NAME(nameParam) \ .name = nameParam, @@ -294,6 +297,16 @@ cfgLogLevelDefault() return (LogLevel)configCommandData[cfgCommand()].logLevelDefault; } +/*********************************************************************************************************************************** +Get max stderr log level -- used to suppress error output for higher log levels, e.g. local and remote commands +***********************************************************************************************************************************/ +LogLevel +cfgLogLevelStdErrMax() +{ + ASSERT_DEBUG_COMMAND_SET(); + return (LogLevel)configCommandData[cfgCommand()].logLevelStdErrMax; +} + /*********************************************************************************************************************************** Ensure that option id is valid ***********************************************************************************************************************************/ diff --git a/src/config/config.h b/src/config/config.h index 7aecb9c37..a83a9061f 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -32,6 +32,7 @@ const char *cfgCommandName(ConfigCommand commandId); bool cfgLogFile(); LogLevel cfgLogLevelDefault(); +LogLevel cfgLogLevelStdErrMax(); const StringList *cfgCommandParam(); diff --git a/src/config/load.c b/src/config/load.c index e85b99c15..2d73ca1b4 100644 --- a/src/config/load.c +++ b/src/config/load.c @@ -29,28 +29,31 @@ cfgLoadParam(unsigned int argListSize, const char *argList[], String *exe) configParse(argListSize, argList); // Initialize logging - if (cfgCommand() != cfgCmdLocal && cfgCommand() != cfgCmdRemote) + LogLevel logLevelConsole = logLevelOff; + LogLevel logLevelStdErr = logLevelOff; + LogLevel logLevelFile = logLevelOff; + bool logTimestamp = true; + + if (cfgOptionValid(cfgOptLogLevelConsole)) + logLevelConsole = logLevelEnum(strPtr(cfgOptionStr(cfgOptLogLevelConsole))); + + if (cfgOptionValid(cfgOptLogLevelStderr)) { - LogLevel logLevelConsole = logLevelOff; - LogLevel logLevelStdErr = logLevelOff; - LogLevel logLevelFile = logLevelOff; - bool logTimestamp = true; + logLevelStdErr = logLevelEnum(strPtr(cfgOptionStr(cfgOptLogLevelStderr))); - if (cfgOptionValid(cfgOptLogLevelConsole)) - logLevelConsole = logLevelEnum(strPtr(cfgOptionStr(cfgOptLogLevelConsole))); - - if (cfgOptionValid(cfgOptLogLevelStderr)) - logLevelStdErr = logLevelEnum(strPtr(cfgOptionStr(cfgOptLogLevelStderr))); - - if (cfgOptionValid(cfgOptLogLevelFile)) - logLevelFile = logLevelEnum(strPtr(cfgOptionStr(cfgOptLogLevelFile))); - - if (cfgOptionValid(cfgOptLogTimestamp)) - logTimestamp = cfgOptionBool(cfgOptLogTimestamp); - - logInit(logLevelConsole, logLevelStdErr, logLevelFile, logTimestamp); + // If configured log level exceeds the max for a command, set it to the max + if (logLevelStdErr > cfgLogLevelStdErrMax()) + logLevelStdErr = cfgLogLevelStdErrMax(); } + if (cfgOptionValid(cfgOptLogLevelFile)) + logLevelFile = logLevelEnum(strPtr(cfgOptionStr(cfgOptLogLevelFile))); + + if (cfgOptionValid(cfgOptLogTimestamp)) + logTimestamp = cfgOptionBool(cfgOptLogTimestamp); + + logInit(logLevelConsole, logLevelStdErr, logLevelFile, logTimestamp); + // Only continue if a command was set. If no command is set then help will be displayed if (cfgCommand() != cfgCmdNone) { @@ -65,7 +68,7 @@ cfgLoadParam(unsigned int argListSize, const char *argList[], String *exe) // Begin the command cmdBegin(); - // If an exe was passed in the use that + // If an exe was passed in then use it if (exe != NULL) cfgExeSet(exe); diff --git a/test/src/module/config/configTest.c b/test/src/module/config/configTest.c index ea7c7d6fa..43fd7fbee 100644 --- a/test/src/module/config/configTest.c +++ b/test/src/module/config/configTest.c @@ -83,8 +83,12 @@ testRun() TEST_RESULT_VOID(cfgCommandSet(cfgCmdBackup), "command set to backup"); TEST_RESULT_INT(cfgLogLevelDefault(), logLevelInfo, "default log level is info"); TEST_RESULT_BOOL(cfgLogFile(), true, "log file is on"); - TEST_RESULT_VOID(cfgCommandSet(cfgCmdInfo), "command set to archive-push"); + TEST_RESULT_VOID(cfgCommandSet(cfgCmdInfo), "command set to info"); TEST_RESULT_INT(cfgLogLevelDefault(), logLevelDebug, "default log level is debug"); + TEST_RESULT_INT(cfgLogLevelStdErrMax(), logLevelTrace, "max stderr log level is trace"); + TEST_RESULT_BOOL(cfgLogFile(), false, "log file is off"); + TEST_RESULT_VOID(cfgCommandSet(cfgCmdLocal), "command set to local"); + TEST_RESULT_INT(cfgLogLevelStdErrMax(), logLevelError, "max stderr log level is error"); TEST_RESULT_BOOL(cfgLogFile(), false, "log file is off"); // ------------------------------------------------------------------------------------------------------------------------- diff --git a/test/src/module/config/loadTest.c b/test/src/module/config/loadTest.c index 8da844f14..d3b84c994 100644 --- a/test/src/module/config/loadTest.c +++ b/test/src/module/config/loadTest.c @@ -24,7 +24,6 @@ testRun() TEST_RESULT_INT(logLevelStdErr, logLevelWarn, "stderr logging is error"); // ------------------------------------------------------------------------------------------------------------------------- - argList = strLstNew(); strLstAdd(argList, strNew("pgbackrest")); strLstAdd(argList, strNew("--host-id=1")); @@ -32,13 +31,15 @@ testRun() strLstAdd(argList, strNew("--command=backup")); strLstAdd(argList, strNew("--stanza=db")); strLstAdd(argList, strNew("--type=backup")); + strLstAdd(argList, strNew("--log-level-stderr=info")); strLstAdd(argList, strNew("local")); TEST_RESULT_VOID(cfgLoad(strLstSize(argList), strLstPtr(argList)), "load local config"); TEST_RESULT_STR(strPtr(cfgExe()), "pgbackrest", "check exe"); - TEST_RESULT_INT(logLevelStdOut, logLevelWarn, "console logging is warn"); - TEST_RESULT_INT(logLevelStdErr, logLevelWarn, "stderr logging is warn"); + TEST_RESULT_INT(logLevelStdOut, logLevelOff, "console logging is off"); + TEST_RESULT_INT(logLevelStdErr, logLevelError, "stderr logging is error"); + TEST_RESULT_INT(logLevelFile, logLevelOff, "file logging is off"); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_VOID(cfgLoadParam(strLstSize(argList), strLstPtr(argList), strNew("pgbackrest2")), "load local config");