target_link_libraries: Allow use with targets in other directories

Previously the command did not allow naming targets on the LHS that
were not created in the calling directory.  Lift this restriction to
enable more flexible use by projects.

Targets named on the RHS will need to be looked up during generation in
the scope of the call site rather than the scope of the LHS target.
Introduce an internal syntax in `[INTERFACE_]LINK_LIBRARIES` properties
to specify target names that need to be looked up in a directory other
than that containing the target on which the property is set.  Add
minimal documentation of the syntax to help users that encounter it.

Unfortunately CMake previously did allow such calls in the case that
only `INTERFACE` libraries are specified, but those libraries would be
looked up in the target's directory rather than the caller's.  Add
policy `CMP0079` to enable the new behavior with new lookup scope in a
compatible way.

Fixes: #17943

9 files changed, 165 insertions, 29 deletions

diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 1bd98e2..efcfaf7 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -5648,11 +5648,38 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
 cmGeneratorTarget::TargetOrString cmGeneratorTarget::ResolveTargetReference(
   std::string const& name) const
 {
+  cmLocalGenerator const* lg = this->LocalGenerator;
+  std::string const* lookupName = &name;
+
+  // When target_link_libraries() is called with a LHS target that is
+  // not created in the calling directory it adds a directory id suffix
+  // that we can use to look up the calling directory.  It is that scope
+  // in which the item name is meaningful.  This case is relatively rare
+  // so we allocate a separate string only when the directory id is present.
+  std::string::size_type pos = name.find(CMAKE_DIRECTORY_ID_SEP);
+  std::string plainName;
+  if (pos != std::string::npos) {
+    // We will look up the plain name without the directory id suffix.
+    plainName = name.substr(0, pos);
+
+    // We will look up in the scope of the directory id.
+    // If we do not recognize the id then leave the original
+    // syntax in place to produce an indicative error later.
+    cmDirectoryId const dirId =
+      name.substr(pos + sizeof(CMAKE_DIRECTORY_ID_SEP) - 1);
+    if (cmLocalGenerator const* otherLG =
+          this->GlobalGenerator->FindLocalGenerator(dirId)) {
+      lg = otherLG;
+      lookupName = &plainName;
+    }
+  }
+
   TargetOrString resolved;
 
-  if (cmGeneratorTarget* tgt =
-        this->LocalGenerator->FindGeneratorTargetToUse(name)) {
+  if (cmGeneratorTarget* tgt = lg->FindGeneratorTargetToUse(*lookupName)) {
     resolved.Target = tgt;
+  } else if (lookupName == &plainName) {
+    resolved.String = std::move(plainName);
   } else {
     resolved.String = name;
   }
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 0aba67c..8a8b3e4 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -1139,11 +1139,14 @@ void cmGlobalGenerator::ClearEnabledLanguages()
 
 void cmGlobalGenerator::CreateLocalGenerators()
 {
+  this->LocalGeneratorSearchIndex.clear();
   cmDeleteAll(this->LocalGenerators);
   this->LocalGenerators.clear();
   this->LocalGenerators.reserve(this->Makefiles.size());
   for (cmMakefile* m : this->Makefiles) {
-    this->LocalGenerators.push_back(this->CreateLocalGenerator(m));
+    cmLocalGenerator* lg = this->CreateLocalGenerator(m);
+    this->LocalGenerators.push_back(lg);
+    this->IndexLocalGenerator(lg);
   }
 }
 
@@ -1661,6 +1664,7 @@ void cmGlobalGenerator::ClearGeneratorMembers()
   this->TargetSearchIndex.clear();
   this->GeneratorTargetSearchIndex.clear();
   this->MakefileSearchIndex.clear();
+  this->LocalGeneratorSearchIndex.clear();
   this->ProjectMap.clear();
   this->RuleHashes.clear();
   this->DirectoryContentMap.clear();
@@ -2130,6 +2134,17 @@ cmMakefile* cmGlobalGenerator::FindMakefile(const std::string& start_dir) const
   return nullptr;
 }
 
+cmLocalGenerator* cmGlobalGenerator::FindLocalGenerator(
+  cmDirectoryId const& id) const
+{
+  LocalGeneratorMap::const_iterator i =
+    this->LocalGeneratorSearchIndex.find(id.String);
+  if (i != this->LocalGeneratorSearchIndex.end()) {
+    return i->second;
+  }
+  return nullptr;
+}
+
 void cmGlobalGenerator::AddAlias(const std::string& name,
                                  std::string const& tgtName)
 {
@@ -2184,6 +2199,12 @@ void cmGlobalGenerator::IndexMakefile(cmMakefile* mf)
     MakefileMap::value_type(mf->GetCurrentSourceDirectory(), mf));
 }
 
+void cmGlobalGenerator::IndexLocalGenerator(cmLocalGenerator* lg)
+{
+  cmDirectoryId id = lg->GetMakefile()->GetDirectoryId();
+  this->LocalGeneratorSearchIndex[id.String] = lg;
+}
+
 cmTarget* cmGlobalGenerator::FindTargetImpl(std::string const& name) const
 {
   TargetMap::const_iterator i = this->TargetSearchIndex.find(name);
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 6b972eb..f240f1d 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -26,6 +26,9 @@
 #  include "cmFileLockPool.h"
 #endif
 
+#define CMAKE_DIRECTORY_ID_SEP "::@"
+
+class cmDirectoryId;
 class cmExportBuildFileGenerator;
 class cmExternalMakefileProjectGenerator;
 class cmGeneratorTarget;
@@ -284,6 +287,7 @@ public:
   bool NameResolvesToFramework(const std::string& libname) const;
 
   cmMakefile* FindMakefile(const std::string& start_dir) const;
+  cmLocalGenerator* FindLocalGenerator(cmDirectoryId const& id) const;
 
   /** Append the subdirectory for the given configuration.  If anything is
       appended the given prefix and suffix will be appended around it, which
@@ -516,6 +520,7 @@ private:
   typedef std::unordered_map<std::string, cmGeneratorTarget*>
     GeneratorTargetMap;
   typedef std::unordered_map<std::string, cmMakefile*> MakefileMap;
+  typedef std::unordered_map<std::string, cmLocalGenerator*> LocalGeneratorMap;
   // Map efficiently from target name to cmTarget instance.
   // Do not use this structure for looping over all targets.
   // It contains both normal and globally visible imported targets.
@@ -527,6 +532,11 @@ private:
   // It may not contain all of them (see note in IndexMakefile method).
   MakefileMap MakefileSearchIndex;
 
+  // Map efficiently from source directory path to cmLocalGenerator instance.
+  // Do not use this structure for looping over all directories.
+  // Its order is not deterministic.
+  LocalGeneratorMap LocalGeneratorSearchIndex;
+
   cmMakefile* TryCompileOuterMakefile;
   // If you add a new map here, make sure it is copied
   // in EnableLanguagesFromGenerator
@@ -585,6 +595,7 @@ private:
                     std::string const& reason) const;
 
   void IndexMakefile(cmMakefile* mf);
+  void IndexLocalGenerator(cmLocalGenerator* lg);
 
   virtual const char* GetBuildIgnoreErrorsFlag() const { return nullptr; }
 
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 5498ad2..28e8195 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -11,6 +11,7 @@
 #include <iterator>
 #include <memory> // IWYU pragma: keep
 #include <sstream>
+#include <stdio.h>
 #include <stdlib.h>
 #include <utility>
 
@@ -48,6 +49,11 @@
 
 class cmMessenger;
 
+cmDirectoryId::cmDirectoryId(std::string s)
+  : String(std::move(s))
+{
+}
+
 // default is not to be building executables
 cmMakefile::cmMakefile(cmGlobalGenerator* globalGenerator,
                        cmStateSnapshot const& snapshot)
@@ -111,6 +117,17 @@ cmMakefile::~cmMakefile()
   cmDeleteAll(this->EvaluationFiles);
 }
 
+cmDirectoryId cmMakefile::GetDirectoryId() const
+{
+  // Use the instance pointer value to uniquely identify this directory.
+  // If we ever need to expose this to CMake language code we should
+  // add a read-only property in cmMakefile::GetProperty.
+  char buf[32];
+  sprintf(buf, "<%p>",
+          static_cast<void const*>(this)); // cast avoids format warning
+  return std::string(buf);
+}
+
 void cmMakefile::IssueMessage(cmake::MessageType t,
                               std::string const& text) const
 {
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 0ab4371..54730b5 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -47,6 +47,14 @@ class cmTest;
 class cmTestGenerator;
 class cmVariableWatch;
 
+/** A type-safe wrapper for a string representing a directory id.  */
+class cmDirectoryId
+{
+public:
+  cmDirectoryId(std::string s);
+  std::string String;
+};
+
 /** \class cmMakefile
  * \brief Process the input CMakeLists.txt file.
  *
@@ -75,6 +83,8 @@ public:
    */
   ~cmMakefile();
 
+  cmDirectoryId GetDirectoryId() const;
+
   bool ReadListFile(const char* filename);
 
   bool ReadDependentFile(const char* filename, bool noPolicyScope = true);
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 91ed924..4ffe803 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -230,7 +230,11 @@ class cmMakefile;
   SELECT(POLICY, CMP0077, "option() honors normal variables.", 3, 13, 0,      \
          cmPolicies::WARN)                                                    \
   SELECT(POLICY, CMP0078, "UseSWIG generates standard target names.", 3, 13,  \
-         0, cmPolicies::WARN)
+         0, cmPolicies::WARN)                                                 \
+  SELECT(                                                                     \
+    POLICY, CMP0079,                                                          \
+    "target_link_libraries allows use with targets in other directories.", 3, \
+    13, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index 927b218..cd40223 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -745,20 +745,27 @@ void cmTarget::GetTllSignatureTraces(std::ostream& s, TLLSignature sig) const
 void cmTarget::AddLinkLibrary(cmMakefile& mf, const std::string& lib,
                               cmTargetLinkLibraryType llt)
 {
+  this->AddLinkLibrary(mf, lib, lib, llt);
+}
+
+void cmTarget::AddLinkLibrary(cmMakefile& mf, std::string const& lib,
+                              std::string const& libRef,
+                              cmTargetLinkLibraryType llt)
+{
   cmTarget* tgt = mf.FindTargetToUse(lib);
   {
     const bool isNonImportedTarget = tgt && !tgt->IsImported();
 
     const std::string libName =
       (isNonImportedTarget && llt != GENERAL_LibraryType)
-      ? targetNameGenex(lib)
-      : lib;
+      ? targetNameGenex(libRef)
+      : libRef;
     this->AppendProperty(
       "LINK_LIBRARIES",
       this->GetDebugGeneratorExpressions(libName, llt).c_str());
   }
 
-  if (cmGeneratorExpression::Find(lib) != std::string::npos ||
+  if (cmGeneratorExpression::Find(lib) != std::string::npos || lib != libRef ||
       (tgt &&
        (tgt->GetType() == cmStateEnums::INTERFACE_LIBRARY ||
         tgt->GetType() == cmStateEnums::OBJECT_LIBRARY)) ||
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index 7a3ab65..1f380df 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -142,6 +142,9 @@ public:
 
   void AddLinkLibrary(cmMakefile& mf, const std::string& lib,
                       cmTargetLinkLibraryType llt);
+  void AddLinkLibrary(cmMakefile& mf, std::string const& lib,
+                      std::string const& libRef, cmTargetLinkLibraryType llt);
+
   enum TLLSignature
   {
     KeywordTLLSignature,
diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx
index 1bbcf46..ad33f98 100644
--- a/Source/cmTargetLinkLibrariesCommand.cxx
+++ b/Source/cmTargetLinkLibrariesCommand.cxx
@@ -359,30 +359,53 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
     }
   }
 
+  bool warnRemoteInterface = false;
+  bool rejectRemoteLinking = false;
+  bool encodeRemoteReference = false;
+  if (this->Makefile != this->Target->GetMakefile()) {
+    // The LHS target was created in another directory.
+    switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP0079)) {
+      case cmPolicies::WARN:
+        warnRemoteInterface = true;
+        CM_FALLTHROUGH;
+      case cmPolicies::OLD:
+        rejectRemoteLinking = true;
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        encodeRemoteReference = true;
+        break;
+    }
+  }
+
+  std::string libRef;
+  if (encodeRemoteReference && !cmSystemTools::FileIsFullPath(lib)) {
+    // This is a library name added by a caller that is not in the
+    // same directory as the target was created.  Add a suffix to
+    // the name to tell ResolveLinkItem to look up the name in the
+    // caller's directory.
+    cmDirectoryId const dirId = this->Makefile->GetDirectoryId();
+    libRef = lib + CMAKE_DIRECTORY_ID_SEP + dirId.String;
+  } else {
+    // This is an absolute path or a library name added by a caller
+    // in the same directory as the target was created.  We can use
+    // the original name directly.
+    libRef = lib;
+  }
+
   // Handle normal case where the command was called with another keyword than
   // INTERFACE / LINK_INTERFACE_LIBRARIES or none at all. (The "LINK_LIBRARIES"
   // property of the target on the LHS shall be populated.)
   if (this->CurrentProcessingState != ProcessingKeywordLinkInterface &&
       this->CurrentProcessingState != ProcessingPlainLinkInterface) {
 
-    // Assure that the target on the LHS was created in the current directory.
-    cmTarget* t =
-      this->Makefile->FindLocalNonAliasTarget(this->Target->GetName());
-    if (!t) {
-      const std::vector<cmTarget*>& importedTargets =
-        this->Makefile->GetOwnedImportedTargets();
-      for (cmTarget* importedTarget : importedTargets) {
-        if (importedTarget->GetName() == this->Target->GetName()) {
-          t = importedTarget;
-          break;
-        }
-      }
-    }
-    if (!t) {
+    if (rejectRemoteLinking) {
       std::ostringstream e;
       e << "Attempt to add link library \"" << lib << "\" to target \""
         << this->Target->GetName()
-        << "\" which is not built in this directory.";
+        << "\" which is not built in this directory.\n"
+        << "This is allowed only when policy CMP0079 is set to NEW.";
       this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
       return false;
     }
@@ -404,7 +427,20 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
       this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
     }
 
-    this->Target->AddLinkLibrary(*this->Makefile, lib, llt);
+    this->Target->AddLinkLibrary(*this->Makefile, lib, libRef, llt);
+  }
+
+  if (warnRemoteInterface) {
+    std::ostringstream w;
+    /* clang-format off */
+    w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0079) << "\n"
+      "Target\n  " << this->Target->GetName() << "\nis not created in this "
+      "directory.  For compatibility with older versions of CMake, link "
+      "library\n  " << lib << "\nwill be looked up in the directory in "
+      "which the target was created rather than in this calling "
+      "directory.";
+    /* clang-format on */
+    this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, w.str());
   }
 
   // Handle (additional) case where the command was called with PRIVATE /
@@ -415,9 +451,9 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
       this->CurrentProcessingState == ProcessingPlainPrivateInterface) {
     if (this->Target->GetType() == cmStateEnums::STATIC_LIBRARY) {
       std::string configLib =
-        this->Target->GetDebugGeneratorExpressions(lib, llt);
-      if (cmGeneratorExpression::IsValidTargetName(lib) ||
-          cmGeneratorExpression::Find(lib) != std::string::npos) {
+        this->Target->GetDebugGeneratorExpressions(libRef, llt);
+      if (cmGeneratorExpression::IsValidTargetName(libRef) ||
+          cmGeneratorExpression::Find(libRef) != std::string::npos) {
         configLib = "$<LINK_ONLY:" + configLib + ">";
       }
       this->Target->AppendProperty("INTERFACE_LINK_LIBRARIES",
@@ -431,7 +467,7 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
   // property of the target on the LHS shall be populated.)
   this->Target->AppendProperty(
     "INTERFACE_LINK_LIBRARIES",
-    this->Target->GetDebugGeneratorExpressions(lib, llt).c_str());
+    this->Target->GetDebugGeneratorExpressions(libRef, llt).c_str());
 
   // Stop processing if called without any keyword.
   if (this->CurrentProcessingState == ProcessingLinkLibraries) {
@@ -464,12 +500,12 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
       for (std::string const& dc : debugConfigs) {
         prop = "LINK_INTERFACE_LIBRARIES_";
         prop += dc;
-        this->Target->AppendProperty(prop, lib.c_str());
+        this->Target->AppendProperty(prop, libRef.c_str());
       }
     }
     if (llt == OPTIMIZED_LibraryType || llt == GENERAL_LibraryType) {
       // Put in the non-DEBUG configuration interfaces.
-      this->Target->AppendProperty("LINK_INTERFACE_LIBRARIES", lib.c_str());
+      this->Target->AppendProperty("LINK_INTERFACE_LIBRARIES", libRef.c_str());
 
       // Make sure the DEBUG configuration interfaces exist so that the
       // general one will not be used as a fall-back.