-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[clang-doc] create a separate cmake file for clang-doc's lit tests #165935
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.
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
Conversation
|
@llvm/pr-subscribers-clang-tools-extra Author: Erick Velez (evelez7) ChangesTo avoid depending on all of the tools in clang-tools-extra, the Full diff: https://github.com/llvm/llvm-project/pull/165935.diff 2 Files Affected:
diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt
index a70d2ef2d92f2..78447e7a00db8 100644
--- a/clang-tools-extra/test/CMakeLists.txt
+++ b/clang-tools-extra/test/CMakeLists.txt
@@ -87,4 +87,7 @@ add_lit_testsuite(check-clang-extra "Running clang-tools-extra/test"
add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
DEPENDS ${CLANG_TOOLS_TEST_DEPS}
+ SKIP "^clang-doc"
)
+
+add_subdirectory(clang-doc)
diff --git a/clang-tools-extra/test/clang-doc/CMakeLists.txt b/clang-tools-extra/test/clang-doc/CMakeLists.txt
new file mode 100644
index 0000000000000..fd2230660bdef
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/CMakeLists.txt
@@ -0,0 +1,6 @@
+# Specialize the clang-doc target to avoid building other projects
+add_lit_testsuite(check-clang-extra-clang-doc "Running clang-doc tests"
+ ${CMAKE_CURRENT_BINARY_DIR}
+ DEPENDS clang-doc
+ DEPENDS ${LLVM_UTILS_DEPS}
+)
|
To avoid depending on all of the tools in clang-tools-extra, the `check-clang-extra-clang-doc` target is specialized in its own CMake file in clang-tools-extra/test/clang-doc. This eliminates around 800 files to be processed when building that target, plus linking every tool.
5cf1030 to
07c3be5
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
If we're doing that, should we also consider having our own unittests target? |
Yes, we should. I want to do that next. The thing is that the |
|
I think having a separate unittest target is also perfectly reasonable to do in a follow up patch. |
Right. I'm not suggesting its a requirement to land. just an observation that we probably should. |
|
|
||
| add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR} | ||
| DEPENDS ${CLANG_TOOLS_TEST_DEPS} | ||
| SKIP "^clang-doc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not something that needs to be done in this patch, but I think we should consider adding CMakeLists.txt files for all other subdirectories and then remove this target altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this as well. I opened #165820. I thought about closing it after merging this but I might keep it open. I don't know how tied the other projects are to that target and CMake file.
…lvm#165935) To avoid depending on all of the tools in clang-tools-extra, the `check-clang-extra-clang-doc` target is specialized in its own CMake file in clang-tools-extra/test/clang-doc. This eliminates around 800 files to be processed when building that target, plus linking every tool. Similar to [llvm#155929](llvm#155929).

To avoid depending on all of the tools in clang-tools-extra, the
check-clang-extra-clang-doctarget is specialized in its own CMakefile in clang-tools-extra/test/clang-doc. This eliminates around 800
files to be processed when building that target, plus linking every
tool. Similar to #155929.