-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Environment Variable Support #83
Merged
+154
−98
Merged
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
726ac9d
Adding `derive_configuration` to allow for config file overrides to c…
fosterbrereton ecbccc7
adding new tests to dogfood
fosterbrereton d1bf6a7
testing a failure
fosterbrereton d266e60
better test reporting
fosterbrereton 84ca0b3
tweak
fosterbrereton f3f1828
tweak
fosterbrereton c3a15b6
tweak
fosterbrereton 20baa98
tweak
fosterbrereton b7669a0
improvements to graceful exit logic
fosterbrereton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,7 +84,42 @@ void open_output_file(const std::string& a, const std::string& b) { | |
|
||
/**************************************************************************************************/ | ||
|
||
void process_orc_config_file(const char* bin_path_string) { | ||
template <typename T> | ||
T parse_enval(std::string&&) = delete; | ||
|
||
template <> | ||
std::string parse_enval(std::string&& x) { | ||
assert(!x.empty()); | ||
return x; | ||
} | ||
|
||
template <> | ||
bool parse_enval(std::string&& x) { | ||
assert(!x.empty()); | ||
return x != "0" && toupper(std::move(x)) != "FALSE"; | ||
} | ||
|
||
template <> | ||
std::size_t parse_enval(std::string&& x) { | ||
assert(!x.empty()); | ||
return std::max<int>(std::atoi(x.c_str()), 0); | ||
} | ||
|
||
template <typename T> | ||
T derive_configuration(const char* key, | ||
const toml::parse_result& settings, | ||
T&& fallback) { | ||
T result = settings[key].value_or(fallback); | ||
std::string envar = toupper(std::string("ORC_") + key); | ||
if (const char* enval = std::getenv(envar.c_str())) { | ||
result = parse_enval<T>(enval); | ||
} | ||
return result; | ||
} | ||
Comment on lines
+108
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This routine is the heart of the PR. Instead of taking values from the settings file directly, |
||
|
||
/**************************************************************************************************/ | ||
|
||
void process_orc_configuration(const char* bin_path_string) { | ||
std::filesystem::path pwd(std::filesystem::current_path()); | ||
std::filesystem::path bin_path(pwd / bin_path_string); | ||
std::filesystem::path config_path; | ||
|
@@ -114,104 +149,106 @@ void process_orc_config_file(const char* bin_path_string) { | |
bin_path = parent; | ||
} | ||
|
||
toml::parse_result settings; | ||
|
||
if (exists(config_path)) { | ||
try { | ||
auto settings = toml::parse_file(config_path.string()); | ||
auto& app_settings = settings::instance(); | ||
|
||
app_settings._graceful_exit = settings["graceful_exit"].value_or(false); | ||
app_settings._max_violation_count = settings["max_error_count"].value_or(0); | ||
app_settings._forward_to_linker = settings["forward_to_linker"].value_or(true); | ||
app_settings._standalone_mode = settings["standalone_mode"].value_or(false); | ||
app_settings._dylib_scan_mode = settings["dylib_scan_mode"].value_or(false); | ||
app_settings._parallel_processing = settings["parallel_processing"].value_or(true); | ||
app_settings._filter_redundant = settings["filter_redundant"].value_or(true); | ||
app_settings._print_object_file_list = settings["print_object_file_list"].value_or(false); | ||
app_settings._relative_output_file = settings["relative_output_file"].value_or(""); | ||
|
||
if (settings["output_file"]) { | ||
std::string fn = *settings["output_file"].value<std::string>(); | ||
open_output_file("", fn); | ||
} | ||
settings = toml::parse_file(config_path.string()); | ||
} catch (const toml::parse_error& err) { | ||
cerr_safe([&](auto& s) { s << "Parsing failed:\n" << err << "\n"; }); | ||
throw std::runtime_error("configuration parsing error"); | ||
} | ||
} else { | ||
cerr_safe([&](auto& s) { s << "ORC config file: not found\n"; }); | ||
} | ||
|
||
if (auto log_level = settings["log_level"].value<std::string>()) { | ||
const auto& level = *log_level; | ||
if (level == "silent") { | ||
app_settings._log_level = settings::log_level::silent; | ||
} else if (level == "warning") { | ||
app_settings._log_level = settings::log_level::warning; | ||
} else if (level == "info") { | ||
app_settings._log_level = settings::log_level::info; | ||
} else if (level == "verbose") { | ||
app_settings._log_level = settings::log_level::verbose; | ||
} else { | ||
// not a known value. Switch to verbose! | ||
app_settings._log_level = settings::log_level::verbose; | ||
cout_safe( | ||
[&](auto& s) { s << "warning: Unknown log level (using verbose)\n"; }); | ||
} | ||
} | ||
auto& app_settings = settings::instance(); | ||
|
||
app_settings._graceful_exit = derive_configuration("graceful_exit", settings, false); | ||
app_settings._max_violation_count = derive_configuration("max_error_count", settings, std::size_t(0)); | ||
app_settings._forward_to_linker = derive_configuration("forward_to_linker", settings, true); | ||
app_settings._standalone_mode = derive_configuration("standalone_mode", settings, false); | ||
app_settings._dylib_scan_mode = derive_configuration("dylib_scan_mode", settings, false); | ||
app_settings._parallel_processing = derive_configuration("parallel_processing", settings, true); | ||
app_settings._filter_redundant = derive_configuration("filter_redundant", settings, true); | ||
app_settings._print_object_file_list = derive_configuration("print_object_file_list", settings, false); | ||
app_settings._relative_output_file = derive_configuration("relative_output_file", settings, std::string()); | ||
|
||
const std::string log_level = derive_configuration("log_level", settings, std::string("warning")); | ||
const std::string output_file = derive_configuration("output_file", settings, std::string()); | ||
const std::string output_file_mode = derive_configuration("output_file_mode", settings, std::string("text")); | ||
|
||
// Do this early so we can log the ensuing output if it happens. | ||
if (!output_file.empty()) { | ||
open_output_file(std::string(), output_file); | ||
} | ||
|
||
if (app_settings._standalone_mode && app_settings._dylib_scan_mode) { | ||
throw std::logic_error( | ||
"Both standalone and dylib scanning mode are enabled. Pick one."); | ||
} | ||
// Do this early, too, so we can _suppress_ the output if we need to. | ||
if (output_file_mode == "text") { | ||
app_settings._output_file_mode = settings::output_file_mode::text; | ||
} else if (output_file_mode == "json") { | ||
app_settings._output_file_mode = settings::output_file_mode::json; | ||
} else if (log_level_at_least(settings::log_level::warning)) { | ||
cout_safe([&](auto& s) { | ||
s << "warning: unknown output_file_mode '" << output_file_mode << "'; using text\n"; | ||
}); | ||
} | ||
|
||
if (app_settings._dylib_scan_mode) { | ||
app_settings._forward_to_linker = false; | ||
} | ||
if (log_level == "silent") { | ||
app_settings._log_level = settings::log_level::silent; | ||
} else if (log_level == "warning") { | ||
app_settings._log_level = settings::log_level::warning; | ||
} else if (log_level == "info") { | ||
app_settings._log_level = settings::log_level::info; | ||
} else if (log_level == "verbose") { | ||
app_settings._log_level = settings::log_level::verbose; | ||
} else { | ||
// not a known value. Switch to verbose! | ||
app_settings._log_level = settings::log_level::verbose; | ||
cout_safe( | ||
[&](auto& s) { s << "warning: unknown log_level '" << log_level << "'; using verbose\n"; }); | ||
} | ||
|
||
auto read_string_list = [&_settings = settings](const char* name) { | ||
std::vector<std::string> result; | ||
if (auto* array = _settings.get_as<toml::array>(name)) { | ||
for (const auto& entry : *array) { | ||
if (auto symbol = entry.value<std::string>()) { | ||
result.push_back(*symbol); | ||
} | ||
} | ||
} | ||
std::sort(result.begin(), result.end()); | ||
return result; | ||
}; | ||
|
||
app_settings._symbol_ignore = read_string_list("symbol_ignore"); | ||
app_settings._violation_report = read_string_list("violation_report"); | ||
app_settings._violation_ignore = read_string_list("violation_ignore"); | ||
|
||
if (!app_settings._violation_report.empty() && | ||
!app_settings._violation_ignore.empty()) { | ||
if (log_level_at_least(settings::log_level::warning)) { | ||
cout_safe([&](auto& s) { | ||
s << "warning: Both `violation_report` and `violation_ignore` lists found\n"; | ||
s << "warning: `violation_report` will be ignored in favor of `violation_ignore`\n"; | ||
}); | ||
} | ||
} | ||
if (app_settings._standalone_mode && app_settings._dylib_scan_mode) { | ||
throw std::logic_error( | ||
"Both standalone and dylib scanning mode are enabled. Pick one."); | ||
} | ||
|
||
if (settings["output_file_mode"]) { | ||
const std::string mode = *settings["output_file_mode"].value<std::string>(); | ||
if (mode == "text") { | ||
app_settings._output_file_mode = settings::output_file_mode::text; | ||
} else if (mode == "json") { | ||
app_settings._output_file_mode = settings::output_file_mode::json; | ||
} else if (log_level_at_least(settings::log_level::warning)) { | ||
cout_safe([&](auto& s) { | ||
s << "warning: unknown `output_file_mode`: " << mode << "\n"; | ||
}); | ||
if (app_settings._dylib_scan_mode) { | ||
app_settings._forward_to_linker = false; | ||
} | ||
|
||
auto read_string_list = [&_settings = settings](const char* name) { | ||
std::vector<std::string> result; | ||
if (auto* array = _settings.get_as<toml::array>(name)) { | ||
for (const auto& entry : *array) { | ||
if (auto symbol = entry.value<std::string>()) { | ||
result.push_back(*symbol); | ||
} | ||
} | ||
} | ||
std::sort(result.begin(), result.end()); | ||
return result; | ||
}; | ||
|
||
if (log_level_at_least(settings::log_level::info)) { | ||
cout_safe([&](auto& s) { | ||
s << "info: ORC config file: " << config_path.string() << "\n"; | ||
}); | ||
} | ||
} catch (const toml::parse_error& err) { | ||
cerr_safe([&](auto& s) { s << "Parsing failed:\n" << err << "\n"; }); | ||
throw std::runtime_error("configuration parsing error"); | ||
app_settings._symbol_ignore = read_string_list("symbol_ignore"); | ||
app_settings._violation_report = read_string_list("violation_report"); | ||
app_settings._violation_ignore = read_string_list("violation_ignore"); | ||
|
||
if (!app_settings._violation_report.empty() && | ||
!app_settings._violation_ignore.empty()) { | ||
if (log_level_at_least(settings::log_level::warning)) { | ||
cout_safe([&](auto& s) { | ||
s << "warning: Both `violation_report` and `violation_ignore` lists found\n"; | ||
s << "warning: `violation_report` will be ignored in favor of `violation_ignore`\n"; | ||
}); | ||
} | ||
} else { | ||
cerr_safe([&](auto& s) { s << "ORC config file: not found\n"; }); | ||
} | ||
|
||
if (log_level_at_least(settings::log_level::info)) { | ||
cout_safe([&](auto& s) { | ||
s << "info: ORC config file: " << config_path.string() << "\n"; | ||
}); | ||
} | ||
} | ||
|
||
|
@@ -483,7 +520,7 @@ int main(int argc, char** argv) try { | |
|
||
signal(SIGINT, interrupt_callback_handler); | ||
|
||
process_orc_config_file(argv[0]); | ||
process_orc_configuration(argv[0]); | ||
|
||
cmdline_results cmdline = process_command_line(argc, argv); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why move semantics instead of
const std::string&
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. the value coming from
getenv
is aconst char*
, which is then converted to astd::string
for this routine. For thestd::string
specialization ofparse_enval
, we can move the incoming string to the result with no additional changes. If the incoming string wereconst std::string&
, it would cost us a copy to make a new one to return by value.