Add policy CMP0019 to skip include/link variable re-expansion

Historically CMake has always expanded ${} variable references in the
values given to include_directories(), link_directories(), and
link_libraries().  This has been unnecessary since general ${}
evaluation syntax was added to the language a LONG time ago, but has
remained for compatibility with VERY early CMake versions.

For a long time the re-expansion was a lightweight operation because it
was only processed once at the directory level and the fast-path of
cmMakefile::ExpandVariablesInString was usually taken because values did
not have any '$' in them.  Then commit d899eb71 (Call
ExpandVariablesInString for each target's INCLUDE_DIRECTORIES,
2012-02-22) made the operation a bit heavier because the expansion is
now needed on a per-target basis.  In the future we will support
generator expressions in INCLUDE_DIRECTORIES with $<> syntax, so the
fast-path in cmMakefile::ExpandVariablesInString will no longer be taken
and re-expansion will be very expensive.

Add policy CMP0019 to skip the re-expansion altogether in NEW behavior.
In OLD behavior perform the expansion but improve the fast-path
heuristic to match ${} but not $<>.  If the policy is not set then warn
if expansion actually does anything.  We expect this to be encountered
very rarely in practice.

Source/cmMakefile.cxx

@@ -814,7 +814,7 @@ bool cmMakefile::NeedBackwardsCompatibility(unsigned int major,
 void cmMakefile::FinalPass()
 {
   // do all the variable expansions here
-  this->ExpandVariables();
+  this->ExpandVariablesCMP0019();
 
   // give all the commands a chance to do something
   // after the file has been parsed before generation
@@ -2122,21 +2122,33 @@ void cmMakefile::AddExtraDirectory(const char* dir)
   this->AuxSourceDirectories.push_back(dir);
 }
 
-
-// expand CMAKE_BINARY_DIR and CMAKE_SOURCE_DIR in the
-// include and library directories.
-
-void cmMakefile::ExpandVariables()
+static bool mightExpandVariablesCMP0019(const char* s)
 {
-  // Now expand variables in the include and link strings
+  return s && *s && strstr(s,"${") && strchr(s,'}');
+}
+
+void cmMakefile::ExpandVariablesCMP0019()
+{
+  // Drop this ancient compatibility behavior with a policy.
+  cmPolicies::PolicyStatus pol = this->GetPolicyStatus(cmPolicies::CMP0019);
+  if(pol != cmPolicies::OLD && pol != cmPolicies::WARN)
+    {
+    return;
+    }
+  cmOStringStream w;
 
-  // May not be necessary anymore... But may need a policy for strict
-  // backwards compatibility
   const char *includeDirs = this->GetProperty("INCLUDE_DIRECTORIES");
-  if (includeDirs)
+  if(mightExpandVariablesCMP0019(includeDirs))
     {
     std::string dirs = includeDirs;
     this->ExpandVariablesInString(dirs, true, true);
+    if(pol == cmPolicies::WARN && dirs != includeDirs)
+      {
+      w << "Evaluated directory INCLUDE_DIRECTORIES\n"
+        << "  " << includeDirs << "\n"
+        << "as\n"
+        << "  " << dirs << "\n";
+      }
     this->SetProperty("INCLUDE_DIRECTORIES", dirs.c_str());
     }
 
@@ -2146,10 +2158,17 @@ void cmMakefile::ExpandVariables()
     {
     cmTarget &t = l->second;
     includeDirs = t.GetProperty("INCLUDE_DIRECTORIES");
-    if (includeDirs)
+    if(mightExpandVariablesCMP0019(includeDirs))
       {
       std::string dirs = includeDirs;
       this->ExpandVariablesInString(dirs, true, true);
+      if(pol == cmPolicies::WARN && dirs != includeDirs)
+        {
+        w << "Evaluated target " << t.GetName() << " INCLUDE_DIRECTORIES\n"
+          << "  " << includeDirs << "\n"
+          << "as\n"
+          << "  " << dirs << "\n";
+        }
       t.SetProperty("INCLUDE_DIRECTORIES", dirs.c_str());
       }
     }
@@ -2157,13 +2176,45 @@ void cmMakefile::ExpandVariables()
   for(std::vector<std::string>::iterator d = this->LinkDirectories.begin();
       d != this->LinkDirectories.end(); ++d)
     {
+    if(mightExpandVariablesCMP0019(d->c_str()))
+      {
+      std::string orig = *d;
       this->ExpandVariablesInString(*d, true, true);
+      if(pol == cmPolicies::WARN && *d != orig)
+        {
+        w << "Evaluated link directory\n"
+          << "  " << orig << "\n"
+          << "as\n"
+          << "  " << *d << "\n";
+        }
+      }
     }
   for(cmTarget::LinkLibraryVectorType::iterator l =
         this->LinkLibraries.begin();
       l != this->LinkLibraries.end(); ++l)
     {
+    if(mightExpandVariablesCMP0019(l->first.c_str()))
+      {
+      std::string orig = l->first;
       this->ExpandVariablesInString(l->first, true, true);
+      if(pol == cmPolicies::WARN && l->first != orig)
+        {
+        w << "Evaluated link library\n"
+          << "  " << orig << "\n"
+          << "as\n"
+          << "  " << l->first << "\n";
+        }
+      }
+    }
+
+  if(!w.str().empty())
+    {
+    cmOStringStream m;
+    m << this->GetPolicies()->GetPolicyWarning(cmPolicies::CMP0019)
+      << "\n"
+      << "The following variable evaluations were encountered:\n"
+      << w.str();
+    this->IssueMessage(cmake::AUTHOR_WARNING, m.str());
     }
 }
 

Source/cmMakefile.h

@@ -693,7 +693,7 @@ public:
   /**
    * Expand variables in the makefiles ivars such as link directories etc
    */
-  void ExpandVariables();
+  void ExpandVariablesCMP0019();
 
   /**
    * Replace variables and #cmakedefine lines in the given string.

Source/cmPolicies.cxx

@@ -491,6 +491,23 @@ cmPolicies::cmPolicies()
     "CMAKE_SHARED_LIBRARY_<Lang>_FLAGS whether it is modified or not and "
     "honor the POSITION_INDEPENDENT_CODE target property.",
     2,8,9,0, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0019, "CMP0019",
+    "Do not re-expand variables in include and link information.",
+    "CMake 2.8.10 and lower re-evaluated values given to the "
+    "include_directories, link_directories, and link_libraries "
+    "commands to expand any leftover variable references at the "
+    "end of the configuration step.  "
+    "This was for strict compatibility with VERY early CMake versions "
+    "because all variable references are now normally evaluated during "
+    "CMake language processing.  "
+    "CMake 2.8.11 and higher prefer to skip the extra evaluation."
+    "\n"
+    "The OLD behavior for this policy is to re-evaluate the values "
+    "for strict compatibility.  "
+    "The NEW behavior for this policy is to leave the values untouched.",
+    2,8,11,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()

Source/cmPolicies.h

@@ -68,6 +68,7 @@ public:
     CMP0018, ///< Ignore language flags for shared libs, and adhere to
     /// POSITION_INDEPENDENT_CODE property and *_COMPILE_OPTIONS_PI{E,C}
     /// instead.
+    CMP0019, ///< No variable re-expansion in include and link info
 
     /** \brief Always the last entry.
      *

Tests/RunCMake/CMP0019/CMP0019-NEW.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0019 NEW)
+include(CMP0019-code.cmake)

Tests/RunCMake/CMP0019/CMP0019-OLD.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0019 OLD)
+include(CMP0019-code.cmake)

Tests/RunCMake/CMP0019/CMP0019-WARN-stderr.txt

@@ -0,0 +1,40 @@
+CMake Warning \(dev\) in CMakeLists.txt:
+  Policy CMP0019 is not set: Do not re-expand variables in include and link
+  information.  Run "cmake --help-policy CMP0019" for policy details.  Use
+  the cmake_policy command to set the policy and suppress this warning.
+
+  The following variable evaluations were encountered:
+
+  Evaluated directory INCLUDE_DIRECTORIES
+
+    /usr/include/\${VAR_INCLUDE};/usr/include/normal
+
+  as
+
+    /usr/include/VAL_INCLUDE;/usr/include/normal
+
+  Evaluated target some_target INCLUDE_DIRECTORIES
+
+    /usr/include/\${VAR_INCLUDE};/usr/include/normal
+
+  as
+
+    /usr/include/VAL_INCLUDE;/usr/include/normal
+
+  Evaluated link directory
+
+    /usr/lib/\${VAR_LINK_DIRS}
+
+  as
+
+    /usr/lib/VAL_LINK_DIRS
+
+  Evaluated link library
+
+    \${VAR_LINK_LIBS}
+
+  as
+
+    VAL_LINK_LIBS
+
+This warning is for project developers.  Use -Wno-dev to suppress it.$

Tests/RunCMake/CMP0019/CMP0019-WARN.cmake

@@ -0,0 +1 @@
+include(CMP0019-code.cmake)

Tests/RunCMake/CMP0019/CMP0019-code.cmake

@@ -0,0 +1,9 @@
+set(VAR_INCLUDE "VAL_INCLUDE")
+set(VAR_LINK_DIRS "VAL_LINK_DIRS")
+set(VAR_LINK_LIBS "VAL_LINK_LIBS")
+add_custom_target(some_target)
+include_directories("/usr/include/\${VAR_INCLUDE}" /usr/include/normal)
+link_directories("/usr/lib/\${VAR_LINK_DIRS}"  /usr/lib/normal)
+link_libraries("\${VAR_LINK_LIBS}" normal)
+add_custom_target(other_target)
+set_property(TARGET other_target PROPERTY INCLUDE_DIRECTORIES "")

Tests/RunCMake/CMP0019/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 2.8)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)

Tests/RunCMake/CMP0019/RunCMakeTest.cmake

@@ -0,0 +1,5 @@
+include(RunCMake)
+
+run_cmake(CMP0019-WARN)
+run_cmake(CMP0019-OLD)
+run_cmake(CMP0019-NEW)

Tests/RunCMake/CMakeLists.txt

@@ -45,6 +45,7 @@ macro(add_RunCMake_test test)
     )
 endmacro()
 
+add_RunCMake_test(CMP0019)
 add_RunCMake_test(GeneratorExpression)
 add_RunCMake_test(TargetPropertyGeneratorExpressions)
 add_RunCMake_test(Languages)