cmArchiveWrite: Consolidate multiple ways to set thread count

Merge use of SetFilterOption() into more abstract thread count
in cmArchiveWrite constructor.

libarchive defaulting of threads for threads == 0 seems to be
configuration dependent. Preemptively default thread count via
std::thread::hardware_concurrency().

Also allow negative values for the thread count in which case
the detected hardware concurrency is also used but the given
absolute thread count is used as an upper limit.

Modules/CPack.cmake

@@ -291,13 +291,21 @@ installers.  The most commonly-used variables are:
 
   Some compression methods used by CPack generators such as Debian or Archive
   may take advantage of multiple CPU cores to speed up compression.
-  ``CPACK_THREADS`` can be set to positive integer to specify how many threads
-  will be used for compression. If it is set to 0, CPack will set it so that
-  all available CPU cores are used.
+  ``CPACK_THREADS`` can be set to specify how many threads will be
+  used for compression.
+
+  A positive integer can be used to specify an exact desired thread count.
+
+  When given a negative integer CPack will use the absolute value
+  as the upper limit but may choose a lower value based on
+  the available hardware concurrency.
+
+  Given 0 CPack will try to use all available CPU cores.
+
   By default ``CPACK_THREADS`` is set to ``1``.
 
-  Currently only ``xz`` compression *may* take advantage of multiple cores. Other
-  compression methods ignore this value and use only one thread.
+  Currently only ``xz`` compression *may* take advantage of multiple cores.
+  Other compression methods ignore this value and use only one thread.
 
   .. versionadded:: 3.21
 

Source/CPack/cmCPackArchiveGenerator.cxx

@@ -2,14 +2,13 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmCPackArchiveGenerator.h"
 
+#include <cstdlib>
 #include <cstring>
 #include <map>
 #include <ostream>
 #include <utility>
 #include <vector>
 
-#include <cm3p/archive.h>
-
 #include "cmCPackComponentGroup.h"
 #include "cmCPackGenerator.h"
 #include "cmCPackLog.h"
@@ -154,15 +153,9 @@ int cmCPackArchiveGenerator::addOneComponentToArchive(
                     << (filename) << ">." << std::endl);                      \
     return 0;                                                                 \
   }                                                                           \
-  cmArchiveWrite archive(gf, this->Compress, this->ArchiveFormat);            \
+  cmArchiveWrite archive(gf, this->Compress, this->ArchiveFormat, 0,          \
+                         this->GetThreadCount());                             \
   do {                                                                        \
-    if (!this->SetArchiveOptions(&archive)) {                                 \
-      cmCPackLogger(cmCPackLog::LOG_ERROR,                                    \
-                    "Problem to set archive options <"                        \
-                      << (filename) << ">, ERROR = " << (archive).GetError()  \
-                      << std::endl);                                          \
-      return 0;                                                               \
-    }                                                                         \
     if (!archive.Open()) {                                                    \
       cmCPackLogger(cmCPackLog::LOG_ERROR,                                    \
                     "Problem to open archive <"                               \
@@ -346,26 +339,16 @@ bool cmCPackArchiveGenerator::SupportsComponentInstallation() const
   return this->IsOn("CPACK_ARCHIVE_COMPONENT_INSTALL");
 }
 
-bool cmCPackArchiveGenerator::SetArchiveOptions(cmArchiveWrite* archive)
+int cmCPackArchiveGenerator::GetThreadCount() const
 {
-#if ARCHIVE_VERSION_NUMBER >= 3004000
-  // Upstream fixed an issue with their integer parsing in 3.4.0 which would
-  // cause spurious errors to be raised from `strtoull`.
-  if (this->Compress == cmArchiveWrite::CompressXZ) {
-    const char* threads = "1";
-
-    // CPACK_ARCHIVE_THREADS overrides CPACK_THREADS
-    if (this->IsSet("CPACK_ARCHIVE_THREADS")) {
-      threads = this->GetOption("CPACK_ARCHIVE_THREADS");
-    } else if (this->IsSet("CPACK_THREADS")) {
-      threads = this->GetOption("CPACK_THREADS");
-    }
+  int threads = 1;
 
-    if (!archive->SetFilterOption("xz", "threads", threads)) {
-      return false;
-    }
+  // CPACK_ARCHIVE_THREADS overrides CPACK_THREADS
+  if (this->IsSet("CPACK_ARCHIVE_THREADS")) {
+    threads = std::atoi(this->GetOption("CPACK_ARCHIVE_THREADS"));
+  } else if (this->IsSet("CPACK_THREADS")) {
+    threads = std::atoi(this->GetOption("CPACK_THREADS"));
   }
-#endif
 
-  return true;
+  return threads;
 }

Source/CPack/cmCPackArchiveGenerator.h

@@ -85,7 +85,7 @@ private:
     return this->OutputExtension.c_str();
   }
 
-  bool SetArchiveOptions(cmArchiveWrite* archive);
+  int GetThreadCount() const;
 
 private:
   cmArchiveWrite::Compress Compress;

Source/CPack/cmCPackDebGenerator.cxx

@@ -130,11 +130,6 @@ DebGenerator::DebGenerator(
                   "Unrecognized number of threads: " << numThreads
                                                      << std::endl);
   }
-
-  if (this->NumThreads < 0) {
-    cmCPackLogger(cmCPackLog::LOG_ERROR,
-                  "Number of threads cannot be negative" << std::endl);
-  }
 }
 
 bool DebGenerator::generate() const

Source/cmArchiveWrite.cxx

@@ -2,11 +2,16 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmArchiveWrite.h"
 
-#include <cstdio>
+#include <cstdlib>
 #include <cstring>
 #include <ctime>
 #include <iostream>
+#include <limits>
 #include <sstream>
+#include <string>
+#include <thread>
+
+#include <cm/algorithm>
 
 #include <cm3p/archive.h>
 #include <cm3p/archive_entry.h>
@@ -144,16 +149,36 @@ cmArchiveWrite::cmArchiveWrite(std::ostream& os, Compress c,
                                cm_archive_error_string(this->Archive));
         return;
       }
+
       {
-        char sNumThreads[8];
-        snprintf(sNumThreads, sizeof(sNumThreads), "%d", numThreads);
-        sNumThreads[7] = '\0'; // for safety
+#if ARCHIVE_VERSION_NUMBER >= 3004000
+        // Upstream fixed an issue with their integer parsing in 3.4.0
+        // which would cause spurious errors to be raised from `strtoull`.
+
+        if (numThreads < 1) {
+          int upperLimit = (numThreads == 0) ? std::numeric_limits<int>::max()
+                                             : std::abs(numThreads);
+
+          numThreads =
+            cm::clamp<int>(std::thread::hardware_concurrency(), 1, upperLimit);
+        }
+
+#  ifdef _AIX
+        // FIXME: Using more than 2 threads creates an empty archive.
+        // Enforce this limit pending further investigation.
+        numThreads = std::min(numThreads, 2);
+#  endif
+
+        std::string sNumThreads = std::to_string(numThreads);
+
         if (archive_write_set_filter_option(this->Archive, "xz", "threads",
-                                            sNumThreads) != ARCHIVE_OK) {
+                                            sNumThreads.c_str()) !=
+            ARCHIVE_OK) {
           this->Error = cmStrCat("archive_compressor_xz_options: ",
                                  cm_archive_error_string(this->Archive));
           return;
         }
+#endif
       }
 
       break;
@@ -425,16 +450,3 @@ bool cmArchiveWrite::AddData(const char* file, size_t size)
   }
   return true;
 }
-
-bool cmArchiveWrite::SetFilterOption(const char* module, const char* key,
-                                     const char* value)
-{
-  if (archive_write_set_filter_option(this->Archive, module, key, value) !=
-      ARCHIVE_OK) {
-    this->Error = "archive_write_set_filter_option: ";
-    this->Error += cm_archive_error_string(this->Archive);
-    return false;
-  }
-
-  return true;
-}

Source/cmArchiveWrite.h

@@ -141,9 +141,6 @@ public:
     this->Gname = "";
   }
 
-  //! Set an option on a filter;
-  bool SetFilterOption(const char* module, const char* key, const char* value);
-
 private:
   bool Okay() const { return this->Error.empty(); }
   bool AddPath(const char* path, size_t skip, const char* prefix,

Tests/RunCMake/CPack/tests/THREADED_ALL/test.cmake

@@ -1,6 +1,6 @@
 install(FILES CMakeLists.txt DESTINATION foo COMPONENT test)
 
-set(CPACK_THREADS 0)
+set(CPACK_THREADS "-4")
 
 if(PACKAGING_TYPE STREQUAL "COMPONENT")
   set(CPACK_COMPONENTS_ALL test)