Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-21 Thread Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL282110: [CMake] Initial support for LLDB.framework (authored by cbieneman). Changed prior to commit: https://reviews.llvm.org/D24749?vs=72097&id=72104#toc Repository: rL LLVM https://reviews.llvm.or

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-21 Thread Todd Fiala via lldb-commits
tfiala added a comment. Okay, LGTM. https://reviews.llvm.org/D24749 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-21 Thread Chris Bieneman via lldb-commits
beanz updated this revision to Diff 72097. beanz added a comment. - Use INSTALL_RPATH and BUILD_WITH_INSTALL_RPATH instead of manually setting the rpath linker flags https://reviews.llvm.org/D24749 Files: CMakeLists.txt cmake/LLDBDependencies.cmake cmake/modules/AddLLDB.cmake cmake/mod

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-21 Thread Pavel Labath via lldb-commits
labath added a comment. In https://reviews.llvm.org/D24749#548778, @tfiala wrote: > I'd say the install rpath change is probably worth doing on the first round. > I think the top-level concept for frameworks is interesting but would be fine > to come in as another change. > > I'd like to start

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-21 Thread Todd Fiala via lldb-commits
tfiala added a comment. I'd say the install rpath change is probably worth doing on the first round. I think the top-level concept for frameworks is interesting but would be fine to come in as another change. I'd like to start being able to use this. Comment at: cmake/module

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-20 Thread Pavel Labath via lldb-commits
labath added inline comments. Comment at: cmake/modules/AddLLDB.cmake:75 @@ -74,1 +74,3 @@ if (PARAM_SHARED) +set(out_dir lib${LLVM_LIBDIR_SUFFIX}) +if(${name} STREQUAL "liblldb" AND LLDB_BUILD_FRAMEWORK) I am wondering whether this wouldn't

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-20 Thread Chris Bieneman via lldb-commits
beanz added a comment. I've tested this on OS X with and without `LLDB_BUILD_FRAMEWORK` set, and check-lldb works in both cases. I would expect it to also work on Linux and Windows. The change should be NFC if `LLDB_BUILD_FRAMEWORK=Off`. Comment at: scripts/Python/finishSwigPy

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-19 Thread Todd Fiala via lldb-commits
On Mon, Sep 19, 2016 at 7:39 PM, Zachary Turner wrote: > Unless someone has a strong desire to keep it and it adds necessary > functionality I would vote for removing it. I think it's fine to remove the other support if nobody else has a good reason to want to keep it, but we're not to that poi

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-19 Thread Zachary Turner via lldb-commits
Unless someone has a strong desire to keep it and it adds necessary functionality I would vote for removing it. It can always be maintained downstream. That being said, let's wait and see if anyone chimes in. I also want to test this tomorrow to make sure nothing breaks. Just to be clear, if i we

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-19 Thread Todd Fiala via lldb-commits
tfiala accepted this revision. tfiala added a subscriber: labath. tfiala added a comment. This revision is now accepted and ready to land. LGTM. Chris - you might want to add @labath to this as well. Comment at: source/API/CMakeLists.txt:9 @@ -8,1 +8,3 @@ +option(LLDB_BUILD_F

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-19 Thread Zachary Turner via lldb-commits
zturner added inline comments. Comment at: scripts/Python/finishSwigPythonLLDB.py:397 @@ -396,3 +396,3 @@ strBuildDir = os.path.join("..", "..", "..", "..") -strSrc = os.path.normcase(os.path.join(strBuildDir, vstrSrcFile)) +strSrc = os.path.normcase(os.pa

[Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

2016-09-19 Thread Chris Bieneman via lldb-commits
beanz created this revision. beanz added reviewers: zturner, tfiala. beanz added a subscriber: lldb-commits. Herald added subscribers: mgorny, beanz. This patch adds a CMake option LLDB_BUILD_FRAMEWORK, which builds libLLDB as a macOS framework instead of as a *nix shared library. With this patc