This is an archive of the discontinued LLVM Phabricator instance.

[CMake] runtimes test targets need to depend on LLVM tools
ClosedPublic

Authored by george.karpenkov on May 10 2017, 9:09 AM.

Details

Summary

Many of the test cases in the runtimes require LLVM's testing tools, to facilitate this working as expected we need to have all the test targets in the runtimes depend on the LLVM testing tools used in the runtimes.

The list of tools I've added here may not be the full or minimally correct list of tools, but this patch is a step in the right direction.

Event Timeline

beanz created this revision.May 10 2017, 9:09 AM
george.karpenkov commandeered this revision.Jul 7 2017, 11:12 AM
george.karpenkov updated this revision to Diff 105663.
george.karpenkov edited reviewers, added: beanz; removed: george.karpenkov.

The updated list should not be overridden.

beanz accepted this revision.Jul 10 2017, 9:19 AM

LGTM.

This revision is now accepted and ready to land.Jul 10 2017, 9:19 AM
This revision was automatically updated to reflect the committed changes.
phosek added a subscriber: phosek.Jul 10 2017, 5:36 PM

@george.karpenkov I was just rebasing D32816 on top of your change and while running tests, I saw errors in CMake. So I tried building building ToT and I'm still seeing the same error:

CMake Error at runtimes/CMakeLists.txt:311 (add_dependencies):
  The dependency target "asan" of target "check-runtimes" does not exist.


CMake Error at runtimes/CMakeLists.txt:311 (add_dependencies):
  The dependency target "asan" of target "check-unwind" does not exist.


CMake Error at runtimes/CMakeLists.txt:311 (add_dependencies):
  The dependency target "asan" of target "check-cxxabi" does not exist.


CMake Error at runtimes/CMakeLists.txt:311 (add_dependencies):
  The dependency target "asan" of target "check-cxx" does not exist.


CMake Error at runtimes/CMakeLists.txt:311 (add_dependencies):
  The dependency target "asan" of target "check-compiler-rt" does not exist.


CMake Error at runtimes/CMakeLists.txt:311 (add_dependencies):
  The dependency target "asan" of target "runtimes-test-depends" does not
  exist.

Looking at the error and your change, I'm not sure if asan should be in the RUNTIME_TEST_DEPENDS list, since asan target is only built as part of the runtimes build and as such isn't available as a dependency until after you've done a runtimes build.

@phosek probably you are right, sorry.

Do you mean it brakes if you put compiler-rt into runtimes out of projects?

Somehow though it works for me, but I'm not entirely sure what sequence of invocations is causing the build of the contents of projects directory.
I've added that as I was trying to move libfuzzer to runtimes, and its tests require asan to be present.

In any case, removing the line.

asan target is going to be defined in SUB_COMPONENTS list in Components.cmake file which is generated by the runtimes build, but runtimes are only built after everything else (that's why Component.cmake is marked as CMAKE_CONFIGURE_DEPENDS to tell CMake that this file is modified during the build). So this is going work if you're doing incremental build (when Components.cmake has already been generated), but it'll break in the clean build. It's pretty convoluted and non-obvious and I wish there was a better solution for this, but I haven't came up with anything better yet.

@phosek somehow, ninja still worked for me on a clean checkout (with projects/compiler-rt added).
But I agree with you it can be a problem, maybe with your invocation.
The question how to run tests in runtimes which depend on sanitizers remain.

beanz added inline comments.Jul 18 2017, 1:44 PM
llvm/trunk/runtimes/CMakeLists.txt
256 ↗(On Diff #105901)

This blob here actually ends up duplicating the targets. During configuration of the runtime targets we generate a file "Components.cmake" that lists the build and check targets for each runtime library, and that gets imported back into the top-level build via the SUB_COMPONENT_CHECK_TARGETS variable.

This is what causes the CMake configuration errors on builds, and we need to remove this.

beanz added inline comments.Jul 18 2017, 1:46 PM
llvm/trunk/runtimes/CMakeLists.txt
256 ↗(On Diff #105901)

Nvm. Please disregard this. I mixed up this patch with a different one.