Page MenuHomePhabricator

Link NetBSD with execinfo (CMAKE build)
ClosedPublic

Authored by krytarowski on Sep 9 2015, 6:56 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

krytarowski updated this revision to Diff 34401.Sep 9 2015, 6:56 PM
krytarowski retitled this revision from to Link NetBSD with execinfo (CMAKE build).
krytarowski updated this object.
krytarowski added a reviewer: joerg.
krytarowski set the repository for this revision to rL LLVM.
krytarowski added a subscriber: lldb-commits.
sas requested changes to this revision.Sep 10 2015, 10:00 AM
sas added a reviewer: sas.
sas added a subscriber: sas.
sas added inline comments.
cmake/LLDBDependencies.cmake
152

Looks like you have a typo. MAKE_SYSTEM_NAME vs. CMAKE_SYSTEM_NAME.

This revision now requires changes to proceed.Sep 10 2015, 10:00 AM
krytarowski edited edge metadata.

typo

sas accepted this revision.Sep 10 2015, 2:42 PM
sas edited edge metadata.
This revision is now accepted and ready to land.Sep 10 2015, 2:42 PM
brucem requested changes to this revision.Sep 10 2015, 5:53 PM
brucem added a reviewer: brucem.
brucem added a subscriber: brucem.
brucem added inline comments.
cmake/LLDBDependencies.cmake
152

This should really be replaced with a cmake-style check for where to find backtrace.

cmake 3.0 added a built-in package to handle this, but I think we still support an older cmake:

http://www.cmake.org/cmake/help/v3.0/module/FindBacktrace.html

Since this is how the cmake build finds this, we don't need to worry about having other build systems here and we can do this in the correct way for cmake.

This revision now requires changes to proceed.Sep 10 2015, 5:53 PM
sas added a comment.Sep 11 2015, 11:52 AM

@brucem, not sure exactly what way forward you are suggesting. LLDB build system supports cmake 2.8.12 and up, so we can't use FindBacktrace unless we bump that to 3.0. Are we recommending we do that?

In D12750#244435, @sas wrote:

@brucem, not sure exactly what way forward you are suggesting. LLDB build system supports cmake 2.8.12 and up, so we can't use FindBacktrace unless we bump that to 3.0. Are we recommending we do that?

Can we do this later, after merging this change at it is? For now I won't be able to test the new approach.

sas accepted this revision.Sep 17 2015, 5:29 PM

I'm fine with this patch the way it is. Unless @brucem refuses, I'll submit it. Improving cmake checks can be dealt with later on.

In D12750#244435, @sas wrote:

@brucem, not sure exactly what way forward you are suggesting. LLDB build system supports cmake 2.8.12 and up, so we can't use FindBacktrace unless we bump that to 3.0. Are we recommending we do that?

Can we do this later, after merging this change at it is? For now I won't be able to test the new approach.

In D12750#248348, @sas wrote:

I'm fine with this patch the way it is. Unless @brucem refuses, I'll submit it. Improving cmake checks can be dealt with later on.

We have got CMake 2.8 set as the minimal requirement.

cmake_minimum_required(VERSION 2.8)

I'm willing to push NetBSD bits first and then refactor for all platforms.

Are you OK with it @brucem?

brucem accepted this revision.Sep 17 2015, 6:18 PM
brucem edited edge metadata.

We can do that other part later ... we don't have to require newer cmake to do it, just provide our own file that does something similar until we update to newer cmake.

This revision is now accepted and ready to land.Sep 17 2015, 6:18 PM
sas closed this revision.Sep 18 2015, 10:17 AM

Committed as r248007.