Merge topic 'ctest-no-make-i'

226df303 CTest: Stop telling 'make' to ignore errors with -i
28e7a135 Help: Fix build_command alternative signature docs
231601b6 build_command: Choose configuration consistently across signatures

25 files changed, 156 insertions, 7 deletions

diff --git a/Help/command/build_command.rst b/Help/command/build_command.rst
index 82a9a42..1298c1f 100644
--- a/Help/command/build_command.rst
+++ b/Help/command/build_command.rst
@@ -19,7 +19,8 @@ Sets the given ``<variable>`` to a command-line string of the form::
 where ``<cmake>`` is the location of the :manual:`cmake(1)` command-line
 tool, and ``<config>`` and ``<target>`` are the values provided to the
 ``CONFIGURATION`` and ``TARGET`` options, if any.  The trailing ``-- -i``
-option is added for Makefile generators.
+option is added for :ref:`Makefile Generators` if policy :policy:`CMP0061`
+is not set to ``NEW``.
 
 When invoked, this ``cmake --build`` command line will launch the
 underlying build system tool.
@@ -32,7 +33,7 @@ This second signature is deprecated, but still available for backwards
 compatibility.  Use the first signature instead.
 
 It sets the given ``<cachevariable>`` to a command-line string as
-above but without the ``--config`` or ``--target`` options.
+above but without the ``--target`` option.
 The ``<makecommand>`` is ignored but should be the full path to
 msdev, devenv, nmake, make or one of the end user build tools
 for legacy invocations.
diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst
index aff696d..e59cce7 100644
--- a/Help/manual/cmake-policies.7.rst
+++ b/Help/manual/cmake-policies.7.rst
@@ -118,3 +118,4 @@ All Policies
    /policy/CMP0058
    /policy/CMP0059
    /policy/CMP0060
+   /policy/CMP0061
diff --git a/Help/policy/CMP0061.rst b/Help/policy/CMP0061.rst
new file mode 100644
index 0000000..069a1ab
--- /dev/null
+++ b/Help/policy/CMP0061.rst
@@ -0,0 +1,24 @@
+CMP0061
+-------
+
+CTest does not by default tell ``make`` to ignore errors (``-i``).
+
+The :command:`ctest_build` and :command:`build_command` commands no
+longer generate build commands for :ref:`Makefile Generators` with
+the ``-i`` option.  Previously this was done to help build as much
+of tested projects as possible.  However, this behavior is not
+consistent with other generators and also causes the return code
+of the ``make`` tool to be meaningless.
+
+Of course users may still add this option manually by setting
+:variable:`CTEST_BUILD_COMMAND` or the ``MAKECOMMAND`` cache entry.
+See the :ref:`CTest Build Step` ``MakeCommand`` setting documentation
+for their effects.
+
+The ``OLD`` behavior for this policy is to add ``-i`` to ``make``
+calls in CTest.  The ``NEW`` behavior for this policy is to not
+add ``-i``.
+
+This policy was introduced in CMake version 3.3.  Unlike most policies,
+CMake version |release| does *not* warn when this policy is not set and
+simply uses OLD behavior.
diff --git a/Help/release/dev/ctest-no-make-i.rst b/Help/release/dev/ctest-no-make-i.rst
new file mode 100644
index 0000000..96da0bd
--- /dev/null
+++ b/Help/release/dev/ctest-no-make-i.rst
@@ -0,0 +1,7 @@
+ctest-no-make-i
+---------------
+
+* The :command:`ctest_build` and :command:`build_command` commands
+  no longer tell ``make`` tools to ignore errors with the ``-i`` option.
+  Previously this was done for :ref:`Makefile Generators` but not others.
+  See policy :policy:`CMP0061`.
diff --git a/Source/CTest/cmCTestBuildCommand.cxx b/Source/CTest/cmCTestBuildCommand.cxx
index 2b36b0a..27e22c4 100644
--- a/Source/CTest/cmCTestBuildCommand.cxx
+++ b/Source/CTest/cmCTestBuildCommand.cxx
@@ -141,7 +141,8 @@ cmCTestGenericHandler* cmCTestBuildCommand::InitializeHandler()
         = this->GlobalGenerator->
         GenerateCMakeBuildCommand(cmakeBuildTarget ? cmakeBuildTarget : "",
           cmakeBuildConfiguration,
-          cmakeBuildAdditionalFlags ? cmakeBuildAdditionalFlags : "", true);
+          cmakeBuildAdditionalFlags ? cmakeBuildAdditionalFlags : "",
+          this->Makefile->IgnoreErrorsCMP0061());
       cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
         "SetMakeCommand:" << buildCommand << "\n", this->Quiet);
       this->CTest->SetCTestConfiguration("MakeCommand", buildCommand.c_str(),
diff --git a/Source/cmBuildCommand.cxx b/Source/cmBuildCommand.cxx
index 988e9a7..62fafa5 100644
--- a/Source/cmBuildCommand.cxx
+++ b/Source/cmBuildCommand.cxx
@@ -106,7 +106,8 @@ bool cmBuildCommand
     }
 
   std::string makecommand = this->Makefile->GetGlobalGenerator()
-      ->GenerateCMakeBuildCommand(target, configuration, "", true);
+    ->GenerateCMakeBuildCommand(target, configuration, "",
+                                this->Makefile->IgnoreErrorsCMP0061());
 
   this->Makefile->AddDefinition(variable, makecommand.c_str());
 
@@ -129,13 +130,14 @@ bool cmBuildCommand
 
   std::string configType = "Release";
   const char* cfg = getenv("CMAKE_CONFIG_TYPE");
-  if ( cfg )
+  if ( cfg && *cfg )
     {
     configType = cfg;
     }
 
   std::string makecommand = this->Makefile->GetGlobalGenerator()
-      ->GenerateCMakeBuildCommand("", configType, "", true);
+    ->GenerateCMakeBuildCommand("", configType, "",
+                                this->Makefile->IgnoreErrorsCMP0061());
 
   if(cacheValue)
     {
diff --git a/Source/cmBuildCommand.h b/Source/cmBuildCommand.h
index 3fb618f..979abc0 100644
--- a/Source/cmBuildCommand.h
+++ b/Source/cmBuildCommand.h
@@ -53,6 +53,8 @@ public:
   virtual std::string GetName() const {return "build_command";}
 
   cmTypeMacro(cmBuildCommand, cmCommand);
+private:
+  bool IgnoreErrors() const;
 };
 
 #endif
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 82add86..4ded936 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -4942,6 +4942,26 @@ void cmMakefile::RecordPolicies(cmPolicies::PolicyMap& pm)
     }
 }
 
+//----------------------------------------------------------------------------
+bool cmMakefile::IgnoreErrorsCMP0061() const
+{
+  bool ignoreErrors = true;
+  switch (this->GetPolicyStatus(cmPolicies::CMP0061))
+    {
+    case cmPolicies::WARN:
+      // No warning for this policy!
+    case cmPolicies::OLD:
+      break;
+    case cmPolicies::REQUIRED_IF_USED:
+    case cmPolicies::REQUIRED_ALWAYS:
+    case cmPolicies::NEW:
+      ignoreErrors = false;
+      break;
+    }
+  return ignoreErrors;
+}
+
+//----------------------------------------------------------------------------
 #define FEATURE_STRING(F) , #F
 static const char * const C_FEATURES[] = {
   0
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 99f2544..e0eef6f 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -415,6 +415,8 @@ public:
   bool HasCMP0054AlreadyBeenReported(
     cmListFileContext context) const;
 
+  bool IgnoreErrorsCMP0061() const;
+
   const char* GetHomeDirectory() const;
   const char* GetHomeOutputDirectory() const;
 
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 63376dd..536dcdc 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -208,6 +208,9 @@ class cmPolicy;
     3, 3, 0, cmPolicies::WARN) \
   SELECT(POLICY, CMP0060, \
     "Link libraries by full path even in implicit directories.", \
+    3, 3, 0, cmPolicies::WARN) \
+  SELECT(POLICY, CMP0061, \
+    "CTest does not by default tell make to ignore errors (-i).", \
     3, 3, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
diff --git a/Tests/RunCMake/build_command/CMP0061-NEW-stderr.txt b/Tests/RunCMake/build_command/CMP0061-NEW-stderr.txt
new file mode 100644
index 0000000..1dde843
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061-NEW-stderr.txt
@@ -0,0 +1,10 @@
+^[^
+]+ --build \. --config "Release"
+[^
+]+ --build \. --config "Release" --target "MyTarget"
+[^
+]+ --build \. --config "Debug"
+[^
+]+ --build \. --config "Debug" --target "MyTarget"
+[^
+]+ --build \. --config "Release"$
diff --git a/Tests/RunCMake/build_command/CMP0061-NEW.cmake b/Tests/RunCMake/build_command/CMP0061-NEW.cmake
new file mode 100644
index 0000000..2e439cb
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061-NEW.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0061 NEW)
+include(CMP0061Common.cmake)
diff --git a/Tests/RunCMake/build_command/CMP0061-OLD-make-stderr.txt b/Tests/RunCMake/build_command/CMP0061-OLD-make-stderr.txt
new file mode 100644
index 0000000..28e0e72
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061-OLD-make-stderr.txt
@@ -0,0 +1,10 @@
+^[^
+]+ --build \. --config "Release" -- -i
+[^
+]+ --build \. --config "Release" --target "MyTarget" -- -i
+[^
+]+ --build \. --config "Debug" -- -i
+[^
+]+ --build \. --config "Debug" --target "MyTarget" -- -i
+[^
+]+ --build \. --config "Release" -- -i$
diff --git a/Tests/RunCMake/build_command/CMP0061-OLD-make.cmake b/Tests/RunCMake/build_command/CMP0061-OLD-make.cmake
new file mode 100644
index 0000000..1542d8c
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061-OLD-make.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0061 OLD)
+include(CMP0061Common.cmake)
diff --git a/Tests/RunCMake/build_command/CMP0061-OLD-other-stderr.txt b/Tests/RunCMake/build_command/CMP0061-OLD-other-stderr.txt
new file mode 100644
index 0000000..1dde843
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061-OLD-other-stderr.txt
@@ -0,0 +1,10 @@
+^[^
+]+ --build \. --config "Release"
+[^
+]+ --build \. --config "Release" --target "MyTarget"
+[^
+]+ --build \. --config "Debug"
+[^
+]+ --build \. --config "Debug" --target "MyTarget"
+[^
+]+ --build \. --config "Release"$
diff --git a/Tests/RunCMake/build_command/CMP0061-OLD-other.cmake b/Tests/RunCMake/build_command/CMP0061-OLD-other.cmake
new file mode 100644
index 0000000..1542d8c
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061-OLD-other.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0061 OLD)
+include(CMP0061Common.cmake)
diff --git a/Tests/RunCMake/build_command/CMP0061Common.cmake b/Tests/RunCMake/build_command/CMP0061Common.cmake
new file mode 100644
index 0000000..50cd7d7
--- /dev/null
+++ b/Tests/RunCMake/build_command/CMP0061Common.cmake
@@ -0,0 +1,10 @@
+build_command(command)
+message("${command}")
+build_command(command TARGET MyTarget)
+message("${command}")
+build_command(command CONFIGURATION Debug)
+message("${command}")
+build_command(command CONFIGURATION Debug TARGET MyTarget)
+message("${command}")
+build_command(cache_command "${CMAKE_MAKE_PROGRAM}")
+message("${cache_command}")
diff --git a/Tests/RunCMake/build_command/RunCMakeTest.cmake b/Tests/RunCMake/build_command/RunCMakeTest.cmake
index eaa1d77..c3bef4c 100644
--- a/Tests/RunCMake/build_command/RunCMakeTest.cmake
+++ b/Tests/RunCMake/build_command/RunCMakeTest.cmake
@@ -1,4 +1,5 @@
 include(RunCMake)
+unset(ENV{CMAKE_CONFIG_TYPE})
 
 run_cmake(ErrorsOFF)
 run_cmake(ErrorsON)
@@ -6,3 +7,10 @@ run_cmake(ErrorsON)
 set(RunCMake_TEST_OPTIONS -DNoProject=1)
 run_cmake(BeforeProject)
 unset(RunCMake_TEST_OPTIONS)
+
+run_cmake(CMP0061-NEW)
+if(RunCMake_GENERATOR MATCHES "Make")
+  run_cmake(CMP0061-OLD-make)
+else()
+  run_cmake(CMP0061-OLD-other)
+endif()
diff --git a/Tests/RunCMake/ctest_build/BuildFailure-CMP0061-OLD-result.txt b/Tests/RunCMake/ctest_build/BuildFailure-CMP0061-OLD-result.txt
new file mode 100644
index 0000000..9cdf4a5
--- /dev/null
+++ b/Tests/RunCMake/ctest_build/BuildFailure-CMP0061-OLD-result.txt
@@ -0,0 +1 @@
+(0|-1|255)
diff --git a/Tests/RunCMake/ctest_build/BuildFailure-CMP0061-OLD-stderr.txt b/Tests/RunCMake/ctest_build/BuildFailure-CMP0061-OLD-stderr.txt
new file mode 100644
index 0000000..af70ac3
--- /dev/null
+++ b/Tests/RunCMake/ctest_build/BuildFailure-CMP0061-OLD-stderr.txt
@@ -0,0 +1,2 @@
+^(Error\(s\) when building project
+)?ctest_build returned zero$
diff --git a/Tests/RunCMake/ctest_build/BuildFailure-result.txt b/Tests/RunCMake/ctest_build/BuildFailure-result.txt
new file mode 100644
index 0000000..b57e2de
--- /dev/null
+++ b/Tests/RunCMake/ctest_build/BuildFailure-result.txt
@@ -0,0 +1 @@
+(-1|255)
diff --git a/Tests/RunCMake/ctest_build/BuildFailure-stderr.txt b/Tests/RunCMake/ctest_build/BuildFailure-stderr.txt
new file mode 100644
index 0000000..1e6ad87
--- /dev/null
+++ b/Tests/RunCMake/ctest_build/BuildFailure-stderr.txt
@@ -0,0 +1,2 @@
+^Error\(s\) when building project
+ctest_build returned non-zero$
diff --git a/Tests/RunCMake/ctest_build/CMakeLists.txt.in b/Tests/RunCMake/ctest_build/CMakeLists.txt.in
index 9ba08e9..82cb7c4 100644
--- a/Tests/RunCMake/ctest_build/CMakeLists.txt.in
+++ b/Tests/RunCMake/ctest_build/CMakeLists.txt.in
@@ -2,3 +2,4 @@ cmake_minimum_required(VERSION 3.1)
 project(CTestBuild@CASE_NAME@ NONE)
 include(CTest)
 add_test(NAME RunCMakeVersion COMMAND "${CMAKE_COMMAND}" --version)
+@CASE_CMAKELISTS_SUFFIX_CODE@
diff --git a/Tests/RunCMake/ctest_build/RunCMakeTest.cmake b/Tests/RunCMake/ctest_build/RunCMakeTest.cmake
index 5826fe4..c6f732c 100644
--- a/Tests/RunCMake/ctest_build/RunCMakeTest.cmake
+++ b/Tests/RunCMake/ctest_build/RunCMakeTest.cmake
@@ -8,3 +8,26 @@ function(run_ctest_build CASE_NAME)
 endfunction()
 
 run_ctest_build(BuildQuiet QUIET)
+
+function(run_BuildFailure)
+  set(CASE_CMAKELISTS_SUFFIX_CODE [[
+add_custom_target(BuildFailure ALL COMMAND command-does-not-exist)
+]])
+  set(CASE_TEST_PREFIX_CODE [[
+cmake_policy(SET CMP0061 NEW)
+]])
+  set(CASE_TEST_SUFFIX_CODE [[
+if (ctest_build_return_value)
+  message("ctest_build returned non-zero")
+else()
+  message("ctest_build returned zero")
+endif()
+]])
+  run_ctest(BuildFailure)
+
+  if (RunCMake_GENERATOR MATCHES "Makefiles")
+    set(CASE_TEST_PREFIX_CODE "")
+    run_ctest(BuildFailure-CMP0061-OLD)
+  endif()
+endfunction()
+run_BuildFailure()
diff --git a/Tests/RunCMake/ctest_build/test.cmake.in b/Tests/RunCMake/ctest_build/test.cmake.in
index 38c8ea1..768f1c6 100644
--- a/Tests/RunCMake/ctest_build/test.cmake.in
+++ b/Tests/RunCMake/ctest_build/test.cmake.in
@@ -1,4 +1,5 @@
 cmake_minimum_required(VERSION 3.1)
+@CASE_TEST_PREFIX_CODE@
 
 set(CTEST_SITE                          "test-site")
 set(CTEST_BUILD_NAME                    "test-build-name")
@@ -12,4 +13,5 @@ set(CTEST_BUILD_CONFIGURATION           "$ENV{CMAKE_CONFIG_TYPE}")
 set(ctest_build_args "@CASE_CTEST_BUILD_ARGS@")
 ctest_start(Experimental)
 ctest_configure()
-ctest_build(${ctest_build_args})
+ctest_build(${ctest_build_args} RETURN_VALUE ctest_build_return_value)
+@CASE_TEST_SUFFIX_CODE@