[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-20 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. BTW, given that we recommend using ninja pretty much everywhere, you might want to include `set(CMAKE_GENERATOR Ninja)` Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61956/new/ https://reviews.llvm.org/D61956 ___

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-17 Thread Don Hinton via Phabricator via lldb-commits
hintonda added a comment. Btw, options on the command line always override what's in the cache. Has nothing to do with FORCE. All FORCE does is make sure the set command actually changes an existing cache value. So it's an ordering issue. If the `-D` comes before the `-C` then using FORCE w

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-17 Thread Phabricator via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL361069: [CMake] Add first CMake cache files (authored by stefan.graenitz, committed by ). Herald added a project: LLVM. He

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-17 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz added a comment. In D61956#1505144 , @xiaobai wrote: > I noticed you have lots of comments that say "Release has different values > for these variables". Yes, I was arguing: //In a previous sketch I had extra variants for development and rele

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-17 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz updated this revision to Diff 27. sgraenitz marked 11 inline comments as done. sgraenitz added a comment. Fix default install locations and add comments on how to use DESTDIR Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61956/new/ ht

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18 +set(CMAKE_BUILD_TYPE RelWithDebInfo) +set(LLVM_ENABLE_MODULES ON CACHE BOOL "") xiaobai wrote: > labath wrote: > > sgraenitz wrote: > > > compnerd wrote: > > > > sgraenitz wro

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment. I noticed you have lots of comments that say "Release has different values for these variables". I think that you could instead guard the Release configuration behind a check. For example: if (CMAKE_BUILD_TYPE MATCHES Release) # Do the release configuration stuff

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18 +set(CMAKE_BUILD_TYPE RelWithDebInfo) +set(LLVM_ENABLE_MODULES ON CACHE BOOL "") sgraenitz wrote: > compnerd wrote: > > sgraenitz wrote: > > > Can / Should we add this? Here? >

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18 +set(CMAKE_BUILD_TYPE RelWithDebInfo) +set(LLVM_ENABLE_MODULES ON CACHE BOOL "") compnerd wrote: > sgraenitz wrote: > > Can / Should we add this? Here? > This is fine to add

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:10 +set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr") +set(LLDB_FRAMEWORK_INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "") + -

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18 +set(CMAKE_BUILD_TYPE RelWithDebInfo) +set(LLVM_ENABLE_MODULES ON CACHE BOOL "") sgraenitz wrote: > Can / Should we add this? Here? This is fine to add assuming that you are b

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz updated this revision to Diff 199853. sgraenitz added a comment. Add back `CACHE STRING ""` for `CMAKE` variables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61956/new/ https://reviews.llvm.org/D61956 Files: lldb/cmake/caches/Apple-l

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D61956#1504761 , @sgraenitz wrote: > Instead lldb could have something like `GreenDragon-lldb-base.cmake` that > provides and explains reasonable default settings for a range of > bots/environments. Actual build bot configs in

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz added a comment. Thanks for your input. In D61956#1504327 , @labath wrote: > I wouldn't want to over-proliferate these but in general I think this is a > good idea. Agreed, caches for most common configurations only. Also we may not over-pro

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz updated this revision to Diff 199817. sgraenitz marked an inline comment as done. sgraenitz added a comment. Rename macOS specific cache file. Add settings and comments. Fix install destinations by appending `Developer` and `SharedFrameworks` respectively. Repository: rG LLVM Github

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I wouldn't want to over-proliferate these, but in general I think this is a good idea. What I would find particularly useful is if we checked in the configurations used by all the bots in here, as that would make it easier to figure out what's going on if one of them bre

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. No objections from me to checking in caches for common configurations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61956/new/ https://reviews.llvm.org/D61956 ___ lldb-co

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-osx.cmake:1 +include(${CMAKE_CURRENT_LIST_DIR}/Apple-lldb-base.cmake) + Filename: Apple-lldb-macOS or apple-lldb-macos? MacOS X has been renamed to OS X which has been renamed to macOS :-)

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

2019-05-15 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz created this revision. sgraenitz added reviewers: xiaobai, compnerd, JDevlieghere, aprantl, labath. Herald added a subscriber: mgorny. Herald added a project: LLDB. CMake cache scripts pre-populate the CMakeCache in a build directory with commonly used settings. The CMake invocation fro