Merge topic 'tll-target-policies'

596b2a8 Disallow linking to utility targets (#13902).
301bb5c Disallow link-to-self (#13947).
05f5fde Disallow invalid target names (#13140)

6 files changed, 164 insertions, 7 deletions

diff --git a/Source/cmAddExecutableCommand.cxx b/Source/cmAddExecutableCommand.cxx
index 5785259..a93e834 100644
--- a/Source/cmAddExecutableCommand.cxx
+++ b/Source/cmAddExecutableCommand.cxx
@@ -69,6 +69,44 @@ bool cmAddExecutableCommand
       }
     }
 
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(exename);
+  if (nameOk && !importTarget && !isAlias)
+    {
+    nameOk = exename.find(":") == std::string::npos;
+    }
+  if (!nameOk)
+    {
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    bool issueMessage = false;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0037))
+      {
+      case cmPolicies::WARN:
+        issueMessage = true;
+      case cmPolicies::OLD:
+        break;
+      case cmPolicies::NEW:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        issueMessage = true;
+        messageType = cmake::FATAL_ERROR;
+      }
+    if (issueMessage)
+      {
+      cmOStringStream e;
+      e << (this->Makefile->GetPolicies()
+            ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
+      e << "The target name \"" << exename << "\" is not valid for certain "
+          "CMake features, such as generator expressions, and may result "
+          "in undefined behavior.";
+      this->Makefile->IssueMessage(messageType, e.str().c_str());
+
+      if (messageType == cmake::FATAL_ERROR)
+        {
+        return false;
+        }
+      }
+    }
+
   // Special modifiers are not allowed with IMPORTED signature.
   if(importTarget
       && (use_win32 || use_macbundle || excludeFromAll))
diff --git a/Source/cmAddLibraryCommand.cxx b/Source/cmAddLibraryCommand.cxx
index 0e61c99..4c591b6 100644
--- a/Source/cmAddLibraryCommand.cxx
+++ b/Source/cmAddLibraryCommand.cxx
@@ -108,6 +108,45 @@ bool cmAddLibraryCommand
       break;
       }
     }
+
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(libName);
+  if (nameOk && !importTarget && !isAlias)
+    {
+    nameOk = libName.find(":") == std::string::npos;
+    }
+  if (!nameOk)
+    {
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    bool issueMessage = false;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0037))
+      {
+      case cmPolicies::WARN:
+        issueMessage = type != cmTarget::INTERFACE_LIBRARY;
+      case cmPolicies::OLD:
+        break;
+      case cmPolicies::NEW:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        issueMessage = true;
+        messageType = cmake::FATAL_ERROR;
+      }
+    if (issueMessage)
+      {
+      cmOStringStream e;
+      e << (this->Makefile->GetPolicies()
+            ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
+      e << "The target name \"" << libName << "\" is not valid for certain "
+          "CMake features, such as generator expressions, and may result "
+          "in undefined behavior.";
+      this->Makefile->IssueMessage(messageType, e.str().c_str());
+
+      if (messageType == cmake::FATAL_ERROR)
+        {
+        return false;
+        }
+      }
+    }
+
   if (isAlias)
     {
     if(!cmGeneratorExpression::IsValidTargetName(libName.c_str()))
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index ab822d3..3881c54 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -286,6 +286,21 @@ cmPolicies::cmPolicies()
     CMP0036, "CMP0036",
     "The build_name command should not be called.",
     3,0,0,0, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0037, "CMP0037",
+    "Target names should match a validity pattern.",
+    3,0,0,0, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0038, "CMP0038",
+    "Targets may not link directly to themselves.",
+    3,0,0,0, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0039, "CMP0039",
+    "Utility targets may not have link dependencies",
+    3,0,0,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 9e72bdc..fc239d4 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -88,6 +88,9 @@ public:
     CMP0034, ///< Disallow command: utility_source
     CMP0035, ///< Disallow command: variable_requires
     CMP0036, ///< Disallow command: build_name
+    CMP0037, ///< Target names should match a validity pattern.
+    CMP0038, ///< Targets may not link directly to themselves
+    CMP0039, ///< Utility targets may not have link dependencies
 
     /** \brief Always the last entry.
      *
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index d0390f7..d5e8420 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -932,12 +932,6 @@ void cmTarget::AddLinkLibrary(cmMakefile& mf,
                               const char *target, const char* lib,
                               LinkLibraryType llt)
 {
-  // Never add a self dependency, even if the user asks for it.
-  if(strcmp( target, lib ) == 0)
-    {
-    return;
-    }
-
   cmTarget *tgt = this->Makefile->FindTargetToUse(lib);
   {
   const bool isNonImportedTarget = tgt && !tgt->IsImported();
@@ -951,7 +945,8 @@ void cmTarget::AddLinkLibrary(cmMakefile& mf,
   }
 
   if (cmGeneratorExpression::Find(lib) != std::string::npos
-      || (tgt && tgt->GetType() == INTERFACE_LIBRARY))
+      || (tgt && tgt->GetType() == INTERFACE_LIBRARY)
+      || (strcmp( target, lib ) == 0))
     {
     return;
     }
@@ -5293,6 +5288,41 @@ void cmTarget::ComputeLinkImplementation(const char* config,
     std::string item = this->CheckCMP0004(*li);
     if(item == this->GetName() || item.empty())
       {
+      if(item == this->GetName())
+        {
+        bool noMessage = false;
+        cmake::MessageType messageType = cmake::FATAL_ERROR;
+        cmOStringStream e;
+        switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0038))
+          {
+          case cmPolicies::WARN:
+            {
+            e << (this->Makefile->GetPolicies()
+                  ->GetPolicyWarning(cmPolicies::CMP0038)) << "\n";
+            messageType = cmake::AUTHOR_WARNING;
+            }
+            break;
+          case cmPolicies::OLD:
+            noMessage = true;
+          case cmPolicies::REQUIRED_IF_USED:
+          case cmPolicies::REQUIRED_ALWAYS:
+          case cmPolicies::NEW:
+            // Issue the fatal message.
+            break;
+          }
+
+        if(!noMessage)
+          {
+          e << "Target \"" << this->GetName() << "\" links to itself.";
+          this->Makefile->GetCMakeInstance()->IssueMessage(messageType,
+                                                        e.str(),
+                                                        this->GetBacktrace());
+          if (messageType == cmake::FATAL_ERROR)
+            {
+            return;
+            }
+          }
+        }
       continue;
       }
     cmTarget *tgt = this->Makefile->FindTargetToUse(li->c_str());
@@ -5335,6 +5365,7 @@ void cmTarget::ComputeLinkImplementation(const char* config,
           }
         }
       }
+
     // The entry is meant for this configuration.
     impl.Libraries.push_back(item);
     }
diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx
index c289459..209609d 100644
--- a/Source/cmTargetLinkLibrariesCommand.cxx
+++ b/Source/cmTargetLinkLibrariesCommand.cxx
@@ -101,6 +101,37 @@ bool cmTargetLinkLibrariesCommand
     return true;
     }
 
+  if (this->Target->GetType() == cmTarget::UTILITY)
+    {
+    const char *modal = 0;
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0039))
+      {
+      case cmPolicies::WARN:
+        modal = "should";
+      case cmPolicies::OLD:
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        modal = "must";
+        messageType = cmake::FATAL_ERROR;
+      }
+    if (modal)
+      {
+      cmOStringStream e;
+      e << this->Makefile->GetPolicies()
+                            ->GetPolicyWarning(cmPolicies::CMP0039) << "\n"
+        "Utility target \"" << this->Target->GetName() << "\" " << modal
+        << " not be used as the target of a target_link_libraries call.";
+      this->Makefile->IssueMessage(messageType, e.str().c_str());
+      if(messageType == cmake::FATAL_ERROR)
+        {
+        return false;
+        }
+      }
+    }
+
   // but we might not have any libs after variable expansion
   if(args.size() < 2)
     {