Merge topic 'forbid-reserved-targets'

3900fcf CMP0037: Extend policy to reserved names and custom targets

Help/policy/CMP0037.rst

@@ -1,19 +1,24 @@
 CMP0037
 -------
 
-Target names should match a validity pattern.
+Target names should not be reserved and should match a validity pattern.
 
-CMake 2.8.12 and lower allowed creating targets using :command:`add_library` and
-:command:`add_executable` with unrestricted choice for the target name.  Newer
-cmake features such as :manual:`cmake-generator-expressions(7)` and some
+CMake 2.8.12 and lower allowed creating targets using :command:`add_library`,
+:command:`add_executable` and :command:`add_custom_target` with unrestricted
+choice for the target name.  Newer cmake features such
+as :manual:`cmake-generator-expressions(7)` and some
 diagnostics expect target names to match a restricted pattern.
 
 Target names may contain upper and lower case letters, numbers, the underscore
 character (_), dot(.), plus(+) and minus(-).  As a special case, ALIAS
 targets and INTERFACE library targets may contain two consequtive colons.
 
-The OLD behavior for this policy is to allow creating targets which do not match
-the validity pattern.  The NEW behavior for this policy is to report an error
+Target names reserved by one or more CMake generators are not allowed.
+Among others these include "all", "help" and "test".
+
+The OLD behavior for this policy is to allow creating targets with
+reserved names or which do not match the validity pattern.
+The NEW behavior for this policy is to report an error
 if an add_* command is used with an invalid target name.
 
 This policy was introduced in CMake version 3.0.0.  CMake version

Source/cmAddCustomTargetCommand.cxx

@@ -11,6 +11,9 @@
 ============================================================================*/
 #include "cmAddCustomTargetCommand.h"
 
+#include "cmGeneratorExpression.h"
+#include "cmGlobalGenerator.h"
+
 // cmAddCustomTargetCommand
 bool cmAddCustomTargetCommand
 ::InitialPass(std::vector<std::string> const& args,
@@ -22,11 +25,13 @@ bool cmAddCustomTargetCommand
     return false;
     }
 
+  std::string targetName = args[0];
+
   // Check the target name.
-  if(args[0].find_first_of("/\\") != args[0].npos)
+  if(targetName.find_first_of("/\\") != targetName.npos)
     {
     cmOStringStream e;
-    e << "called with invalid target name \"" << args[0]
+    e << "called with invalid target name \"" << targetName
       << "\".  Target names may not contain a slash.  "
       << "Use ADD_CUSTOM_COMMAND to generate files.";
     this->SetError(e.str().c_str());
@@ -138,16 +143,59 @@ bool cmAddCustomTargetCommand
       }
     }
 
-  std::string::size_type pos = args[0].find_first_of("#<>");
-  if(pos != args[0].npos)
+  std::string::size_type pos = targetName.find_first_of("#<>");
+  if(pos != targetName.npos)
     {
     cmOStringStream msg;
-    msg << "called with target name containing a \"" << args[0][pos]
+    msg << "called with target name containing a \"" << targetName[pos]
         << "\".  This character is not allowed.";
     this->SetError(msg.str().c_str());
     return false;
     }
 
+  // Some requirements on custom target names already exist
+  // and have been checked at this point.
+  // The following restrictions overlap but depend on policy CMP0037.
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(targetName) &&
+    !cmGlobalGenerator::IsReservedTarget(targetName);
+  if (nameOk)
+    {
+    nameOk = targetName.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 \"" << targetName <<
+          "\" is reserved or 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;
+        }
+      }
+    }
+
   // Store the last command line finished.
   if(!currentLine.empty())
     {
@@ -158,7 +206,7 @@ bool cmAddCustomTargetCommand
   // Enforce name uniqueness.
   {
   std::string msg;
-  if(!this->Makefile->EnforceUniqueName(args[0], msg, true))
+  if(!this->Makefile->EnforceUniqueName(targetName, msg, true))
     {
     this->SetError(msg.c_str());
     return false;
@@ -176,7 +224,7 @@ bool cmAddCustomTargetCommand
   // Add the utility target to the makefile.
   bool escapeOldStyle = !verbatim;
   cmTarget* target =
-    this->Makefile->AddUtilityCommand(args[0].c_str(), excludeFromAll,
+    this->Makefile->AddUtilityCommand(targetName.c_str(), excludeFromAll,
                                       working_directory.c_str(), depends,
                                       commandLines, escapeOldStyle, comment);
 

Source/cmAddExecutableCommand.cxx

@@ -69,7 +69,9 @@ bool cmAddExecutableCommand
       }
     }
 
-  bool nameOk = cmGeneratorExpression::IsValidTargetName(exename);
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(exename) &&
+    !cmGlobalGenerator::IsReservedTarget(exename);
+
   if (nameOk && !importTarget && !isAlias)
     {
     nameOk = exename.find(":") == std::string::npos;
@@ -95,7 +97,8 @@ bool cmAddExecutableCommand
       cmOStringStream e;
       e << (this->Makefile->GetPolicies()
             ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
-      e << "The target name \"" << exename << "\" is not valid for certain "
+      e << "The target name \"" << exename <<
+          "\" is reserved or not valid for certain "
           "CMake features, such as generator expressions, and may result "
           "in undefined behavior.";
       this->Makefile->IssueMessage(messageType, e.str().c_str());

Source/cmAddLibraryCommand.cxx

@@ -109,7 +109,9 @@ bool cmAddLibraryCommand
       }
     }
 
-  bool nameOk = cmGeneratorExpression::IsValidTargetName(libName);
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(libName) &&
+    !cmGlobalGenerator::IsReservedTarget(libName);
+
   if (nameOk && !importTarget && !isAlias)
     {
     nameOk = libName.find(":") == std::string::npos;
@@ -135,7 +137,8 @@ bool cmAddLibraryCommand
       cmOStringStream e;
       e << (this->Makefile->GetPolicies()
             ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
-      e << "The target name \"" << libName << "\" is not valid for certain "
+      e << "The target name \"" << libName <<
+          "\" is reserved or not valid for certain "
           "CMake features, such as generator expressions, and may result "
           "in undefined behavior.";
       this->Makefile->IssueMessage(messageType, e.str().c_str());

Source/cmGlobalGenerator.cxx

@@ -2466,6 +2466,37 @@ void cmGlobalGenerator::AddTarget(cmTarget* t)
     }
 }
 
+bool cmGlobalGenerator::IsReservedTarget(std::string const& name)
+{
+  // The following is a list of targets reserved
+  // by one or more of the cmake generators.
+
+  // Adding additional targets to this list will require a policy!
+  const char* reservedTargets[] =
+  {
+    "all", "ALL_BUILD",
+    "help",
+    "install", "INSTALL",
+    "preinstall",
+    "clean",
+    "edit_cache",
+    "rebuild_cache",
+    "test", "RUN_TESTS",
+    "package", "PACKAGE",
+    "package_source",
+    "ZERO_CHECK",
+    0
+  };
+
+  for(const char** reservedTarget = reservedTargets;
+    *reservedTarget; ++reservedTarget)
+    {
+    if(name == *reservedTarget) return true;
+    }
+
+  return false;
+}
+
 void cmGlobalGenerator::SetExternalMakefileProjectGenerator(
                             cmExternalMakefileProjectGenerator *extraGenerator)
 {

Source/cmGlobalGenerator.h

@@ -239,6 +239,8 @@ public:
 
   void AddTarget(cmTarget* t);
 
+  static bool IsReservedTarget(std::string const& name);
+
   virtual const char* GetAllTargetName()         const { return "ALL_BUILD"; }
   virtual const char* GetInstallTargetName()       const { return "INSTALL"; }
   virtual const char* GetInstallLocalTargetName()  const { return 0; }

Source/cmPolicies.cxx

@@ -289,7 +289,7 @@ cmPolicies::cmPolicies()
 
   this->DefinePolicy(
     CMP0037, "CMP0037",
-    "Target names should match a validity pattern.",
+    "Target names should not be reserved and should match a validity pattern.",
     3,0,0,0, cmPolicies::WARN);
 
   this->DefinePolicy(

Source/cmPolicies.h

@@ -88,7 +88,8 @@ public:
     CMP0034, ///< Disallow command: utility_source
     CMP0035, ///< Disallow command: variable_requires
     CMP0036, ///< Disallow command: build_name
-    CMP0037, ///< Target names should match a validity pattern.
+    CMP0037, ///< Target names should not be reserved and
+    /// should match a validity pattern.
     CMP0038, ///< Targets may not link directly to themselves
     CMP0039, ///< Utility targets may not have link dependencies
 

Tests/RunCMake/CMP0037/CMP0037-NEW-colon-stderr.txt

@@ -1,19 +1,35 @@
 CMake Error at CMP0037-NEW-colon.cmake:4 \(add_library\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "lib:colon" is not valid for certain CMake features, such
-  as generator expressions, and may result in undefined behavior.
+  The target name "lib:colon" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 +
 CMake Error at CMP0037-NEW-colon.cmake:5 \(add_executable\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "exe:colon" is not valid for certain CMake features, such
-  as generator expressions, and may result in undefined behavior.
+  The target name "exe:colon" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
++
+CMake Error at CMP0037-NEW-colon.cmake:6 \(add_custom_target\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "custom:colon" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)

Tests/RunCMake/CMP0037/CMP0037-NEW-colon.cmake

@@ -3,3 +3,4 @@ cmake_policy(SET CMP0037 NEW)
 
 add_library("lib:colon" empty.cpp)
 add_executable("exe:colon" empty.cpp)
+add_custom_target("custom:colon")

Tests/RunCMake/CMP0037/CMP0037-NEW-reserved-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/CMP0037/CMP0037-NEW-reserved-stderr.txt

@@ -0,0 +1,33 @@
+CMake Error at CMP0037-NEW-reserved.cmake:4 \(add_library\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "all" is reserved or not valid for certain CMake features,
+  such as generator expressions, and may result in undefined behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
++
+CMake Error at CMP0037-NEW-reserved.cmake:5 \(add_executable\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "clean" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
++
+CMake Error at CMP0037-NEW-reserved.cmake:6 \(add_custom_target\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "help" is reserved or not valid for certain CMake features,
+  such as generator expressions, and may result in undefined behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/CMP0037/CMP0037-NEW-reserved.cmake

@@ -0,0 +1,6 @@
+
+cmake_policy(SET CMP0037 NEW)
+
+add_library(all empty.cpp)
+add_executable(clean empty.cpp)
+add_custom_target(help)

Tests/RunCMake/CMP0037/CMP0037-NEW-space-stderr.txt

@@ -1,19 +1,35 @@
 CMake Error at CMP0037-NEW-space.cmake:4 \(add_library\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "lib with spaces" is not valid for certain CMake features,
-  such as generator expressions, and may result in undefined behavior.
+  The target name "lib with spaces" is reserved or not valid for certain
+  CMake features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 +
 CMake Error at CMP0037-NEW-space.cmake:5 \(add_executable\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "exe with spaces" is not valid for certain CMake features,
-  such as generator expressions, and may result in undefined behavior.
+  The target name "exe with spaces" is reserved or not valid for certain
+  CMake features, such as generator expressions, and may result in undefined
+  behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
++
+CMake Error at CMP0037-NEW-space.cmake:6 \(add_custom_target\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "custom with spaces" is reserved or not valid for certain
+  CMake features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)

Tests/RunCMake/CMP0037/CMP0037-NEW-space.cmake

@@ -3,3 +3,4 @@ cmake_policy(SET CMP0037 NEW)
 
 add_library("lib with spaces" empty.cpp)
 add_executable("exe with spaces" empty.cpp)
+add_custom_target("custom with spaces")

Tests/RunCMake/CMP0037/CMP0037-OLD-reserved-result.txt

@@ -0,0 +1 @@
+0

Tests/RunCMake/CMP0037/CMP0037-OLD-reserved-stderr.txt

@@ -0,0 +1 @@
+^$

Tests/RunCMake/CMP0037/CMP0037-OLD-reserved.cmake

@@ -0,0 +1,6 @@
+
+cmake_policy(SET CMP0037 OLD)
+
+add_library(all empty.cpp)
+add_executable(clean empty.cpp)
+add_custom_target(help)

Tests/RunCMake/CMP0037/CMP0037-OLD-space.cmake

@@ -3,3 +3,4 @@ cmake_policy(SET CMP0037 OLD)
 
 add_library("lib with spaces" empty.cpp)
 add_executable("exe with spaces" empty.cpp)
+add_custom_target("custom with spaces")

Tests/RunCMake/CMP0037/CMP0037-WARN-colon-stderr.txt

@@ -1,21 +1,38 @@
 CMake Warning \(dev\) at CMP0037-WARN-colon.cmake:2 \(add_library\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "lib:colon" is not valid for certain CMake features, such
-  as generator expressions, and may result in undefined behavior.
+  The target name "lib:colon" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.
 +
 CMake Warning \(dev\) at CMP0037-WARN-colon.cmake:3 \(add_executable\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "exe:colon" is not valid for certain CMake features, such
-  as generator expressions, and may result in undefined behavior.
+  The target name "exe:colon" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
++
+CMake Warning \(dev\) at CMP0037-WARN-colon.cmake:4 \(add_custom_target\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "custom:colon" is reserved or not valid for certain CMake
+  features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.

Tests/RunCMake/CMP0037/CMP0037-WARN-colon.cmake

@@ -1,3 +1,4 @@
 
 add_library("lib:colon" empty.cpp)
 add_executable("exe:colon" empty.cpp)
+add_custom_target("custom:colon")

Tests/RunCMake/CMP0037/CMP0037-WARN-space-stderr.txt

@@ -1,21 +1,37 @@
 CMake Warning \(dev\) at CMP0037-WARN-space.cmake:2 \(add_library\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "lib with spaces" is not valid for certain CMake features,
-  such as generator expressions, and may result in undefined behavior.
+  The target name "lib with spaces" is reserved or not valid for certain
+  CMake features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.
 +
 CMake Warning \(dev\) at CMP0037-WARN-space.cmake:3 \(add_executable\):
-  Policy CMP0037 is not set: Target names should match a validity pattern.
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
-  command to set the policy and suppress this warning.
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
 
-  The target name "exe with spaces" is not valid for certain CMake features,
-  such as generator expressions, and may result in undefined behavior.
+  The target name "exe with spaces" is reserved or not valid for certain
+  CMake features, such as generator expressions, and may result in undefined
+  behavior.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.
++
+CMake Warning \(dev\) at CMP0037-WARN-space.cmake:4 \(add_custom_target\):
+  Policy CMP0037 is not set: Target names should not be reserved and should
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  The target name "custom with spaces" is reserved or not valid for certain
+  CMake features, such as generator expressions, and may result in undefined
+  behavior.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/CMP0037/CMP0037-WARN-space.cmake

@@ -1,3 +1,4 @@
 
 add_library("lib with spaces" empty.cpp)
 add_executable("exe with spaces" empty.cpp)
+add_custom_target("custom with spaces")

Tests/RunCMake/CMP0037/RunCMakeTest.cmake

@@ -8,3 +8,6 @@ run_cmake(CMP0037-NEW-colon)
 if(NOT (WIN32 AND "${RunCMake_GENERATOR}" MATCHES "Make"))
   run_cmake(CMP0037-WARN-colon)
 endif()
+
+run_cmake(CMP0037-OLD-reserved)
+run_cmake(CMP0037-NEW-reserved)