DPL: fix handling of boolean options

Author: ktf

Framework/Core/CMakeLists.txt

@@ -263,5 +263,5 @@ O2_GENERATE_TESTS(
   COMMAND_LINE_ARGS
   --global-config require-me
   # Note: the group switch makes process consumer parse only the group arguments
-  --consumer "--global-config consumer-config --local-option hello-aliceo2"
+  --consumer "--global-config consumer-config --local-option hello-aliceo2 --a-boolean3 --an-int2 20 --a-double2 22."
 )

Framework/Core/src/BoostOptionsRetriever.cxx

@@ -51,7 +51,7 @@ BoostOptionsRetriever::BoostOptionsRetriever(std::vector<ConfigParamSpec> const&
         options = options(name, bpo::value<std::string>()->default_value(spec.defaultValue.get<const char *>()), help);
         break;
       case VariantType::Bool:
-        options = options(name, bpo::value<bool>()->default_value(spec.defaultValue.get<bool>()), help);
+        options = options(name, bpo::value<bool>()->zero_tokens()->default_value(spec.defaultValue.get<bool>()), help);
         break;
       case VariantType::Unknown:
       case VariantType::Empty:

Framework/Core/src/ConfigParamsHelper.cxx

@@ -63,22 +63,27 @@ void ConfigParamsHelper::populateBoostProgramOptions(
 
 /// populate boost program options making all options of type string
 /// this is used for filtering the command line argument
-bool ConfigParamsHelper::prepareOptionsDescription(const std::vector<ConfigParamSpec> &spec,
+bool ConfigParamsHelper::prepareOptionsDescription(const std::vector<ConfigParamSpec>& spec,
                                                    boost::program_options::options_description& options,
-                                                   boost::program_options::options_description vetos
-			  )
+                                                   boost::program_options::options_description vetos)
 {
   bool haveOption = false;
-  for (const auto & configSpec : spec) {
+  for (const auto& configSpec : spec) {
     // skip everything found in the veto definition
-    if (vetos.find_nothrow(configSpec.name, false)) continue;
+    if (vetos.find_nothrow(configSpec.name, false))
+      continue;
     haveOption = true;
     std::stringstream defaultValue;
     defaultValue << configSpec.defaultValue;
-    options.add_options()
-      (configSpec.name.c_str(),
-       bpo::value<std::string>()->default_value(defaultValue.str().c_str()),
-       configSpec.help.c_str());
+    if (configSpec.type != VariantType::Bool) {
+      options.add_options()(configSpec.name.c_str(),
+                            bpo::value<std::string>()->default_value(defaultValue.str().c_str()),
+                            configSpec.help.c_str());
+    } else {
+      options.add_options()(configSpec.name.c_str(),
+                            bpo::value<bool>()->zero_tokens()->default_value(configSpec.defaultValue.get<bool>()),
+                            configSpec.help.c_str());
+    }
   }
 
   return haveOption;

Framework/Core/src/DeviceSpecHelpers.cxx

@@ -628,8 +628,6 @@ void DeviceSpecHelpers::prepareArguments(int argc, char** argv, bool defaultQuie
         // and is not defaulted
         const auto* description = odesc.find_nothrow(varit.first, false);
         if (description && varmap.count(varit.first)) {
-          tmpArgs.emplace_back("--");
-          tmpArgs.back() += varit.first;
           // check the semantics of the value
           auto semantic = description->semantic();
           const char* optarg = "";
@@ -638,11 +636,16 @@ void DeviceSpecHelpers::prepareArguments(int argc, char** argv, bool defaultQuie
             // multitoken, zero_token and composing
             // currently only the simple case is supported
             assert(semantic->min_tokens() <= 1);
-            assert(semantic->max_tokens() && semantic->min_tokens());
+            //assert(semantic->max_tokens() && semantic->min_tokens());
             if (semantic->min_tokens() > 0) {
+              tmpArgs.emplace_back("--");
+              tmpArgs.back() += varit.first;
               // add the token
               tmpArgs.emplace_back(varit.second.as<std::string>());
               optarg = tmpArgs.back().c_str();
+            } else if (semantic->min_tokens() == 0 && varit.second.as<bool>()) {
+              tmpArgs.emplace_back("--");
+              tmpArgs.back() += varit.first;
             }
           }
           control.options.insert(std::make_pair(varit.first, optarg));

Framework/Core/test/test_ProcessorOptions.cxx

@@ -62,6 +62,21 @@ WorkflowSpec defineDataProcessing(ConfigContext const&)
           // read back the option from the command line, see CMakeLists.txt
           auto configstring = ic.options().get<std::string>("global-config");
           auto anotheroption = ic.options().get<std::string>("local-option");
+          auto aBoolean = ic.options().get<bool>("a-boolean");
+          auto aBoolean2 = ic.options().get<bool>("a-boolean2");
+          auto aBoolean3 = ic.options().get<bool>("a-boolean3");
+          auto anInt = ic.options().get<int>("an-int");
+          auto anInt2 = ic.options().get<int>("an-int2");
+          auto aDouble = ic.options().get<double>("a-double");
+          auto aDouble2 = ic.options().get<double>("a-double2");
+
+          ASSERT_ERROR(aBoolean == true);
+          ASSERT_ERROR(aBoolean2 == false);
+          ASSERT_ERROR(aBoolean3 == true);
+          ASSERT_ERROR(anInt == 10);
+          ASSERT_ERROR(anInt2 == 20);
+          ASSERT_ERROR(aDouble == 11.);
+          ASSERT_ERROR(aDouble2 == 22.);
           ASSERT_ERROR(configstring == "consumer-config");
           ASSERT_ERROR(anotheroption == "hello-aliceo2");
 
@@ -74,6 +89,13 @@ WorkflowSpec defineDataProcessing(ConfigContext const&)
       {
         ConfigParamSpec{ "global-config", VariantType::String, { "A global config option for all processor specs" } },
         ConfigParamSpec{ "local-option", VariantType::String, { "Option only valid for this processor spec" } },
+        ConfigParamSpec{ "a-boolean", VariantType::Bool, true, { "A boolean which we pick by default" } },
+        ConfigParamSpec{ "a-boolean2", VariantType::Bool, false, { "Another boolean which we pick by default" } },
+        ConfigParamSpec{ "a-boolean3", VariantType::Bool, false, { "Another boolean which we pick from the outside options" } },
+        ConfigParamSpec{ "an-int", VariantType::Int, 10, { "An int for which we pick up the default" } },
+        ConfigParamSpec{ "an-int2", VariantType::Int, 1, { "An int for which we pick up the override" } },
+        ConfigParamSpec{ "a-double", VariantType::Double, 11., { "A double for which we pick up the override" } },
+        ConfigParamSpec{ "a-double2", VariantType::Double, 12., { "A double for which we pick up the override" } },
       },
     }
   };