Merge topic 'refactor-target-construction'

9353d991 cmTarget: Remove unused support for partial construction
fa3897b2 cmGlobalGenerator: Refactor global target construction
916d8445 cmGlobalGenerator: Split CreateDefaultGlobalTargets implementation

4 files changed, 154 insertions, 157 deletions

diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index e85d80e..64c9870 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -1060,16 +1060,16 @@ void cmGlobalGenerator::Configure()
   this->ConfigureDoneCMP0026AndCMP0024 = true;
 
   // Put a copy of each global target in every directory.
-  cmTargets globalTargets;
-  this->CreateDefaultGlobalTargets(&globalTargets);
+  std::vector<GlobalTargetInfo> globalTargets;
+  this->CreateDefaultGlobalTargets(globalTargets);
 
   for (unsigned int i = 0; i < this->Makefiles.size(); ++i) {
     cmMakefile* mf = this->Makefiles[i];
     cmTargets* targets = &(mf->GetTargets());
-    cmTargets::iterator tit;
-    for (tit = globalTargets.begin(); tit != globalTargets.end(); ++tit) {
+    for (std::vector<GlobalTargetInfo>::iterator gti = globalTargets.begin();
+         gti != globalTargets.end(); ++gti) {
       targets->insert(
-        cmTargets::value_type(tit->first, tit->second.CopyForDirectory(mf)));
+        cmTargets::value_type(gti->Name, this->CreateGlobalTarget(*gti, mf)));
     }
   }
 
@@ -2069,15 +2069,27 @@ inline std::string removeQuotes(const std::string& s)
   return s;
 }
 
-void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
+void cmGlobalGenerator::CreateDefaultGlobalTargets(
+  std::vector<GlobalTargetInfo>& targets)
+{
+  this->AddGlobalTarget_Package(targets);
+  this->AddGlobalTarget_PackageSource(targets);
+  this->AddGlobalTarget_Test(targets);
+  this->AddGlobalTarget_EditCache(targets);
+  this->AddGlobalTarget_RebuildCache(targets);
+  this->AddGlobalTarget_Install(targets);
+}
+
+void cmGlobalGenerator::AddGlobalTarget_Package(
+  std::vector<GlobalTargetInfo>& targets)
 {
   cmMakefile* mf = this->Makefiles[0];
   const char* cmakeCfgIntDir = this->GetCMakeCFGIntDir();
-
-  // CPack
-  std::string workingDir = mf->GetCurrentBinaryDirectory();
-  cmCustomCommandLines cpackCommandLines;
-  std::vector<std::string> depends;
+  GlobalTargetInfo gti;
+  gti.Name = this->GetPackageTargetName();
+  gti.Message = "Run CPack packaging tool...";
+  gti.UsesTerminal = true;
+  gti.WorkingDir = mf->GetCurrentBinaryDirectory();
   cmCustomCommandLine singleLine;
   singleLine.push_back(cmSystemTools::GetCPackCommand());
   if (cmakeCfgIntDir && *cmakeCfgIntDir && cmakeCfgIntDir[0] != '.') {
@@ -2086,60 +2098,61 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
   }
   singleLine.push_back("--config");
   std::string configFile = mf->GetCurrentBinaryDirectory();
-  ;
   configFile += "/CPackConfig.cmake";
   std::string relConfigFile = "./CPackConfig.cmake";
   singleLine.push_back(relConfigFile);
-  cpackCommandLines.push_back(singleLine);
+  gti.CommandLines.push_back(singleLine);
   if (this->GetPreinstallTargetName()) {
-    depends.push_back(this->GetPreinstallTargetName());
+    gti.Depends.push_back(this->GetPreinstallTargetName());
   } else {
     const char* noPackageAll =
       mf->GetDefinition("CMAKE_SKIP_PACKAGE_ALL_DEPENDENCY");
     if (!noPackageAll || cmSystemTools::IsOff(noPackageAll)) {
-      depends.push_back(this->GetAllTargetName());
+      gti.Depends.push_back(this->GetAllTargetName());
     }
   }
   if (cmSystemTools::FileExists(configFile.c_str())) {
-    targets->insert(cmTargets::value_type(
-      this->GetPackageTargetName(),
-      this->CreateGlobalTarget(this->GetPackageTargetName(),
-                               "Run CPack packaging tool...",
-                               &cpackCommandLines, depends, workingDir.c_str(),
-                               /*uses_terminal*/ true)));
-  }
-  // CPack source
+    targets.push_back(gti);
+  }
+}
+
+void cmGlobalGenerator::AddGlobalTarget_PackageSource(
+  std::vector<GlobalTargetInfo>& targets)
+{
+  cmMakefile* mf = this->Makefiles[0];
   const char* packageSourceTargetName = this->GetPackageSourceTargetName();
   if (packageSourceTargetName) {
-    cpackCommandLines.erase(cpackCommandLines.begin(),
-                            cpackCommandLines.end());
-    singleLine.erase(singleLine.begin(), singleLine.end());
-    depends.erase(depends.begin(), depends.end());
+    GlobalTargetInfo gti;
+    gti.Name = packageSourceTargetName;
+    gti.Message = "Run CPack packaging tool for source...";
+    gti.WorkingDir = mf->GetCurrentBinaryDirectory();
+    gti.UsesTerminal = true;
+    cmCustomCommandLine singleLine;
     singleLine.push_back(cmSystemTools::GetCPackCommand());
     singleLine.push_back("--config");
-    configFile = mf->GetCurrentBinaryDirectory();
-    ;
+    std::string configFile = mf->GetCurrentBinaryDirectory();
     configFile += "/CPackSourceConfig.cmake";
-    relConfigFile = "./CPackSourceConfig.cmake";
+    std::string relConfigFile = "./CPackSourceConfig.cmake";
     singleLine.push_back(relConfigFile);
     if (cmSystemTools::FileExists(configFile.c_str())) {
       singleLine.push_back(configFile);
-      cpackCommandLines.push_back(singleLine);
-      targets->insert(cmTargets::value_type(
-        packageSourceTargetName,
-        this->CreateGlobalTarget(
-          packageSourceTargetName, "Run CPack packaging tool for source...",
-          &cpackCommandLines, depends, workingDir.c_str(),
-          /*uses_terminal*/ true)));
+      gti.CommandLines.push_back(singleLine);
+      targets.push_back(gti);
     }
   }
+}
 
-  // Test
+void cmGlobalGenerator::AddGlobalTarget_Test(
+  std::vector<GlobalTargetInfo>& targets)
+{
+  cmMakefile* mf = this->Makefiles[0];
+  const char* cmakeCfgIntDir = this->GetCMakeCFGIntDir();
   if (mf->IsOn("CMAKE_TESTING_ENABLED")) {
-    cpackCommandLines.erase(cpackCommandLines.begin(),
-                            cpackCommandLines.end());
-    singleLine.erase(singleLine.begin(), singleLine.end());
-    depends.erase(depends.begin(), depends.end());
+    GlobalTargetInfo gti;
+    gti.Name = this->GetTestTargetName();
+    gti.Message = "Running tests...";
+    gti.UsesTerminal = true;
+    cmCustomCommandLine singleLine;
     singleLine.push_back(cmSystemTools::GetCTestCommand());
     singleLine.push_back("--force-new-ctest-process");
     if (cmakeCfgIntDir && *cmakeCfgIntDir && cmakeCfgIntDir[0] != '.') {
@@ -2150,21 +2163,19 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     {
       singleLine.push_back("$(ARGS)");
     }
-    cpackCommandLines.push_back(singleLine);
-    targets->insert(cmTargets::value_type(
-      this->GetTestTargetName(),
-      this->CreateGlobalTarget(this->GetTestTargetName(), "Running tests...",
-                               &cpackCommandLines, depends, CM_NULLPTR,
-                               /*uses_terminal*/ true)));
+    gti.CommandLines.push_back(singleLine);
+    targets.push_back(gti);
   }
+}
 
-  // Edit Cache
+void cmGlobalGenerator::AddGlobalTarget_EditCache(
+  std::vector<GlobalTargetInfo>& targets)
+{
   const char* editCacheTargetName = this->GetEditCacheTargetName();
   if (editCacheTargetName) {
-    cpackCommandLines.erase(cpackCommandLines.begin(),
-                            cpackCommandLines.end());
-    singleLine.erase(singleLine.begin(), singleLine.end());
-    depends.erase(depends.begin(), depends.end());
+    GlobalTargetInfo gti;
+    gti.Name = editCacheTargetName;
+    cmCustomCommandLine singleLine;
 
     // Use generator preference for the edit_cache rule if it is defined.
     std::string edit_cmd = this->GetEditCacheCommand();
@@ -2172,45 +2183,46 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
       singleLine.push_back(edit_cmd);
       singleLine.push_back("-H$(CMAKE_SOURCE_DIR)");
       singleLine.push_back("-B$(CMAKE_BINARY_DIR)");
-      cpackCommandLines.push_back(singleLine);
-      targets->insert(cmTargets::value_type(
-        editCacheTargetName,
-        this->CreateGlobalTarget(
-          editCacheTargetName, "Running CMake cache editor...",
-          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
+      gti.Message = "Running CMake cache editor...";
+      gti.UsesTerminal = true;
+      gti.CommandLines.push_back(singleLine);
     } else {
       singleLine.push_back(cmSystemTools::GetCMakeCommand());
       singleLine.push_back("-E");
       singleLine.push_back("echo");
       singleLine.push_back("No interactive CMake dialog available.");
-      cpackCommandLines.push_back(singleLine);
-      targets->insert(cmTargets::value_type(
-        editCacheTargetName,
-        this->CreateGlobalTarget(
-          editCacheTargetName, "No interactive CMake dialog available...",
-          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ false)));
+      gti.Message = "No interactive CMake dialog available...";
+      gti.UsesTerminal = false;
+      gti.CommandLines.push_back(singleLine);
     }
+
+    targets.push_back(gti);
   }
+}
 
-  // Rebuild Cache
+void cmGlobalGenerator::AddGlobalTarget_RebuildCache(
+  std::vector<GlobalTargetInfo>& targets)
+{
   const char* rebuildCacheTargetName = this->GetRebuildCacheTargetName();
   if (rebuildCacheTargetName) {
-    cpackCommandLines.erase(cpackCommandLines.begin(),
-                            cpackCommandLines.end());
-    singleLine.erase(singleLine.begin(), singleLine.end());
-    depends.erase(depends.begin(), depends.end());
+    GlobalTargetInfo gti;
+    gti.Name = rebuildCacheTargetName;
+    gti.Message = "Running CMake to regenerate build system...";
+    gti.UsesTerminal = true;
+    cmCustomCommandLine singleLine;
     singleLine.push_back(cmSystemTools::GetCMakeCommand());
     singleLine.push_back("-H$(CMAKE_SOURCE_DIR)");
     singleLine.push_back("-B$(CMAKE_BINARY_DIR)");
-    cpackCommandLines.push_back(singleLine);
-    targets->insert(cmTargets::value_type(
-      rebuildCacheTargetName,
-      this->CreateGlobalTarget(
-        rebuildCacheTargetName, "Running CMake to regenerate build system...",
-        &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
+    gti.CommandLines.push_back(singleLine);
+    targets.push_back(gti);
   }
+}
 
-  // Install
+void cmGlobalGenerator::AddGlobalTarget_Install(
+  std::vector<GlobalTargetInfo>& targets)
+{
+  cmMakefile* mf = this->Makefiles[0];
+  const char* cmakeCfgIntDir = this->GetCMakeCFGIntDir();
   bool skipInstallRules = mf->IsOn("CMAKE_SKIP_INSTALL_RULES");
   if (this->InstallTargetEnabled && skipInstallRules) {
     this->CMakeInstance->IssueMessage(
@@ -2220,9 +2232,6 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
   } else if (this->InstallTargetEnabled && !skipInstallRules) {
     if (!cmakeCfgIntDir || !*cmakeCfgIntDir || cmakeCfgIntDir[0] == '.') {
       std::set<std::string>* componentsSet = &this->InstallComponents;
-      cpackCommandLines.erase(cpackCommandLines.begin(),
-                              cpackCommandLines.end());
-      depends.erase(depends.begin(), depends.end());
       std::ostringstream ostr;
       if (!componentsSet->empty()) {
         ostr << "Available install components are: ";
@@ -2230,25 +2239,25 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
       } else {
         ostr << "Only default component available";
       }
-      singleLine.push_back(ostr.str());
-      targets->insert(cmTargets::value_type(
-        "list_install_components",
-        this->CreateGlobalTarget("list_install_components", ostr.str().c_str(),
-                                 &cpackCommandLines, depends, CM_NULLPTR,
-                                 /*uses_terminal*/ false)));
+      GlobalTargetInfo gti;
+      gti.Name = "list_install_components";
+      gti.Message = ostr.str();
+      gti.UsesTerminal = false;
+      targets.push_back(gti);
     }
     std::string cmd = cmSystemTools::GetCMakeCommand();
-    cpackCommandLines.erase(cpackCommandLines.begin(),
-                            cpackCommandLines.end());
-    singleLine.erase(singleLine.begin(), singleLine.end());
-    depends.erase(depends.begin(), depends.end());
+    GlobalTargetInfo gti;
+    gti.Name = this->GetInstallTargetName();
+    gti.Message = "Install the project...";
+    gti.UsesTerminal = true;
+    cmCustomCommandLine singleLine;
     if (this->GetPreinstallTargetName()) {
-      depends.push_back(this->GetPreinstallTargetName());
+      gti.Depends.push_back(this->GetPreinstallTargetName());
     } else {
       const char* noall =
         mf->GetDefinition("CMAKE_SKIP_INSTALL_ALL_DEPENDENCY");
       if (!noall || cmSystemTools::IsOff(noall)) {
-        depends.push_back(this->GetAllTargetName());
+        gti.Depends.push_back(this->GetAllTargetName());
       }
     }
     if (mf->GetDefinition("CMake_BINARY_DIR") &&
@@ -2273,46 +2282,39 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     }
     singleLine.push_back("-P");
     singleLine.push_back("cmake_install.cmake");
-    cpackCommandLines.push_back(singleLine);
-    targets->insert(cmTargets::value_type(
-      this->GetInstallTargetName(),
-      this->CreateGlobalTarget(this->GetInstallTargetName(),
-                               "Install the project...", &cpackCommandLines,
-                               depends, CM_NULLPTR, /*uses_terminal*/ true)));
+    gti.CommandLines.push_back(singleLine);
+    targets.push_back(gti);
 
     // install_local
     if (const char* install_local = this->GetInstallLocalTargetName()) {
+      gti.Name = install_local;
+      gti.Message = "Installing only the local directory...";
+      gti.UsesTerminal = true;
+      gti.CommandLines.clear();
+
       cmCustomCommandLine localCmdLine = singleLine;
 
       localCmdLine.insert(localCmdLine.begin() + 1,
                           "-DCMAKE_INSTALL_LOCAL_ONLY=1");
-      cpackCommandLines.erase(cpackCommandLines.begin(),
-                              cpackCommandLines.end());
-      cpackCommandLines.push_back(localCmdLine);
 
-      targets->insert(cmTargets::value_type(
-        install_local,
-        this->CreateGlobalTarget(
-          install_local, "Installing only the local directory...",
-          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
+      gti.CommandLines.push_back(localCmdLine);
+      targets.push_back(gti);
     }
 
     // install_strip
     const char* install_strip = this->GetInstallStripTargetName();
     if ((install_strip != CM_NULLPTR) && (mf->IsSet("CMAKE_STRIP"))) {
+      gti.Name = install_strip;
+      gti.Message = "Installing the project stripped...";
+      gti.UsesTerminal = true;
+      gti.CommandLines.clear();
+
       cmCustomCommandLine stripCmdLine = singleLine;
 
       stripCmdLine.insert(stripCmdLine.begin() + 1,
                           "-DCMAKE_INSTALL_DO_STRIP=1");
-      cpackCommandLines.erase(cpackCommandLines.begin(),
-                              cpackCommandLines.end());
-      cpackCommandLines.push_back(stripCmdLine);
-
-      targets->insert(cmTargets::value_type(
-        install_strip,
-        this->CreateGlobalTarget(
-          install_strip, "Installing the project stripped...",
-          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
+      gti.CommandLines.push_back(stripCmdLine);
+      targets.push_back(gti);
     }
   }
 }
@@ -2346,14 +2348,12 @@ bool cmGlobalGenerator::UseFolderProperty()
   return false;
 }
 
-cmTarget cmGlobalGenerator::CreateGlobalTarget(
-  const std::string& name, const char* message,
-  const cmCustomCommandLines* commandLines, std::vector<std::string> depends,
-  const char* workingDirectory, bool uses_terminal)
+cmTarget cmGlobalGenerator::CreateGlobalTarget(GlobalTargetInfo const& gti,
+                                               cmMakefile* mf)
 {
   // Package
-  cmTarget target(name, cmState::GLOBAL_TARGET, cmTarget::VisibilityNormal,
-                  CM_NULLPTR);
+  cmTarget target(gti.Name, cmState::GLOBAL_TARGET, cmTarget::VisibilityNormal,
+                  mf);
   target.SetProperty("EXCLUDE_FROM_ALL", "TRUE");
 
   std::vector<std::string> no_outputs;
@@ -2361,12 +2361,14 @@ cmTarget cmGlobalGenerator::CreateGlobalTarget(
   std::vector<std::string> no_depends;
   // Store the custom command in the target.
   cmCustomCommand cc(CM_NULLPTR, no_outputs, no_byproducts, no_depends,
-                     *commandLines, CM_NULLPTR, workingDirectory);
-  cc.SetUsesTerminal(uses_terminal);
+                     gti.CommandLines, CM_NULLPTR, gti.WorkingDir.c_str());
+  cc.SetUsesTerminal(gti.UsesTerminal);
   target.AddPostBuildCommand(cc);
-  target.SetProperty("EchoString", message);
-  std::vector<std::string>::iterator dit;
-  for (dit = depends.begin(); dit != depends.end(); ++dit) {
+  if (!gti.Message.empty()) {
+    target.SetProperty("EchoString", gti.Message.c_str());
+  }
+  for (std::vector<std::string>::const_iterator dit = gti.Depends.begin();
+       dit != gti.Depends.end(); ++dit) {
     target.AddUtility(*dit);
   }
 
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 1e1479a..f7b2e59 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -402,11 +402,30 @@ protected:
   bool IsExcluded(cmLocalGenerator* root, cmLocalGenerator* gen) const;
   bool IsExcluded(cmLocalGenerator* root, cmGeneratorTarget* target) const;
   virtual void InitializeProgressMarks() {}
-  void CreateDefaultGlobalTargets(cmTargets* targets);
-  cmTarget CreateGlobalTarget(const std::string& name, const char* message,
-                              const cmCustomCommandLines* commandLines,
-                              std::vector<std::string> depends,
-                              const char* workingDir, bool uses_terminal);
+
+  struct GlobalTargetInfo
+  {
+    std::string Name;
+    std::string Message;
+    cmCustomCommandLines CommandLines;
+    std::vector<std::string> Depends;
+    std::string WorkingDir;
+    bool UsesTerminal;
+    GlobalTargetInfo()
+      : UsesTerminal(false)
+    {
+    }
+  };
+
+  void CreateDefaultGlobalTargets(std::vector<GlobalTargetInfo>& targets);
+
+  void AddGlobalTarget_Package(std::vector<GlobalTargetInfo>& targets);
+  void AddGlobalTarget_PackageSource(std::vector<GlobalTargetInfo>& targets);
+  void AddGlobalTarget_Test(std::vector<GlobalTargetInfo>& targets);
+  void AddGlobalTarget_EditCache(std::vector<GlobalTargetInfo>& targets);
+  void AddGlobalTarget_RebuildCache(std::vector<GlobalTargetInfo>& targets);
+  void AddGlobalTarget_Install(std::vector<GlobalTargetInfo>& targets);
+  cmTarget CreateGlobalTarget(GlobalTargetInfo const& gti, cmMakefile* mf);
 
   std::string FindMakeProgramFile;
   std::string ConfiguredFilesPath;
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index d964f00..1b6c93e 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -62,10 +62,10 @@ public:
 cmTarget::cmTarget(std::string const& name, cmState::TargetType type,
                    Visibility vis, cmMakefile* mf)
 {
-  assert(mf || type == cmState::GLOBAL_TARGET);
+  assert(mf);
   this->Name = name;
   this->TargetTypeValue = type;
-  this->Makefile = CM_NULLPTR;
+  this->Makefile = mf;
   this->HaveInstallRule = false;
   this->DLLPlatform = false;
   this->IsAndroid = false;
@@ -82,25 +82,6 @@ cmTarget::cmTarget(std::string const& name, cmState::TargetType type,
     this->RecordDependencies = false;
   }
 
-  if (mf) {
-    this->SetMakefile(mf);
-  }
-}
-
-cmTarget cmTarget::CopyForDirectory(cmMakefile* mf) const
-{
-  assert(this->GetType() == cmState::GLOBAL_TARGET);
-  assert(this->GetMakefile() == CM_NULLPTR);
-  cmTarget result(*this);
-  result.SetMakefile(mf);
-  return result;
-}
-
-void cmTarget::SetMakefile(cmMakefile* mf)
-{
-  // Set our makefile.
-  this->Makefile = mf;
-
   // Check whether this is a DLL platform.
   this->DLLPlatform =
     (this->Makefile->IsOn("WIN32") || this->Makefile->IsOn("CYGWIN") ||
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index ebc92f3..4b182bb 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -88,9 +88,6 @@ public:
   ///! Set/Get the name of the target
   const std::string& GetName() const { return this->Name; }
 
-  /** Get a copy of this target adapted for the given directory.  */
-  cmTarget CopyForDirectory(cmMakefile* mf) const;
-
   /** Get the cmMakefile that owns this target.  */
   cmMakefile* GetMakefile() const { return this->Makefile; }
 
@@ -283,8 +280,6 @@ public:
   };
 
 private:
-  void SetMakefile(cmMakefile* mf);
-
   bool HandleLocationPropertyPolicy(cmMakefile* context) const;
 
   const char* GetSuffixVariableInternal(bool implib) const;