This is an archive of the discontinued LLVM Phabricator instance.

Fix cmake build on OSX after r249434.
ClosedPublic

Authored by dawn on Oct 7 2015, 2:46 PM.

Details

Summary

This fixes the cmake build on OSX when building in a new workspace. It reverts r249434 and instead fixes the issue as proposed in lldb-dev in message 'cmake question and [PATCH] for Undefined symbol "_liblldb_coreVersionString"'. It gets the warning:

ninja: warning: multiple rules generate tools/lldb/source/LLDB_vers.c. builds involving this target will not be correct; continuing anyway

(which no one was able to suggest a fix for in lldb-dev), but at least it fixes the problem without breaking the entire build :)

Diff Detail

Repository
rL LLVM

Event Timeline

dawn updated this revision to Diff 36798.Oct 7 2015, 2:46 PM
dawn retitled this revision from to Fix cmake build on OSX after r249434..
dawn updated this object.
dawn added reviewers: sas, clayborg, zturner.
dawn set the repository for this revision to rL LLVM.
dawn added a subscriber: lldb-commits.
zturner edited edge metadata.Oct 7 2015, 2:50 PM

I can't verify that this works on MacOSX at the moment. Did you verify that this works on other platforms as well?

dawn added a comment.Oct 7 2015, 5:17 PM

I can't verify that this works on MacOSX at the moment. Did you verify that this works on other platforms as well?

I've confirmed it works on MacOSX - I'll try to test Linux later today if no one beats me to it.

dawn added a comment.Oct 7 2015, 7:25 PM

Works fine on Linux too.

zturner added a subscriber: zturner.Oct 7 2015, 7:28 PM

Ok, lgtm then.

dawn added a comment.Oct 7 2015, 8:24 PM

Can someone accept this patch so I can commit please?

lgtm is just as good as hitting accept in Phabricator, I just can't login
to Phab right now so I'm using email. Feel free to commit

brucem added a subscriber: brucem.Oct 7 2015, 8:36 PM
brucem added inline comments.
tools/lldb-server/CMakeLists.txt
42 ↗(On Diff #36798)

Why is this needed?

dawn added inline comments.Oct 8 2015, 5:08 AM
tools/lldb-server/CMakeLists.txt
42 ↗(On Diff #36798)

You get undefined symbol otherwise (if you build from scratch).

dawn added inline comments.Oct 8 2015, 5:22 AM
tools/lldb-server/CMakeLists.txt
42 ↗(On Diff #36798)

I should clarify - lldb-server needs to link with LLDB_vers.c or you get unresolved symbol (that's the fix at line 38). This sets up the dependency - see example in source/API/CMakeLists.txt.

dawn added a reviewer: brucem.Oct 8 2015, 5:30 AM
This revision was automatically updated to reflect the committed changes.
brucem edited edge metadata.Oct 8 2015, 5:38 AM

I think what needs to happen is that you take the add_custom_command and then create a target that will execute the command and only do it once ... I'll provide an example from something else shortly.

(I was about to request changes here when you committed it and closed the revision.)

brucem added a comment.Oct 8 2015, 5:42 AM

The way to do this correctly is something like

ADD_CUSTOM_COMMAND(
  OUTPUT whatever.output.file.c
  COMMAND ... whatever ...
  DEPENDS ... whatever ...
)
ADD_CUSTOM_TARGET(whatever-target-name
                  ALL
                  DEPENDS whatever.output.file.c)
dawn added a comment.Oct 8 2015, 5:44 AM

I went ahead and committed this since the build was completely broken - I'd be happy to apply any suggestions folks make here in subsequent commits.

brucem added a comment.Oct 8 2015, 5:46 AM

I don't know what's broken for you. My build hasn't had any issues on Mac OS X using cmake.

dawn added a comment.Oct 8 2015, 5:53 AM

The way to do this correctly is something like
...

Can you open a new patch which shows your suggested changes please? Or at least clarify how the add_custom_command should look in this case and where it should go?

dawn added a comment.Oct 8 2015, 5:59 AM

I don't know what's broken for you. My build hasn't had any issues on Mac OS X using cmake.

In a clean workspace, cmake would fail because LLDB_vers.c hadn't been created yet. You get the error:

CMake Error at cmake/modules/AddLLVM.cmake:401 (add_library):
  Cannot find source file:

    /Users/testuser/build/workspace/LLVM-Clang-LLDB_master_release_OSX/llvm/build_ninja/tools/lldb/source/LLDB_vers.c

  Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp
  .hxx .in .txx
Call Stack (most recent call first):
  tools/lldb/cmake/modules/AddLLDB.cmake:59 (llvm_add_library)
  tools/lldb/source/API/CMakeLists.txt:9 (add_lldb_library)
dawn added a comment.Oct 8 2015, 6:03 AM

FYI - our Jenkins master build just picked up the change and is finally building again - yay!! :)