Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-doc][cmake] Copy assets to build directory #95187

Merged
merged 3 commits into from
Jun 14, 2024
Merged

[clang-doc][cmake] Copy assets to build directory #95187

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
753c6d4
[clang-doc][cmake] Copy assets to build directory
ilovepi Jun 12, 2024
d250a9a
Use helper utility to copy files
ilovepi Jun 13, 2024
c989ca3
Remove message argument
ilovepi Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions clang-tools-extra/clang-doc/tool/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,38 @@ target_link_libraries(clang-doc
clangDoc
)

install(FILES ../assets/clang-doc-default-stylesheet.css
DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
COMPONENT clang-doc)

install(FILES ../assets/index.js
DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
COMPONENT clang-doc)
set(assets
index.js
clang-doc-default-stylesheet.css
)

set(asset_dir "${CMAKE_CURRENT_SOURCE_DIR}/../assets")
set(resource_dir "${CMAKE_BINARY_DIR}/share/clang")
set(out_files)

function(copy_files_to_dst src_dir dst_dir file)
set(src "${src_dir}/${file}")
set(dst "${dst_dir}/${file}")
add_custom_command(OUTPUT ${dst}
DEPENDS ${src}
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
COMMENT "Copying ${file} to ${dst_dir}"
)
list(APPEND out_files ${dst})
set(out_files ${out_files} PARENT_SCOPE)
endfunction(copy_files_to_dst)

foreach(f ${assets})
install(FILES ${asset_dir}/${f}
DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
COMPONENT clang-doc)
copy_files_to_dst(${asset_dir} ${resource_dir} ${f})
endforeach(f)

add_custom_target(copy-clang-doc-assets
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer having a rule per file since most systems don't have a reliable way to tell if the directory content has changed. The approach we usually use to to have one add_custom_command per file with a single add_custom_target that depends on all output files, see for example

llvm-project/libcxx/include/CMakeLists.txt

Lines 1024 to 1043 in 092dbfa

foreach(f ${files})
set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}")
set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}")
add_custom_command(OUTPUT ${dst}
DEPENDS ${src}
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
COMMENT "Copying CXX header ${f}")
list(APPEND _all_includes "${dst}")
endforeach()
# Generate the IWYU mapping. This depends on all header files but it's also considered as an
# "include" for dependency tracking.
add_custom_command(OUTPUT "${LIBCXX_GENERATED_INCLUDE_DIR}/libcxx.imp"
COMMAND "${Python3_EXECUTABLE}" "${LIBCXX_SOURCE_DIR}/utils/generate_iwyu_mapping.py" "-o" "${LIBCXX_GENERATED_INCLUDE_DIR}/libcxx.imp"
DEPENDS ${_all_includes}
COMMENT "Generate the mapping file for include-what-you-use"
)
list(APPEND _all_includes "${LIBCXX_GENERATED_INCLUDE_DIR}/libcxx.imp")
add_custom_target(generate-cxx-headers ALL DEPENDS ${_all_includes})
or https://github.com/llvm/llvm-project/blob/092dbfaad257885692fa64559e9eb43a5c466798/clang/lib/Headers/CMakeLists.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I’ll take a pass at adapting one of those for use here. If I think I can generalize it, I’ll make another PR to add it as a utility in the llvm cmake module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is addressed now. I more or less cribed directly from the clang header example. Initially I tried to have the function be a bit more generic and put it in llvm/cmake/modules/CopyLLVMFiles.cmake, but my normal methods of including don't seem to work in clang-doc. Likely I'm just holding that part wrong, but I don't see anything in clang-tools-extra using include(AddLLVM) or some other module.

Happy to refactor if you have some other suggestions.

DEPENDS ${out_files}
COMMENT "Copying Clang-Doc Assets"
)
set_target_properties(copy-clang-doc-assets PROPERTIES FOLDER "Clang-Doc/Assets")
add_dependencies(clang-doc copy-clang-doc-assets)
Loading