Wed Aug 15 07:35:51 2007 Google Inc. <opensource@google.com>

	* google-gflags: version 0.6
	* Deal correctly with case that libpthread is not linked in (csilvers)
	* Update Makefile/tests so we pass "make distcheck" (csilvers)
	* Document and test that last assignment to a flag wins (wan)


git-svn-id: https://gflags.googlecode.com/svn/trunk@17 6586e3c6-dcc4-952a-343f-ff74eb82781d

5 files changed, 137 insertions, 27 deletions

diff --git a/src/gflags.cc b/src/gflags.cc
index 54956a3..1210434 100644
--- a/src/gflags.cc
+++ b/src/gflags.cc
@@ -367,27 +367,20 @@ CommandLineFlag::~CommandLineFlag() {
 
 const char* CommandLineFlag::CleanFileName() const {
   // Compute top-level directory & file that this appears in
-  // search full path backwards.  Set kMaxSlashes = 5,
-  // as the current code has <= 4 levels of dirs.
-  //   E.g. .../froogle/wrapping/autowrap/clustering/*.cc
-  // Also, stop going backwards at "/google3/"; and skip by the first slash.
+  // search full path backwards.
+  // Stop going backwards at kGoogle; and skip by the first slash.
   // E.g.
   //   filename_where_defined = "froogle/wrapping/autowrap/clustering/**.cc"
   //   filename_where_defined = "file/util/fileutil.cc"
-  static const int kMaxSlashes = 5;    // one more than max dir levels
   static const char kGoogle[] = "";    // can set this to whatever
 
   if (sizeof(kGoogle)-1 == 0)          // no prefix to strip
     return filename();
 
   const char* clean_name = filename() + strlen(filename()) - 1;
-  int slashes = 0;
   while ( clean_name > filename() ) {
     if (*clean_name == PATH_SEPARATOR) {
-      ++slashes;
-      if (slashes == kMaxSlashes) {
-        break;    // no dirs now are deeper than this
-      } else if (strncmp(clean_name, kGoogle, sizeof(kGoogle)-1) == 0) {
+      if (strncmp(clean_name, kGoogle, sizeof(kGoogle)-1) == 0) {
         // ".../google/base/logging.cc" ==> "base/logging.cc"
         clean_name += sizeof(kGoogle)-1;    // past "/google/"
         break;
@@ -486,6 +479,7 @@ class FlagRegistry {
   pthread_mutex_t lock_;
   static FlagRegistry* global_registry_;   // a singleton registry
   static pthread_once_t global_registry_once_;
+  static int global_registry_once_nothreads_;   // when we don't link pthreads
 
   static void InitGlobalRegistry();
 
@@ -636,13 +630,21 @@ bool FlagRegistry::SetFlagLocked(CommandLineFlag* flag,
 // Get the singleton FlagRegistry object
 FlagRegistry* FlagRegistry::global_registry_ = NULL;
 pthread_once_t FlagRegistry::global_registry_once_ = PTHREAD_ONCE_INIT;
+int FlagRegistry::global_registry_once_nothreads_ = 0;
 
 void FlagRegistry::InitGlobalRegistry() {
   global_registry_ = new FlagRegistry;
 }
 
+// We want to use pthread_once here, for safety, but have to worry about
+// whether libpthread is linked in or not.
 FlagRegistry* FlagRegistry::GlobalRegistry() {
-  pthread_once(&global_registry_once_, &FlagRegistry::InitGlobalRegistry);
+  if (pthread_once) {   // means we're running with pthreads
+    pthread_once(&global_registry_once_, &FlagRegistry::InitGlobalRegistry);
+  } else {              // not running with pthreads: we're the only thread
+    if (global_registry_once_nothreads_++ == 0)
+      InitGlobalRegistry();
+  }
   return global_registry_;
 }
 
diff --git a/src/gflags_unittest.cc b/src/gflags_unittest.cc
index b594581..3ec0333 100644
--- a/src/gflags_unittest.cc
+++ b/src/gflags_unittest.cc
@@ -46,9 +46,16 @@
 using std::vector;
 using std::string;
 
+// Returns the number of elements in an array.  We don't use the safer
+// version in base/basictypes.h as commandlineflags is open-sourced.
+#define GET_ARRAY_SIZE(arr) (sizeof(arr)/sizeof(*(arr)))
+
 DECLARE_string(tryfromenv);   // in commandlineflags.cc
 
 DEFINE_string(test_tmpdir, "/tmp/gflags_unittest", "Dir we use for temp files");
+DEFINE_string(srcdir, google::StringFromEnv("SRCDIR", "."),
+              "Source-dir root, needed to find gflags_unittest_flagfile");
+
 
 DEFINE_bool(test_bool, false, "tests bool-ness");
 DEFINE_int32(test_int32, -1, "");
@@ -964,7 +971,7 @@ TEST(DeprecatedFunctionsTest, ReadFromFlagsFile) {
   r = ReadFromFlagsFile(filename, GetArgv0(), true);
   EXPECT_EQ(true, r);
   EXPECT_EQ(-10, FLAGS_test_int32);
-} // unnamed namespace
+}
 
 TEST(DeprecatedFunctionsTest, ReadFromFlagsFileFailure) {
   FLAGS_test_int32 = -20;
@@ -985,13 +992,114 @@ TEST(FlagsSetBeforeInitGoogleTest, TryFromEnv) {
   EXPECT_EQ("pre-set", FLAGS_test_tryfromenv);
 }
 
+// The following test case verifies that ParseCommandLineFlags() and
+// ParseCommandLineNonHelpFlags() uses the last definition of a flag
+// in case it's defined more than once.
+
+DEFINE_int32(test_flag, -1, "used for testing commandlineflags.cc");
+
+// Returns the definition of the --flagfile flag to be used in the tests.
+const char* GetFlagFileFlag() {
+  static const string flagfile_flag = string("--flagfile=")
+      + FLAGS_srcdir + "/src/gflags_unittest_flagfile";
+
+  return flagfile_flag.c_str();
+}
+
+// Parses and returns the --test_flag flag.
+// If with_help is true, calls ParseCommandLineFlags; otherwise calls
+// ParseCommandLineNonHelpFlags.
+int32 ParseTestFlag(bool with_help, int argc, const char** const_argv) {
+  FlagSaver fs;  // Restores the flags before returning.
+
+  // Makes a copy of the input array s.t. it can be reused
+  // (ParseCommandLineFlags() will alter the array).
+  char** const argv_save = new char*[argc + 1];
+  char** argv = argv_save;
+  memcpy(argv, const_argv, sizeof(*argv)*(argc + 1));
+
+  if (with_help) {
+    ParseCommandLineFlags(&argc, &argv, true);
+  } else {
+    ParseCommandLineNonHelpFlags(&argc, &argv, true);
+  }
+
+  delete[] argv_save;
+  return FLAGS_test_flag;
+}
+
+TEST(ParseCommandLineFlagsUsesLastDefinitionTest,
+     WhenFlagIsDefinedTwiceOnCommandLine) {
+  const char* argv[] = {
+    "my_test",
+    "--test_flag=1",
+    "--test_flag=2",
+    NULL,
+  };
+
+  EXPECT_EQ(2, ParseTestFlag(true, GET_ARRAY_SIZE(argv) - 1, argv));
+  EXPECT_EQ(2, ParseTestFlag(false, GET_ARRAY_SIZE(argv) - 1, argv));
+}
+
+TEST(ParseCommandLineFlagsUsesLastDefinitionTest,
+     WhenFlagIsDefinedTwiceInFlagFile) {
+  const char* argv[] = {
+    "my_test",
+    GetFlagFileFlag(),
+    NULL,
+  };
+
+  EXPECT_EQ(2, ParseTestFlag(true, GET_ARRAY_SIZE(argv) - 1, argv));
+  EXPECT_EQ(2, ParseTestFlag(false, GET_ARRAY_SIZE(argv) - 1, argv));
+}
+
+TEST(ParseCommandLineFlagsUsesLastDefinitionTest,
+     WhenFlagIsDefinedInCommandLineAndThenFlagFile) {
+  const char* argv[] = {
+    "my_test",
+    "--test_flag=0",
+    GetFlagFileFlag(),
+    NULL,
+  };
+
+  EXPECT_EQ(2, ParseTestFlag(true, GET_ARRAY_SIZE(argv) - 1, argv));
+  EXPECT_EQ(2, ParseTestFlag(false, GET_ARRAY_SIZE(argv) - 1, argv));
+}
+
+TEST(ParseCommandLineFlagsUsesLastDefinitionTest,
+     WhenFlagIsDefinedInFlagFileAndThenCommandLine) {
+  const char* argv[] = {
+    "my_test",
+    GetFlagFileFlag(),
+    "--test_flag=3",
+    NULL,
+  };
+
+  EXPECT_EQ(3, ParseTestFlag(true, GET_ARRAY_SIZE(argv) - 1, argv));
+  EXPECT_EQ(3, ParseTestFlag(false, GET_ARRAY_SIZE(argv) - 1, argv));
+}
+
+TEST(ParseCommandLineFlagsUsesLastDefinitionTest,
+     WhenFlagIsDefinedInCommandLineAndFlagFileAndThenCommandLine) {
+  const char* argv[] = {
+    "my_test",
+    "--test_flag=0",
+    GetFlagFileFlag(),
+    "--test_flag=3",
+    NULL,
+  };
+
+  EXPECT_EQ(3, ParseTestFlag(true, GET_ARRAY_SIZE(argv) - 1, argv));
+  EXPECT_EQ(3, ParseTestFlag(false, GET_ARRAY_SIZE(argv) - 1, argv));
+}
+
 static int Main(int argc, char **argv) {
   // We need to call SetArgv before InitGoogle, so our "test" argv will
   // win out over this executable's real argv.  That makes running this
   // test with a real --help flag kinda annoying, unfortunately.
   const char* test_argv[] = { "/test/argv/for/gflags_unittest",
                               "argv 2", "3rd argv", "argv #4" };
-  SetArgv(sizeof(test_argv)/sizeof(*test_argv), test_argv);
+  SetArgv(GET_ARRAY_SIZE(test_argv), test_argv);
 
   // The first arg is the usage message, also important for testing.
   string usage_message = (string(GetArgv0()) +
diff --git a/src/gflags_unittest.sh b/src/gflags_unittest.sh
index 68dc353..76d63b3 100755
--- a/src/gflags_unittest.sh
+++ b/src/gflags_unittest.sh
@@ -38,12 +38,13 @@
 
 if [ -z "$1" ]
 then
-    echo "USAGE: $0 <unittest exe> [tmpdir]"
+    echo "USAGE: $0 <unittest exe> [top_srcdir] [tmpdir]"
     exit 1
 fi
 
 EXE=$1
-TMPDIR=${2:-/tmp/gflags}
+SRCDIR=${2:-./}
+TMPDIR=${3:-/tmp/gflags}
 
 # $1: line-number $2: expected return code.  $3: substring of expected output.
 # $4: a substring you *don't* expect to find in the output.  $5+ flags
@@ -57,7 +58,8 @@ Expect() {
   local unexpected_output="$1"
   shift
 
-  $EXE "$@" > "$TMPDIR/test.$line_number" 2>&1
+  # We always add --srcdir=$SRCDIR because it's needed for correctness
+  $EXE --srcdir="$SRCDIR" "$@" > "$TMPDIR/test.$line_number" 2>&1
   local actual_rc=$?
   if [ $actual_rc != $expected_rc ]; then
     echo "Test on line $line_number failed:" \
diff --git a/src/gflags_unittest_flagfile b/src/gflags_unittest_flagfile
new file mode 100644
index 0000000..f4fa0c4
--- /dev/null
+++ b/src/gflags_unittest_flagfile
@@ -0,0 +1,2 @@
+--test_flag=1
+--test_flag=2
diff --git a/src/google/gflags.h.in b/src/google/gflags.h.in
index 5b9519e..8dc257a 100644
--- a/src/google/gflags.h.in
+++ b/src/google/gflags.h.in
@@ -197,9 +197,6 @@ extern std::string SetCommandLineOptionWithMode(const char* name, const char* va
 // usage example above, the compiler would complain that it's an
 // unused variable.
 
-// This is a trick to work with autoconf, which sets a var to "yes" or "no"
-#define HAVE___ATTRIBUTE__yes 1  // will only be referenced if autoconf says yes
-
 class FlagSaver {
  public:
   FlagSaver();
@@ -210,19 +207,14 @@ class FlagSaver {
 
   FlagSaver(const FlagSaver&);  // no copying!
   void operator=(const FlagSaver&);
-}
-// swig seems to have trouble with __attribute__ for some reason
-#if !defined SWIG && defined HAVE___ATTRIBUTE__@ac_cv___attribute__@
-__attribute__ ((unused))
-#endif
-;
+} @ac_cv___attribute__unused@;
 
 // --------------------------------------------------------------------
 // Some deprecated or hopefully-soon-to-be-deprecated functions.
 
 // This is often used for logging.  TODO(csilvers): figure out a better way
 extern std::string CommandlineFlagsIntoString();
-// DEPRECATED.  Usually where this is used, a FlagSaver should be used instead.
+// Usually where this is used, a FlagSaver should be used instead.
 extern bool ReadFlagsFromString(const std::string& flagfilecontents,
                                 const char* prog_name,
                                 bool errors_are_fatal); // uses SET_FLAGS_VALUE
@@ -263,6 +255,8 @@ extern void SetUsageMessage(const std::string& usage);
 
 // Looks for flags in argv and parses them.  Rearranges argv to put
 // flags first, or removes them entirely if remove_flags is true.
+// If a flag is defined more than once in the command line or flag
+// file, the last definition is used.
 // See top-of-file for more details on this function.
 #ifndef SWIG   // In swig, use ParseCommandLineFlagsScript() instead.
 extern uint32 ParseCommandLineFlags(int *argc, char*** argv,
@@ -277,6 +271,8 @@ extern uint32 ParseCommandLineFlags(int *argc, char*** argv,
 // e.g. SetCommandLineOptionWithMode calls) between the time of
 // command line parsing and the time of dumping help information for
 // the flags as a result of command line parsing.
+// If a flag is defined more than once in the command line or flag
+// file, the last definition is used.
 extern uint32 ParseCommandLineNonHelpFlags(int *argc, char*** argv,
                                            bool remove_flags);
 // This is actually defined in commandlineflags_reporting.cc.
@@ -360,7 +356,7 @@ class FlagRegisterer {
       new (dfl_##name.store) namespc type(value));                             \
     namespc type& FLAGS_##name =                                               \
       *(reinterpret_cast<namespc type*>(cur_##name.store));                    \
-    char FLAGS_no##name;                                                       \
+    char FLAGS_no##name @ac_cv___attribute__unused@;                           \
   }                                                                            \
   using Flag_Names_##type::FLAGS_##name