From 110f39c128a09a22fbc421a762724e6c9f63e749 Mon Sep 17 00:00:00 2001 From: Valentin Seitz Date: Fri, 5 Jul 2024 06:51:52 +0200 Subject: [PATCH 1/4] Refactoring the Env variable handling a bit: - Make the getValue() unwrap the optional if its required. --- src/backends/detection/detection.hpp | 5 ++++- .../dlb/dlb_talp_tree/dlb_talp_tree.hpp | 16 +++++++------- src/backends/extrae/extrae_type_stack.hpp | 4 ++-- src/delegator.cpp | 21 +++++++++---------- src/utils/environment_variable.hpp | 11 ++++++---- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/backends/detection/detection.hpp b/src/backends/detection/detection.hpp index e1d8758..7fc2f82 100644 --- a/src/backends/detection/detection.hpp +++ b/src/backends/detection/detection.hpp @@ -42,7 +42,10 @@ class DetectionStrategy : public ProperlyNestedAnnotationStrategy { "DETECTION_VERBOSE", "Enables the verbose mode in the detection backend", true); - bool has_errors_ = false; + std::mutex starts_mutex_; + std::mutex ends_mutex_; + EnvironmentVariable verbose_mode_ = + EnvironmentVariable("DETECTION_VERBOSE", "Enables the verbose mode in the detection backend"); public: DetectionStrategy(); diff --git a/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp b/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp index f013186..8954e1e 100644 --- a/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp +++ b/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp @@ -33,21 +33,19 @@ class DLBTalpTreeStrategy : public ProperlyNestedAnnotationStrategy { // Configuration variables inline static const std::string def_json_file_name_ = "nesmik_talp_tree.json"; - EnvironmentVariable env_json_file_name_ = EnvironmentVariable< - std::string>( + EnvironmentVariable env_json_file_name_ = EnvironmentVariable< + std::string,false>( "JSON_FILE", - "Set the name of the output JSON file. (default: nesmik_talp_tree.json)", - false); + "Set the name of the output JSON file. (default: nesmik_talp_tree.json)"); inline static const bool def_enable_json_ = true; - EnvironmentVariable env_enable_json_ = EnvironmentVariable( - "JSON_OUTPUT", "Enables output to a JSON file.", false); + EnvironmentVariable env_enable_json_ = EnvironmentVariable( + "JSON_OUTPUT", "Enables output to a JSON file."); inline static const bool def_enable_ascii_ = false; - EnvironmentVariable env_enable_ascii_ = EnvironmentVariable( + EnvironmentVariable env_enable_ascii_ = EnvironmentVariable( "ASCII_OUTPUT", - "Enables human readable output to standard output. (default: off)", - false); + "Enables human readable output to standard output. (default: off)"); public: DLBTalpTreeStrategy(); diff --git a/src/backends/extrae/extrae_type_stack.hpp b/src/backends/extrae/extrae_type_stack.hpp index 207894a..eebac31 100644 --- a/src/backends/extrae/extrae_type_stack.hpp +++ b/src/backends/extrae/extrae_type_stack.hpp @@ -25,9 +25,9 @@ class ExtraeTypeStackStrategy : public ProperlyNestedAnnotationStrategy { static const std::string paraverConfigOverviewWindow; static const std::string paraverConfigLevelWindow; - EnvironmentVariable write_config_file_ = EnvironmentVariable( + EnvironmentVariable write_config_file_ = EnvironmentVariable( "EXTRAE_WRITE_CONFIG_FILE", - "Write corresponding Paraver .cfg file if set to True", false); + "Write corresponding Paraver .cfg file if set to True"); inline static bool didInitialize{false}; diff --git a/src/delegator.cpp b/src/delegator.cpp index ea73f97..0907f20 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -89,17 +89,16 @@ void Delegator::Init(std::string_view nesting_mode, std::string_view backend) { break; } - if (stringsAreCaseInsensitiveEqual("ENV", std::string(backend))) { - // We instantiate the variable here, because its only required if the - // backend is set to "env". - EnvironmentVariable env_backend = - EnvironmentVariable( - "BACKEND", - "Set the backend to use, when init with \"env\" backend name.", - true); - - // Unwraping the std::optional is safe because the variable is required. - backend = env_backend.getValue().value(); + if (stringsAreCaseInsensitiveEqual("ENV",std::string(backend))) { + // We instanciate the variable here, because its only required if the + // backend is set to "env". + EnvironmentVariable env_backend = EnvironmentVariable< + std::string,true>( + "BACKEND", + "Set the backend to use, when init with \"env\" backend name."); + + + backend = env_backend.getValue(); } // next choose backend diff --git a/src/utils/environment_variable.hpp b/src/utils/environment_variable.hpp index edf98f3..c51509c 100644 --- a/src/utils/environment_variable.hpp +++ b/src/utils/environment_variable.hpp @@ -16,7 +16,7 @@ std::optional fromEnvString(const std::string &env_string); template <> std::optional fromEnvString(const std::string &env_string); -template +template class EnvironmentVariable { inline static std::string PREFIX = "NESMIK_"; @@ -25,7 +25,7 @@ class EnvironmentVariable { Upon construction it will do the getenv */ EnvironmentVariable(const std::string &variable_name, - const std::string &description, bool required = false) + const std::string &description) : variable_name_(variable_name), description_(description) { const std::string variable_to_query = EnvironmentVariable::PREFIX + variable_name_; @@ -48,8 +48,11 @@ class EnvironmentVariable { std::string description_; std::optional value_; - public: - std::optional getValue() const { return value_; } +public: + auto getValue() const { + if constexpr(required) {return value_.value();} + else {return value_;} + } bool isSet() const { return value_.has_value(); } }; -- GitLab From 6b1456caf2dedc7abb6b9ffbea098940745c17ba Mon Sep 17 00:00:00 2001 From: Valentin Seitz Date: Fri, 5 Jul 2024 07:13:52 +0200 Subject: [PATCH 2/4] clang format --- src/utils/environment_variable.hpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/utils/environment_variable.hpp b/src/utils/environment_variable.hpp index c51509c..e5d3dba 100644 --- a/src/utils/environment_variable.hpp +++ b/src/utils/environment_variable.hpp @@ -11,16 +11,14 @@ template std::optional fromEnvString(const std::string &env_string) = delete; -template <> -std::optional fromEnvString(const std::string &env_string); -template <> -std::optional fromEnvString(const std::string &env_string); +template <> std::optional fromEnvString(const std::string &env_string); +template <> std::optional fromEnvString(const std::string &env_string); template class EnvironmentVariable { inline static std::string PREFIX = "NESMIK_"; - public: +public: /* Upon construction it will do the getenv */ @@ -49,9 +47,12 @@ class EnvironmentVariable { std::optional value_; public: - auto getValue() const { - if constexpr(required) {return value_.value();} - else {return value_;} + auto getValue() const { + if constexpr (required) { + return value_.value(); + } else { + return value_; + } } bool isSet() const { return value_.has_value(); } }; -- GitLab From 84a2ca335c8119cc0b759edeb618982b0cfcdf36 Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Fri, 5 Jul 2024 08:24:26 +0200 Subject: [PATCH 3/4] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: JOAN VINYALS YLLA CATALA --- src/delegator.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/delegator.cpp b/src/delegator.cpp index 0907f20..13e873b 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -92,8 +92,7 @@ void Delegator::Init(std::string_view nesting_mode, std::string_view backend) { if (stringsAreCaseInsensitiveEqual("ENV",std::string(backend))) { // We instanciate the variable here, because its only required if the // backend is set to "env". - EnvironmentVariable env_backend = EnvironmentVariable< - std::string,true>( + EnvironmentVariable env_backend( "BACKEND", "Set the backend to use, when init with \"env\" backend name."); -- GitLab From b7ab13cb993a34feda6be63fdda2ebf710a8cb8a Mon Sep 17 00:00:00 2001 From: Valentin Seitz Date: Sun, 7 Jul 2024 19:28:01 +0200 Subject: [PATCH 4/4] Fixed small problems using formattiing --- src/backends/detection/detection.hpp | 10 +++++---- .../dlb/dlb_talp_tree/dlb_talp_tree.hpp | 21 +++++++++++-------- src/backends/extrae/extrae_type_stack.hpp | 7 ++++--- src/delegator.cpp | 17 +++++++-------- src/utils/environment_variable.hpp | 12 ++++++----- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/backends/detection/detection.hpp b/src/backends/detection/detection.hpp index 7fc2f82..b07b0b5 100644 --- a/src/backends/detection/detection.hpp +++ b/src/backends/detection/detection.hpp @@ -42,10 +42,12 @@ class DetectionStrategy : public ProperlyNestedAnnotationStrategy { "DETECTION_VERBOSE", "Enables the verbose mode in the detection backend", true); - std::mutex starts_mutex_; - std::mutex ends_mutex_; - EnvironmentVariable verbose_mode_ = - EnvironmentVariable("DETECTION_VERBOSE", "Enables the verbose mode in the detection backend"); + std::mutex starts_mutex_; + std::mutex ends_mutex_; + EnvironmentVariable verbose_mode_ = + EnvironmentVariable( + "DETECTION_VERBOSE", + "Enables the verbose mode in the detection backend"); public: DetectionStrategy(); diff --git a/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp b/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp index 8954e1e..56f124b 100644 --- a/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp +++ b/src/backends/dlb/dlb_talp_tree/dlb_talp_tree.hpp @@ -33,19 +33,22 @@ class DLBTalpTreeStrategy : public ProperlyNestedAnnotationStrategy { // Configuration variables inline static const std::string def_json_file_name_ = "nesmik_talp_tree.json"; - EnvironmentVariable env_json_file_name_ = EnvironmentVariable< - std::string,false>( - "JSON_FILE", - "Set the name of the output JSON file. (default: nesmik_talp_tree.json)"); + EnvironmentVariable env_json_file_name_ = + EnvironmentVariable( + "JSON_FILE", + "Set the name of the output JSON file. (default: " + "nesmik_talp_tree.json)"); inline static const bool def_enable_json_ = true; - EnvironmentVariable env_enable_json_ = EnvironmentVariable( - "JSON_OUTPUT", "Enables output to a JSON file."); + EnvironmentVariable env_enable_json_ = + EnvironmentVariable("JSON_OUTPUT", + "Enables output to a JSON file."); inline static const bool def_enable_ascii_ = false; - EnvironmentVariable env_enable_ascii_ = EnvironmentVariable( - "ASCII_OUTPUT", - "Enables human readable output to standard output. (default: off)"); + EnvironmentVariable env_enable_ascii_ = + EnvironmentVariable( + "ASCII_OUTPUT", + "Enables human readable output to standard output. (default: off)"); public: DLBTalpTreeStrategy(); diff --git a/src/backends/extrae/extrae_type_stack.hpp b/src/backends/extrae/extrae_type_stack.hpp index eebac31..30ffb3c 100644 --- a/src/backends/extrae/extrae_type_stack.hpp +++ b/src/backends/extrae/extrae_type_stack.hpp @@ -25,9 +25,10 @@ class ExtraeTypeStackStrategy : public ProperlyNestedAnnotationStrategy { static const std::string paraverConfigOverviewWindow; static const std::string paraverConfigLevelWindow; - EnvironmentVariable write_config_file_ = EnvironmentVariable( - "EXTRAE_WRITE_CONFIG_FILE", - "Write corresponding Paraver .cfg file if set to True"); + EnvironmentVariable write_config_file_ = + EnvironmentVariable( + "EXTRAE_WRITE_CONFIG_FILE", + "Write corresponding Paraver .cfg file if set to True"); inline static bool didInitialize{false}; diff --git a/src/delegator.cpp b/src/delegator.cpp index 13e873b..259e7f0 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -89,15 +89,14 @@ void Delegator::Init(std::string_view nesting_mode, std::string_view backend) { break; } - if (stringsAreCaseInsensitiveEqual("ENV",std::string(backend))) { - // We instanciate the variable here, because its only required if the - // backend is set to "env". - EnvironmentVariable env_backend( - "BACKEND", - "Set the backend to use, when init with \"env\" backend name."); - - - backend = env_backend.getValue(); + if (stringsAreCaseInsensitiveEqual("ENV", std::string(backend))) { + // We instantiate the variable here, because its only required if the + // backend is set to "env". + EnvironmentVariable env_backend( + "BACKEND", + "Set the backend to use, when init with \"env\" backend name."); + + backend = env_backend.getValue(); } // next choose backend diff --git a/src/utils/environment_variable.hpp b/src/utils/environment_variable.hpp index e5d3dba..ea04550 100644 --- a/src/utils/environment_variable.hpp +++ b/src/utils/environment_variable.hpp @@ -11,14 +11,16 @@ template std::optional fromEnvString(const std::string &env_string) = delete; -template <> std::optional fromEnvString(const std::string &env_string); -template <> std::optional fromEnvString(const std::string &env_string); +template <> +std::optional fromEnvString(const std::string &env_string); +template <> +std::optional fromEnvString(const std::string &env_string); -template +template class EnvironmentVariable { inline static std::string PREFIX = "NESMIK_"; -public: + public: /* Upon construction it will do the getenv */ @@ -46,7 +48,7 @@ public: std::string description_; std::optional value_; -public: + public: auto getValue() const { if constexpr (required) { return value_.value(); -- GitLab