diff --git a/doc/xml/release.xml b/doc/xml/release.xml index c70243a91..0123a3e7a 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -97,6 +97,16 @@

Add StringId type.

+ + + + + + + + +

Add cfgOptionDisplay()/cfgOptionIdxDisplay().

+
diff --git a/src/command/archive/push/push.c b/src/command/archive/push/push.c index 8226c32dd..c614a0a2b 100644 --- a/src/command/archive/push/push.c +++ b/src/command/archive/push/push.c @@ -390,7 +390,7 @@ cmdArchivePush(void) { THROW_FMT( ArchiveTimeoutError, "unable to push WAL file '%s' to the archive asynchronously after %s second(s)", - strZ(archiveFile), strZ(strNewDbl((double)cfgOptionInt64(cfgOptArchiveTimeout) / MSEC_PER_SEC))); + strZ(archiveFile), strZ(cfgOptionDisplay(cfgOptArchiveTimeout))); } // Log success diff --git a/src/command/backup/backup.c b/src/command/backup/backup.c index 4f900f815..51500556b 100644 --- a/src/command/backup/backup.c +++ b/src/command/backup/backup.c @@ -409,7 +409,7 @@ backupBuildIncrPrior(const InfoBackup *infoBackup) { LOG_WARN_FMT( "%s backup cannot alter " CFGOPT_COMPRESS_TYPE " option to '%s', reset to value in %s", - strZ(cfgOptionStr(cfgOptType)), strZ(cfgOptionStr(cfgOptCompressType)), strZ(backupLabelPrior)); + strZ(cfgOptionDisplay(cfgOptType)), strZ(cfgOptionDisplay(cfgOptCompressType)), strZ(backupLabelPrior)); // Set the compression type back to whatever was in the prior backup. This is not strictly needed since we // could store compression type on a per file basis, but it seems simplest and safest for now. @@ -433,8 +433,7 @@ backupBuildIncrPrior(const InfoBackup *infoBackup) { LOG_WARN_FMT( "%s backup cannot alter hardlink option to '%s', reset to value in %s", - strZ(cfgOptionStr(cfgOptType)), cvtBoolToConstZ(cfgOptionBool(cfgOptRepoHardlink)), - strZ(backupLabelPrior)); + strZ(cfgOptionDisplay(cfgOptType)), strZ(cfgOptionDisplay(cfgOptRepoHardlink)), strZ(backupLabelPrior)); cfgOptionSet(cfgOptRepoHardlink, cfgSourceParam, VARBOOL(manifestPriorData->backupOptionHardLink)); } @@ -455,7 +454,7 @@ backupBuildIncrPrior(const InfoBackup *infoBackup) { LOG_WARN_FMT( "%s backup cannot alter '" CFGOPT_CHECKSUM_PAGE "' option to '%s', reset to '%s' from %s", - strZ(cfgOptionStr(cfgOptType)), cvtBoolToConstZ(cfgOptionBool(cfgOptChecksumPage)), + strZ(cfgOptionDisplay(cfgOptType)), strZ(cfgOptionDisplay(cfgOptChecksumPage)), cvtBoolToConstZ(checksumPagePrior), strZ(manifestData(result)->backupLabel)); } @@ -466,7 +465,7 @@ backupBuildIncrPrior(const InfoBackup *infoBackup) } else { - LOG_WARN_FMT("no prior backup exists, %s backup has been changed to full", strZ(cfgOptionStr(cfgOptType))); + LOG_WARN_FMT("no prior backup exists, %s backup has been changed to full", strZ(cfgOptionDisplay(cfgOptType))); cfgOptionSet(cfgOptType, cfgSourceParam, VARSTR(backupTypeStr(backupTypeFull))); } } @@ -725,7 +724,7 @@ backupResumeFind(const Manifest *manifest, const String *cipherPassBackup) { reason = strNewFmt( "new backup type '%s' does not match resumable backup type '%s'", - strZ(cfgOptionStr(cfgOptType)), strZ(backupTypeStr(manifestResumeData->backupType))); + strZ(cfgOptionDisplay(cfgOptType)), strZ(backupTypeStr(manifestResumeData->backupType))); } // Check prior backup label ??? Do we really care about the prior backup label? else if (!strEq(manifestResumeData->backupLabelPrior, manifestData(manifest)->backupLabelPrior)) @@ -742,7 +741,7 @@ backupResumeFind(const Manifest *manifest, const String *cipherPassBackup) { reason = strNewFmt( "new compression '%s' does not match resumable compression '%s'", - strZ(cfgOptionStr(cfgOptCompressType)), + strZ(cfgOptionDisplay(cfgOptCompressType)), strZ(compressTypeStr(manifestResumeData->backupOptionCompressType))); } else diff --git a/src/command/check/common.c b/src/command/check/common.c index a0f6e8915..7208d4548 100644 --- a/src/command/check/common.c +++ b/src/command/check/common.c @@ -65,7 +65,7 @@ checkDbConfig(const unsigned int pgVersion, const unsigned int pgIdx, const Db * DbMismatchError, "version '%s' and path '%s' queried from cluster do not match version '%s' and '%s' read from '%s/" PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL "'\nHINT: the %s and %s settings likely reference different clusters.", strZ(pgVersionToStr(dbVersion)), strZ(dbPath), strZ(pgVersionToStr(pgVersion)), - strZ(cfgOptionIdxStr(cfgOptPgPath, pgIdx)), strZ(cfgOptionIdxStr(cfgOptPgPath, pgIdx)), + strZ(cfgOptionIdxDisplay(cfgOptPgPath, pgIdx)), strZ(cfgOptionIdxDisplay(cfgOptPgPath, pgIdx)), cfgOptionIdxName(cfgOptPgPath, pgIdx), cfgOptionIdxName(cfgOptPgPort, pgIdx)); } diff --git a/src/command/command.c b/src/command/command.c index d3f06b308..d6fdfb557 100644 --- a/src/command/command.c +++ b/src/command/command.c @@ -126,17 +126,11 @@ cmdOption(void) { valueList = strLstNewVarLst(cfgOptionIdxLst(optionId, optionIdx)); } - // Generate time value - else if (cfgParseOptionType(optionId) == cfgOptTypeTime) - { - valueList = strLstNew(); - strLstAdd(valueList, strNewDbl((double)cfgOptionIdxInt64(optionId, optionIdx) / MSEC_PER_SEC)); - } // Else only one value else { valueList = strLstNew(); - strLstAdd(valueList, varStrForce(cfgOptionIdx(optionId, optionIdx))); + strLstAdd(valueList, cfgOptionIdxDisplay(optionId, optionIdx)); } // Output options and values diff --git a/src/command/control/common.c b/src/command/control/common.c index 1afe27d40..fcf76b584 100644 --- a/src/command/control/common.c +++ b/src/command/control/common.c @@ -33,7 +33,7 @@ lockStopTest(void) if (cfgOptionTest(cfgOptStanza)) { if (storageExistsP(storageLocal(), lockStopFileName(cfgOptionStr(cfgOptStanza)))) - THROW_FMT(StopError, "stop file exists for stanza %s", strZ(cfgOptionStr(cfgOptStanza))); + THROW_FMT(StopError, "stop file exists for stanza %s", strZ(cfgOptionDisplay(cfgOptStanza))); } // Check all stanzas diff --git a/src/command/control/start.c b/src/command/control/start.c index 6c61a28e4..6d7444bf3 100644 --- a/src/command/control/start.c +++ b/src/command/control/start.c @@ -30,7 +30,7 @@ cmdStart(void) { LOG_WARN_FMT( "stop file does not exist%s", - (cfgOptionTest(cfgOptStanza) ? strZ(strNewFmt(" for stanza %s", strZ(cfgOptionStr(cfgOptStanza)))) : "")); + (cfgOptionTest(cfgOptStanza) ? strZ(strNewFmt(" for stanza %s", strZ(cfgOptionDisplay(cfgOptStanza)))) : "")); } } MEM_CONTEXT_TEMP_END(); diff --git a/src/command/control/stop.c b/src/command/control/stop.c index e698766a8..9efaaa557 100644 --- a/src/command/control/stop.c +++ b/src/command/control/stop.c @@ -101,7 +101,7 @@ cmdStop(void) { LOG_WARN_FMT( "stop file already exists for %s", - cfgOptionTest(cfgOptStanza) ? strZ(strNewFmt("stanza %s", strZ(cfgOptionStr(cfgOptStanza)))) : "all stanzas"); + cfgOptionTest(cfgOptStanza) ? strZ(strNewFmt("stanza %s", strZ(cfgOptionDisplay(cfgOptStanza)))) : "all stanzas"); } } MEM_CONTEXT_TEMP_END(); diff --git a/src/command/help/help.c b/src/command/help/help.c index 9ca5e3aa8..746e0a6ad 100644 --- a/src/command/help/help.c +++ b/src/command/help/help.c @@ -12,7 +12,7 @@ Help Command #include "common/io/fdWrite.h" #include "common/memContext.h" #include "common/type/pack.h" -#include "config/config.h" +#include "config/config.intern.h" #include "config/parse.h" #include "version.h" @@ -205,10 +205,8 @@ helpRenderValue(const Variant *value, ConfigOptionType type) result = resultTemp; } - else if (type == cfgOptTypeTime) - result = strNewDbl((double)varInt64(value) / MSEC_PER_SEC); else - result = varStrForce(value); + result = cfgOptionDisplayVar(value, type); } FUNCTION_LOG_RETURN_CONST(STRING, result); diff --git a/src/command/local/local.c b/src/command/local/local.c index d146fe2bf..7579e1f01 100644 --- a/src/command/local/local.c +++ b/src/command/local/local.c @@ -37,7 +37,7 @@ cmdLocal(int fdRead, int fdWrite) MEM_CONTEXT_TEMP_BEGIN() { - String *name = strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u", cfgOptionUInt(cfgOptProcess)); + String *name = strNewFmt(PROTOCOL_SERVICE_LOCAL "-%s", strZ(cfgOptionDisplay(cfgOptProcess))); IoRead *read = ioFdReadNew(name, fdRead, cfgOptionUInt64(cfgOptProtocolTimeout)); ioReadOpen(read); IoWrite *write = ioFdWriteNew(name, fdWrite, cfgOptionUInt64(cfgOptProtocolTimeout)); diff --git a/src/command/remote/remote.c b/src/command/remote/remote.c index 24d5a11c0..b215cc1ed 100644 --- a/src/command/remote/remote.c +++ b/src/command/remote/remote.c @@ -35,7 +35,7 @@ cmdRemote(int fdRead, int fdWrite) MEM_CONTEXT_TEMP_BEGIN() { - String *name = strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u", cfgOptionUInt(cfgOptProcess)); + String *name = strNewFmt(PROTOCOL_SERVICE_REMOTE "-%s", strZ(cfgOptionDisplay(cfgOptProcess))); IoRead *read = ioFdReadNew(name, fdRead, cfgOptionUInt64(cfgOptProtocolTimeout)); ioReadOpen(read); IoWrite *write = ioFdWriteNew(name, fdWrite, cfgOptionUInt64(cfgOptProtocolTimeout)); diff --git a/src/command/repo/get.c b/src/command/repo/get.c index 92967a834..87225df82 100644 --- a/src/command/repo/get.c +++ b/src/command/repo/get.c @@ -48,7 +48,7 @@ storageGetProcess(IoWrite *destination) { THROW_FMT( OptionInvalidValueError, "absolute path '%s' is not in base path '%s'", strZ(file), - strZ(cfgOptionStr(cfgOptRepoPath))); + strZ(cfgOptionDisplay(cfgOptRepoPath))); } // Get the relative part of the file @@ -95,7 +95,7 @@ storageGetProcess(IoWrite *destination) { THROW_FMT( OptionInvalidValueError, "stanza name '%s' given in option doesn't match the given path", - strZ(cfgOptionStr(cfgOptStanza))); + strZ(cfgOptionDisplay(cfgOptStanza))); } // Archive path diff --git a/src/command/restore/restore.c b/src/command/restore/restore.c index e9b77ff80..5d9d17e79 100644 --- a/src/command/restore/restore.c +++ b/src/command/restore/restore.c @@ -82,7 +82,7 @@ restorePathValidate(void) // The PGDATA directory must exist // ??? We should remove this requirement in a separate commit. What's the harm in creating the dir assuming we have perms? if (!storagePathExistsP(storagePg(), NULL)) - THROW_FMT(PathMissingError, "$PGDATA directory '%s' does not exist", strZ(cfgOptionStr(cfgOptPgPath))); + THROW_FMT(PathMissingError, "$PGDATA directory '%s' does not exist", strZ(cfgOptionDisplay(cfgOptPgPath))); // PostgreSQL must not be running if (storageExistsP(storagePg(), PG_FILE_POSTMASTERPID_STR)) @@ -92,7 +92,7 @@ restorePathValidate(void) "unable to restore while PostgreSQL is running\n" "HINT: presence of '" PG_FILE_POSTMASTERPID "' in '%s' indicates PostgreSQL is running.\n" "HINT: remove '" PG_FILE_POSTMASTERPID "' only if PostgreSQL is not running.", - strZ(cfgOptionStr(cfgOptPgPath))); + strZ(cfgOptionDisplay(cfgOptPgPath))); } // If the restore will be destructive attempt to verify that PGDATA looks like a valid PostgreSQL directory @@ -103,7 +103,7 @@ restorePathValidate(void) "--delta or --force specified but unable to find '" PG_FILE_PGVERSION "' or '" BACKUP_MANIFEST_FILE "' in '%s' to" " confirm that this is a valid $PGDATA directory. --delta and --force have been disabled and if any files" " exist in the destination directories the restore will be aborted.", - strZ(cfgOptionStr(cfgOptPgPath))); + strZ(cfgOptionDisplay(cfgOptPgPath))); // Disable delta and force so restore will fail if the directories are not empty cfgOptionSet(cfgOptDelta, cfgSourceDefault, VARBOOL(false)); @@ -391,7 +391,7 @@ restoreBackupSet(void) LOG_WARN_FMT( "unable to find backup set with stop time less than '%s', repo%u: latest backup set will be used", - strZ(cfgOptionStr(cfgOptTarget)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, result.repoIdx)); + strZ(cfgOptionDisplay(cfgOptTarget)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, result.repoIdx)); } else THROW(BackupSetInvalidError, "no backup set found to restore"); diff --git a/src/command/stanza/create.c b/src/command/stanza/create.c index b197abb61..bc955e0b3 100644 --- a/src/command/stanza/create.c +++ b/src/command/stanza/create.c @@ -44,7 +44,7 @@ cmdStanzaCreate(void) for (unsigned int repoIdx = 0; repoIdx < cfgOptionGroupIdxTotal(cfgOptGrpRepo); repoIdx++) { LOG_INFO_FMT( - CFGCMD_STANZA_CREATE " for stanza '%s' on repo%u", strZ(cfgOptionStr(cfgOptStanza)), + CFGCMD_STANZA_CREATE " for stanza '%s' on repo%u", strZ(cfgOptionDisplay(cfgOptStanza)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, repoIdx)); const Storage *storageRepoReadStanza = storageRepoIdx(repoIdx); @@ -133,7 +133,7 @@ cmdStanzaCreate(void) if (sourceFile == NULL) { LOG_INFO_FMT( - "stanza '%s' already exists on repo%u and is valid", strZ(cfgOptionStr(cfgOptStanza)), + "stanza '%s' already exists on repo%u and is valid", strZ(cfgOptionDisplay(cfgOptStanza)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, repoIdx)); } } diff --git a/src/command/stanza/delete.c b/src/command/stanza/delete.c index c0217e8dd..ec0b58d53 100644 --- a/src/command/stanza/delete.c +++ b/src/command/stanza/delete.c @@ -79,7 +79,7 @@ stanzaDelete(const Storage *storageRepoWriteStanza, const StringList *archiveLis THROW_FMT( FileMissingError, "stop file does not exist for stanza '%s'\n" "HINT: has the pgbackrest stop command been run on this server for this stanza?", - strZ(cfgOptionStr(cfgOptStanza))); + strZ(cfgOptionDisplay(cfgOptStanza))); } // If a force has not been issued and Postgres is running, then error @@ -88,9 +88,9 @@ stanzaDelete(const Storage *storageRepoWriteStanza, const StringList *archiveLis THROW_FMT( PgRunningError, PG_FILE_POSTMASTERPID " exists - looks like " PG_NAME " is running. " "To delete stanza '%s' on repo%u, shut down " PG_NAME " for stanza '%s' and try again, or use --force.", - strZ(cfgOptionStr(cfgOptStanza)), + strZ(cfgOptionDisplay(cfgOptStanza)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, cfgOptionGroupIdxDefault(cfgOptGrpRepo)), - strZ(cfgOptionStr(cfgOptStanza))); + strZ(cfgOptionDisplay(cfgOptStanza))); } // Delete the archive info files diff --git a/src/command/stanza/upgrade.c b/src/command/stanza/upgrade.c index 3ba5c4e18..bab7e54e6 100644 --- a/src/command/stanza/upgrade.c +++ b/src/command/stanza/upgrade.c @@ -41,7 +41,7 @@ cmdStanzaUpgrade(void) for (unsigned int repoIdx = 0; repoIdx < cfgOptionGroupIdxTotal(cfgOptGrpRepo); repoIdx++) { LOG_INFO_FMT( - CFGCMD_STANZA_UPGRADE " for stanza '%s' on repo%u", strZ(cfgOptionStr(cfgOptStanza)), + CFGCMD_STANZA_UPGRADE " for stanza '%s' on repo%u", strZ(cfgOptionDisplay(cfgOptStanza)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, repoIdx)); const Storage *storageRepoReadStanza = storageRepoIdx(repoIdx); @@ -100,7 +100,7 @@ cmdStanzaUpgrade(void) if (!(infoArchiveUpgrade || infoBackupUpgrade)) { LOG_INFO_FMT( - "stanza '%s' on repo%u is already up to date", strZ(cfgOptionStr(cfgOptStanza)), + "stanza '%s' on repo%u is already up to date", strZ(cfgOptionDisplay(cfgOptStanza)), cfgOptionGroupIdxToKey(cfgOptGrpRepo, repoIdx)); } } diff --git a/src/config/config.c b/src/config/config.c index 4153e838e..361e40198 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -584,7 +584,10 @@ cfgOptionDefaultSet(ConfigOption optionId, const Variant *defaultValue) for (unsigned int optionIdx = 0; optionIdx < cfgOptionIdxTotal(optionId); optionIdx++) { if (configLocal->option[optionId].index[optionIdx].source == cfgSourceDefault) + { configLocal->option[optionId].index[optionIdx].value = configLocal->option[optionId].defaultValue; + configLocal->option[optionId].index[optionIdx].display = NULL; + } } } MEM_CONTEXT_END(); @@ -592,6 +595,81 @@ cfgOptionDefaultSet(ConfigOption optionId, const Variant *defaultValue) FUNCTION_TEST_RETURN_VOID(); } + +/**********************************************************************************************************************************/ +const String * +cfgOptionDisplayVar(const Variant *const value, const ConfigOptionType optionType) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(VARIANT, value); + FUNCTION_TEST_PARAM(UINT, optionType); + FUNCTION_TEST_END(); + + ASSERT(value != NULL); + ASSERT(optionType != cfgOptTypeHash && optionType != cfgOptTypeList); + + if (varType(value) == varTypeString) + { + FUNCTION_TEST_RETURN(varStr(value)); + } + else if (optionType == cfgOptTypeBoolean) + { + FUNCTION_TEST_RETURN(varBool(value) ? TRUE_STR : FALSE_STR); + } + else if (optionType == cfgOptTypeTime) + { + FUNCTION_TEST_RETURN(strNewDbl((double)varInt64(value) / MSEC_PER_SEC)); + } + + FUNCTION_TEST_RETURN(varStrForce(value)); +} + +const String * +cfgOptionIdxDisplay(const ConfigOption optionId, const unsigned int optionIdx) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(ENUM, optionId); + FUNCTION_TEST_PARAM(UINT, optionIdx); + FUNCTION_TEST_END(); + + ASSERT(optionId < CFG_OPTION_TOTAL); + ASSERT(configLocal != NULL); + ASSERT( + (!configLocal->option[optionId].group && optionIdx == 0) || + (configLocal->option[optionId].group && optionIdx < + configLocal->optionGroup[configLocal->option[optionId].groupId].indexTotal)); + + // Check that the option is valid for the current command + if (!cfgOptionValid(optionId)) + THROW_FMT(AssertError, "option '%s' is not valid for the current command", cfgOptionIdxName(optionId, optionIdx)); + + // If there is already a display value set then return that + ConfigOptionValue *const option = &configLocal->option[optionId].index[optionIdx]; + + if (option->display != NULL) + FUNCTION_TEST_RETURN(option->display); + + // Generate the display value based on the type + MEM_CONTEXT_BEGIN(configLocal->memContext) + { + option->display = cfgOptionDisplayVar(option->value, cfgParseOptionType(optionId)); + } + MEM_CONTEXT_END(); + + + FUNCTION_TEST_RETURN(option->display); +} + +const String * +cfgOptionDisplay(const ConfigOption optionId) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(ENUM, optionId); + FUNCTION_TEST_END(); + + FUNCTION_TEST_RETURN(cfgOptionIdxDisplay(optionId, cfgOptionIdxDefault(optionId))); +} + /**********************************************************************************************************************************/ String * cfgOptionHostPort(ConfigOption optionId, unsigned int *port) @@ -1183,6 +1261,9 @@ cfgOptionIdxSet(ConfigOption optionId, unsigned int optionIdx, ConfigSource sour } else configLocal->option[optionId].index[optionIdx].value = NULL; + + // Clear the display value, which will be generated when needed + configLocal->option[optionId].index[optionIdx].display = NULL; } MEM_CONTEXT_END(); diff --git a/src/config/config.h b/src/config/config.h index 783680585..3d087a14a 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -137,6 +137,13 @@ unsigned int cfgOptionIdxUInt(ConfigOption optionId, unsigned int optionIdx); uint64_t cfgOptionUInt64(ConfigOption optionId); uint64_t cfgOptionIdxUInt64(ConfigOption optionId, unsigned int optionIdx); +// Format the configuration value for display to the user or passing on a command line. If the value was set by the user via the +// command line, config, etc., then that exact value will be displayed. This makes it easier for the user to recognize the value and +// saves having to format it into something reasonable, especially for time and size option types. Note that cfgOptTypeHash and +// cfgOptTypeList option types are not supported by these functions, but they are generally not displayed to the user as a whole. +const String *cfgOptionDisplay(const ConfigOption optionId); +const String *cfgOptionIdxDisplay(const ConfigOption optionId, const unsigned int optionIdx); + // Option name by id const char *cfgOptionName(ConfigOption optionId); const char *cfgOptionIdxName(ConfigOption optionId, unsigned int optionIdx); diff --git a/src/config/config.intern.h b/src/config/config.intern.h index eee9196a5..4fe9d4d62 100644 --- a/src/config/config.intern.h +++ b/src/config/config.intern.h @@ -10,6 +10,7 @@ The general-purpose functions for querying the current configuration are found i #define CONFIG_CONFIG_INTERN_H #include "config/config.h" +#include "config/parse.auto.h" /*********************************************************************************************************************************** The maximum number of keys that an indexed option can have, e.g. pg256-path would be the maximum pg-path option @@ -25,6 +26,7 @@ typedef struct ConfigOptionValue bool negate; // Is the option negated? bool reset; // Is the option reset? unsigned int source; // Where the option came from, i.e. ConfigSource enum + const String *display; // Current display value, if any. Used for messages, etc. const Variant *value; // Value } ConfigOptionValue; @@ -87,6 +89,10 @@ unsigned int cfgOptionGroupId(ConfigOption optionId); /*********************************************************************************************************************************** Option Functions ***********************************************************************************************************************************/ +// Format a variant for display using the supplied option type. cfgOptionDisplay()/cfgOptionIdxDisplay() should be used whenever +// possible, but sometimes the variant needs to be manipulated before being formatted. +const String *cfgOptionDisplayVar(const Variant *const value, const ConfigOptionType optionType); + // Convert the key used in the original configuration to a group index. This is used when an option key must be translated into the // local group index, e.g. during parsing or when getting the value of specific options from a remote. unsigned int cfgOptionKeyToIdx(ConfigOption optionId, unsigned int key); diff --git a/src/config/exec.c b/src/config/exec.c index a0d6eccc5..1b3b93a05 100644 --- a/src/config/exec.c +++ b/src/config/exec.c @@ -110,16 +110,11 @@ cfgExecParam(ConfigCommand commandId, ConfigCommandRole commandRoleId, const Key { valueList = strLstNewVarLst(varVarLst(value)); } - else if (cfgParseOptionType(optionId) == cfgOptTypeTime) - { - valueList = strLstNew(); - strLstAdd(valueList, strNewDbl((double)varInt64(value) / MSEC_PER_SEC)); - } // Else only one value else { valueList = strLstNew(); - strLstAdd(valueList, varStrForce(value)); + strLstAdd(valueList, cfgOptionDisplayVar(value, cfgParseOptionType(optionId))); } // Output options and values diff --git a/src/config/load.c b/src/config/load.c index ec6e9bf35..442368f9c 100644 --- a/src/config/load.c +++ b/src/config/load.c @@ -101,7 +101,7 @@ cfgLoadUpdateOption(void) OptionInvalidValueError, "local repo%u and repo%u paths are both '%s' but must be different", cfgOptionGroupIdxToKey(cfgOptGrpRepo, optionIdx), cfgOptionGroupIdxToKey(cfgOptGrpRepo, repoIdx), - strZ(cfgOptionIdxStr(cfgOptRepoPath, repoIdx))); + strZ(cfgOptionIdxDisplay(cfgOptRepoPath, repoIdx))); } } } @@ -145,8 +145,8 @@ cfgLoadUpdateOption(void) OptionInvalidValueError, "'%s' is not valid for '" CFGOPT_PROTOCOL_TIMEOUT "' option\nHINT '" CFGOPT_PROTOCOL_TIMEOUT "' option (%s)" " should be greater than '" CFGOPT_DB_TIMEOUT "' option (%s).", - strZ(varStrForce(cfgOption(cfgOptProtocolTimeout))), strZ(varStrForce(cfgOption(cfgOptProtocolTimeout))), - strZ(varStrForce(cfgOption(cfgOptDbTimeout)))); + strZ(cfgOptionDisplay(cfgOptProtocolTimeout)), strZ(cfgOptionDisplay(cfgOptProtocolTimeout)), + strZ(cfgOptionDisplay(cfgOptDbTimeout))); } } @@ -186,7 +186,7 @@ cfgLoadUpdateOption(void) "option '%s' is not set for '%s=%s', the repository may run out of space" "\nHINT: to retain full backups indefinitely (without warning), set option '%s' to the maximum.", cfgOptionIdxName(cfgOptRepoRetentionFull, optionIdx), cfgOptionIdxName(cfgOptRepoRetentionFullType, optionIdx), - strZ(cfgOptionIdxStr(cfgOptRepoRetentionFullType, optionIdx)), + strZ(cfgOptionIdxDisplay(cfgOptRepoRetentionFullType, optionIdx)), cfgOptionIdxName(cfgOptRepoRetentionFull, optionIdx)); } } @@ -277,7 +277,7 @@ cfgLoadUpdateOption(void) "\nHINT: RFC-2818 forbids dots in wildcard matches." "\nHINT: TLS/SSL verification cannot proceed with this bucket name." "\nHINT: remove dots from the bucket name.", - strZ(cfgOptionIdxStr(cfgOptRepoS3Bucket, repoIdx)), cfgOptionIdxName(cfgOptRepoS3Bucket, repoIdx)); + strZ(cfgOptionIdxDisplay(cfgOptRepoS3Bucket, repoIdx)), cfgOptionIdxName(cfgOptRepoS3Bucket, repoIdx)); } } diff --git a/src/config/parse.c b/src/config/parse.c index 85d015934..2f547bb7c 100644 --- a/src/config/parse.c +++ b/src/config/parse.c @@ -1667,6 +1667,13 @@ configParse(const Storage *storage, unsigned int argListSize, const char *argLis { int64_t valueInt64 = 0; + // Preserve original value to display + MEM_CONTEXT_BEGIN(config->memContext) + { + configOptionValue->display = strDup(value); + } + MEM_CONTEXT_END(); + // Check that the value can be converted TRY_BEGIN() { @@ -1726,7 +1733,7 @@ configParse(const Storage *storage, unsigned int argListSize, const char *argLis cfgParseOptionKeyIdxName(optionId, optionKeyIdx)); } } - // Else if path make sure it is valid + // Else if string make sure it is valid else { // Make sure it is long enough to be a path @@ -1737,6 +1744,7 @@ configParse(const Storage *storage, unsigned int argListSize, const char *argLis cfgParseOptionKeyIdxName(optionId, optionKeyIdx)); } + // If path make sure it is valid if (optionType == cfgOptTypePath) { // Make sure it starts with / @@ -1877,7 +1885,7 @@ configParse(const Storage *storage, unsigned int argListSize, const char *argLis if (index == cfgOptionGroupIdxTotal(groupId)) { THROW_FMT( - OptionInvalidValueError, "key '%u' is not valid for '%s' option", cfgOptionUInt(defaultOptionId), + OptionInvalidValueError, "key '%s' is not valid for '%s' option", strZ(cfgOptionDisplay(defaultOptionId)), cfgOptionName(defaultOptionId)); } diff --git a/test/src/module/config/loadTest.c b/test/src/module/config/loadTest.c index 0152bdc4f..425afe492 100644 --- a/test/src/module/config/loadTest.c +++ b/test/src/module/config/loadTest.c @@ -217,8 +217,8 @@ testRun(void) hrnCfgArgRawZ(argList, cfgOptProtocolTimeout, "50.5"); TEST_ERROR( harnessCfgLoad(cfgCmdCheck, argList), OptionInvalidValueError, - "'50500' is not valid for 'protocol-timeout' option\n" - "HINT 'protocol-timeout' option (50500) should be greater than 'db-timeout' option (100000000)."); + "'50.5' is not valid for 'protocol-timeout' option\n" + "HINT 'protocol-timeout' option (50.5) should be greater than 'db-timeout' option (100000)."); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("very small protocol-timeout triggers db-timeout special handling"); diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index f38237282..29eb33015 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -1337,6 +1337,7 @@ testRun(void) TEST_RESULT_STR_Z(cfgOptionIdxStr(cfgOptPgSocketPath, 0), "/path/to/socket", " pg1-socket-path is set"); TEST_RESULT_INT(cfgOptionIdxSource(cfgOptPgSocketPath, 0), cfgSourceConfig, " pg1-socket-path is config param"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptOnline), false, " online not is set"); + TEST_RESULT_STR_Z(cfgOptionDisplay(cfgOptOnline), "false", " online display is false"); TEST_RESULT_INT(cfgOptionSource(cfgOptOnline), cfgSourceParam, " online is source param"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptStartFast), false, " start-fast not is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptStartFast), cfgSourceConfig, " start-fast is config param"); @@ -1346,6 +1347,7 @@ testRun(void) TEST_RESULT_BOOL(cfgOptionTest(cfgOptArchiveCheck), false, " archive-check is not set"); TEST_RESULT_BOOL(cfgOptionTest(cfgOptArchiveCopy), false, " archive-copy is not set"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptRepoHardlink), true, " repo-hardlink is set"); + TEST_RESULT_STR_Z(cfgOptionDisplay(cfgOptRepoHardlink), "true", " repo-hardlink display is true"); TEST_RESULT_INT(cfgOptionSource(cfgOptRepoHardlink), cfgSourceConfig, " repo-hardlink is source config"); TEST_RESULT_INT(cfgOptionInt(cfgOptCompressLevel), 3, " compress-level is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptCompressLevel), cfgSourceConfig, " compress-level is source config"); @@ -1357,6 +1359,7 @@ testRun(void) TEST_RESULT_INT(cfgOptionInt64(cfgOptBufferSize), 65536, " buffer-size is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptBufferSize), cfgSourceConfig, " backup-standby is source config"); TEST_RESULT_UINT(cfgOptionUInt64(cfgOptDbTimeout), 1800000, " db-timeout is set"); + TEST_RESULT_STR_Z(cfgOptionDisplay(cfgOptDbTimeout), "1800", " db-timeout display"); TEST_RESULT_UINT(cfgOptionUInt64(cfgOptProtocolTimeout), 3600000, " protocol-timeout is set"); TEST_RESULT_UINT(cfgOptionIdxUInt(cfgOptPgPort, 1), 5432, " pg2-port is set"); TEST_RESULT_UINT(cfgOptionIdxUInt64(cfgOptPgPort, 1), 5432, " pg2-port is set"); @@ -1369,13 +1372,16 @@ testRun(void) TEST_RESULT_PTR(cfgOptionDefault(cfgOptPgHost), NULL, " pg-host default is NULL"); TEST_RESULT_STR_Z(varStr(cfgOptionDefault(cfgOptLogLevelConsole)), "warn", " log-level-console default is warn"); TEST_RESULT_INT(varInt64(cfgOptionDefault(cfgOptPgPort)), 5432, " pg-port default is 5432"); + TEST_RESULT_STR_Z(cfgOptionDisplay(cfgOptPgPort), "5432", " pg-port display is 5432"); TEST_RESULT_INT(varInt64(cfgOptionDefault(cfgOptDbTimeout)), 1800000, " db-timeout default is 1800000"); TEST_RESULT_VOID(cfgOptionDefaultSet(cfgOptPgSocketPath, VARSTRDEF("/default")), " set pg-socket-path default"); TEST_RESULT_STR_Z(cfgOptionIdxStr(cfgOptPgSocketPath, 0), "/path/to/socket", " pg1-socket-path unchanged"); TEST_RESULT_STR_Z(cfgOptionIdxStr(cfgOptPgSocketPath, 1), "/default", " pg2-socket-path is new default"); + TEST_RESULT_STR_Z(cfgOptionIdxDisplay(cfgOptPgSocketPath, 1), "/default", " pg2-socket-path display"); TEST_ERROR(cfgOptionDefaultValue(cfgOptDbInclude), AssertError, "default value not available for option type 3"); + TEST_ERROR(cfgOptionDisplay(cfgOptTarget), AssertError, "option 'target' is not valid for the current command"); TEST_ERROR(cfgOptionLst(cfgOptDbInclude), AssertError, "option 'db-include' is not valid for the current command"); TEST_ERROR(cfgOptionKv(cfgOptPgPath), AssertError, "option 'pg1-path' is type 4 but 3 was requested"); @@ -1423,6 +1429,7 @@ testRun(void) TEST_RESULT_UINT(cfgOptionUInt64(cfgOptArchivePushQueueMax), 4503599627370496, "archive-push-queue-max is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptArchivePushQueueMax), cfgSourceParam, " archive-push-queue-max is source config"); TEST_RESULT_INT(cfgOptionIdxInt64(cfgOptBufferSize, 0), 2097152, "buffer-size is set to bytes from MB"); + TEST_RESULT_STR_Z(cfgOptionDisplay(cfgOptBufferSize), "2MB", "buffer-size display is 2MB"); TEST_RESULT_INT(cfgOptionSource(cfgOptBufferSize), cfgSourceParam, " buffer-size is source config"); TEST_RESULT_PTR(cfgOption(cfgOptSpoolPath), NULL, " spool-path is not set"); TEST_RESULT_INT(cfgOptionSource(cfgOptSpoolPath), cfgSourceDefault, " spool-path is source default");