ENH: Enforce global target name uniqueness.

- Error if imported target is involved in conflict
  - Error for non-imported target conflict unless
    CMAKE_BACKWARDS_COMPATIBILITY <= 2.4
  - Include OUTPUT_NAME property in error message
  - Update add_executable and add_library command documentation

Source/cmAddCustomTargetCommand.cxx

@@ -158,6 +158,14 @@ bool cmAddCustomTargetCommand
     currentLine.clear();
     }
 
+  // Enforce name uniqueness.
+  std::string msg;
+  if(!this->Makefile->EnforceUniqueName(args[0], msg))
+    {
+    this->SetError(msg.c_str());
+    return false;
+    }
+
   // Add the utility target to the makefile.
   bool escapeOldStyle = !verbatim;
   this->Makefile->AddUtilityCommand(args[0].c_str(), excludeFromAll,

Source/cmAddExecutableCommand.cxx

@@ -82,12 +82,11 @@ bool cmAddExecutableCommand
     return false;
     }
 
-  // Check for an existing target with this name.
+  // Handle imported target creation.
-  cmTarget* existing = this->Makefile->FindTargetToUse(exename.c_str());
   if(importTarget)
     {
     // Make sure the target does not already exist.
-    if(existing)
+    if(this->Makefile->FindTargetToUse(exename.c_str()))
       {
       cmOStringStream e;
       e << "cannot create imported target \"" << exename
@@ -100,21 +99,14 @@ bool cmAddExecutableCommand
     this->Makefile->AddImportedTarget(exename.c_str(), cmTarget::EXECUTABLE);
     return true;
     }
-  else
+
+  // Enforce name uniqueness.
+  std::string msg;
+  if(!this->Makefile->EnforceUniqueName(exename, msg))
     {
-    // Make sure the target does not conflict with an imported target.
+    this->SetError(msg.c_str());
-    // This should really enforce global name uniqueness for targets
-    // built within the project too, but that may break compatiblity
-    // with projects in which it was accidentally working.
-    if(existing && existing->IsImported())
-      {
-      cmOStringStream e;
-      e << "cannot create target \"" << exename
-        << "\" because an imported target with the same name already exists.";
-      this->SetError(e.str().c_str());
     return false;
     }
-    }
 
   if (s == args.end())
     {

Source/cmAddExecutableCommand.h

@@ -63,33 +63,36 @@ public:
   virtual const char* GetFullDocumentation()
     {
     return
-      "  add_executable(exename [WIN32] [MACOSX_BUNDLE] [EXCLUDE_FROM_ALL]\n"
+      "  add_executable(<name> [WIN32] [MACOSX_BUNDLE] [EXCLUDE_FROM_ALL]\n"
       "                 source1 source2 ... sourceN)\n"
-      "This command adds an executable target to the current directory.  "
+      "Adds an executable target called <name> to be built from the "
-      "The executable will be built from the list of source files "
+      "source files listed in the command invocation.  "
-      "specified.\n"
+      "The <name> corresponds to the logical target name and must be "
-      "After specifying the executable name, WIN32 and/or MACOSX_BUNDLE can "
+      "globally unique within a project.  "
-      "be specified. WIN32 indicates that the executable (when compiled on "
+      "The actual file name of the executable built is constructed based on "
-      "windows) is a windows app (using WinMain) not a console app "
+      "conventions of the native platform "
-      "(using main). The variable CMAKE_MFC_FLAG be used if the windows app "
+      "(such as <name>.exe or just <name>).  "
-      "uses MFC. This variable can be set to the following values:\n"
+      "\n"
-      " 0: Use Standard Windows Libraries\n"
+      "By default the executable file will be created in the build tree "
-      " 1: Use MFC in a Static Library\n"
+      "directory corresponding to the source tree directory in which "
-      " 2: Use MFC in a Shared DLL\n"
+      "the command was invoked.  "
-      "MACOSX_BUNDLE indicates that when build on Mac OSX, executable should "
+      "See documentation of the RUNTIME_OUTPUT_DIRECTORY "
-      "be in the bundle form. The MACOSX_BUNDLE also allows several "
+      "target property to change this location.  "
-      "variables to be specified:\n"
+      "See documentation of the OUTPUT_NAME target property to change "
-      "  MACOSX_BUNDLE_INFO_STRING\n"
+      "the <name> part of the final file name.  "
-      "  MACOSX_BUNDLE_ICON_FILE\n"
+      "\n"
-      "  MACOSX_BUNDLE_GUI_IDENTIFIER\n"
+      "If WIN32 is given the property WIN32_EXECUTABLE will be set on the "
-      "  MACOSX_BUNDLE_LONG_VERSION_STRING\n"
+      "target created.  "
-      "  MACOSX_BUNDLE_BUNDLE_NAME\n"
+      "See documentation of that target property for details."
-      "  MACOSX_BUNDLE_SHORT_VERSION_STRING\n"
+      "\n"
-      "  MACOSX_BUNDLE_BUNDLE_VERSION\n"
+      "If MACOSX_BUNDLE is given the corresponding property will be "
-      "  MACOSX_BUNDLE_COPYRIGHT\n"
+      "set on the created target.  "
-      "If EXCLUDE_FROM_ALL is given the target will not be built by default. "
+      "See documentation of the MACOSX_BUNDLE target property for details."
-      "It will be built only if the user explicitly builds the target or "
+      "\n"
-      "another target that requires the target depends on it."
+      "If EXCLUDE_FROM_ALL is given the corresponding property will be "
+      "set on the created target.  "
+      "See documentation of the EXCLUDE_FROM_ALL target property for "
+      "details."
       "\n"
       "The add_executable command can also create IMPORTED executable "
       "targets using this signature:\n"

Source/cmAddLibraryCommand.cxx

@@ -109,12 +109,11 @@ bool cmAddLibraryCommand
     return false;
     }
 
-  // Check for an existing target with this name.
+  // Handle imported target creation.
-  cmTarget* existing = this->Makefile->FindTargetToUse(libName.c_str());
   if(importTarget)
     {
     // Make sure the target does not already exist.
-    if(existing)
+    if(this->Makefile->FindTargetToUse(libName.c_str()))
       {
       cmOStringStream e;
       e << "cannot create imported target \"" << libName
@@ -127,21 +126,14 @@ bool cmAddLibraryCommand
     this->Makefile->AddImportedTarget(libName.c_str(), type);
     return true;
     }
-  else
+
+  // Enforce name uniqueness.
+  std::string msg;
+  if(!this->Makefile->EnforceUniqueName(libName, msg))
     {
-    // Make sure the target does not conflict with an imported target.
+    this->SetError(msg.c_str());
-    // This should really enforce global name uniqueness for targets
-    // built within the project too, but that may break compatiblity
-    // with projects in which it was accidentally working.
-    if(existing && existing->IsImported())
-      {
-      cmOStringStream e;
-      e << "cannot create target \"" << libName
-        << "\" because an imported target with the same name already exists.";
-      this->SetError(e.str().c_str());
     return false;
     }
-    }
 
   if (s == args.end())
     {

Source/cmAddLibraryCommand.h

@@ -62,18 +62,41 @@ public:
   virtual const char* GetFullDocumentation()
     {
     return
-      "  add_library(libname [SHARED | STATIC | MODULE] [EXCLUDE_FROM_ALL]\n"
+      "  add_library(<name> [STATIC | SHARED | MODULE] [EXCLUDE_FROM_ALL]\n"
       "              source1 source2 ... sourceN)\n"
-      "Adds a library target.  SHARED, STATIC or MODULE keywords are used "
+      "Adds a library target called <name> to be built from the "
-      "to set the library type.  If the keyword MODULE appears, the library "
+      "source files listed in the command invocation.  "
-      "type is set to MH_BUNDLE on systems which use dyld.  On systems "
+      "The <name> corresponds to the logical target name and must be "
-      "without dyld, MODULE is treated like SHARED.  If no keywords appear "
+      "globally unique within a project.  "
-      " as the second argument, the type defaults to the current value of "
+      "The actual file name of the library built is constructed based on "
-      "BUILD_SHARED_LIBS.  If this variable is not set, the type defaults "
+      "conventions of the native platform "
-      "to STATIC.\n"
+      "(such as lib<name>.a or <name>.lib)."
-      "If EXCLUDE_FROM_ALL is given the target will not be built by default. "
+      "\n"
-      "It will be built only if the user explicitly builds the target or "
+      "STATIC, SHARED, or MODULE may be given to specify the type of library "
-      "another target that requires the target depends on it."
+      "to be created.  "
+      "STATIC libraries are archives of object files for use when linking "
+      "other targets.  "
+      "SHARED libraries are linked dynamically and loaded at runtime.  "
+      "MODULE libraries are plugins that are not linked into other targets "
+      "but may be loaded dynamically at runtime using dlopen-like "
+      "functionality.  "
+      "If no type is given explicitly the type is STATIC or SHARED based "
+      "on whether the current value of the variable BUILD_SHARED_LIBS is "
+      "true."
+      "\n"
+      "By default the library file will be created in the build tree "
+      "directory corresponding to the source tree directory in which "
+      "the command was invoked.  "
+      "See documentation of the ARCHIVE_OUTPUT_DIRECTORY, "
+      "LIBRARY_OUTPUT_DIRECTORY, and RUNTIME_OUTPUT_DIRECTORY "
+      "target properties to change this location.  "
+      "See documentation of the OUTPUT_NAME target property to change "
+      "the <name> part of the final file name.  "
+      "\n"
+      "If EXCLUDE_FROM_ALL is given the corresponding property will be "
+      "set on the created target.  "
+      "See documentation of the EXCLUDE_FROM_ALL target property for "
+      "details."
       "\n"
       "The add_library command can also create IMPORTED library "
       "targets using this signature:\n"

Source/cmMakefile.cxx

@@ -3138,3 +3138,42 @@ cmTarget* cmMakefile::FindTargetToUse(const char* name)
   // Look for a target built in this project.
   return this->LocalGenerator->GetGlobalGenerator()->FindTarget(0, name);
 }
+
+//----------------------------------------------------------------------------
+bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg)
+{
+  if(cmTarget* existing = this->FindTargetToUse(name.c_str()))
+    {
+    // The name given conflicts with an existing target.  Produce an
+    // error in a compatible way.
+    if(existing->IsImported())
+      {
+      // Imported targets were not supported in previous versions.
+      // This is new code, so we can make it an error.
+      cmOStringStream e;
+      e << "cannot create target \"" << name
+        << "\" because an imported target with the same name already exists.";
+      msg = e.str();
+      return false;
+      }
+    else if(!this->NeedBackwardsCompatibility(2, 4))
+      {
+      // The conflict is with a non-imported target.  Produce an error
+      // that tells the user how to work around the problem.
+      cmOStringStream e;
+      e << "cannot create target \"" << name
+        << "\" because another target with the same name already exists.  "
+        << "Logical target names must be globally unique.  "
+        << "Consider using the OUTPUT_NAME target property to create "
+        << "two targets with the same physical name while keeping logical "
+        << "names distinct.\n"
+        << "If you are building an older project it is possible that "
+        << "it violated this rule but was working accidentally.  "
+        << "Set CMAKE_BACKWARDS_COMPATIBILITY to 2.4 or lower to disable "
+        << "this error.";
+      msg = e.str();
+      return false;
+      }
+    }
+  return true;
+}

Source/cmMakefile.h

@@ -129,6 +129,11 @@ public:
                                   unsigned int minor,
                                   unsigned int patch = 0xFFu);
 
+  /**
+   * Help enforce global target name uniqueness.
+   */
+  bool EnforceUniqueName(std::string const& name, std::string& msg);
+
   /**
    * Perform FinalPass, Library dependency analysis etc before output of the
    * makefile.  

Source/cmTarget.cxx

@@ -496,10 +496,21 @@ void cmTarget::DefineProperties(cmake *cm)
 
   cm->DefineProperty
     ("WIN32_EXECUTABLE", cmProperty::TARGET,
-     "Used to specify Windows executable with a WinMain entry point.",
+     "Build an executable with a WinMain entry point on windows.",
-     "This can be set to indicate that a target is a Windows executable "
+     "When this property is set to true the executable when linked "
-     "in contrast to a console application for example. This changes "
+     "on Windows will be created with a WinMain() entry point instead "
-     "how the executable will be linked.");
+     "of of just main()."
+     "This makes it a GUI executable instead of a console application.  "
+     "See the CMAKE_MFC_FLAG variable documentation to configure use "
+     "of MFC for WinMain executables.");
+
+  cm->DefineProperty
+    ("MACOSX_BUNDLE", cmProperty::TARGET,
+     "Build an executable as an application bundle on Mac OS X.",
+     "When this property is set to true the executable when built "
+     "on Mac OS X will be created as an application bundle.  "
+     "This makes it a GUI executable that can be launched from "
+     "the Finder.");
 
   cm->DefineProperty
     ("ENABLE_EXPORTS", cmProperty::TARGET,