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

7 files changed, 202 insertions, 38 deletions

diff --git a/Source/cmComputeLinkInformation.cxx b/Source/cmComputeLinkInformation.cxx
index a3f8bab..f43cb55 100644
--- a/Source/cmComputeLinkInformation.cxx
+++ b/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.";
 }
 
 //----------------------------------------------------------------------------
diff --git a/Source/cmComputeLinkInformation.h b/Source/cmComputeLinkInformation.h
index df22d53..31d0e33 100644
--- a/Source/cmComputeLinkInformation.h
+++ b/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;
diff --git a/Source/cmDocumentVariables.cxx b/Source/cmDocumentVariables.cxx
index ca1b0f2..a644749 100644
--- a/Source/cmDocumentVariables.cxx
+++ b/Source/cmDocumentVariables.cxx
@@ -784,24 +784,6 @@ void cmDocumentVariables::DefineVariables(cmake* cm)
      "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!).",
      "If this is set to TRUE, then the CMake will use "
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 6d59bfe..06133bc 100644
--- a/Source/cmPolicies.cxx
+++ b/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()
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 088d554..4ca6a6b 100644
--- a/Source/cmPolicies.h
+++ b/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.
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index 429c5f9..5811285 100644
--- a/Source/cmTarget.cxx
+++ b/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);
 }
 
 //----------------------------------------------------------------------------
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index a8b2863..82d9b67 100644
--- a/Source/cmTarget.h
+++ b/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;