[compiler-rt] [cmake] Create dummy gtest target for stand-alone builds

[mgorny]

Summary

Create a dummy 'gtest' target to satisfy the test dependencies while
doing stand-alone builds. Historically those dependencies were
conditional to non-stand-alone builds but the refactoring
in rL310971 has made them unconditional, breaking stand-alone builds
as a result.

Given the added complexity of maintaining a large number of conditionals
scattered around the different CMakeLists.txt files and the likeliness
of people mistakenly adding more unconditional dependencies, adding
a dummy target seems an easier way forward.

[george.karpenkov]

breaking stand-alone builds as a result

That's a strong statement. Could you clarify? We have a lot of buildbots performing standalone builds, and they are still green.

and the likeliness of people mistakenly adding more unconditional dependencies

That's a good point, however I'm not sure how your change would fix the problem.
As far as I remember targets in compiler-rt had quite a few dependencies which required checking for whether it is a standalone build.

In general, I'm not sure what this change would achieve, and the added complexity can always cause more bugs in the future.

[mgorny]

In D38444#886138, @george.karpenkov wrote:

breaking stand-alone builds as a result

That's a strong statement. Could you clarify? We have a lot of buildbots performing standalone builds, and they are still green.

I didn't know anyone actually added bots doing that. Are you sure we're talking about the same meaning of 'stand-alone'? Stand-alone == out of LLVM, against installed copy of LLVM.

ninja -v -j16 -l0 check-all
ninja: error: '/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999/lib/asan/tests/gtest', needed by 'lib/asan/tests/dynamic/Asan-i386-calls-Dynamic-Test', missing and no known rule to make it

It's as broken as it could be since it depends on target that does not exist.

and the likeliness of people mistakenly adding more unconditional dependencies

That's a good point, however I'm not sure how your change would fix the problem.
As far as I remember targets in compiler-rt had quite a few dependencies which required checking for whether it is a standalone build.

In general, I'm not sure what this change would achieve, and the added complexity can always cause more bugs in the future.

My goal is to make things work again.

[george.karpenkov]

@mgorny I've replied via email, but the message didn't seem to appear here.

From my (maybe limited) understanding, running tests on standalone compiler-rt builds was never something which was supported, as that required a fresh compiler.
I've just tried running them, and for me even check-* targets don't exist.

How do you create the standalone build? I've checked out compiler-rt separately, and ran

cmake -GNinja -DLLVM_CONFIG=/Users/george/code/llvm/release-build/bin/llvm-config  ../.

but the resulting build directory does not have any check-* targets.

In the refactoring I did try to make the dependence conditional: all compilation goes through sanitizer_test_compile,
but looking into the sanitizer_test_compile macro there is an obvious bug.
Would it be better to fix that instead?

[mgorny]

In D38444#895403, @george.karpenkov wrote:

@mgorny I've replied via email, but the message didn't seem to appear here.

From my (maybe limited) understanding, running tests on standalone compiler-rt builds was never something which was supported, as that required a fresh compiler.
I've just tried running them, and for me even check-* targets don't exist.

How do you create the standalone build? I've checked out compiler-rt separately, and ran

cmake -GNinja -DLLVM_CONFIG=/Users/george/code/llvm/release-build/bin/llvm-config  ../.

Our command-line is:

cmake -C /var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999_build/gentoo_common_config.cmake -G Ninja -DCMAKE_INSTALL_PREFIX=/usr -DCOMPILER_RT_INSTALL_PATH=/usr/lib/clang/6.0.0 -DCOMPILER_RT_OUTPUT_DIR=/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999_build/lib/clang/6.0.0 -DCOMPILER_RT_INCLUDE_TESTS=yes -DCOMPILER_RT_BUILD_BUILTINS=OFF -DCOMPILER_RT_BUILD_LIBFUZZER=ON -DCOMPILER_RT_BUILD_SANITIZERS=ON -DCOMPILER_RT_BUILD_XRAY=ON -DLLVM_MAIN_SRC_DIR=/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/llvm -DLLVM_EXTERNAL_LIT=/usr/bin/lit -DLLVM_LIT_ARGS=-vv -DCOMPILER_RT_TEST_COMPILER=/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999_build/lib/llvm/6/bin/clang -DCOMPILER_RT_TEST_CXX_COMPILER=/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999_build/lib/llvm/6/bin/clang++ -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_USER_MAKE_RULES_OVERRIDE=/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999_build/gentoo_rules.cmake -DCMAKE_TOOLCHAIN_FILE=/var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999_build/gentoo_toolchain.cmake  /var/tmp/portage/sys-libs/compiler-rt-sanitizers-9999/work/compiler-rt-sanitizers-9999

but the resulting build directory does not have any check-* targets.

I think the -DCOMPILER_RT_INCLUDE_TESTS=yes may be relevant here.

In the refactoring I did try to make the dependence conditional: all compilation goes through sanitizer_test_compile,
but looking into the sanitizer_test_compile macro there is an obvious bug.
Would it be better to fix that instead?

Indeed there is. I'll check if fixing that solves the issue.

[mgorny]

Ok, I think I've found a better solution. Will start submitting patches soonish.

[mgorny]

D38838 addresses the typo. Now that I know how it's supposed to work, D38839 and D38840 address the issues, and fix it for me.

cmake/Modules/AddCompilerRT.cmake

 set(COMPILER_RT_GTEST_SOURCE ${COMPILER_RT_GTEST_PATH}/src/gtest-all.cc)
 set(COMPILER_RT_GTEST_CFLAGS
   -DGTEST_NO_LLVM_RAW_OSTREAM=1
   -DGTEST_HAS_RTTI=0
   -I${COMPILER_RT_GTEST_PATH}/include
   -I${COMPILER_RT_GTEST_PATH}
 )
 
+if(COMPILER_RT_STANDALONE_BUILD)
+  # Add a dummy target to satisfy dependencies on GTest when building
+  # stand-alone. GTest itself is provided via COMPILER_RT_GTEST_SOURCES.
+  add_custom_target(gtest)
+endif()
+
 append_list_if(COMPILER_RT_DEBUG -DSANITIZER_DEBUG=1 COMPILER_RT_UNITTEST_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WCOVERED_SWITCH_DEFAULT_FLAG -Wno-covered-switch-default COMPILER_RT_UNITTEST_CFLAGS)
 
 if(MSVC)
   # clang doesn't support exceptions on Windows yet.
   list(APPEND COMPILER_RT_UNITTEST_CFLAGS -D_HAS_EXCEPTIONS=0)
 
   # We should teach clang to understand "#pragma intrinsic", see PR19898.