[libc] Add a convenience CMake rule to add testsuites.

[sivachandra]

Summary

This rule helps avoid repeated setting of check-libc's dependency on the
various testsuites.

[abrachet]

This patch is fine but my build fails (unrelated to this patch)

make[3]: *** No rule to make target 'projects/libc/src/errno/CMakeFiles/__errno_location_objects.dir/errno_location.cpp.o', needed by 'projects/libc/src/errno/__errno_location_raw.o'.  Stop.

But this happened before applying this patch (and after too). On CMake 3.7 and 3.16. Does it build for you?

libc/test/src/errno/CMakeLists.txt

1

It seems like the rest of LLVM would call this LibcErrnoUnittests, but I don't care too much for the formatting just thought I would bring it up.

[sivachandra]

In D72353#1808713, @abrachet wrote:

This patch is fine but my build fails (unrelated to this patch)

make[3]: *** No rule to make target 'projects/libc/src/errno/CMakeFiles/__errno_location_objects.dir/errno_location.cpp.o', needed by 'projects/libc/src/errno/__errno_location_raw.o'.  Stop.

But this happened before applying this patch (and after too). On CMake 3.7 and 3.16. Does it build for you?

I confess I never built with make. I have always tested with Ninja, and it still works. I use CMake 3.13.4.

I tried with make, and indeed it fails in exactly the same fashion. Will dig into this separately. That it works with Ninja seems to indicate a CMake problem.

libc/test/src/errno/CMakeLists.txt

1

I picked this naming style from libcxx. It has targets with names cxx_abi_headers, cxx_external_threads, etc., and test targets like libcxx_test_objects.

TBF, I do not like either of these naming styles. I prefer target names which reflect the source layout. For example, one should not need to list a suites like this at all. Each directory in the test subtree should get its own suite automatically. Its fully qualified name should be something like libc.test.src.errno.all. Individual targets, like the errno_test listed below, will automatically be part of the suite for the directory and have a fully qualified name like libc.test.src.errno.errno_test.

It is possible to implement such a system in CMake, and a project where it was implemented successfully is this: https://github.com/google/pytype

[abrachet]

libc/test/src/errno/CMakeLists.txt

1

What do you think about something like I wrote up in D72383? It's pretty awkward for OS/architecture specific tests and doesn't handle it as well as here.

For reference, it changes the names to create these targets:

abrachet@Alexs-MacBook-Pro ~/D/l/build> ninja help | grep "libc\."
libc.test.config.linux.x86_64.all: phony
libc.test.config.linux.x86_64.syscall_test: phony
libc.test.src.errno.all: phony
libc.test.src.errno.errno_test: phony
libc.test.src.string.all: phony
libc.test.src.string.strcat_test: phony
libc.test.src.string.strcpy_test: phony
libc.test.src.sys.mman.all: phony
libc.test.src.sys.mman.mmap_test: phony

[sivachandra]

libc/test/src/errno/CMakeLists.txt

1

I think it captures the overall idea. Few details can be chiseled finer may be.

About the awkwardness, it is probably subjective. But, I tend to go with conventions which are explicit and deducible. For example, to run mmap test, I just do:

$> ninja libc.test.src.sys.mman.mmap_test

Likewise, to run all linux x86_64 tests we have very explicit target and I don't need to refer to the CMake files to see what target we need to run/build:

$> ninja libc.test.config.linux.x86_64.all

[tschuett]

Unfortunately, this fails on OSX when ${LIBC_TARGET_OS} == "darwin" in libc/test/config/CMakeLists.txt. Windows might have a similar problem. I have no intent building libc. It fails with having libc in LLVM_ENABLE_PROJECTS.

[sivachandra]

In D72353#1809629, @tschuett wrote:

Unfortunately, this fails on OSX when ${LIBC_TARGET_OS} == "darwin" in libc/test/config/CMakeLists.txt. Windows might have a similar problem. I have no intent building libc. It fails with having libc in LLVM_ENABLE_PROJECTS.

Darwin and Windows support is still to be done. We will need a darwin config directory and many other darwin pieces for this to work on darwin.

libc/cmake/modules/LLVMLibCRules.cmake

function(add_libc_unittest target_name)
   )
   if(LIBC_UNITTEST_SUITE)
     add_dependencies(
       ${LIBC_UNITTEST_SUITE}
       ${target_name}
     )
   endif()
 endfunction(add_libc_unittest)
+
+function(add_libc_testsuite suite_name)
+  add_custom_target(${suite_name})
+  add_dependencies(check-libc ${suite_name})
+endfunction(add_libc_testsuite)

libc/test/CMakeLists.txt

-add_custom_target(check_libc)
+add_custom_target(check-libc)
 
 add_subdirectory(config)
 add_subdirectory(src)

libc/test/config/CMakeLists.txt

-add_subdirectory(linux)
+add_subdirectory(${LIBC_TARGET_OS})

libc/test/config/linux/CMakeLists.txt

-add_custom_target(libc_linux_tests)
-add_dependencies(check_libc libc_linux_tests)
+add_libc_testsuite(libc_linux_tests)
 
-add_subdirectory(x86_64)
+add_subdirectory(${LIBC_TARGET_MACHINE})

libc/test/config/linux/x86_64/CMakeLists.txt

-add_custom_target(libc_linux_x86_64_tests)
-add_dependencies(libc_linux_tests libc_linux_x86_64_tests)
-
 add_libc_unittest(
   libc_linux_x86_64_syscall_unittest
-  SUITE libc_linux_x86_64_tests
+  SUITE libc_linux_tests
   SRCS syscall_test.cpp
   DEPENDS
    syscall_impl_h
    support_common_h
 )

libc/test/src/errno/CMakeLists.txt

-add_custom_target(libc_errno_unittests)
-add_dependencies(check_libc libc_errno_unittests)
+add_libc_testsuite(libc_errno_unittests)
 
 add_libc_unittest(
   errno_test
   SUITE
     libc_errno_unittests
   SRCS
     errno_test.cpp
   DEPENDS
     __errno_location
 )

libc/test/src/string/CMakeLists.txt

-add_custom_target(libc_string_unittests)
-add_dependencies(check_libc libc_string_unittests)
+add_libc_testsuite(libc_string_unittests)
 
 add_libc_unittest(
   strcat_test
   SUITE
     libc_string_unittests
   SRCS
     strcat_test.cpp
   DEPENDS

libc/test/src/sys/mman/CMakeLists.txt

-add_custom_target(libc_sys_mman_unittests)
-add_dependencies(check_libc libc_sys_mman_unittests)
+add_libc_testsuite(libc_sys_mman_unittests)
 
 add_libc_unittest(
   mmap_test
   SUITE
     libc_sys_mman_unittests
   SRCS
     mmap_test.cpp
   DEPENDS
     mmap
     munmap
     __errno_location
 )