Repository: mesos
Updated Branches:
  refs/heads/1.0.x 914bb5f2f -> 041249f69


Allowed all flags load methods to specify a prefix.

This also refactors the prefix logic into one place, so that's nice.
Required for the actual fix for passing work_dir in local.

Review: https://reviews.apache.org/r/50002/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/839537a7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/839537a7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/839537a7

Branch: refs/heads/1.0.x
Commit: 839537a77ec94f9e92aad03797f40c3567838398
Parents: 914bb5f
Author: Ammar Askar <am...@ammaraskar.com>
Authored: Wed Aug 10 16:09:40 2016 -0700
Committer: Vinod Kone <vinodk...@gmail.com>
Committed: Mon Oct 17 14:41:03 2016 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/flags/flags.hpp | 85 +++++++++++++----------
 3rdparty/stout/tests/flags_tests.cpp         | 34 +++++++++
 2 files changed, 82 insertions(+), 37 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/839537a7/3rdparty/stout/include/stout/flags/flags.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/flags/flags.hpp 
b/3rdparty/stout/include/stout/flags/flags.hpp
index c9f4cc5..e39d157 100644
--- a/3rdparty/stout/include/stout/flags/flags.hpp
+++ b/3rdparty/stout/include/stout/flags/flags.hpp
@@ -68,7 +68,7 @@ public:
       bool unknowns = false,
       bool duplicates = false);
 
-  // Load any flags from the environment as above but remove processed
+  // Loads any flags from the environment as above but remove processed
   // flags from 'argv' and update 'argc' appropriately. For example:
   //
   // argv = ["/path/program", "--arg1", "hi", "--arg2", "--", "bye"]
@@ -83,13 +83,29 @@ public:
       bool unknowns = false,
       bool duplicates = false);
 
+  // Loads flags from the given map where the keys represent the flag
+  // name and the values represent the flag values. For example:
+  //
+  // values = { 'arg1': 'hi',
+  //            'arg2': 'bye' }
+  //
+  // Optionally, if `prefix` is specified, flags will also be loaded
+  // from environment variables with the given prefix.
+  // Note that if a flag exists in both the environment and the values map,
+  // the latter takes precedence.
   virtual Try<Warnings> load(
       const std::map<std::string, Option<std::string>>& values,
-      bool unknowns = false);
+      bool unknowns = false,
+      const Option<std::string>& prefix = None());
 
+  // Loads flags from the map and optionally the environment vars
+  // if a prefix is specified. Follows the behavior of the
+  // method `load(values, unknowns, prefix)` above in terms
+  // of precedence and load order.
   virtual Try<Warnings> load(
       const std::map<std::string, std::string>& values,
-      bool unknowns = false);
+      bool unknowns = false,
+      const Option<std::string>& prefix = None());
 
   // Returns a string describing the flags, preceded by a "usage
   // message" that will be prepended to that description (see
@@ -373,9 +389,10 @@ protected:
 
 private:
   Try<Warnings> load(
-      const Multimap<std::string, Option<std::string>>& values,
+      Multimap<std::string, Option<std::string>>& values,
       bool unknowns = false,
-      bool duplicates = false);
+      bool duplicates = false,
+      const Option<std::string>& prefix = None());
 
   // Maps flag's name to flag.
   std::map<std::string, Flag> flags_;
@@ -784,19 +801,7 @@ inline Try<Warnings> FlagsBase::load(
     values.put(name, value);
   }
 
-  if (prefix.isSome()) {
-    // Merge in flags from the environment. Command-line
-    // flags take precedence over environment flags.
-    foreachpair (const std::string& name,
-                 const Option<std::string>& value,
-                 extract(prefix.get())) {
-      if (!values.contains(name)) {
-        values.put(name, value);
-      }
-    }
-  }
-
-  return load(values, unknowns, duplicates);
+  return load(values, unknowns, duplicates, prefix);
 }
 
 
@@ -852,19 +857,7 @@ inline Try<Warnings> FlagsBase::load(
     values.put(name, value);
   }
 
-  if (prefix.isSome()) {
-    // Merge in flags from the environment. Command-line
-    // flags take precedence over environment flags.
-    foreachpair (const std::string& name,
-                 const Option<std::string>& value,
-                 extract(prefix.get())) {
-      if (!values.contains(name)) {
-        values.put(name, value);
-      }
-    }
-  }
-
-  Try<Warnings> result = load(values, unknowns, duplicates);
+  Try<Warnings> result = load(values, unknowns, duplicates, prefix);
 
   // Update 'argc' and 'argv' if we successfully loaded the flags.
   if (!result.isError()) {
@@ -888,7 +881,8 @@ inline Try<Warnings> FlagsBase::load(
 
 inline Try<Warnings> FlagsBase::load(
     const std::map<std::string, Option<std::string>>& values,
-    bool unknowns)
+    bool unknowns,
+    const Option<std::string>& prefix)
 {
   Multimap<std::string, Option<std::string>> values_;
   foreachpair (const std::string& name,
@@ -896,29 +890,46 @@ inline Try<Warnings> FlagsBase::load(
                values) {
     values_.put(name, value);
   }
-  return load(values_, unknowns);
+  return load(values_, unknowns, false, prefix);
 }
 
 
 inline Try<Warnings> FlagsBase::load(
     const std::map<std::string, std::string>& values,
-    bool unknowns)
+    bool unknowns,
+    const Option<std::string>& prefix)
 {
   Multimap<std::string, Option<std::string>> values_;
   foreachpair (const std::string& name, const std::string& value, values) {
     values_.put(name, Some(value));
   }
-  return load(values_, unknowns);
+  return load(values_, unknowns, false, prefix);
 }
 
 
 inline Try<Warnings> FlagsBase::load(
-    const Multimap<std::string, Option<std::string>>& values,
+    Multimap<std::string, Option<std::string>>& values,
     bool unknowns,
-    bool duplicates)
+    bool duplicates,
+    const Option<std::string>& prefix)
 {
   Warnings warnings;
 
+  if (prefix.isSome()) {
+    // Merge in flags from the environment. Values in the
+    // map take precedence over environment flags.
+    //
+    // Other overloads parse command line flags into
+    // the values map and pass them into this method.
+    foreachpair (const std::string& name,
+                 const Option<std::string>& value,
+                 extract(prefix.get())) {
+      if (!values.contains(name)) {
+        values.put(name, value);
+      }
+    }
+  }
+
   foreachpair (const std::string& name,
                const Option<std::string>& value,
                values) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/839537a7/3rdparty/stout/tests/flags_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/flags_tests.cpp 
b/3rdparty/stout/tests/flags_tests.cpp
index 77f3a6a..77014e0 100644
--- a/3rdparty/stout/tests/flags_tests.cpp
+++ b/3rdparty/stout/tests/flags_tests.cpp
@@ -514,20 +514,54 @@ TEST(FlagsTest, DuplicatesFromEnvironment)
   TestFlags flags;
 
   os::setenv("FLAGSTEST_name1", "ben folds");
+  os::setenv("FLAGSTEST_name2", "50");
 
   const char* argv[] = {
     "/path/to/program",
     "--name1=billy joel"
   };
 
+  // `load(prefix, argc, argv)`.
   Try<Warnings> load = flags.load("FLAGSTEST_", arraySize(argv), argv);
   EXPECT_SOME(load);
   EXPECT_EQ(0, load->warnings.size());
 
   // The environment variables are overwritten by command line flags.
   EXPECT_EQ(flags.name1, "billy joel");
+  EXPECT_EQ(flags.name2, 50);
+
+  {
+    flags = TestFlags();
+    std::map<std::string, std::string> values;
+    values["name1"] = "billy joel";
+
+    // `load(map<string, string>, unknowns, prefix)`.
+    load = flags.load(values, false, "FLAGSTEST_");
+    EXPECT_SOME(load);
+    EXPECT_EQ(0, load->warnings.size());
+
+    EXPECT_EQ(flags.name1, "billy joel");
+    EXPECT_EQ(flags.name2, 50);
+  }
+
+  {
+    flags = TestFlags();
+    std::map<std::string, Option<std::string>> values;
+    values["name1"] = "billy joel";
+    values["name2"] = "51";
+
+    // `load(map<string, Option<string>, unknowns, prefix)`.
+    load = flags.load(values, false, "FLAGSTEST_");
+
+    EXPECT_SOME(load);
+    EXPECT_EQ(0, load->warnings.size());
+
+    EXPECT_EQ(flags.name1, "billy joel");
+    EXPECT_EQ(flags.name2, 51);
+  }
 
   os::unsetenv("FLAGSTEST_name1");
+  os::unsetenv("FLAGSTEST_name2");
 }
 
 

Reply via email to