Merge topic 'byproducts_make_clean'

80e2f8ee0c Ninja,Makefile: Add tests for handling of byproducts by clean operations
c7f1ed03d7 Help: Add release note for make clean and byproducts
4220962d18 Makefile: Add build events byproducts to clean rules
182d9597ec Makefile: Add custom command byproducts to clean rules
9c2b393cb7 Tests: Update CustomCommandWorkingDirectory to handle in-source byproducts

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !2334

9 files changed, 212 insertions, 4 deletions

diff --git a/Help/release/dev/byproducts_make_clean.rst b/Help/release/dev/byproducts_make_clean.rst
new file mode 100644
index 0000000..54df77d
--- /dev/null
+++ b/Help/release/dev/byproducts_make_clean.rst
@@ -0,0 +1,5 @@
+byproducts_make_clean
+---------------------
+
+* The :ref:`Makefile Generators` learned to remove custom command and
+  custom target byproducts during ``make clean``.
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index c8dc392..f423560 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -181,6 +181,36 @@ void cmMakefileTargetGenerator::WriteTargetBuildRules()
           this->LocalGenerator->MaybeConvertToRelativePath(currentBinDir,
                                                            output));
       }
+      const std::vector<std::string>& byproducts = ccg.GetByproducts();
+      for (std::string const& byproduct : byproducts) {
+        this->CleanFiles.push_back(
+          this->LocalGenerator->MaybeConvertToRelativePath(currentBinDir,
+                                                           byproduct));
+      }
+    }
+  }
+
+  // Add byproducts from build events to the clean rules
+  if (clean) {
+    std::vector<cmCustomCommand> buildEventCommands =
+      this->GeneratorTarget->GetPreBuildCommands();
+
+    buildEventCommands.insert(
+      buildEventCommands.end(),
+      this->GeneratorTarget->GetPreLinkCommands().begin(),
+      this->GeneratorTarget->GetPreLinkCommands().end());
+    buildEventCommands.insert(
+      buildEventCommands.end(),
+      this->GeneratorTarget->GetPostBuildCommands().begin(),
+      this->GeneratorTarget->GetPostBuildCommands().end());
+
+    for (const auto& be : buildEventCommands) {
+      const std::vector<std::string>& byproducts = be.GetByproducts();
+      for (std::string const& byproduct : byproducts) {
+        this->CleanFiles.push_back(
+          this->LocalGenerator->MaybeConvertToRelativePath(currentBinDir,
+                                                           byproduct));
+      }
     }
   }
   std::vector<cmSourceFile const*> headerSources;
diff --git a/Tests/CustomCommandWorkingDirectory/CMakeLists.txt b/Tests/CustomCommandWorkingDirectory/CMakeLists.txt
index 5495a9b..2e12a78 100644
--- a/Tests/CustomCommandWorkingDirectory/CMakeLists.txt
+++ b/Tests/CustomCommandWorkingDirectory/CMakeLists.txt
@@ -9,17 +9,17 @@ add_custom_command(
 )
 
 set_source_files_properties(
-  "${TestWorkingDir_BINARY_DIR}/customTarget.c"
+  "${TestWorkingDir_BINARY_DIR}/customTarget1.c"
   "${TestWorkingDir_BINARY_DIR}/customTarget2.c"
   PROPERTIES GENERATED 1)
 
 add_executable(working "${TestWorkingDir_BINARY_DIR}/working.c"
-  "${TestWorkingDir_BINARY_DIR}/customTarget.c")
+  "${TestWorkingDir_BINARY_DIR}/customTarget1.c")
 
 add_custom_target(
   Custom ALL
-  COMMAND "${CMAKE_COMMAND}" -E copy_if_different ./customTarget.c "${TestWorkingDir_BINARY_DIR}/customTarget.c"
-  BYPRODUCTS "${TestWorkingDir_BINARY_DIR}/customTarget.c"
+  COMMAND "${CMAKE_COMMAND}" -E copy_if_different ./customTarget.c "${TestWorkingDir_BINARY_DIR}/customTarget1.c"
+  BYPRODUCTS "${TestWorkingDir_BINARY_DIR}/customTarget1.c"
   WORKING_DIRECTORY  "${TestWorkingDir_SOURCE_DIR}"
 )
 
diff --git a/Tests/RunCMake/Byproducts/CMakeLists.txt b/Tests/RunCMake/Byproducts/CMakeLists.txt
new file mode 100644
index 0000000..bf2ef15
--- /dev/null
+++ b/Tests/RunCMake/Byproducts/CMakeLists.txt
@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.10)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)
diff --git a/Tests/RunCMake/Byproducts/CleanByproducts.cmake b/Tests/RunCMake/Byproducts/CleanByproducts.cmake
new file mode 100644
index 0000000..85d9582
--- /dev/null
+++ b/Tests/RunCMake/Byproducts/CleanByproducts.cmake
@@ -0,0 +1,93 @@
+cmake_minimum_required(VERSION 3.10)
+project(CleanByproducts)
+
+# Configurable parameters
+set(TEST_CLEAN_NO_CUSTOM FALSE CACHE BOOL "Value for the CLEAN_NO_CUSTOM PROPERTY")
+set(TEST_BUILD_EVENTS TRUE CACHE BOOL "Create byproducts with build events")
+set(TEST_CUSTOM_TARGET TRUE CACHE BOOL "Create a byproduct with a custom target")
+set(TEST_CUSTOM_COMMAND TRUE CACHE BOOL "Create a byproduct with a custom command")
+
+set_property(DIRECTORY PROPERTY CLEAN_NO_CUSTOM ${TEST_CLEAN_NO_CUSTOM})
+
+macro(add_build_event)
+  set(oneValueArgs EVENT)
+
+  cmake_parse_Arguments(ABE "" "${oneValueArgs}" "" ${ARGN})
+
+  # Create two byproducts and only declare one
+  add_custom_command(TARGET foo
+    ${ABE_EVENT}
+    COMMAND ${CMAKE_COMMAND} -E touch foo.${ABE_EVENT}
+    COMMAND ${CMAKE_COMMAND} -E touch foo.${ABE_EVENT}.notdeclared
+    COMMENT "Creating byproducts with ${ABE_EVENT}"
+    BYPRODUCTS foo.${ABE_EVENT}
+  )
+
+  # The nondeclared byproduct should always be present
+  list(APPEND EXPECTED_PRESENT foo.${ABE_EVENT}.notdeclared)
+
+  # If CLEAN_NO_CUSTOM is set, the declared byproduct should be present
+  if(TEST_CLEAN_NO_CUSTOM)
+    list(APPEND EXPECTED_PRESENT foo.${ABE_EVENT})
+  else()
+    list(APPEND EXPECTED_DELETED foo.${ABE_EVENT})
+  endif()
+endmacro()
+
+add_executable(foo foo.cpp)
+
+# Test build events
+if(TEST_BUILD_EVENTS)
+  add_build_event(EVENT "PRE_BUILD" ENABLE ${TEST_PRE_BUILD})
+  add_build_event(EVENT "PRE_LINK" ENABLE ${TEST_PRE_LINK})
+  add_build_event(EVENT "POST_BUILD" ENABLE ${TEST_POST_BUILD})
+endif()
+
+# Custom command that generates byproducts
+if(TEST_CUSTOM_COMMAND)
+  file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/bar.cpp.in "void bar() {}\n")
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/bar.cpp
+    COMMAND ${CMAKE_COMMAND} -E touch foo.customcommand
+    COMMAND ${CMAKE_COMMAND} -E touch foo.customcommand.notdeclared
+    COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/bar.cpp.in ${CMAKE_CURRENT_BINARY_DIR}/bar.cpp
+    BYPRODUCTS foo.customcommand
+    COMMENT "Creating byproducts with a custom command"
+  )
+
+  # The nondeclared byproduct should always be present
+  list(APPEND EXPECTED_PRESENT "foo.customcommand.notdeclared")
+
+  # If CLEAN_NO_CUSTOM is set, both the output and byproduct should be present
+  if(TEST_CLEAN_NO_CUSTOM)
+    list(APPEND EXPECTED_PRESENT "bar.cpp")
+    list(APPEND EXPECTED_PRESENT "foo.customcommand")
+  else()
+    list(APPEND EXPECTED_DELETED "bar.cpp")
+    list(APPEND EXPECTED_DELETED "foo.customcommand")
+  endif()
+
+  target_sources(foo PUBLIC "${CMAKE_CURRENT_BINARY_DIR}/bar.cpp")
+endif()
+
+# Custom target that generates byproducts
+if(TEST_CUSTOM_TARGET)
+  add_custom_target(foo_file ALL
+    DEPENDS foo
+    COMMAND ${CMAKE_COMMAND} -E touch foo.customtarget
+    COMMAND ${CMAKE_COMMAND} -E touch foo.customtarget.notdeclared
+    BYPRODUCTS foo.customtarget
+    COMMENT "Creating byproducts with a custom target"
+  )
+
+  # The nondeclared byproduct should always be present
+  list(APPEND EXPECTED_PRESENT "foo.customtarget.notdeclared")
+
+  # If CLEAN_NO_CUSTOM is set, the declared byproduct should be present
+  if(TEST_CLEAN_NO_CUSTOM)
+    list(APPEND EXPECTED_PRESENT "foo.customtarget")
+  else()
+    list(APPEND EXPECTED_DELETED "foo.customtarget")
+  endif()
+endif()
+
+configure_file(files.cmake.in files.cmake)
diff --git a/Tests/RunCMake/Byproducts/RunCMakeTest.cmake b/Tests/RunCMake/Byproducts/RunCMakeTest.cmake
new file mode 100644
index 0000000..a7584ee
--- /dev/null
+++ b/Tests/RunCMake/Byproducts/RunCMakeTest.cmake
@@ -0,0 +1,58 @@
+include(RunCMake)
+
+function(run_CleanByproducts case)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CleanByproducts-${case}-build)
+  set(RunCMake_TEST_OPTIONS "${ARGN}")
+
+  run_cmake(CleanByproducts)
+  set(RunCMake_TEST_NO_CLEAN 1)
+
+  run_cmake_command(CleanByProducts-build ${CMAKE_COMMAND} --build .)
+  include("${RunCMake_TEST_BINARY_DIR}/files.cmake")
+
+  message("Checking that all expected files are present")
+  check_files(EXPECTED_PRESENT "${RunCMake_TEST_BINARY_DIR}" TRUE)
+  check_files(EXPECTED_DELETED "${RunCMake_TEST_BINARY_DIR}" TRUE)
+
+  run_cmake_command(CleanByProducts-clean ${CMAKE_COMMAND} --build . --target clean)
+
+  message("Checking that only the expected files are present after cleaning")
+  check_files(EXPECTED_PRESENT "${RunCMake_TEST_BINARY_DIR}" TRUE)
+  check_files(EXPECTED_DELETED "${RunCMake_TEST_BINARY_DIR}" FALSE)
+endfunction()
+
+function(check_files list path has_to_exist)
+  foreach(file IN LISTS ${list})
+    message("Checking ${file}")
+    set(file_exists FALSE)
+    if(EXISTS "${path}/${file}")
+      set(file_exists TRUE)
+    endif()
+
+    if(file_exists AND NOT has_to_exist)
+      message(FATAL_ERROR "${file} should have been deleted")
+    elseif(NOT file_exists AND has_to_exist)
+      message(FATAL_ERROR "${file} does not exist")
+    elseif(file_exists AND has_to_exist)
+      message("${file} found as expected")
+    elseif(NOT file_exists AND NOT has_to_exist)
+      message("${file} deleted as expected")
+    endif()
+
+  endforeach()
+endfunction()
+
+
+# Iterate through all possible test values
+set(counter 0)
+foreach(test_clean_no_custom TRUE FALSE)
+  foreach(test_build_events TRUE FALSE)
+    foreach(test_custom_command TRUE FALSE)
+      foreach(test_custom_target TRUE FALSE)
+        math(EXPR counter "${counter} + 1")
+        message("Test ${counter} - CLEAN_NO_CUSTOM: ${test_clean_no_custom}, Build events: ${test_build_events}, Custom command: ${test_custom_command}, Custom target: ${test_custom_target}")
+        run_CleanByproducts("buildevents${counter}" -DCLEAN_NO_CUSTOM=${test_clean_no_custom} -DTEST_BUILD_EVENTS=${test_build_events} -DTEST_CUSTOM_COMMAND=${test_custom_command} -DTEST_CUSTOM_TARGET=${test_custom_target})
+      endforeach()
+    endforeach()
+  endforeach()
+endforeach()
diff --git a/Tests/RunCMake/Byproducts/files.cmake.in b/Tests/RunCMake/Byproducts/files.cmake.in
new file mode 100644
index 0000000..a7d4831
--- /dev/null
+++ b/Tests/RunCMake/Byproducts/files.cmake.in
@@ -0,0 +1,2 @@
+set(EXPECTED_PRESENT "@EXPECTED_PRESENT@")
+set(EXPECTED_DELETED "@EXPECTED_DELETED@")
diff --git a/Tests/RunCMake/Byproducts/foo.cpp b/Tests/RunCMake/Byproducts/foo.cpp
new file mode 100644
index 0000000..d47cb91
--- /dev/null
+++ b/Tests/RunCMake/Byproducts/foo.cpp
@@ -0,0 +1,14 @@
+int bar(int y)
+{
+  return y * 6;
+}
+
+int foo(int x)
+{
+  return x * bar(x);
+}
+
+int main()
+{
+  return foo(4);
+}
diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index 080d0d0..90681b9 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -144,6 +144,9 @@ endif()
 add_RunCMake_test(AndroidTestUtilities)
 add_RunCMake_test(BuildDepends)
 if(UNIX AND "${CMAKE_GENERATOR}" MATCHES "Unix Makefiles|Ninja")
+  add_RunCMake_test(Byproducts)
+endif()
+if(UNIX AND "${CMAKE_GENERATOR}" MATCHES "Unix Makefiles|Ninja")
   add_RunCMake_test(CompilerChange)
 endif()
 add_RunCMake_test(CompilerNotFound)