From 77131ff8020c540a3c4dd6ab56fb7c5d59228a3a Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 15 Nov 2019 16:44:19 -0500 Subject: [PATCH] refactor: Remove settings merge reverse precedence code This commit has no effect on behavior because as of https://github.com/bitcoin/bitcoin/pull/17493 it's not possible to specify multiple values for single value settings in the config file. --- src/common/settings.cpp | 11 +---------- src/test/argsman_tests.cpp | 6 +++--- src/test/settings_tests.cpp | 11 ++--------- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 046afca15d7..c7ce034992f 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -159,15 +159,6 @@ SettingsValue GetSetting(const Settings& settings, // even though normal non-negated values there would be ignored. const bool never_ignore_negated_setting = span.last_negated(); - // Weird behavior preserved for backwards compatibility: Take first - // assigned value instead of last. In general, later settings take - // precedence over early settings, but for backwards compatibility in - // the config file the precedence is reversed for all settings except - // chain type settings. - const bool reverse_precedence = - (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && - !get_chain_type; - // Weird behavior preserved for backwards compatibility: Negated // -regtest and -testnet arguments which you would expect to override // values set in the configuration file are currently accepted but @@ -190,7 +181,7 @@ SettingsValue GetSetting(const Settings& settings, if (skip_negated_command_line && span.last_negated()) return; if (!span.empty()) { - result = reverse_precedence ? span.begin()[0] : span.end()[-1]; + result = span.end()[-1]; done = true; } else if (span.last_negated()) { result = false; diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp index f2ff11ac8a4..ee769a65f13 100644 --- a/src/test/argsman_tests.cpp +++ b/src/test/argsman_tests.cpp @@ -1067,7 +1067,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) BOOST_CHECK_EQUAL(test_args.GetArg("-a", "xxx"), ""); BOOST_CHECK_EQUAL(test_args.GetArg("-b", "xxx"), "1"); - BOOST_CHECK_EQUAL(test_args.TestArgString("-ccc", "xxx"), "argument"); + BOOST_CHECK_EQUAL(test_args.TestArgString("-ccc", "xxx"), "multiple"); BOOST_CHECK_EQUAL(test_args.GetArg("-d", "xxx"), "e"); BOOST_CHECK_EQUAL(test_args.GetArg("-fff", "xxx"), "0"); BOOST_CHECK_EQUAL(test_args.GetArg("-ggg", "xxx"), "1"); @@ -1133,7 +1133,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) // section-specific setting BOOST_CHECK(test_args.TestArgString("-h", "xxx") == "1"); // section takes priority for multiple values - BOOST_CHECK(test_args.TestArgString("-ccc", "xxx") == "extend1"); + BOOST_CHECK(test_args.TestArgString("-ccc", "xxx") == "extend2"); // check multiple values works const std::vector sec1_ccc_expected = {"extend1","extend2","argument","multiple"}; const auto& sec1_ccc_res = test_args.GetArgs("-ccc"); @@ -1216,7 +1216,7 @@ BOOST_AUTO_TEST_CASE(util_GetArg) BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest4", false), true); BOOST_CHECK_EQUAL(testArgs.GetArg("pritest1", "default"), "b"); - BOOST_CHECK_EQUAL(testArgs.GetArg("pritest2", "default"), "a"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest2", "default"), "b"); BOOST_CHECK_EQUAL(testArgs.GetArg("pritest3", "default"), "a"); BOOST_CHECK_EQUAL(testArgs.GetArg("pritest4", "default"), "b"); } diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index 95f38fc0ce6..447fd49f915 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -127,15 +127,8 @@ BOOST_AUTO_TEST_CASE(Simple) settings.command_line_options["name"].emplace_back("val2"); settings.ro_config["section"]["name"].emplace_back(2); - // The last given arg takes precedence when specified via commandline. + // The last given arg takes precedence when multiple values are present CheckValues(settings, R"("val2")", R"(["val1","val2",2])"); - - common::Settings settings2; - settings2.ro_config["section"]["name"].emplace_back("val2"); - settings2.ro_config["section"]["name"].emplace_back("val3"); - - // The first given arg takes precedence when specified via config file. - CheckValues(settings2, R"("val2")", R"(["val2","val3"])"); } // Confirm that a high priority setting overrides a lower priority setting even @@ -263,7 +256,7 @@ BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup) // Results file is formatted like: // // || GetSetting() | GetSettingsList() | OnlyHasDefaultSectionSetting() - BOOST_CHECK_EQUAL(out_sha_hex, "79db02d74e3e193196541b67c068b40ebd0c124a24b3ecbe9cbf7e85b1c4ba7a"); + BOOST_CHECK_EQUAL(out_sha_hex, "10a2293bfd447ee128f80714bac5e8abe83df2a518980e383a8d590319d32a00"); } BOOST_AUTO_TEST_SUITE_END()