CMP0037: Extend policy to reserved names and custom targets

Teach add_custom_target to check the policy too.  Extend the policy to
disallow reserved target names that we use for builtin targets like
"all".

Extend the RunCMake.CMP0037 test to cover these cases.

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
+CMake 2.8.12 and lower allowed creating targets using :command:`add_library`,
-:command:`add_executable` with unrestricted choice for the target name.  Newer
+:command:`add_executable` and :command:`add_custom_target` with unrestricted
-cmake features such as :manual:`cmake-generator-expressions(7)` and some
+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
+Target names reserved by one or more CMake generators are not allowed.
-the validity pattern.  The NEW behavior for this policy is to report an error
+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("#<>");
+  std::string::size_type pos = targetName.find_first_of("#<>");
-  if(pos != args[0].npos)
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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
+  The target name "lib:colon" is reserved or not valid for certain CMake
-  as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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
+  The target name "exe:colon" is reserved or not valid for certain CMake
-  as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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,
+  The target name "lib with spaces" is reserved or not valid for certain
-  such as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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,
+  The target name "exe with spaces" is reserved or not valid for certain
-  such as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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
+  The target name "lib:colon" is reserved or not valid for certain CMake
-  as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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
+  The target name "exe:colon" is reserved or not valid for certain CMake
-  as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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,
+  The target name "lib with spaces" is reserved or not valid for certain
-  such as generator expressions, and may result in undefined behavior.
+  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.
+  Policy CMP0037 is not set: Target names should not be reserved and should
-  Run "cmake --help-policy CMP0037" for policy details.  Use the cmake_policy
+  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
-  command to set the policy and suppress this warning.
+  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,
+  The target name "exe with spaces" is reserved or not valid for certain
-  such as generator expressions, and may result in undefined behavior.
+  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)