瀏覽代碼

Clang-CXX: copy into the dyndep output on success

When `clang-scan-deps` fails to scan (e.g., bad source syntax, junk
flags, etc.), the redirection unconditionally updates the file. If this
fails, the `.ddi` file timestamp is updated. If the state is then
reverted (e.g., the command line returns to the state of the last
successful build), the updated file is not useful, but `ninja` does not
rerun because:

- the command hash matches the last successful run
- the output file is newer than its inputs

However, since the `.ddi` file has been updated with bogus contents from
a failed scan, collation fails as the `rules` array is empty (or
incomplete from a batch scan).

If `clang-scan-deps` were properly aware of its output file, it could
use this to not write the file if any inner scan fails. Requested in
https://github.com/llvm/llvm-project/issues/72875.

See: https://github.com/llvm/llvm-project/issues/72875
Fixes: #25429
Ben Boeckel 1 年之前
父節點
當前提交
77a7edb73f
共有 2 個文件被更改,包括 18 次插入3 次删除
  1. 16 1
      Modules/Compiler/Clang-CXX.cmake
  2. 2 2
      Tests/RunCMake/CXXModules/examples/scan_properties/CMakeLists.txt

+ 16 - 1
Modules/Compiler/Clang-CXX.cmake

@@ -32,6 +32,13 @@ endif()
 
 if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 16.0)
   if("x${CMAKE_CXX_COMPILER_FRONTEND_VARIANT}" STREQUAL "xGNU")
+    if (CMAKE_HOST_WIN32)
+      # `rename` doesn't overwrite and doesn't retry in case of "target file is
+      # busy".
+      set(_clang_scan_deps_mv "\"${CMAKE_COMMAND}\" -E rename")
+    else ()
+      set(_clang_scan_deps_mv "mv")
+    endif ()
     string(CONCAT CMAKE_CXX_SCANDEP_SOURCE
       "\"${CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS}\""
       " -format=p1689"
@@ -40,7 +47,15 @@ if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 16.0)
       " -x c++ <SOURCE> -c -o <OBJECT>"
       " -MT <DYNDEP_FILE>"
       " -MD -MF <DEP_FILE>"
-      " > <DYNDEP_FILE>")
+      # Write to a temporary file. If the scan fails, we do not want to update
+      # the actual output file as `ninja` (at least) assumes that failed
+      # commands either delete or leave output files alone. See Issue#25419.
+      " > <DYNDEP_FILE>.tmp"
+      # We cannot use `copy_if_different` as the rule does not have a feature
+      # analogous to `ninja`'s `restat = 1`. It would also leave behind the
+      # `.tmp` file.
+      " && ${_clang_scan_deps_mv} <DYNDEP_FILE>.tmp <DYNDEP_FILE>")
+    unset(_clang_scan_deps_mv)
     set(CMAKE_CXX_MODULE_MAP_FORMAT "clang")
     set(CMAKE_CXX_MODULE_MAP_FLAG "@<MODULE_MAP_FILE>")
     set(CMAKE_CXX_MODULE_BMI_ONLY_FLAG "--precompile")

+ 2 - 2
Tests/RunCMake/CXXModules/examples/scan_properties/CMakeLists.txt

@@ -13,8 +13,8 @@ endif ()
 # if not.
 string(APPEND CMAKE_CXX_MODULE_MAP_FLAG
   " -DCMAKE_SCANNED_THIS_SOURCE")
-string(APPEND CMAKE_CXX_SCANDEP_SOURCE
-  " -DCMAKE_SCANNED_THIS_SOURCE")
+string(REPLACE "<DEFINES>" "<DEFINES> -DCMAKE_SCANNED_THIS_SOURCE"
+  CMAKE_CXX_SCANDEP_SOURCE "${CMAKE_CXX_SCANDEP_SOURCE}")
 
 set_property(SOURCE always_scan.cxx
   PROPERTY CXX_SCAN_FOR_MODULES 1)