[CMake] Fix some breakages when using ninja multi config #65451
Conversation
|
@llvmbot Is there any mechanism to classify this PR? |
|
@EugeneZelenko Is there any mechanism to classify and deal with this PR? |
EugeneZelenko
added
cmake
|
@ur4t: You could also look into file history and add some of developers who changed file in past as reviewers. |
|
@mgorny It seems that the most recently modification on |
|
I'm not really an expert on this. @MaskRay perhaps? |
|
@MaskRay Would you like to participate in reviewing this PR? FYI, building This PR fixes this issue #63800 by adding multiple configurations compatibility like other |
I am not familiar with CMake... but this patch does address a problem. I suggest that you add to the description the following instruction and also some description from #63800.
Use the preciser term |
llvm/tools/llvm-shlib/CMakeLists.txt
Outdated
| ${CMAKE_CURRENT_SOURCE_DIR}/simple_version_script.map.in | ||
| ${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map) | ||
|
|
||
| if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".") |
There was a problem hiding this comment.
Generally we prefer
if (...) {
} else {
}
over
if (!...) {
} else {
}
CMake is similar.
There was a problem hiding this comment.
The "if" style is adjusted in 72e1dc0.
MaskRay
changed the title
|
@MaskRay This tiny PR has been staled for months even years. Could we please merge it? |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-x86 Author: None (ur4t) ChangesWhen using multi-config generator to build This patch adds multi-config compatibility when configuring Fixes #63800. Full diff: https://github.com/llvm/llvm-project/pull/65451.diff 2 Files Affected:
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index 43124111b7ba5..a3a505bcb7f88 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -395,12 +395,25 @@ set(riscv_generated_files)
function(copy_header_to_output_dir src_dir file)
set(src ${src_dir}/${file})
- set(dst ${output_dir}/${file})
- add_custom_command(OUTPUT ${dst}
- DEPENDS ${src}
- COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
- COMMENT "Copying clang's ${file}...")
- list(APPEND out_files ${dst})
+ if("${CMAKE_CFG_INTDIR}" STREQUAL ".")
+ set(dst ${output_dir}/${file})
+ add_custom_command(OUTPUT ${dst}
+ DEPENDS ${src}
+ COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
+ COMMENT "Copying clang's ${file}...")
+ list(APPEND out_files ${dst})
+ else()
+ foreach(BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
+ # Replace the special string with a per config directory.
+ string(REPLACE ${CMAKE_CFG_INTDIR} ${BUILD_MODE} per_conf_output_dir ${output_dir})
+ set(dst ${per_conf_output_dir}/${file})
+ add_custom_command(OUTPUT ${dst}
+ DEPENDS ${src}
+ COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
+ COMMENT "Copying clang's ${file}...")
+ list(APPEND out_files ${dst})
+ endforeach()
+ endif()
set(out_files ${out_files} PARENT_SCOPE)
endfunction(copy_header_to_output_dir)
diff --git a/llvm/tools/llvm-shlib/CMakeLists.txt b/llvm/tools/llvm-shlib/CMakeLists.txt
index ede3c5034e045..a5b0cab0f1ce5 100644
--- a/llvm/tools/llvm-shlib/CMakeLists.txt
+++ b/llvm/tools/llvm-shlib/CMakeLists.txt
@@ -45,10 +45,19 @@ if(LLVM_BUILD_LLVM_DYLIB)
if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
set(LIB_NAMES -Wl,-all_load ${LIB_NAMES})
else()
- configure_file(
- ${CMAKE_CURRENT_SOURCE_DIR}/simple_version_script.map.in
- ${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map)
-
+ if("${CMAKE_CFG_INTDIR}" STREQUAL ".")
+ configure_file(
+ ${CMAKE_CURRENT_SOURCE_DIR}/simple_version_script.map.in
+ ${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map)
+ else()
+ foreach(BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
+ # Replace the special string with a per config directory.
+ string(REPLACE ${CMAKE_CFG_INTDIR} ${BUILD_MODE} PER_CONF_LIBRARY_DIR ${LLVM_LIBRARY_DIR})
+ configure_file(
+ ${CMAKE_CURRENT_SOURCE_DIR}/simple_version_script.map.in
+ ${PER_CONF_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map)
+ endforeach()
+ endif()
if(MSVC)
target_link_directories(LLVM PRIVATE ${LLVM_LIBRARY_DIR})
foreach(library ${LIB_NAMES})
@@ -156,7 +165,10 @@ if(LLVM_BUILD_LLVM_C_DYLIB AND MSVC)
# Need to separate lib names with newlines.
string(REPLACE ";" "\n" FILE_CONTENT "${FULL_LIB_NAMES}")
- if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+ if("${CMAKE_CFG_INTDIR}" STREQUAL ".")
+ # Write out the full lib names into file to be read by the python script.
+ file(WRITE ${LIBSFILE} "${FILE_CONTENT}")
+ else()
foreach(BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
# Replace the special string with a per config directory.
string(REPLACE ${CMAKE_CFG_INTDIR} ${BUILD_MODE} PER_CONF_CONTENT "${FILE_CONTENT}")
@@ -166,9 +178,6 @@ if(LLVM_BUILD_LLVM_C_DYLIB AND MSVC)
# ${CMAKE_CFG_INTDIR} correctly and select the right one.
file(WRITE ${LLVM_BINARY_DIR}/${BUILD_MODE}/libllvm-c.args "${PER_CONF_CONTENT}")
endforeach()
- else()
- # Write out the full lib names into file to be read by the python script.
- file(WRITE ${LIBSFILE} "${FILE_CONTENT}")
endif()
# Generate the exports file dynamically.
|
ur4t
changed the title
|
Could you rebase this patch? In addition, please unselect the hidden email feature per suggestion on https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it/74223 |
|
@MaskRay I have unselected the hidden email feature and updated the branch. Is it necessary to reset email of my old commits? |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/16701 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2342 Here is the relevant piece of the build log for the reference |
ur4t
deleted the
fix-llvm-shlib-ninja-multi-config-github-issue-63800
branch
When using multi-config generator to build
libLLVM.solikecmake -G 'Ninja Multi-Config' -Sllvm -B/tmp/out/ninja-multi -DCMAKE_CONFIGURATION_TYPES='Debug;Release' -DLLVM_LINK_LLVM_DYLIB=on -DLLVM_TARGETS_TO_BUILD=host && cmake --build /tmp/out/ninja-multi --config Debug,lldcomplainserror: cannot find version script /tmp/out/ninja-multi/Debug/lib/tools/llvm-shlib/simple_version_script.map.This patch adds multi-config compatibility when configuring
simple_version_script.map.Fixes #63800.
When using multi-config generator, clang's headers is not copied to proper directories, which is fixed as well.