[CMAKE] Fix 'clean' target not working

[pdhaliwal]

Summary

cmake was still considering the empty value of ${fake_version_inc}
even if it was not defined.

[lebedev.ri]

Thank you for looking into it, i also encountered ninja clean not working.

[phosek]

llvm/include/llvm/Support/CMakeLists.txt

26

Can you use ${generated_files} here as well for OUTPUT and avoid duplicating this command?

[pdhaliwal]

llvm/include/llvm/Support/CMakeLists.txt

26

Hi, I had initially tried using ${generated_files} for OUTPUT as well but then ninja started complaining with following error (only in cases where llvm_vc was not defined)

ninja: error: 'include/llvm/Support/VCSRevision.h', needed by 'include/llvm/Support/CMakeFiles/llvm_vcsrevision_h', missing and no known rule to make it

[vsapsai]

With this change ninja clean works without errors for me (also regular ninja, ninja install). Approving from the functional perspective only, don't know CMake good enough to say if it can benefit from any improvements.

[pdhaliwal]

@phosek Let me know if this is good to land.

[tstellar]

This commit appears to be causing an error when I run CMake:

CMake Error:

Running

 '/usr/bin/ninja-build' '-C' '/home/tstellar/llvm-project/llvm-build' '-t' 'cleandead'

failed with:

 ninja: error: remove(include/llvm/Support): Directory not empty

CMake Generate step failed. Build files cannot be regenerated correctly.

[lebedev.ri]

In D82847#2211661, @tstellar wrote:

This commit appears to be causing an error when I run CMake:

CMake Error:

Running

 '/usr/bin/ninja-build' '-C' '/home/tstellar/llvm-project/llvm-build' '-t' 'cleandead'

failed with:

 ninja: error: remove(include/llvm/Support): Directory not empty

CMake Generate step failed. Build files cannot be regenerated correctly.

That can't be it. I was seeing that very error before this patch,
which is why i was happy to see this patch, hoping that it would fix it.

[tstellar]

Reverting this patch seems to fix the error for me. Are you still seeing this same error in trunk?

[lebedev.ri]

In D82847#2211668, @tstellar wrote:

Reverting this patch seems to fix the error for me. Are you still seeing this same error in trunk?

Just tried, and nope, works fine now. @pdhaliwal - thanks, i think!
Perhaps it's cache invalidation issue, and you need a fresh build dir?

llvm/include/llvm/Support/CMakeLists.txt

if(LLVM_APPEND_VC_REV)
   # A fake version file and is not expected to exist. It is being used to
   # force regeneration of VCSRevision.h for source directory with no write
   # permission available.
   if (NOT llvm_vc)
     set(fake_version_inc "${CMAKE_CURRENT_BINARY_DIR}/__FakeVCSRevision.h")
   endif()
 endif()
 
+set(generated_files "${version_inc}")
+if (fake_version_inc)
+  list(APPEND generated_files "${fake_version_inc}")
+endif()
+
 # Create custom target to generate the VC revision include.
+if (fake_version_inc)
   add_custom_command(OUTPUT "${version_inc}" "${fake_version_inc}"
     DEPENDS "${llvm_vc}" "${generate_vcs_version_script}"
     COMMAND ${CMAKE_COMMAND} "-DNAMES=LLVM"
                              "-DLLVM_SOURCE_DIR=${llvm_source_dir}"
                              "-DHEADER_FILE=${version_inc}"
                              -P "${generate_vcs_version_script}")
+else()
+  add_custom_command(OUTPUT "${version_inc}"
+    DEPENDS "${llvm_vc}" "${generate_vcs_version_script}"
+    COMMAND ${CMAKE_COMMAND} "-DNAMES=LLVM"
+                             "-DLLVM_SOURCE_DIR=${llvm_source_dir}"
+                             "-DHEADER_FILE=${version_inc}"
+                             -P "${generate_vcs_version_script}")
+endif()
 
 # Mark the generated header as being generated.
 set_source_files_properties("${version_inc}"
   PROPERTIES GENERATED TRUE
              HEADER_FILE_ONLY TRUE)
 
-add_custom_target(llvm_vcsrevision_h ALL DEPENDS "${version_inc}" "${fake_version_inc}")
+add_custom_target(llvm_vcsrevision_h ALL DEPENDS "${generated_files}")
 set_target_properties(llvm_vcsrevision_h PROPERTIES FOLDER "Misc")