Merge topic 'out-of-dir-link-list'

f0e67da061 target_link_libraries: Fix out-of-dir linking of a list of targets
acee629103 cmTargetLinkLibrariesCommand: Move HandleLibrary to helper struct
ba675f1ecc Tests: Enable CMP0022 in ExportImport out-of-dir linking case

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !4226

1 files changed, 118 insertions, 82 deletions

diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx
index 1d9ecc2..df751da 100644
--- a/Source/cmTargetLinkLibrariesCommand.cxx
+++ b/Source/cmTargetLinkLibrariesCommand.cxx
@@ -5,6 +5,8 @@
 #include <cstring>
 #include <memory>
 #include <sstream>
+#include <unordered_set>
+#include <utility>
 
 #include "cmExecutionStatus.h"
 #include "cmGeneratorExpression.h"
@@ -35,15 +37,30 @@ enum ProcessingState
 
 const char* LinkLibraryTypeNames[3] = { "general", "debug", "optimized" };
 
+struct TLL
+{
+  cmMakefile& Makefile;
+  cmTarget* Target;
+  bool WarnRemoteInterface = false;
+  bool RejectRemoteLinking = false;
+  bool EncodeRemoteReference = false;
+  std::string DirectoryId;
+  std::unordered_set<std::string> Props;
+
+  TLL(cmMakefile& mf, cmTarget* target);
+  ~TLL();
+
+  bool HandleLibrary(ProcessingState currentProcessingState,
+                     const std::string& lib, cmTargetLinkLibraryType llt);
+  void AppendProperty(std::string const& prop, std::string const& value);
+  void AffectsProperty(std::string const& prop);
+};
+
 } // namespace
 
 static void LinkLibraryTypeSpecifierWarning(cmMakefile& mf, int left,
                                             int right);
 
-static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
-                          ProcessingState currentProcessingState,
-                          const std::string& lib, cmTargetLinkLibraryType llt);
-
 bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
                                   cmExecutionStatus& status)
 {
@@ -148,6 +165,8 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
     return true;
   }
 
+  TLL tll(mf, target);
+
   // Keep track of link configuration specifiers.
   cmTargetLinkLibraryType llt = GENERAL_LibraryType;
   bool haveLLT = false;
@@ -246,7 +265,7 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
     } else if (haveLLT) {
       // The link type was specified by the previous argument.
       haveLLT = false;
-      if (!HandleLibrary(mf, target, currentProcessingState, args[i], llt)) {
+      if (!tll.HandleLibrary(currentProcessingState, args[i], llt)) {
         return false;
       }
     } else {
@@ -267,7 +286,7 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
           llt = OPTIMIZED_LibraryType;
         }
       }
-      if (!HandleLibrary(mf, target, currentProcessingState, args[i], llt)) {
+      if (!tll.HandleLibrary(currentProcessingState, args[i], llt)) {
         return false;
       }
     }
@@ -310,21 +329,48 @@ static void LinkLibraryTypeSpecifierWarning(cmMakefile& mf, int left,
       "\" instead of a library name.  The first specifier will be ignored."));
 }
 
-static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
-                          ProcessingState currentProcessingState,
-                          const std::string& lib, cmTargetLinkLibraryType llt)
+namespace {
+
+TLL::TLL(cmMakefile& mf, cmTarget* target)
+  : Makefile(mf)
+  , Target(target)
+{
+  if (&this->Makefile != this->Target->GetMakefile()) {
+    // The LHS target was created in another directory.
+    switch (this->Makefile.GetPolicyStatus(cmPolicies::CMP0079)) {
+      case cmPolicies::WARN:
+        this->WarnRemoteInterface = true;
+        CM_FALLTHROUGH;
+      case cmPolicies::OLD:
+        this->RejectRemoteLinking = true;
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        this->EncodeRemoteReference = true;
+        break;
+    }
+  }
+  if (this->EncodeRemoteReference) {
+    cmDirectoryId const dirId = this->Makefile.GetDirectoryId();
+    this->DirectoryId = cmStrCat(CMAKE_DIRECTORY_ID_SEP, dirId.String);
+  }
+}
+
+bool TLL::HandleLibrary(ProcessingState currentProcessingState,
+                        const std::string& lib, cmTargetLinkLibraryType llt)
 {
-  if (target->GetType() == cmStateEnums::INTERFACE_LIBRARY &&
+  if (this->Target->GetType() == cmStateEnums::INTERFACE_LIBRARY &&
       currentProcessingState != ProcessingKeywordLinkInterface) {
-    mf.IssueMessage(
+    this->Makefile.IssueMessage(
       MessageType::FATAL_ERROR,
       "INTERFACE library can only be used with the INTERFACE keyword of "
       "target_link_libraries");
     return false;
   }
-  if (target->IsImported() &&
+  if (this->Target->IsImported() &&
       currentProcessingState != ProcessingKeywordLinkInterface) {
-    mf.IssueMessage(
+    this->Makefile.IssueMessage(
       MessageType::FATAL_ERROR,
       "IMPORTED library can only be used with the INTERFACE keyword of "
       "target_link_libraries");
@@ -339,11 +385,12 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
      currentProcessingState == ProcessingKeywordLinkInterface)
     ? cmTarget::KeywordTLLSignature
     : cmTarget::PlainTLLSignature;
-  if (!target->PushTLLCommandTrace(sig, mf.GetExecutionContext())) {
+  if (!this->Target->PushTLLCommandTrace(
+        sig, this->Makefile.GetExecutionContext())) {
     std::ostringstream e;
     const char* modal = nullptr;
     MessageType messageType = MessageType::AUTHOR_WARNING;
-    switch (mf.GetPolicyStatus(cmPolicies::CMP0023)) {
+    switch (this->Makefile.GetPolicyStatus(cmPolicies::CMP0023)) {
       case cmPolicies::WARN:
         e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0023) << "\n";
         modal = "should";
@@ -364,77 +411,38 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
       e << "The " << existingSig
         << " signature for target_link_libraries has "
            "already been used with the target \""
-        << target->GetName()
+        << this->Target->GetName()
         << "\".  All uses of target_link_libraries with a target " << modal
         << " be either all-keyword or all-plain.\n";
-      target->GetTllSignatureTraces(e,
-                                    sig == cmTarget::KeywordTLLSignature
-                                      ? cmTarget::PlainTLLSignature
-                                      : cmTarget::KeywordTLLSignature);
-      mf.IssueMessage(messageType, e.str());
+      this->Target->GetTllSignatureTraces(e,
+                                          sig == cmTarget::KeywordTLLSignature
+                                            ? cmTarget::PlainTLLSignature
+                                            : cmTarget::KeywordTLLSignature);
+      this->Makefile.IssueMessage(messageType, e.str());
       if (messageType == MessageType::FATAL_ERROR) {
         return false;
       }
     }
   }
 
-  bool warnRemoteInterface = false;
-  bool rejectRemoteLinking = false;
-  bool encodeRemoteReference = false;
-  if (&mf != target->GetMakefile()) {
-    // The LHS target was created in another directory.
-    switch (mf.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 = mf.GetDirectoryId();
-    // FIXME: The "lib" may be a genex with a list inside it.
-    // After expansion this id will only attach to the last entry,
-    // or may attach to an empty string!  We will need another way
-    // to encode this that can apply to a whole list.  See issue #20204.
-    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 (currentProcessingState != ProcessingKeywordLinkInterface &&
       currentProcessingState != ProcessingPlainLinkInterface) {
 
-    if (rejectRemoteLinking) {
-      mf.IssueMessage(
+    if (this->RejectRemoteLinking) {
+      this->Makefile.IssueMessage(
         MessageType::FATAL_ERROR,
         cmStrCat("Attempt to add link library \"", lib, "\" to target \"",
-                 target->GetName(),
+                 this->Target->GetName(),
                  "\" which is not built in this "
                  "directory.\nThis is allowed only when policy CMP0079 "
                  "is set to NEW."));
       return false;
     }
 
-    cmTarget* tgt = mf.GetGlobalGenerator()->FindTarget(lib);
+    cmTarget* tgt = this->Makefile.GetGlobalGenerator()->FindTarget(lib);
 
     if (tgt && (tgt->GetType() != cmStateEnums::STATIC_LIBRARY) &&
         (tgt->GetType() != cmStateEnums::SHARED_LIBRARY) &&
@@ -442,7 +450,7 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
         (tgt->GetType() != cmStateEnums::OBJECT_LIBRARY) &&
         (tgt->GetType() != cmStateEnums::INTERFACE_LIBRARY) &&
         !tgt->IsExecutableWithExports()) {
-      mf.IssueMessage(
+      this->Makefile.IssueMessage(
         MessageType::FATAL_ERROR,
         cmStrCat(
           "Target \"", lib, "\" of type ",
@@ -452,15 +460,16 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
           "executables with the ENABLE_EXPORTS property set."));
     }
 
-    target->AddLinkLibrary(mf, lib, libRef, llt);
+    this->AffectsProperty("LINK_LIBRARIES");
+    this->Target->AddLinkLibrary(this->Makefile, lib, llt);
   }
 
-  if (warnRemoteInterface) {
-    mf.IssueMessage(
+  if (this->WarnRemoteInterface) {
+    this->Makefile.IssueMessage(
       MessageType::AUTHOR_WARNING,
       cmStrCat(
         cmPolicies::GetPolicyWarning(cmPolicies::CMP0079), "\nTarget\n  ",
-        target->GetName(),
+        this->Target->GetName(),
         "\nis not created in this "
         "directory.  For compatibility with older versions of CMake, link "
         "library\n  ",
@@ -475,15 +484,15 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
   // STATIC library.)
   if (currentProcessingState == ProcessingKeywordPrivateInterface ||
       currentProcessingState == ProcessingPlainPrivateInterface) {
-    if (target->GetType() == cmStateEnums::STATIC_LIBRARY ||
-        target->GetType() == cmStateEnums::OBJECT_LIBRARY) {
+    if (this->Target->GetType() == cmStateEnums::STATIC_LIBRARY ||
+        this->Target->GetType() == cmStateEnums::OBJECT_LIBRARY) {
       std::string configLib =
-        target->GetDebugGeneratorExpressions(libRef, llt);
+        this->Target->GetDebugGeneratorExpressions(lib, llt);
       if (cmGeneratorExpression::IsValidTargetName(lib) ||
           cmGeneratorExpression::Find(lib) != std::string::npos) {
         configLib = "$<LINK_ONLY:" + configLib + ">";
       }
-      target->AppendProperty("INTERFACE_LINK_LIBRARIES", configLib);
+      this->AppendProperty("INTERFACE_LINK_LIBRARIES", configLib);
     }
     return true;
   }
@@ -491,8 +500,8 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
   // Handle general case where the command was called with another keyword than
   // PRIVATE / LINK_PRIVATE or none at all. (The "INTERFACE_LINK_LIBRARIES"
   // property of the target on the LHS shall be populated.)
-  target->AppendProperty("INTERFACE_LINK_LIBRARIES",
-                         target->GetDebugGeneratorExpressions(libRef, llt));
+  this->AppendProperty("INTERFACE_LINK_LIBRARIES",
+                       this->Target->GetDebugGeneratorExpressions(lib, llt));
 
   // Stop processing if called without any keyword.
   if (currentProcessingState == ProcessingLinkLibraries) {
@@ -500,13 +509,13 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
   }
   // Stop processing if policy CMP0022 is set to NEW.
   const cmPolicies::PolicyStatus policy22Status =
-    target->GetPolicyStatusCMP0022();
+    this->Target->GetPolicyStatusCMP0022();
   if (policy22Status != cmPolicies::OLD &&
       policy22Status != cmPolicies::WARN) {
     return true;
   }
   // Stop processing if called with an INTERFACE library on the LHS.
-  if (target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
+  if (this->Target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
     return true;
   }
 
@@ -516,7 +525,7 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
   {
     // Get the list of configurations considered to be DEBUG.
     std::vector<std::string> debugConfigs =
-      mf.GetCMakeInstance()->GetDebugConfigs();
+      this->Makefile.GetCMakeInstance()->GetDebugConfigs();
     std::string prop;
 
     // Include this library in the link interface for the target.
@@ -524,22 +533,49 @@ static bool HandleLibrary(cmMakefile& mf, cmTarget* target,
       // Put in the DEBUG configuration interfaces.
       for (std::string const& dc : debugConfigs) {
         prop = cmStrCat("LINK_INTERFACE_LIBRARIES_", dc);
-        target->AppendProperty(prop, libRef);
+        this->AppendProperty(prop, lib);
       }
     }
     if (llt == OPTIMIZED_LibraryType || llt == GENERAL_LibraryType) {
       // Put in the non-DEBUG configuration interfaces.
-      target->AppendProperty("LINK_INTERFACE_LIBRARIES", libRef);
+      this->AppendProperty("LINK_INTERFACE_LIBRARIES", lib);
 
       // Make sure the DEBUG configuration interfaces exist so that the
       // general one will not be used as a fall-back.
       for (std::string const& dc : debugConfigs) {
         prop = cmStrCat("LINK_INTERFACE_LIBRARIES_", dc);
-        if (!target->GetProperty(prop)) {
-          target->SetProperty(prop, "");
+        if (!this->Target->GetProperty(prop)) {
+          this->Target->SetProperty(prop, "");
         }
       }
     }
   }
   return true;
 }
+
+void TLL::AppendProperty(std::string const& prop, std::string const& value)
+{
+  this->AffectsProperty(prop);
+  this->Target->AppendProperty(prop, value);
+}
+
+void TLL::AffectsProperty(std::string const& prop)
+{
+  if (!this->EncodeRemoteReference) {
+    return;
+  }
+  // Add a wrapper to the expression to tell LookupLinkItems to look up
+  // names in the caller's directory.
+  if (this->Props.insert(prop).second) {
+    this->Target->AppendProperty(prop, this->DirectoryId);
+  }
+}
+
+TLL::~TLL()
+{
+  for (std::string const& prop : this->Props) {
+    this->Target->AppendProperty(prop, CMAKE_DIRECTORY_ID_SEP);
+  }
+}
+
+} // namespace