cleanup flags.h, write options to some channel, stdout in cmdline

Summary: fixes #61
fixes #62
in fb services will call printOptions(LOG(INFO));

{need fb side work/will break there/this is a start/proposal}

Closes #63

Reviewed By: @uddipta

Differential Revision: D2397161

Pulled By: @ldemailly

CMakeLists.txt

@@ -22,7 +22,7 @@ cmake_minimum_required(VERSION 3.2)
 # There is no C per se in WDT but if you use CXX only here many checks fail
 # Version is Major.Minor.YYMMDDX for up to 10 releases per day
 # Minor currently is also the protocol version - has to match with Protocol.cpp
-project("WDT" LANGUAGES C CXX VERSION 1.16.1508210)
+project("WDT" LANGUAGES C CXX VERSION 1.16.1508310)
 
 # On MacOS this requires the latest (master) CMake (and/or CMake 3.1.1/3.2)
 set(CMAKE_CXX_STANDARD 11)
@@ -206,7 +206,7 @@ if (BUILD_TESTING)
  enable_testing()
 
  # Extra code that we use in tests
- add_library(wdt4tests
+ add_library(wdt4tests_min
  "${FOLLY_SOURCE_DIR}/folly/FileUtil.cpp" # used by Random used by tests
  "${FOLLY_SOURCE_DIR}/folly/Random.cpp" # used indirectly by tests
  )
@@ -238,14 +238,20 @@ if (BUILD_TESTING)
  # ${GMOCK_PREFIX}/src/gmock/gmock-all.cc
  # ${GMOCK_PREFIX}/src/gmock/gmock_main.cc)
 
- add_dependencies(wdt4tests gmock)
+ add_dependencies(wdt4tests_min gmock)
 
  # ${BINARY_DIR}/libgmock.a works everywhere except xcode...
 # so ugly weird hack generating warnings about unknown dir for now:
- target_link_libraries(wdt4tests
+ target_link_libraries(wdt4tests_min
  "-L ${BINARY_DIR} -L ${BINARY_DIR}/Debug -lgmock"
- wdtlib
+ wdtlib_min
+ )
+
+ add_library(wdt4tests
+ WdtFlags.cpp
  )
+ target_link_libraries(wdt4tests wdt4tests_min)
+
 
  add_executable(protocol_test ProtocolTest.cpp)
  target_link_libraries(protocol_test wdt4tests)
@@ -259,6 +265,15 @@ if (BUILD_TESTING)
  target_link_libraries(file_reader_test wdt4tests)
  add_test(NAME FileReaderTests COMMAND file_reader_test)
 
+ add_executable(option_type_test_long_flags OptionTypeTest.cpp)
+ target_link_libraries(option_type_test_long_flags wdt4tests)
+
+ add_executable(option_type_test_short_flags OptionTypeTest.cpp WdtFlags.cpp)
+ set_target_properties(option_type_test_short_flags PROPERTIES
+ COMPILE_DEFINITIONS "STANDALONE_APP")
+ target_link_libraries(option_type_test_short_flags wdt4tests_min)
+
+
  add_test(NAME WdtRandGenTest COMMAND
  "${CMAKE_CURRENT_SOURCE_DIR}/wdt_rand_gen_test.sh")
 
@@ -268,5 +283,7 @@ if (BUILD_TESTING)
  add_test(NAME WdtBasicE2Exfs COMMAND
  "${CMAKE_CURRENT_SOURCE_DIR}/wdt_e2e_xfs_test.sh")
 
+ add_test(NAME WdtOptionsTypeTests COMMAND
+ "${CMAKE_CURRENT_SOURCE_DIR}/wdt_option_type_test.sh")
 
 endif(BUILD_TESTING)

DirectorySourceQueue.cpp

@@ -111,7 +111,7 @@ bool DirectorySourceQueue::setRootDir(const string &newRootDir) {
  if (dir.back() != '/') {
  dir.push_back('/');
  }
- if ( dir != rootDir_ ) {
+ if (dir != rootDir_) {
  rootDir_.assign(dir);
  LOG(INFO) << "Root dir now " << rootDir_;
  }
@@ -202,7 +202,7 @@ string DirectorySourceQueue::resolvePath(const string &path) {
  char *resolvedPath = realpath(path.c_str(), nullptr);
  if (!resolvedPath) {
  PLOG(ERROR) << "Couldn't resolve " << path;
- return result; // empty string == error
+ return result;  // empty string == error
  }
  result.assign(resolvedPath);
  free(resolvedPath);

OptionTypeTest.cpp

@@ -11,6 +11,8 @@
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
+#include "WdtFlagsMacros.h"
+
 /*
  * Tests in this file can not be run in the same process. That is because we
  * rely on is_default property of gflags to determine whether a flag has been
@@ -24,12 +26,9 @@
 namespace facebook {
 namespace wdt {
 
-const std::string NUM_PORTS_FLAG =
- WdtFlags::getFlagNameFromOptionName("num_ports");
-const std::string BLOCK_SIZE_FLAG =
- WdtFlags::getFlagNameFromOptionName("block_size_mbytes");
-const std::string OPTION_TYPE_FLAG =
- WdtFlags::getFlagNameFromOptionName("option_type");
+const std::string NUM_PORTS_FLAG = WDT_FLAG_STR(num_ports);
+const std::string BLOCK_SIZE_FLAG = WDT_FLAG_STR(block_size_mbytes);
+const std::string OPTION_TYPE_FLAG = WDT_FLAG_STR(option_type);
 
 void overrideTest1(const std::string &optionType) {
  const auto &options = WdtOptions::get();

WdtConfig.h

@@ -8,9 +8,9 @@
 
 #define WDT_VERSION_MAJOR 1
 #define WDT_VERSION_MINOR 16
-#define WDT_VERSION_BUILD 1508210
+#define WDT_VERSION_BUILD 1508310
 // Add -fbcode to version str
-#define WDT_VERSION_STR "1.16.1508210-fbcode"
+#define WDT_VERSION_STR "1.16.1508310-fbcode"
 // Tie minor and proto version
 #define WDT_PROTOCOL_VERSION WDT_VERSION_MINOR
 

WdtFlags.cpp

@@ -7,38 +7,30 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  */
 #include "WdtFlags.h"
-#include "WdtFlags.cpp.inc"
+#include "WdtOptions.h"
+
+#include <gflags/gflags.h>
 #include <glog/logging.h>
+#include <iostream>
 #include "Protocol.h"
-#include <folly/Conv.h>
 
-FLAG_DEFINITION(string, PREFIX(option_type),
- facebook::wdt::WdtOptions::FLASH_OPTION_TYPE,
- "WDT option type. Options are initialized to different values "
- "depending on the type. Individual options can still be "
- "changed using specific flags.")
+#include "WdtFlags.cpp.inc"
+
+WDT_FLAG_DEFINITION(
+ string, WDT_FLAG_SYM(option_type),
+ facebook::wdt::WdtOptions::FLASH_OPTION_TYPE,
+ "WDT option type. Options are initialized to different values "
+ "depending on the type. Individual options can still be changed using "
+ "specific flags. Use -" WDT_FLAG_STR(print_options) " to see values")
 
 namespace facebook {
 namespace wdt {
 
-const std::string FLAGS_PREFIX = "wdt_";
+const std::string FLAGS_PREFIX = WDT_TOSTR(WDT_LONG_PREFIX);
 
-void WdtFlags::initializeFromFlags() {
- LOG(INFO) << "Running WDT " << Protocol::getFullVersion();
-#define ASSIGN_OPT
-#include "WdtFlags.cpp.inc" //nolint
-#undef ASSIGN_OPT
- std::set<std::string> userSpecifiedFlags = getUserSpecifiedOptions();
- WdtOptions::getMutable().modifyOptions(FLAGS_OPTION_TYPE, userSpecifiedFlags);
-}
-
-void WdtFlags::printOptions() {
-#define PRINT_OPT
-#include "WdtFlags.cpp.inc" //nolint
-#undef PRINT_OPT
-}
+// Internal utilities
 
-std::string WdtFlags::getOptionNameFromFlagName(const std::string &flagName) {
+std::string getOptionNameFromFlagName(const std::string &flagName) {
 #ifndef STANDALONE_APP
  // extra wdt_ prefix is added in this case
  if (flagName.compare(0, FLAGS_PREFIX.size(), FLAGS_PREFIX) == 0) {
@@ -49,17 +41,9 @@ std::string WdtFlags::getOptionNameFromFlagName(const std::string &flagName) {
  return flagName;
 }
 
-std::string WdtFlags::getFlagNameFromOptionName(const std::string &optionName) {
-#ifndef STANDALONE_APP
- // extra wdt_ prefix has to be added
- std::string flagName;
- folly::toAppend(FLAGS_PREFIX, optionName, &flagName);
- return flagName;
-#endif
- return optionName;
-}
+// getFlagNameFromOptionName is WDT_FLAG_STR()
 
-std::set<std::string> WdtFlags::getUserSpecifiedOptions() {
+std::set<std::string> getUserSpecifiedOptions() {
  std::set<std::string> userSpecifiedFlags;
  std::vector<google::CommandLineFlagInfo> allFlags;
  google::GetAllFlags(&allFlags);
@@ -75,5 +59,22 @@ std::set<std::string> WdtFlags::getUserSpecifiedOptions() {
  }
  return userSpecifiedFlags;
 }
+
+void WdtFlags::initializeFromFlags() {
+ LOG(INFO) << "Running WDT " << Protocol::getFullVersion();
+#define ASSIGN_OPT
+#include "WdtFlags.cpp.inc" //nolint
+#undef ASSIGN_OPT
+ std::set<std::string> userSpecifiedFlags = getUserSpecifiedOptions();
+ WdtOptions::getMutable().modifyOptions(WDT_FLAG_VAR(option_type),
+ userSpecifiedFlags);
+}
+
+void WdtFlags::printOptions(std::ostream &out) {
+ out << "Options current value:" << std::endl;
+#define PRINT_OPT
+#include "WdtFlags.cpp.inc" //nolint
+#undef PRINT_OPT
+}
 }
 }

WdtFlags.h

@@ -7,39 +7,20 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  */
 #pragma once
-#include <gflags/gflags.h>
-#include <iostream>
-#include "WdtOptions.h"
-#define DECLARE_ONLY
-#include "WdtFlags.cpp.inc"
-#undef DECLARE_ONLY
-
-#define FLAGS_OPTION_TYPE FLAG_VALUE(option_type)
-FLAG_DECLARATION(string, PREFIX(option_type))
+#include <ostream>
 
 namespace facebook {
 namespace wdt {
 class WdtFlags {
  public:
  /**
  * Set the values of options in WdtOptions from corresponding flags
+ * TODO: return the options (or set the ones passed in)
  */
  static void initializeFromFlags();
 
- static void printOptions();
-
- /**
- * Returns option name from flag name
- */
- static std::string getOptionNameFromFlagName(const std::string &flagName);
-
- /**
- * Returns flag name from option name
- */
- static std::string getFlagNameFromOptionName(const std::string &optionName);
-
- /// returns list of options specified in the cmd line by the user
- static std::set<std::string> getUserSpecifiedOptions();
+ /// TODO change this to take the options returned above
+ static void printOptions(std::ostream &out);
 };
 }
 }

WdtFlagsMacros.h

@@ -1,46 +1,72 @@
 /// override-include-guard
+
+// If you make any changes to this - use g++ -E to check the generated code
+
+/// Which options object to use (we can reuse those macros for fbonly options)
 #ifndef OPTIONS
 #define OPTIONS WdtOptions
 #endif
 
+// Short symbol A is a field inside the Options struct
+// The flag name is either the short one DEFINE_type(A,...) or
+// prefixed by wdt_ so we play nice with others when making a
+// library (long flag)
+#define WDT_READ_OPT(A) facebook::wdt::OPTIONS::get().A
+#define WDT_WRITE_OPT(A) facebook::wdt::OPTIONS::getMutable().A
+
+// Generic macros to concat and stringify:
+// Turns wdt_ and foo into wdt_foo
+#define WDT_CONCAT1(a, b) a##b
+#define WDT_CONCAT(a, b) WDT_CONCAT1(a, b)
+// Turns a symbol into a string literal ie foo to "foo"
+// Needs two steps so WDT_TOSTR(WDT_CONCAT(wdt_,foo)) gives "wdt_foo"
+// and not "WDT_CONCAT(wdt_,foo)"
+#define WDT_TOSTR1(x) #x
+#define WDT_TOSTR(x) WDT_TOSTR1(x)
+
+#define WDT_LONG_PREFIX wdt_
+
 #ifndef STANDALONE_APP
-#define PREFIX(argument) wdt_##argument
+#define WDT_PREFIX(argument) WDT_CONCAT(WDT_LONG_PREFIX, argument)
 #else
-#define PREFIX(argument) argument
+#define WDT_PREFIX(argument) argument
 #endif
-#define VALUE(A) facebook::wdt::OPTIONS::get().A
 
-#define VALUE_X(argument) FLAGS_##argument
-#define CAT(argument) VALUE_X(argument)
-#define FLAG_VALUE(argument) CAT(PREFIX(argument))
+// Symbol. eg wdt_foo
+#define WDT_FLAG_SYM(A) WDT_PREFIX(A)
+// String version eg "wdt_foo"
+#define WDT_FLAG_STR(A) WDT_TOSTR(WDT_FLAG_SYM(A))
+// Flag variable eg FLAGS_wdt_foo
+#define WDT_FLAG_VAR(A) WDT_CONCAT(FLAGS_, WDT_FLAG_SYM(A))
 
 #ifdef WDT_OPT
 #undef WDT_OPT
 #endif
 
+/// Setup variants to replace WDT_OPT by the right code depending
+/// on the mode/context. Trailing semi colon is expected to be in the .inc
 #ifdef ASSIGN_OPT
 // Assign option from flags
-#define WDT_OPT(argument, type, description) \
- facebook::wdt::OPTIONS::getMutable().argument = FLAG_VALUE(argument);
+#define WDT_OPT(A, type, description) WDT_WRITE_OPT(A) = WDT_FLAG_VAR(A)
 #else
 #ifdef PRINT_OPT
 // print options
-#define WDT_OPT(argument, type, description) \
- LOG(INFO) << #argument << " " << facebook::wdt::OPTIONS::get().argument;
+#define WDT_OPT(A, type, description) \
+ out << WDT_TOSTR(A) << " " << WDT_READ_OPT(A) << std::endl
 #else
-
-#define FLAG_DECLARATION(type, argument) DECLARE_##type(argument);
-#define FLAG_DEFINITION(type, argument, value, description) \
+// google flag define or declare:
+#define WDT_FLAG_DECLARATION(type, argument) DECLARE_##type(argument);
+#define WDT_FLAG_DEFINITION(type, argument, value, description) \
  DEFINE_##type(argument, value, description);
 
 #ifdef DECLARE_ONLY
 // declare flags
-#define WDT_OPT(argument, type, description) \
- FLAG_DECLARATION(type, PREFIX(argument))
+#define WDT_OPT(A, type, description) \
+ WDT_FLAG_DECLARATION(type, WDT_FLAG_SYM(A))
 #else
 // define flags
-#define WDT_OPT(argument, type, description) \
- FLAG_DEFINITION(type, PREFIX(argument), VALUE(argument), description)
+#define WDT_OPT(A, type, description) \
+ WDT_FLAG_DEFINITION(type, WDT_FLAG_SYM(A), WDT_READ_OPT(A), description)
 #endif
 #endif
 #endif

wdtCmdLine.cpp

@@ -23,6 +23,7 @@
 #define ADDITIONAL_SENDER_SETUP
 #endif
 
+// This can be the fbonly version (extended flags/options)
 #ifndef FLAGS
 #define FLAGS WdtFlags
 #endif
@@ -117,15 +118,15 @@ int main(int argc, char *argv[]) {
  usage.append(google::ProgramInvocationShortName());
  usage.append(" # for a server/receiver\n\t");
  usage.append(google::ProgramInvocationShortName());
- usage.append(" -destination host # for a sender");
+ usage.append(" -connection_url url_produced_by_receiver # for a sender");
  google::SetUsageMessage(usage);
  google::ParseCommandLineFlags(&argc, &argv, true);
  google::InitGoogleLogging(argv[0]);
  signal(SIGPIPE, SIG_IGN);
 
  FLAGS::initializeFromFlags();
  if (FLAGS_print_options) {
- FLAGS::printOptions();
+ FLAGS::printOptions(std::cout);
  return 0;
  }