Ninja: Transform DEPFILEs with policy CMP0116

Fixes: #21267

Help/command/add_custom_command.rst

@@ -239,6 +239,11 @@ The options are:
   command itself.
   Using ``DEPFILE`` with other generators than Ninja is an error.
 
+  If the ``DEPFILE`` argument is relative, it should be relative to
+  :variable:`CMAKE_CURRENT_BINARY_DIR`, and any relative paths inside the
+  ``DEPFILE`` should also be relative to :variable:`CMAKE_CURRENT_BINARY_DIR`
+  (see policy :policy:`CMP0116`.)
+
 Build Events
 ^^^^^^^^^^^^
 

Help/manual/cmake-policies.7.rst

@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.20
 .. toctree::
    :maxdepth: 1
 
+   CMP0116: Ninja generators transform DEPFILEs from add_custom_command(). </policy/CMP0116>
    CMP0115: Source file extensions must be explicit. </policy/CMP0115>
 
 Policies Introduced by CMake 3.19

Help/policy/CMP0116.rst

@@ -0,0 +1,38 @@
+CMP0116
+-------
+
+.. versionadded:: 3.20
+
+Ninja generators transform ``DEPFILE`` s from :command:`add_custom_command`.
+
+In CMake 3.19 and below, files given to the ``DEPFILE`` argument of
+:command:`add_custom_command` were passed directly to Ninja's ``depfile``
+variable without any path resolution. This meant that if
+:command:`add_custom_command` was called from a subdirectory (created by
+:command:`add_subdirectory`), the ``DEPFILE`` argument would have to be either
+an absolute path or a path relative to :variable:`CMAKE_BINARY_DIR`, rather
+than :variable:`CMAKE_CURRENT_BINARY_DIR`. In addition, no transformation was
+done on the file listed in ``DEPFILE``, which meant that the paths within the
+``DEPFILE`` had the same restrictions.
+
+Starting with CMake 3.20, the ``DEPFILE`` argument is relative to
+:variable:`CMAKE_CURRENT_BINARY_DIR` (unless it is absolute), and the paths in
+the ``DEPFILE`` are also relative to :variable:`CMAKE_CURRENT_BINARY_DIR`.
+CMake automatically transforms the paths in the ``DEPFILE`` (unless they are
+absolute) after the custom command is run. The file listed in ``DEPFILE`` is
+not modified in any way. Instead, CMake writes the transformation to its own
+internal file, and passes this internal file to Ninja's ``depfile`` variable.
+This transformation happens regardless of whether or not ``DEPFILE`` is
+relative, and regardless of whether or not :command:`add_custom_command` is
+called from a subdirectory.
+
+The ``OLD`` behavior for this policy is to pass the ``DEPFILE`` to Ninja
+unaltered. The ``NEW`` behavior for this policy is to transform the ``DEPFILE``
+after running the custom command.
+
+This policy was introduced in CMake version 3.20.  Unlike most policies,
+CMake version |release| does *not* warn by default when this policy is not set
+(unless ``DEPFILE`` is used in a subdirectory) and simply uses ``OLD``
+behavior.  See documentation of the
+:variable:`CMAKE_POLICY_WARNING_CMP0116 <CMAKE_POLICY_WARNING_CMP<NNNN>>`
+variable to control the warning.

Help/release/dev/ninja-depfile-transformation.rst

@@ -0,0 +1,5 @@
+ninja-depfile-transformation
+----------------------------
+
+* Ninja generators now transform ``DEPFILE`` s from
+  :command:`add_custom_command`. See policy :policy:`CMP0116` for details.

Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst

@@ -27,6 +27,8 @@ warn by default:
   policy :policy:`CMP0102`.
 * ``CMAKE_POLICY_WARNING_CMP0112`` controls the warning for
   policy :policy:`CMP0112`.
+* ``CMAKE_POLICY_WARNING_CMP0116`` controls the warning for
+  policy :policy:`CMP0116`.
 
 This variable should not be set by a project in CMake code.  Project
 developers running CMake may set this variable in their cache to

Source/cmGlobalNinjaGenerator.h

@@ -24,6 +24,7 @@
 #include "cmNinjaTypes.h"
 #include "cmPolicies.h"
 #include "cmStringAlgorithms.h"
+#include "cmTransformDepfile.h"
 
 class cmCustomCommand;
 class cmGeneratorTarget;
@@ -211,6 +212,10 @@ public:
   const char* GetCleanTargetName() const override { return "clean"; }
 
   bool SupportsCustomCommandDepfile() const override { return true; }
+  cm::optional<cmDepfileFormat> DepfileFormat() const override
+  {
+    return cmDepfileFormat::GccDepfile;
+  }
 
   virtual cmGeneratedFileStream* GetImplFileStream(
     const std::string& /*config*/) const

Source/cmLocalNinjaGenerator.cxx

@@ -22,7 +22,9 @@
 #include "cmGlobalNinjaGenerator.h"
 #include "cmLocalGenerator.h"
 #include "cmMakefile.h"
+#include "cmMessageType.h"
 #include "cmNinjaTargetGenerator.h"
+#include "cmPolicies.h"
 #include "cmProperty.h"
 #include "cmRulePlaceholderExpander.h"
 #include "cmSourceFile.h"
@@ -573,7 +575,20 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement(
     return;
   }
 
-  cmCustomCommandGenerator ccg(*cc, config, this);
+  bool transformDepfile = false;
+  auto cmp0116 = this->GetPolicyStatus(cmPolicies::CMP0116);
+  switch (cmp0116) {
+    case cmPolicies::OLD:
+    case cmPolicies::WARN:
+      break;
+    case cmPolicies::REQUIRED_IF_USED:
+    case cmPolicies::REQUIRED_ALWAYS:
+    case cmPolicies::NEW:
+      transformDepfile = true;
+      break;
+  }
+
+  cmCustomCommandGenerator ccg(*cc, config, this, transformDepfile);
 
   const std::vector<std::string>& outputs = ccg.GetOutputs();
   const std::vector<std::string>& byproducts = ccg.GetByproducts();
@@ -623,10 +638,36 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement(
     cmCryptoHash hash(cmCryptoHash::AlgoSHA256);
     customStep += hash.HashString(ninjaOutputs[0]).substr(0, 7);
 
+    std::string depfile = cc->GetDepfile();
+    if (!depfile.empty()) {
+      switch (cmp0116) {
+        case cmPolicies::WARN:
+          if (this->GetCurrentBinaryDirectory() !=
+                this->GetBinaryDirectory() ||
+              this->Makefile->PolicyOptionalWarningEnabled(
+                "CMAKE_POLICY_WARNING_CMP0116")) {
+            this->GetCMakeInstance()->IssueMessage(
+              MessageType::AUTHOR_WARNING,
+              cmPolicies::GetPolicyWarning(cmPolicies::CMP0116),
+              cc->GetBacktrace());
+          }
+          CM_FALLTHROUGH;
+        case cmPolicies::OLD:
+          break;
+        case cmPolicies::REQUIRED_IF_USED:
+        case cmPolicies::REQUIRED_ALWAYS:
+        case cmPolicies::NEW:
+          cmSystemTools::MakeDirectory(
+            cmStrCat(this->GetBinaryDirectory(), "/CMakeFiles/d"));
+          depfile = ccg.GetInternalDepfile();
+          break;
+      }
+    }
+
     gg->WriteCustomCommandBuild(
       this->BuildCommandLine(cmdLines, customStep),
       this->ConstructComment(ccg), "Custom command for " + ninjaOutputs[0],
-      cc->GetDepfile(), cc->GetJobPool(), cc->GetUsesTerminal(),
+      depfile, cc->GetJobPool(), cc->GetUsesTerminal(),
       /*restat*/ !symbolic || !byproducts.empty(), ninjaOutputs, config,
       ninjaDeps, orderOnlyDeps);
   }

Source/cmPolicies.h

@@ -342,7 +342,10 @@ class cmMakefile;
          "ExternalProject step targets fully adopt their steps.", 3, 19, 0,   \
          cmPolicies::WARN)                                                    \
   SELECT(POLICY, CMP0115, "Source file extensions must be explicit.", 3, 20,  \
-         0, cmPolicies::WARN)
+         0, cmPolicies::WARN)                                                 \
+  SELECT(POLICY, CMP0116,                                                     \
+         "Ninja generators transform DEPFILEs from add_custom_command().", 3, \
+         20, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \

Tests/RunCMake/CMP0116/CMP0116-NEW-NOWARN.cmake

@@ -0,0 +1,3 @@
+set(depdir)
+
+include(Common.cmake)

Tests/RunCMake/CMP0116/CMP0116-NEW-WARN.cmake

@@ -0,0 +1,3 @@
+set(depdir)
+
+include(Common.cmake)

Tests/RunCMake/CMP0116/CMP0116-OLD-NOWARN.cmake

@@ -0,0 +1,3 @@
+set(depdir Subdirectory/)
+
+include(Common.cmake)

Tests/RunCMake/CMP0116/CMP0116-OLD-WARN.cmake

@@ -0,0 +1,3 @@
+set(depdir Subdirectory/)
+
+include(Common.cmake)

Tests/RunCMake/CMP0116/CMP0116-WARN-NOWARN-stderr.txt

@@ -0,0 +1,7 @@
+^(CMake Warning \(dev\) at Subdirectory/CMakeLists\.txt:[0-9]+ \(add_custom_command\):
+  Policy CMP0116 is not set: Ninja generators transform DEPFILEs from
+  add_custom_command\(\)\.  Run "cmake --help-policy CMP0116" for policy
+  details\.  Use the cmake_policy command to set the policy and suppress this
+  warning\.
+This warning is for project developers\.  Use -Wno-dev to suppress it\.
+*)+$

Tests/RunCMake/CMP0116/CMP0116-WARN-NOWARN.cmake

@@ -0,0 +1,3 @@
+set(depdir Subdirectory/)
+
+include(Common.cmake)

Tests/RunCMake/CMP0116/CMP0116-WARN-WARN-stderr.txt

@@ -0,0 +1,16 @@
+^(CMake Warning \(dev\) at Common\.cmake:[0-9]+ \(add_custom_command\):
+  Policy CMP0116 is not set: Ninja generators transform DEPFILEs from
+  add_custom_command\(\)\.  Run "cmake --help-policy CMP0116" for policy
+  details\.  Use the cmake_policy command to set the policy and suppress this
+  warning\.
+Call Stack \(most recent call first\):
+  CMP0116-WARN-WARN\.cmake:[0-9]+ \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.
++)+(CMake Warning \(dev\) at Subdirectory/CMakeLists\.txt:[0-9]+ \(add_custom_command\):
+  Policy CMP0116 is not set: Ninja generators transform DEPFILEs from
+  add_custom_command\(\)\.  Run "cmake --help-policy CMP0116" for policy
+  details\.  Use the cmake_policy command to set the policy and suppress this
+  warning\.
+This warning is for project developers\.  Use -Wno-dev to suppress it\.
+*)+$

Tests/RunCMake/CMP0116/CMP0116-WARN-WARN.cmake

@@ -0,0 +1,3 @@
+set(depdir Subdirectory/)
+
+include(Common.cmake)

Tests/RunCMake/CMP0116/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.18)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)

Tests/RunCMake/CMP0116/Common.cmake

@@ -0,0 +1,8 @@
+add_custom_command(
+  OUTPUT top.txt
+  COMMAND ${CMAKE_COMMAND} -DOUTFILE=top.txt -DINFILE=topdep.txt -DDEPFILE=top.txt.d -DSTAMPFILE=topstamp.txt -DDEPDIR= -P ${CMAKE_SOURCE_DIR}/WriteDepfile.cmake
+  DEPFILE top.txt.d
+  )
+add_custom_target(top ALL DEPENDS top.txt)
+
+add_subdirectory(Subdirectory)

Tests/RunCMake/CMP0116/RunCMakeTest.cmake

@@ -0,0 +1,49 @@
+include(RunCMake)
+
+function(run_cmp0116 status warn)
+  if(warn)
+    set(name CMP0116-${status}-WARN)
+  else()
+    set(name CMP0116-${status}-NOWARN)
+  endif()
+  set(RunCMake_TEST_OPTIONS
+    -DCMAKE_POLICY_WARNING_CMP0116:BOOL=${warn}
+    )
+  if(NOT status STREQUAL "WARN")
+    list(APPEND RunCMake_TEST_OPTIONS
+      -DCMAKE_POLICY_DEFAULT_CMP0116:STRING=${status}
+      )
+  endif()
+
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${name}-build)
+  run_cmake(${name})
+  unset(RunCMake_TEST_OPTIONS)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  set(RunCMake-check-file check.cmake)
+
+  file(TOUCH "${RunCMake_TEST_BINARY_DIR}/topdep.txt")
+  file(TOUCH "${RunCMake_TEST_BINARY_DIR}/Subdirectory/subdep.txt")
+  set(cmp0116_step 1)
+  run_cmake_command(${name}-build1 ${CMAKE_COMMAND} --build . --config Debug)
+  file(REMOVE "${RunCMake_TEST_BINARY_DIR}/topstamp.txt")
+  file(REMOVE "${RunCMake_TEST_BINARY_DIR}/Subdirectory/substamp.txt")
+  execute_process(COMMAND ${CMAKE_COMMAND} -E sleep 1.25)
+
+  file(TOUCH "${RunCMake_TEST_BINARY_DIR}/topdep.txt")
+  file(TOUCH "${RunCMake_TEST_BINARY_DIR}/Subdirectory/subdep.txt")
+  set(cmp0116_step 2)
+  run_cmake_command(${name}-build2 ${CMAKE_COMMAND} --build . --config Debug)
+  file(REMOVE "${RunCMake_TEST_BINARY_DIR}/topstamp.txt")
+  file(REMOVE "${RunCMake_TEST_BINARY_DIR}/Subdirectory/substamp.txt")
+  execute_process(COMMAND ${CMAKE_COMMAND} -E sleep 1.25)
+
+  set(cmp0116_step 3)
+  run_cmake_command(${name}-build3 ${CMAKE_COMMAND} --build . --config Debug)
+endfunction()
+
+run_cmp0116(WARN OFF)
+run_cmp0116(OLD OFF)
+run_cmp0116(NEW OFF)
+run_cmp0116(WARN ON)
+run_cmp0116(OLD ON)
+run_cmp0116(NEW ON)

Tests/RunCMake/CMP0116/Subdirectory/CMakeLists.txt

@@ -0,0 +1,6 @@
+add_custom_command(
+  OUTPUT sub.txt
+  COMMAND ${CMAKE_COMMAND} -DOUTFILE=sub.txt -DINFILE=subdep.txt -DDEPFILE=sub.txt.d -DSTAMPFILE=substamp.txt -DDEPDIR=${depdir} -P ${CMAKE_SOURCE_DIR}/WriteDepfile.cmake
+  DEPFILE ${depdir}sub.txt.d
+  )
+add_custom_target(sub ALL DEPENDS sub.txt)

Tests/RunCMake/CMP0116/WriteDepfile.cmake

@@ -0,0 +1,3 @@
+file(TOUCH "${OUTFILE}")
+file(TOUCH "${STAMPFILE}")
+file(WRITE "${DEPFILE}" "${DEPDIR}${OUTFILE}: ${DEPDIR}${INFILE}\n")

Tests/RunCMake/CMP0116/check.cmake

@@ -0,0 +1,18 @@
+function(check_exists file)
+  if(NOT EXISTS "${file}")
+    string(APPEND RunCMake_TEST_FAILED "${file} does not exist\n")
+  endif()
+  set(RunCMake_TEST_FAILED "${RunCMake_TEST_FAILED}" PARENT_SCOPE)
+endfunction()
+
+function(check_not_exists file)
+  if(EXISTS "${file}")
+    string(APPEND RunCMake_TEST_FAILED "${file} exists\n")
+  endif()
+  set(RunCMake_TEST_FAILED "${RunCMake_TEST_FAILED}" PARENT_SCOPE)
+endfunction()
+
+if(cmp0116_step EQUAL 3)
+  check_not_exists("${RunCMake_TEST_BINARY_DIR}/topstamp.txt")
+  check_not_exists("${RunCMake_TEST_BINARY_DIR}/Subdirectory/substamp.txt")
+endif()

Tests/RunCMake/CMakeLists.txt

@@ -126,6 +126,9 @@ endif()
 add_RunCMake_test(CMP0106)
 add_RunCMake_test(CMP0111)
 add_RunCMake_test(CMP0115)
+if(CMAKE_GENERATOR MATCHES "Ninja")
+  add_RunCMake_test(CMP0116)
+endif()
 
 # The test for Policy 65 requires the use of the
 # CMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS variable, which both the VS and Xcode