1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2026-06-20 01:17:49 +02:00

Improve logging of default for options with an unresolved dependency.

Options with unresolved dependencies can have an implied default specified. This makes the code a bit simpler since we don't need to check for option validity.

However, there was an edge case where if an option was specified in the config file and ultimately the dependency was not resolved then the option would not be marked as default and therefore show up in the option logging at the beginning of a command. The default value was correct so everything operated as expected but the logging was confusing.

In the case of an implied default, reinitialize the option struct so that any leftover settings will be reset.
This commit is contained in:
David Steele
2025-11-28 14:27:36 +05:30
committed by GitHub
parent 28bfa20ba3
commit 38ac6387cb
3 changed files with 30 additions and 4 deletions
+12
View File
@@ -52,6 +52,18 @@
<p>Remove support for <proper>PostgreSQL</proper> <id>9.5</id>.</p>
</release-item>
<release-item>
<github-issue id="2707"/>
<github-pull-request id="2708"/>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="stefan.fercot"/>
</release-item-contributor-list>
<p>Improve logging of default for options with an unresolved dependency.</p>
</release-item>
</release-improvement-list>
</release-core-list>
+8 -4
View File
@@ -2764,10 +2764,14 @@ cfgParse(const Storage *const storage, const unsigned int argListSize, const cha
// Else apply the default for the unresolved dependency, if it exists
else if (dependResult.defaultExists)
{
configOptionValue->set = true;
configOptionValue->value = dependResult.defaultValue;
configOptionValue->defaultValue = optionalRules.defaultRaw;
configOptionValue->display = optionalRules.defaultRaw;
// Fully reinitialize since it might have been left partially set if dependency was not resolved
*configOptionValue = (ConfigOptionValue)
{
.set = true,
.value = dependResult.defaultValue,
.defaultValue = optionalRules.defaultRaw,
.display = optionalRules.defaultRaw,
};
}
pckReadFree(optionalRules.pack);
+10
View File
@@ -1859,6 +1859,12 @@ testRun(void)
"\n"
"[global:backup]\n"
"repo1-hardlink=y\n"
"repo2-type=s3\n" // Set to S3 to test unresolved default below
"repo2-symlink=n\n" // This will be unresolved and default to false
"repo2-s3-bucket=x\n" // Required for S3 repo
"repo2-s3-endpoint=x\n" // Required for S3 repo
"repo2-s3-region=x\n" // Required for S3 repo
"repo2-s3-key-type=auto\n" // Required for S3 repo
"bogus=bogus\n"
"no-delta=y\n"
"reset-delta=y\n"
@@ -1968,6 +1974,10 @@ testRun(void)
TEST_RESULT_STR(cfgOptionStrNull(cfgOptPgHost), NULL, "pg2-host is NULL");
TEST_ERROR(cfgOptionIdxStr(cfgOptPgHost, 1), AssertError, "option 'pg2-host' is null but non-null was requested");
TEST_RESULT_UINT(cfgOptionUInt64(cfgOptIoTimeout), 60000, "io-timeout is set");
TEST_RESULT_UINT(cfgOptionIdxStrId(cfgOptRepoType, 1), STRID6("s3", 0x7d31), "repo2-type is s3");
TEST_RESULT_UINT(cfgOptionIdxBool(cfgOptRepoSymlink, 1), false, "repo2-symlink is not set");
TEST_RESULT_UINT(cfgOptionIdxSource(cfgOptRepoSymlink, 1), cfgSourceDefault, "repo2-symlink is default");
TEST_RESULT_UINT(cfgOptionIdxNegate(cfgOptRepoSymlink, 1), false, "repo2-symlink is not negated");
TEST_RESULT_STR_Z(cfgOptionIdxDefaultValue(cfgOptBackupStandby, 0), "n", "backup-standby default is false");
TEST_RESULT_STR_Z(cfgOptionIdxDefaultValue(cfgOptBackupStandby, 0), "n", "backup-standby default is false (again)");