ENH: Convert CMAKE_LINK_OLD_PATHS to policy CMP0003.

  - Policy is WARN by default so projects will build
    as they did in 2.4 without user intervention
  - Remove CMAKE_LINK_OLD_PATHS variable since it was
    never in a release and the policy supercedes it
  - Report target creation backtrace in warning message
    since policy should be set by that point

Modules/VTKCompatibility.cmake

@@ -31,11 +31,6 @@ SET(VTK_WGLEXT_FILE "${VTK_SOURCE_DIR}/Utilities/ParseOGLExt/headers/wglext.h"
 # work around an old bug in VTK
 SET(TIFF_RIGHT_VERSION 1)
 
-# vtkRendering links to X11 with "-lXt ${X11_LIBRARIES}" because CMake
-# 2.4 and below did not provide the X11_Xt_LIB variable.  We need the
-# linker search path compatiblity feature.
-SET(CMAKE_LINK_OLD_PATHS 1)
-
 # for very old VTK (versions prior to 4.2)
 MACRO(SOURCE_FILES)
   message (FATAL_ERROR "You are trying to build a very old version of VTK (prior to VTK 4.2). To do this you need to use CMake 2.0 as it was the last version of CMake to support VTK 4.0.")

Source/cmComputeLinkInformation.cxx

@@ -396,9 +396,6 @@ cmComputeLinkInformation
       ->SetImplicitDirectories(this->ImplicitLinkDirs);
     }
 
-  // Initial state.
-  this->HaveUserFlagItem = false;
-
   // Decide whether to enable compatible library search path mode.
   // There exists code that effectively does
   //
@@ -410,12 +407,18 @@ cmComputeLinkInformation
   // because -L/path/to would be added by the -L/-l split for A.  In
   // order to support such projects we need to add the directories
   // containing libraries linked with a full path to the -L path.
-  this->OldLinkDirMode = false;
-  if(this->Makefile->IsOn("CMAKE_LINK_OLD_PATHS") ||
-     this->Makefile->GetLocalGenerator()
-     ->NeedBackwardsCompatibility(2, 4))
+  this->OldLinkDirMode =
+    this->Target->GetPolicyStatusCMP0003() != cmPolicies::NEW;
+  if(this->OldLinkDirMode)
     {
-    this->OldLinkDirMode = true;
+    // Construct a mask to not bother with this behavior for link
+    // directories already specified by the user.
+    std::vector<std::string> const& dirs = this->Target->GetLinkDirectories();
+    for(std::vector<std::string>::const_iterator di = dirs.begin();
+        di != dirs.end(); ++di)
+      {
+      this->OldLinkDirMask.insert(*di);
+      }
     }
 }
 
@@ -537,7 +540,10 @@ bool cmComputeLinkInformation::Compute()
     }
 
   // Finish setting up linker search directories.
-  this->FinishLinkerSearchDirectories();
+  if(!this->FinishLinkerSearchDirectories())
+    {
+    return false;
+    }
 
   return true;
 }
@@ -1037,7 +1043,9 @@ void cmComputeLinkInformation::AddFullItem(std::string const& item)
 
   // For compatibility with CMake 2.4 include the item's directory in
   // the linker search path.
-  if(this->OldLinkDirMode)
+  if(this->OldLinkDirMode &&
+     this->OldLinkDirMask.find(cmSystemTools::GetFilenamePath(item)) ==
+     this->OldLinkDirMask.end())
     {
     this->OldLinkDirItems.push_back(item);
     }
@@ -1161,7 +1169,7 @@ void cmComputeLinkInformation::AddUserItem(std::string const& item)
   else if(item[0] == '-' || item[0] == '$' || item[0] == '`')
     {
     // This is a linker option provided by the user.
-    this->HaveUserFlagItem = true;
+    this->OldUserFlagItems.push_back(item);
 
     // Restore the target link type since this item does not specify
     // one.
@@ -1174,7 +1182,7 @@ void cmComputeLinkInformation::AddUserItem(std::string const& item)
   else
     {
     // This is a name specified by the user.
-    this->HaveUserFlagItem = true;
+    this->OldUserFlagItems.push_back(item);
 
     // We must ask the linker to search for a library with this name.
     // Restore the target link type since this item does not specify
@@ -1304,18 +1312,115 @@ void cmComputeLinkInformation::AddSharedLibNoSOName(std::string const& item)
 }
 
 //----------------------------------------------------------------------------
-void cmComputeLinkInformation::FinishLinkerSearchDirectories()
+bool cmComputeLinkInformation::FinishLinkerSearchDirectories()
 {
   // Support broken projects if necessary.
-  if(this->HaveUserFlagItem && this->OldLinkDirMode)
+  if(this->OldLinkDirItems.empty() || this->OldUserFlagItems.empty() ||
+     !this->OldLinkDirMode)
     {
-    for(std::vector<std::string>::const_iterator
-          i = this->OldLinkDirItems.begin();
-        i != this->OldLinkDirItems.end(); ++i)
+    return true;
+    }
+
+  // Enforce policy constraints.
+  switch(this->Target->GetPolicyStatusCMP0003())
+    {
+    case cmPolicies::WARN:
+      {
+      cmOStringStream w;
+      w << (this->Makefile->GetPolicies()
+            ->GetPolicyWarning(cmPolicies::CMP0003)) << "\n";
+      this->PrintLinkPolicyDiagnosis(w);
+      this->CMakeInstance->IssueMessage(cmake::AUTHOR_WARNING, w.str(),
+                                        this->Target->GetBacktrace());
+      }
+    case cmPolicies::OLD:
+      // OLD behavior is to add the paths containing libraries with
+      // known full paths as link directories.
+      break;
+    case cmPolicies::NEW:
+      // Should never happen due to assignment of OldLinkDirMode
+      return true;
+      break;
+    case cmPolicies::REQUIRED_IF_USED:
+    case cmPolicies::REQUIRED_ALWAYS:
+      {
+      cmOStringStream e;
+      e << (this->Makefile->GetPolicies()->
+            GetRequiredPolicyError(cmPolicies::CMP0003)) << "\n";
+      this->PrintLinkPolicyDiagnosis(e);
+      this->CMakeInstance->IssueMessage(cmake::FATAL_ERROR, e.str(),
+                                        this->Target->GetBacktrace());
+      return false;
+      }
+      break;
+    }
+
+  // Add the link directories for full path items.
+  for(std::vector<std::string>::const_iterator
+        i = this->OldLinkDirItems.begin();
+      i != this->OldLinkDirItems.end(); ++i)
+    {
+    this->OrderLinkerSearchPath->AddLinkLibrary(*i);
+    }
+  return true;
+}
+
+//----------------------------------------------------------------------------
+void cmComputeLinkInformation::PrintLinkPolicyDiagnosis(std::ostream& os)
+{
+  // Name the target.
+  os << "Target \"" << this->Target->GetName() << "\" ";
+
+  // List the items that would add paths in old behavior.
+  os << " links to some items with known full path:\n";
+  for(std::vector<std::string>::const_iterator
+        i = this->OldLinkDirItems.begin();
+      i != this->OldLinkDirItems.end(); ++i)
+    {
+    os << "  " << *i << "\n";
+    }
+
+  // List the items that might need the old-style paths.
+  os << "and to some items with no path known:\n";
+  {
+  // Format the list of unknown items to be as short as possible while
+  // still fitting in the allowed width (a true solution would be the
+  // bin packing problem if we were allowed to change the order).
+  std::string::size_type max_size = 76;
+  std::string line;
+  const char* sep = "  ";
+  for(std::vector<std::string>::const_iterator
+        i = this->OldUserFlagItems.begin();
+      i != this->OldUserFlagItems.end(); ++i)
+    {
+    // If the addition of another item will exceed the limit then
+    // output the current line and reset it.  Note that the separator
+    // is either " " or ", " which is always 2 characters.
+    if(!line.empty() && (line.size() + i->size() + 2) > max_size)
       {
-      this->OrderLinkerSearchPath->AddLinkLibrary(*i);
+      os << line << "\n";
+      sep = "  ";
+      line = "";
       }
+    line += sep;
+    line += *i;
+
+    // Convert to the other separator.
+    sep = ", ";
     }
+  if(!line.empty())
+    {
+    os << line << "\n";
+    }
+  }
+
+  // Tell the user what is wrong.
+  os << "The linker will search for libraries in the second list.  "
+     << "Finding them may depend on linker search paths earlier CMake "
+     << "versions added as an implementation detail for linking to the "
+     << "libraries in the first list.  "
+     << "For compatibility CMake is including the extra linker search "
+     << "paths, but policy CMP0003 should be set by the project.";
 }
 
 //----------------------------------------------------------------------------

Source/cmComputeLinkInformation.h

@@ -153,13 +153,15 @@ private:
 
   // Linker search path computation.
   cmOrderDirectories* OrderLinkerSearchPath;
-  void FinishLinkerSearchDirectories();
+  bool FinishLinkerSearchDirectories();
+  void PrintLinkPolicyDiagnosis(std::ostream&);
   std::set<cmStdString> ImplicitLinkDirs;
 
   // Linker search path compatibility mode.
+  std::set<cmStdString> OldLinkDirMask;
   std::vector<std::string> OldLinkDirItems;
+  std::vector<std::string> OldUserFlagItems;
   bool OldLinkDirMode;
-  bool HaveUserFlagItem;
 
   // Runtime path computation.
   cmOrderDirectories* OrderRuntimeSearchPath;

Source/cmDocumentVariables.cxx

@@ -783,24 +783,6 @@ void cmDocumentVariables::DefineVariables(cmake* cm)
      "The flag used before a library file path is given to the linker.  "
      "This is needed only on very few platforms.", false,
      "Variables that Control the Build");
-  cm->DefineProperty
-    ("CMAKE_LINK_OLD_PATHS", cmProperty::VARIABLE,
-     "Enable linker search path compatibility mode.",
-     "This option enables linking compatibility mode for broken projects.  "
-     "There exists code that effectively does\n"
-     "  target_link_libraries(myexe /path/to/libA.so -lB)\n"
-     "where -lB is meant to link to /path/to/libB.so.  This is broken "
-     "because it specifies -lB without adding \"/path/to\" to the linker "
-     "search path with the link_directories command.  With CMake 2.4 and "
-     "below the code worked accidentally because \"/path/to\" would be "
-     "added to the linker search path by its implementation of linking to "
-     "/path/to/libA.so (which passed -L/path/to -lA to the linker).  "
-     "This option tells CMake to add the directories containing libraries "
-     "specified with a full path to the linker search path if the link "
-     "line contains any items like -lB.  "
-     "The behavior is also enabled if CMAKE_BACKWARDS_COMPATIBILITY is "
-     "set to 2.4 or lower.", false,
-     "Variables that Control the Build");
   cm->DefineProperty
     ("CMAKE_USE_RELATIVE_PATHS", cmProperty::VARIABLE,
      "Use relative paths (May not work!).",

Source/cmPolicies.cxx

@@ -139,6 +139,67 @@ cmPolicies::cmPolicies()
     "Makefiles generator).",
     2,6,0, cmPolicies::WARN
     );
+
+  this->DefinePolicy(
+    CMP0003, "CMP0003",
+    "Libraries linked via full path no longer produce linker search paths.",
+    "This policy affects how libraries whose full paths are NOT known "
+    "are found at link time, but was created due to a change in how CMake "
+    "deals with libraries whose full paths are known.  "
+    "Consider the code\n"
+    "  target_link_libraries(myexe /path/to/libA.so)\n"
+    "CMake 2.4 and below implemented linking to libraries whose full paths "
+    "are known by splitting them on the link line into separate components "
+    "consisting of the linker search path and the library name.  "
+    "The example code might have produced something like\n"
+    "  ... -L/path/to -lA ...\n"
+    "in order to link to library A.  "
+    "An analysis was performed to order multiple link directories such that "
+    "the linker would find library A in the desired location, but there "
+    "are cases in which this does not work.  "
+    "CMake versions 2.6 and above use the more reliable approach of passing "
+    "the full path to libraries directly to the linker in most cases.  "
+    "The example code now produces something like\n"
+    "  ... /path/to/libA.so ....\n"
+    "Unfortunately this change can break code like\n"
+    "  target_link_libraries(myexe /path/to/libA.so B)\n"
+    "where \"B\" is meant to find \"/path/to/libB.so\".  "
+    "This code is wrong because the user is asking the linker to find "
+    "library B but has not provided a linker search path (which may be "
+    "added with the link_directories command).  "
+    "However, with the old linking implementation the code would work "
+    "accidentally because the linker search path added for library A "
+    "allowed library B to be found."
+    "\n"
+    "In order to support projects depending on linker search paths "
+    "added by linking to libraries with known full paths, the OLD "
+    "behavior for this policy will add the linker search paths even "
+    "though they are not needed for their own libraries.  "
+    "When this policy is set to OLD, CMake will produce a link line such as\n"
+    "  ... -L/path/to /path/to/libA.so -lB ...\n"
+    "which will allow library B to be found as it was previously.  "
+    "When this policy is set to NEW, CMake will produce a link line such as\n"
+    "  ... /path/to/libA.so -lB ...\n"
+    "which more accurately matches what the project specified."
+    "\n"
+    "The setting for this policy used when generating the link line is that "
+    "in effect when the target is created by an add_executable or "
+    "add_library command.  For the example described above, the code\n"
+    "  cmake_policy(SET CMP0003 OLD) # or cmake_policy(VERSION 2.4)\n"
+    "  add_executable(myexe myexe.c)\n"
+    "  target_link_libraries(myexe /path/to/libA.so B)\n"
+    "will work and suppress the warning for this policy.  "
+    "It may also be updated to work with the corrected linking approach:\n"
+    "  cmake_policy(SET CMP0003 NEW) # or cmake_policy(VERSION 2.6)\n"
+    "  link_directories(/path/to) # needed to find library B\n"
+    "  add_executable(myexe myexe.c)\n"
+    "  target_link_libraries(myexe /path/to/libA.so B)\n"
+    "Even better, library B may be specified with a full path:\n"
+    "  add_executable(myexe myexe.c)\n"
+    "  target_link_libraries(myexe /path/to/libA.so /path/to/libB.so)\n"
+    "When all items on the link line have known paths CMake does not check "
+    "this policy so it has no effect.",
+    2,6,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()

Source/cmPolicies.h

@@ -43,6 +43,7 @@ public:
     CMP0000, // Policy version specification
     CMP0001, // Ignore old compatibility variable
     CMP0002, // Target names must be unique
+    CMP0003, // Linking does not include extra -L paths
 
     // Always the last entry.  Useful mostly to avoid adding a comma
     // the last policy when adding a new one.

Source/cmTarget.cxx

@@ -53,6 +53,7 @@ public:
 cmTarget::cmTarget()
 {
   this->Makefile = 0;
+  this->PolicyStatusCMP0003 = cmPolicies::WARN;
   this->LinkLibrariesAnalyzed = false;
   this->HaveInstallRule = false;
   this->DLLPlatform = false;
@@ -726,6 +727,10 @@ void cmTarget::SetMakefile(cmMakefile* mf)
 
   // Save the backtrace of target construction.
   this->Makefile->GetBacktrace(this->Internal->Backtrace);
+
+  // Record current policies for later use.
+  this->PolicyStatusCMP0003 =
+    this->Makefile->GetPolicyStatus(cmPolicies::CMP0003);
 }
 
 //----------------------------------------------------------------------------

Source/cmTarget.h

@@ -19,6 +19,7 @@
 
 #include "cmCustomCommand.h"
 #include "cmPropertyMap.h"
+#include "cmPolicies.h"
 
 class cmake;
 class cmMakefile;
@@ -105,6 +106,10 @@ public:
   void SetMakefile(cmMakefile *mf);
   cmMakefile *GetMakefile() const { return this->Makefile;};
 
+  /** Get the status of policy CMP0003 when the target was created.  */
+  cmPolicies::PolicyStatus GetPolicyStatusCMP0003() const
+    { return this->PolicyStatusCMP0003; }
+
   /**
    * Get the list of the custom commands for this target
    */
@@ -530,6 +535,9 @@ private:
   // always be set.
   cmMakefile* Makefile;
 
+  // Policy status recorded when target was created.
+  cmPolicies::PolicyStatus PolicyStatusCMP0003;
+
   // Internal representation details.
   friend class cmTargetInternals;
   cmTargetInternalPointer Internal;