CPackIFW: Refactor cmCPackIFWGenerator::PackageFiles

Split PackageFiles() into four methods, for increased readability:

- cmCPackIFWGenerator::BuildRepogenCommand
- cmCPackIFWGenerator::BuildBinaryCreatorCommand
- cmCPackIFWGenerator::RunRepogen
- cmCPackIFWGenerator::RunBinaryCreator

Fixes: #22744

2 files changed, 203 insertions, 166 deletions

diff --git a/Source/CPack/IFW/cmCPackIFWGenerator.cxx b/Source/CPack/IFW/cmCPackIFWGenerator.cxx
index b375ba6..9b33eec 100644
--- a/Source/CPack/IFW/cmCPackIFWGenerator.cxx
+++ b/Source/CPack/IFW/cmCPackIFWGenerator.cxx
@@ -38,202 +38,233 @@ int cmCPackIFWGenerator::PackageFiles()
   std::string ifwTLD = this->GetOption("CPACK_TOPLEVEL_DIRECTORY");
   std::string ifwTmpFile = cmStrCat(ifwTLD, "/IFWOutput.log");
 
-  // Run repogen
-  if (!this->Installer.RemoteRepositories.empty()) {
-    std::vector<std::string> ifwCmd;
-    std::string ifwArg;
+  // Create repositories
+  if (!this->RunRepogen(ifwTmpFile)) {
+    return 0;
+  }
 
-    ifwCmd.emplace_back(this->RepoGen);
+  // Create installer
+  if (!this->RunBinaryCreator(ifwTmpFile)) {
+    return 0;
+  }
 
-    if (this->IsVersionLess("2.0.0")) {
-      ifwCmd.emplace_back("-c");
-      ifwCmd.emplace_back(this->toplevel + "/config/config.xml");
-    }
+  return 1;
+}
 
-    ifwCmd.emplace_back("-p");
-    ifwCmd.emplace_back(this->toplevel + "/packages");
+std::vector<std::string> cmCPackIFWGenerator::BuildRepogenCommand()
+{
+  std::vector<std::string> ifwCmd;
+  std::string ifwArg;
 
-    if (!this->PkgsDirsVector.empty()) {
-      for (std::string const& it : this->PkgsDirsVector) {
-        ifwCmd.emplace_back("-p");
-        ifwCmd.emplace_back(it);
-      }
+  ifwCmd.emplace_back(this->RepoGen);
+
+  if (this->IsVersionLess("2.0.0")) {
+    ifwCmd.emplace_back("-c");
+    ifwCmd.emplace_back(this->toplevel + "/config/config.xml");
+  }
+
+  ifwCmd.emplace_back("-p");
+  ifwCmd.emplace_back(this->toplevel + "/packages");
+
+  if (!this->PkgsDirsVector.empty()) {
+    for (std::string const& it : this->PkgsDirsVector) {
+      ifwCmd.emplace_back("-p");
+      ifwCmd.emplace_back(it);
     }
+  }
 
-    if (!this->RepoDirsVector.empty()) {
-      if (!this->IsVersionLess("3.1")) {
-        for (std::string const& rd : this->RepoDirsVector) {
-          ifwCmd.emplace_back("--repository");
-          ifwCmd.emplace_back(rd);
-        }
-      } else {
-        cmCPackIFWLogger(WARNING,
-                         "The \"CPACK_IFW_REPOSITORIES_DIRECTORIES\" "
-                           << "variable is set, but content will be skipped, "
-                           << "because this feature available only since "
-                           << "QtIFW 3.1. Please update your QtIFW instance."
-                           << std::endl);
+  if (!this->RepoDirsVector.empty()) {
+    if (!this->IsVersionLess("3.1")) {
+      for (std::string const& rd : this->RepoDirsVector) {
+        ifwCmd.emplace_back("--repository");
+        ifwCmd.emplace_back(rd);
       }
+    } else {
+      cmCPackIFWLogger(WARNING,
+                       "The \"CPACK_IFW_REPOSITORIES_DIRECTORIES\" "
+                         << "variable is set, but content will be skipped, "
+                         << "because this feature available only since "
+                         << "QtIFW 3.1. Please update your QtIFW instance."
+                         << std::endl);
     }
+  }
 
-    if (!this->OnlineOnly && !this->DownloadedPackages.empty()) {
-      ifwCmd.emplace_back("-i");
-      auto it = this->DownloadedPackages.begin();
-      ifwArg = (*it)->Name;
+  if (!this->OnlineOnly && !this->DownloadedPackages.empty()) {
+    ifwCmd.emplace_back("-i");
+    auto it = this->DownloadedPackages.begin();
+    ifwArg = (*it)->Name;
+    ++it;
+    while (it != this->DownloadedPackages.end()) {
+      ifwArg += "," + (*it)->Name;
       ++it;
-      while (it != this->DownloadedPackages.end()) {
-        ifwArg += "," + (*it)->Name;
-        ++it;
-      }
-      ifwCmd.emplace_back(ifwArg);
-    }
-    ifwCmd.emplace_back(this->toplevel + "/repository");
-    cmCPackIFWLogger(VERBOSE,
-                     "Execute: " << cmSystemTools::PrintSingleCommand(ifwCmd)
-                                 << std::endl);
-    std::string output;
-    int retVal = 1;
-    cmCPackIFWLogger(OUTPUT, "- Generate repository" << std::endl);
-    bool res = cmSystemTools::RunSingleCommand(
-      ifwCmd, &output, &output, &retVal, nullptr, this->GeneratorVerbose,
-      cmDuration::zero());
-    if (!res || retVal) {
-      cmGeneratedFileStream ofs(ifwTmpFile);
-      ofs << "# Run command: " << cmSystemTools::PrintSingleCommand(ifwCmd)
-          << std::endl
-          << "# Output:" << std::endl
-          << output << std::endl;
-      cmCPackIFWLogger(
-        ERROR,
-        "Problem running IFW command: "
-          << cmSystemTools::PrintSingleCommand(ifwCmd) << std::endl
-          << "Please check \"" << ifwTmpFile << "\" for errors" << std::endl);
-      return 0;
     }
+    ifwCmd.emplace_back(ifwArg);
+  }
+  ifwCmd.emplace_back(this->toplevel + "/repository");
 
-    if (!this->Repository.RepositoryUpdate.empty() &&
-        !this->Repository.PatchUpdatesXml()) {
-      cmCPackIFWLogger(WARNING,
-                       "Problem patch IFW \"Updates\" "
-                         << "file: \"" << this->toplevel
-                         << "/repository/Updates.xml\"" << std::endl);
-    }
+  return ifwCmd;
+}
 
-    cmCPackIFWLogger(OUTPUT,
-                     "- repository: \"" << this->toplevel
-                                        << "/repository\" generated"
-                                        << std::endl);
+int cmCPackIFWGenerator::RunRepogen(const std::string& ifwTmpFile)
+{
+  if (this->Installer.RemoteRepositories.empty()) {
+    return 1;
+  }
+
+  std::vector<std::string> ifwCmd = this->BuildRepogenCommand();
+  cmCPackIFWLogger(VERBOSE,
+                   "Execute: " << cmSystemTools::PrintSingleCommand(ifwCmd)
+                               << std::endl);
+  std::string output;
+  int retVal = 1;
+  cmCPackIFWLogger(OUTPUT, "- Generate repository" << std::endl);
+  bool res = cmSystemTools::RunSingleCommand(ifwCmd, &output, &output, &retVal,
+                                             nullptr, this->GeneratorVerbose,
+                                             cmDuration::zero());
+  if (!res || retVal) {
+    cmGeneratedFileStream ofs(ifwTmpFile);
+    ofs << "# Run command: " << cmSystemTools::PrintSingleCommand(ifwCmd)
+        << std::endl
+        << "# Output:" << std::endl
+        << output << std::endl;
+    cmCPackIFWLogger(
+      ERROR,
+      "Problem running IFW command: "
+        << cmSystemTools::PrintSingleCommand(ifwCmd) << std::endl
+        << "Please check \"" << ifwTmpFile << "\" for errors" << std::endl);
+    return 0;
   }
 
-  // Run binary creator
-  {
-    std::vector<std::string> ifwCmd;
-    std::string ifwArg;
-
-    ifwCmd.emplace_back(this->BinCreator);
+  if (!this->Repository.RepositoryUpdate.empty() &&
+      !this->Repository.PatchUpdatesXml()) {
+    cmCPackIFWLogger(WARNING,
+                     "Problem patch IFW \"Updates\" "
+                       << "file: \"" << this->toplevel
+                       << "/repository/Updates.xml\"" << std::endl);
+  }
 
-    ifwCmd.emplace_back("-c");
-    ifwCmd.emplace_back(this->toplevel + "/config/config.xml");
+  cmCPackIFWLogger(OUTPUT,
+                   "- repository: \"" << this->toplevel
+                                      << "/repository\" generated"
+                                      << std::endl);
+  return 1;
+}
 
-    if (!this->Installer.Resources.empty()) {
-      ifwCmd.emplace_back("-r");
-      auto it = this->Installer.Resources.begin();
-      std::string path = this->toplevel + "/resources/";
-      ifwArg = path + *it;
+std::vector<std::string> cmCPackIFWGenerator::BuildBinaryCreatorCommmand()
+{
+  std::vector<std::string> ifwCmd;
+  std::string ifwArg;
+
+  ifwCmd.emplace_back(this->BinCreator);
+
+  ifwCmd.emplace_back("-c");
+  ifwCmd.emplace_back(this->toplevel + "/config/config.xml");
+
+  if (!this->Installer.Resources.empty()) {
+    ifwCmd.emplace_back("-r");
+    auto it = this->Installer.Resources.begin();
+    std::string path = this->toplevel + "/resources/";
+    ifwArg = path + *it;
+    ++it;
+    while (it != this->Installer.Resources.end()) {
+      ifwArg += "," + path + *it;
       ++it;
-      while (it != this->Installer.Resources.end()) {
-        ifwArg += "," + path + *it;
-        ++it;
-      }
-      ifwCmd.emplace_back(ifwArg);
     }
+    ifwCmd.emplace_back(ifwArg);
+  }
 
-    ifwCmd.emplace_back("-p");
-    ifwCmd.emplace_back(this->toplevel + "/packages");
+  ifwCmd.emplace_back("-p");
+  ifwCmd.emplace_back(this->toplevel + "/packages");
 
-    if (!this->PkgsDirsVector.empty()) {
-      for (std::string const& it : this->PkgsDirsVector) {
-        ifwCmd.emplace_back("-p");
-        ifwCmd.emplace_back(it);
-      }
+  if (!this->PkgsDirsVector.empty()) {
+    for (std::string const& it : this->PkgsDirsVector) {
+      ifwCmd.emplace_back("-p");
+      ifwCmd.emplace_back(it);
     }
+  }
 
-    if (!this->RepoDirsVector.empty()) {
-      if (!this->IsVersionLess("3.1")) {
-        for (std::string const& rd : this->RepoDirsVector) {
-          ifwCmd.emplace_back("--repository");
-          ifwCmd.emplace_back(rd);
-        }
-      } else {
-        cmCPackIFWLogger(WARNING,
-                         "The \"CPACK_IFW_REPOSITORIES_DIRECTORIES\" "
-                           << "variable is set, but content will be skipped, "
-                           << "because this feature available only since "
-                           << "QtIFW 3.1. Please update your QtIFW instance."
-                           << std::endl);
+  if (!this->RepoDirsVector.empty()) {
+    if (!this->IsVersionLess("3.1")) {
+      for (std::string const& rd : this->RepoDirsVector) {
+        ifwCmd.emplace_back("--repository");
+        ifwCmd.emplace_back(rd);
       }
+    } else {
+      cmCPackIFWLogger(WARNING,
+                       "The \"CPACK_IFW_REPOSITORIES_DIRECTORIES\" "
+                         << "variable is set, but content will be skipped, "
+                         << "because this feature available only since "
+                         << "QtIFW 3.1. Please update your QtIFW instance."
+                         << std::endl);
     }
+  }
 
-    if (this->OnlineOnly) {
-      ifwCmd.emplace_back("--online-only");
-    } else if (!this->DownloadedPackages.empty() &&
-               !this->Installer.RemoteRepositories.empty()) {
-      ifwCmd.emplace_back("-e");
-      auto it = this->DownloadedPackages.begin();
-      ifwArg = (*it)->Name;
+  if (this->OnlineOnly) {
+    ifwCmd.emplace_back("--online-only");
+  } else if (!this->DownloadedPackages.empty() &&
+             !this->Installer.RemoteRepositories.empty()) {
+    ifwCmd.emplace_back("-e");
+    auto it = this->DownloadedPackages.begin();
+    ifwArg = (*it)->Name;
+    ++it;
+    while (it != this->DownloadedPackages.end()) {
+      ifwArg += "," + (*it)->Name;
       ++it;
-      while (it != this->DownloadedPackages.end()) {
-        ifwArg += "," + (*it)->Name;
-        ++it;
-      }
-      ifwCmd.emplace_back(ifwArg);
-    } else if (!this->DependentPackages.empty()) {
-      ifwCmd.emplace_back("-i");
-      ifwArg.clear();
-      // Binary
-      auto bit = this->BinaryPackages.begin();
-      while (bit != this->BinaryPackages.end()) {
-        ifwArg += (*bit)->Name + ",";
-        ++bit;
-      }
-      // Depend
-      auto it = this->DependentPackages.begin();
-      ifwArg += it->second.Name;
-      ++it;
-      while (it != this->DependentPackages.end()) {
-        ifwArg += "," + it->second.Name;
-        ++it;
-      }
-      ifwCmd.emplace_back(ifwArg);
     }
-    // TODO: set correct name for multipackages
-    if (!this->packageFileNames.empty()) {
-      ifwCmd.emplace_back(this->packageFileNames[0]);
-    } else {
-      ifwCmd.emplace_back("installer" + this->OutputExtension);
+    ifwCmd.emplace_back(ifwArg);
+  } else if (!this->DependentPackages.empty()) {
+    ifwCmd.emplace_back("-i");
+    ifwArg.clear();
+    // Binary
+    auto bit = this->BinaryPackages.begin();
+    while (bit != this->BinaryPackages.end()) {
+      ifwArg += (*bit)->Name + ",";
+      ++bit;
     }
-    cmCPackIFWLogger(VERBOSE,
-                     "Execute: " << cmSystemTools::PrintSingleCommand(ifwCmd)
-                                 << std::endl);
-    std::string output;
-    int retVal = 1;
-    cmCPackIFWLogger(OUTPUT, "- Generate package" << std::endl);
-    bool res = cmSystemTools::RunSingleCommand(
-      ifwCmd, &output, &output, &retVal, nullptr, this->GeneratorVerbose,
-      cmDuration::zero());
-    if (!res || retVal) {
-      cmGeneratedFileStream ofs(ifwTmpFile);
-      ofs << "# Run command: " << cmSystemTools::PrintSingleCommand(ifwCmd)
-          << std::endl
-          << "# Output:" << std::endl
-          << output << std::endl;
-      cmCPackIFWLogger(
-        ERROR,
-        "Problem running IFW command: "
-          << cmSystemTools::PrintSingleCommand(ifwCmd) << std::endl
-          << "Please check \"" << ifwTmpFile << "\" for errors" << std::endl);
-      return 0;
+    // Depend
+    auto it = this->DependentPackages.begin();
+    ifwArg += it->second.Name;
+    ++it;
+    while (it != this->DependentPackages.end()) {
+      ifwArg += "," + it->second.Name;
+      ++it;
     }
+    ifwCmd.emplace_back(ifwArg);
+  }
+  // TODO: set correct name for multipackages
+  if (!this->packageFileNames.empty()) {
+    ifwCmd.emplace_back(this->packageFileNames[0]);
+  } else {
+    ifwCmd.emplace_back("installer" + this->OutputExtension);
+  }
+
+  return ifwCmd;
+}
+
+int cmCPackIFWGenerator::RunBinaryCreator(const std::string& ifwTmpFile)
+{
+  std::vector<std::string> ifwCmd = this->BuildBinaryCreatorCommmand();
+  cmCPackIFWLogger(VERBOSE,
+                   "Execute: " << cmSystemTools::PrintSingleCommand(ifwCmd)
+                               << std::endl);
+  std::string output;
+  int retVal = 1;
+  cmCPackIFWLogger(OUTPUT, "- Generate package" << std::endl);
+  bool res = cmSystemTools::RunSingleCommand(ifwCmd, &output, &output, &retVal,
+                                             nullptr, this->GeneratorVerbose,
+                                             cmDuration::zero());
+  if (!res || retVal) {
+    cmGeneratedFileStream ofs(ifwTmpFile);
+    ofs << "# Run command: " << cmSystemTools::PrintSingleCommand(ifwCmd)
+        << std::endl
+        << "# Output:" << std::endl
+        << output << std::endl;
+    cmCPackIFWLogger(
+      ERROR,
+      "Problem running IFW command: "
+        << cmSystemTools::PrintSingleCommand(ifwCmd) << std::endl
+        << "Please check \"" << ifwTmpFile << "\" for errors" << std::endl);
+    return 0;
   }
 
   return 1;
diff --git a/Source/CPack/IFW/cmCPackIFWGenerator.h b/Source/CPack/IFW/cmCPackIFWGenerator.h
index 024d25d..902ebaf 100644
--- a/Source/CPack/IFW/cmCPackIFWGenerator.h
+++ b/Source/CPack/IFW/cmCPackIFWGenerator.h
@@ -140,6 +140,12 @@ protected:
   std::map<cmCPackComponentGroup*, cmCPackIFWPackage*> GroupPackages;
 
 private:
+  std::vector<std::string> BuildRepogenCommand();
+  int RunRepogen(const std::string& ifwTmpFile);
+
+  std::vector<std::string> BuildBinaryCreatorCommmand();
+  int RunBinaryCreator(const std::string& ifwTmpFile);
+
   std::string RepoGen;
   std::string BinCreator;
   std::string FrameworkVersion;