ENH: Improve FILE GLOB_RECURSE handling of symlinks with a new CMake policy. CMP0009 establishes NEW default behavior of not recursing through symlinks. OLD default behavior or explicit FOLLOW_SYMLINKS argument to FILE GLOB_RECURSE will still recurse through symlinks.

6 files changed, 89 insertions, 11 deletions

diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index bbd8396..032d603 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -668,18 +668,39 @@ bool cmFileCommand::HandleGlobCommand(std::vector<std::string> const& args,
   i++;
   cmsys::Glob g;
   g.SetRecurse(recurse);
+
+  bool explicitFollowSymlinks = false;
+  cmPolicies::PolicyStatus status =
+    this->Makefile->GetPolicyStatus(cmPolicies::CMP0009);
+  if(recurse)
+    {
+    switch(status)
+      {
+      case cmPolicies::NEW:
+        g.RecurseThroughSymlinksOff();
+        break;
+      case cmPolicies::OLD:
+      case cmPolicies::WARN:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        g.RecurseThroughSymlinksOn();
+        break;
+      }
+    }
+
   std::string output = "";
   bool first = true;
   for ( ; i != args.end(); ++i )
     {
-    if ( *i == "RECURSE_SYMLINKS_OFF" )
+    if ( recurse && (*i == "FOLLOW_SYMLINKS") )
       {
-      g.RecurseThroughSymlinksOff();
+      explicitFollowSymlinks = true;
+      g.RecurseThroughSymlinksOn();
       ++i;
       if ( i == args.end() )
         {
         this->SetError(
-          "GLOB requires a glob expression after RECURSE_SYMLINKS_OFF");
+          "GLOB_RECURSE requires a glob expression after FOLLOW_SYMLINKS");
         return false;
         }
       }
@@ -732,6 +753,37 @@ bool cmFileCommand::HandleGlobCommand(std::vector<std::string> const& args,
       first = false;
       }
     }
+
+  if(recurse && !explicitFollowSymlinks)
+    {
+    switch (status)
+      {
+      case cmPolicies::NEW:
+        // Correct behavior, yay!
+        break;
+      case cmPolicies::OLD:
+        // Probably not really the expected behavior, but the author explicitly
+        // asked for the old behavior... no warning.
+      case cmPolicies::WARN:
+        // Possibly unexpected old behavior *and* we actually traversed
+        // symlinks without being explicitly asked to: warn the author.
+        if(g.GetFollowedSymlinkCount() != 0)
+          {
+          this->Makefile->IssueMessage(cmake::AUTHOR_WARNING,
+            this->Makefile->GetPolicies()->
+              GetPolicyWarning(cmPolicies::CMP0009));
+          }
+        break;
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        this->SetError("policy CMP0009 error");
+        this->Makefile->IssueMessage(cmake::FATAL_ERROR,
+          this->Makefile->GetPolicies()->
+            GetRequiredPolicyError(cmPolicies::CMP0009));
+        return false;
+      }
+    }
+
   this->Makefile->AddDefinition(variable.c_str(), output.c_str());
   return true;
 }
diff --git a/Source/cmFileCommand.h b/Source/cmFileCommand.h
index 9343a63..1b0c2ff 100644
--- a/Source/cmFileCommand.h
+++ b/Source/cmFileCommand.h
@@ -77,7 +77,7 @@ public:
       "       [NO_HEX_CONVERSION])\n"
       "  file(GLOB variable [RELATIVE path] [globbing expressions]...)\n"
       "  file(GLOB_RECURSE variable [RELATIVE path] \n"
-      "       [RECURSE_SYMLINKS_OFF] [globbing expressions]...)\n"
+      "       [FOLLOW_SYMLINKS] [globbing expressions]...)\n"
       "  file(REMOVE [file1 ...])\n"
       "  file(REMOVE_RECURSE [file1 ...])\n"
       "  file(MAKE_DIRECTORY [directory1 directory2 ...])\n"
@@ -124,11 +124,11 @@ public:
       "   *.cxx      - match all files with extension cxx\n"
       "   *.vt?      - match all files with extension vta,...,vtz\n"
       "   f[3-5].txt - match files f3.txt, f4.txt, f5.txt\n"
-      "GLOB_RECURSE will generate similar list as the regular GLOB, except "
+      "GLOB_RECURSE will generate a list similar to the regular GLOB, except "
       "it will traverse all the subdirectories of the matched directory and "
-      "match the files. Subdirectories that are symlinks are traversed by "
-      "default to match the behavior of older CMake releases. Use "
-      "RECURSE_SYMLINKS_OFF to prevent recursion through symlinks.\n"
+      "match the files. Subdirectories that are symlinks are only traversed "
+      "if FOLLOW_SYMLINKS is given or cmake policy CMP0009 is not set to NEW. "
+      "See cmake --help-policy CMP0009 for more information.\n"
       "Examples of recursive globbing include:\n"
       "   /dir/*.py  - match all python files in /dir and subdirectories\n"
       "MAKE_DIRECTORY will create the given directories, also if their parent "
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 640e0e7..5d8c71f 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -307,6 +307,22 @@ cmPolicies::cmPolicies()
     "The NEW behavior for this policy is to trust the given path and "
     "pass it directly to the native build tool unchanged.",
     2,6,1, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0009, "CMP0009",
+    "FILE GLOB_RECURSE calls should not follow symlinks by default.",
+    "In CMake 2.6.1 and below, FILE GLOB_RECURSE calls would follow "
+    "through symlinks, sometimes coming up with unexpectedly large "
+    "result sets because of symlinks to top level directories that "
+    "contain hundreds of thousands of files."
+    "\n"
+    "This policy determines whether or not to follow symlinks "
+    "encountered during a FILE GLOB_RECURSE call. "
+    "The OLD behavior for this policy is to follow the symlinks. "
+    "The NEW behavior for this policy is not to follow the symlinks "
+    "by default, but only if FOLLOW_SYMLINKS is given as an additional "
+    "argument to the FILE command.",
+    2,6,2, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()
@@ -384,7 +400,7 @@ bool cmPolicies::ApplyPolicyVersion(cmMakefile *mf,
       "In order to get compatibility features supporting versions earlier "
       "than 2.4 set policy CMP0001 to OLD to tell CMake to check the "
       "CMAKE_BACKWARDS_COMPATIBILITY variable.  "
-      "One way to so this is to set the policy version to 2.4 exactly."
+      "One way to do this is to set the policy version to 2.4 exactly."
       );
     return false;
     }
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 5284034..06079b0 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -49,6 +49,7 @@ public:
     CMP0006, // BUNDLE install rules needed for MACOSX_BUNDLE targets
     CMP0007, // list command handling of empty elements
     CMP0008, // Full-path libraries must be a valid library file name
+    CMP0009, // GLOB_RECURSE should not follow symlinks by default
 
     // Always the last entry.  Useful mostly to avoid adding a comma
     // the last policy when adding a new one.
diff --git a/Source/kwsys/Glob.cxx b/Source/kwsys/Glob.cxx
index 80fdf29..f5dc512 100644
--- a/Source/kwsys/Glob.cxx
+++ b/Source/kwsys/Glob.cxx
@@ -67,6 +67,7 @@ Glob::Glob()
   this->RecurseThroughSymlinks = true;
     // RecurseThroughSymlinks is true by default for backwards compatibility,
     // not because it's a good idea...
+  this->FollowedSymlinkCount = 0;
 }
 
 //----------------------------------------------------------------------------
@@ -266,9 +267,13 @@ void Glob::RecurseDirectory(kwsys_stl::string::size_type start,
       }
     if ( kwsys::SystemTools::FileIsDirectory(realname.c_str()) )
       {
-      if (!kwsys::SystemTools::FileIsSymlink(realname.c_str()) ||
-        this->RecurseThroughSymlinks)
+      bool isSymLink = kwsys::SystemTools::FileIsSymlink(realname.c_str());
+      if (!isSymLink || this->RecurseThroughSymlinks)
         {
+        if (isSymLink)
+          {
+          ++this->FollowedSymlinkCount;
+          }
         this->RecurseDirectory(start+1, realname, dir_only);
         }
       }
diff --git a/Source/kwsys/Glob.hxx.in b/Source/kwsys/Glob.hxx.in
index a66dd6f..fb4eac6 100644
--- a/Source/kwsys/Glob.hxx.in
+++ b/Source/kwsys/Glob.hxx.in
@@ -64,6 +64,9 @@ public:
   void SetRecurseThroughSymlinks(bool i) { this->RecurseThroughSymlinks = i; }
   bool GetRecurseThroughSymlinks() { return this->RecurseThroughSymlinks; }
 
+  //! Get the number of symlinks followed through recursion
+  unsigned int GetFollowedSymlinkCount() { return this->FollowedSymlinkCount; }
+
   //! Set relative to true to only show relative path to files.
   void SetRelative(const char* dir);
   const char* GetRelative();
@@ -98,6 +101,7 @@ protected:
   bool Recurse;
   kwsys_stl::string Relative;
   bool RecurseThroughSymlinks;
+  unsigned int FollowedSymlinkCount;
 
 private:
   Glob(const Glob&);  // Not implemented.