Merge topic 'jobs-positive-value'

6ad6993 cmake: --build -j <jobs> should not accept 0. Acked-by: Kitware Robot <[email protected]> Merge-request: !3255
Mariusko82 · May 3, 2019 · 9713154 · 9713154
2 parents cd285b7 + 6ad6993
commit 9713154
Show file tree

Hide file tree

Showing 11 changed files with 48 additions and 3 deletions.
diff --git a/Help/manual/cmake.1.rst b/Help/manual/cmake.1.rst
@@ -286,6 +286,9 @@ following options:
   The :envvar:`CMAKE_BUILD_PARALLEL_LEVEL` environment variable, if set,
   specifies a default parallel level when this option is not given.
 
+  Some native build tools always build in parallel.  The use of ``<jobs>``
+  value of ``1`` can be used to limit to a single job.
+
 ``--target <tgt>..., -t <tgt>...``
   Build ``<tgt>`` instead of default targets.  May be specified multiple times.
 

diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx
@@ -24,6 +24,7 @@
 #endif
 
 #include <cassert>
+#include <climits>
 #include <ctype.h>
 #include <iostream>
 #include <string.h>
@@ -69,7 +70,7 @@ static const char* cmDocumentationUsageNote[][2] = {
     "  --config <cfg> = For multi-configuration tools, choose <cfg>.\n"       \
     "  --clean-first  = Build target 'clean' first, then build.\n"            \
     "                   (To clean only, use --target 'clean'.)\n"             \
-    " --verbose, -v   = Enable verbose output - if supported - including\n"   \
+    "  --verbose, -v  = Enable verbose output - if supported - including\n"   \
     "                   the build commands to be executed. \n"                \
     "  --             = Pass remaining options to the native tool.\n"
 
@@ -395,7 +396,14 @@ int extract_job_number(int& index, char const* current, char const* next,
   if (jobString.empty()) {
     jobs = cmake::DEFAULT_BUILD_PARALLEL_LEVEL;
   } else if (cmSystemTools::StringToULong(jobString.c_str(), &numJobs)) {
-    jobs = int(numJobs);
+    if (numJobs == 0) {
+      std::cerr
+        << "The <jobs> value requires a positive integer argument.\n\n";
+    } else if (numJobs > INT_MAX) {
+      std::cerr << "The <jobs> value is too large.\n\n";
+    } else {
+      jobs = int(numJobs);
+    }
   } else {
     std::cerr << "'" << command.substr(0, len_of_flag) << "' invalid number '"
               << jobString << "' given.\n\n";
@@ -507,7 +515,17 @@ static int do_build(int ac, char const* const* av)
       } else {
         unsigned long numJobs = 0;
         if (cmSystemTools::StringToULong(parallel.c_str(), &numJobs)) {
-          jobs = int(numJobs);
+          if (numJobs == 0) {
+            std::cerr << "The CMAKE_BUILD_PARALLEL_LEVEL environment variable "
+                         "requires a positive integer argument.\n\n";
+            dir.clear();
+          } else if (numJobs > INT_MAX) {
+            std::cerr << "The CMAKE_BUILD_PARALLEL_LEVEL environment variable "
+                         "is too large.\n\n";
+            dir.clear();
+          } else {
+            jobs = int(numJobs);
+          }
         } else {
           std::cerr << "'CMAKE_BUILD_PARALLEL_LEVEL' environment variable\n"
                     << "invalid number '" << parallel << "' given.\n\n";

diff --git a/Tests/RunCMake/CommandLine/BuildDir--build--parallel-large-result.txt b/Tests/RunCMake/CommandLine/BuildDir--build--parallel-large-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build--parallel-large-stderr.txt b/Tests/RunCMake/CommandLine/BuildDir--build--parallel-large-stderr.txt
@@ -0,0 +1,3 @@
+^The <jobs> value is too large\.
++
+Usage: cmake --build <dir> \[options\] \[-- \[native-options\]\]
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build--parallel-zero-result.txt b/Tests/RunCMake/CommandLine/BuildDir--build--parallel-zero-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build--parallel-zero-stderr.txt b/Tests/RunCMake/CommandLine/BuildDir--build--parallel-zero-stderr.txt
@@ -0,0 +1,3 @@
+^The <jobs> value requires a positive integer argument\.
++
+Usage: cmake --build <dir> \[options\] \[-- \[native-options\]\]
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build-jobs-large-result.txt b/Tests/RunCMake/CommandLine/BuildDir--build-jobs-large-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build-jobs-large-stderr.txt b/Tests/RunCMake/CommandLine/BuildDir--build-jobs-large-stderr.txt
@@ -0,0 +1,3 @@
+^The <jobs> value is too large\.
++
+Usage: cmake --build <dir> \[options\] \[-- \[native-options\]\]
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build-jobs-zero-result.txt b/Tests/RunCMake/CommandLine/BuildDir--build-jobs-zero-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/CommandLine/BuildDir--build-jobs-zero-stderr.txt b/Tests/RunCMake/CommandLine/BuildDir--build-jobs-zero-stderr.txt
@@ -0,0 +1,3 @@
+^The <jobs> value requires a positive integer argument\.
++
+Usage: cmake --build <dir> \[options\] \[-- \[native-options\]\]
diff --git a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake
@@ -149,6 +149,14 @@ function(run_BuildDir)
     ${CMAKE_COMMAND} --build BuildDir-build --parallel2)
   run_cmake_command(BuildDir--build--parallel-no-space-good-number-trailing--target ${CMAKE_COMMAND} -E chdir ..
     ${CMAKE_COMMAND} --build BuildDir-build --parallel2 --target CustomTarget)
+  run_cmake_command(BuildDir--build-jobs-zero ${CMAKE_COMMAND} -E chdir ..
+    ${CMAKE_COMMAND} --build BuildDir-build -j 0)
+  run_cmake_command(BuildDir--build--parallel-zero ${CMAKE_COMMAND} -E chdir ..
+    ${CMAKE_COMMAND} --build BuildDir-build --parallel 0)
+  run_cmake_command(BuildDir--build-jobs-large ${CMAKE_COMMAND} -E chdir ..
+    ${CMAKE_COMMAND} --build BuildDir-build -j 4294967293)
+  run_cmake_command(BuildDir--build--parallel-large ${CMAKE_COMMAND} -E chdir ..
+    ${CMAKE_COMMAND} --build BuildDir-build --parallel 4294967293)
 
   # No default jobs for Xcode and FreeBSD build command
   if(NOT RunCMake_GENERATOR MATCHES "Xcode" AND NOT CMAKE_SYSTEM_NAME MATCHES "FreeBSD")