sgraenitz added a comment. Thanks for your input.
In D61956#1504327 <https://reviews.llvm.org/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-proliferate includes across caches. In a previous sketch I had extra variants for development and release builds. I preferred to keep only one variant for now and make notes for release builds instead, because actual release configurations are likely more complex anyway. For downstream repositories I am not sure what's the best approach, but I'd tend to: create their own cache files, include one upstream cache file and add downstream options. > 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 breaks and the reason is not obvious... Interesting idea. This would go in the direction of CI systems with checked-in build/test configurations (like `.travis.yml` or `.gitlab-ci.yml`)? In my experience this works great with docker containers, because they provide a stable environment, but I remember well that debugging Travis CI builds was a big time sink before containerization. Hence, I wonder whether it comes with drawbacks in our case. We don't use containers exclusively. We do have local differences between machines and bot configurations smooth them out explicitly. I am not fond of putting thing like this in a cache file: -DPYTHON_EXECUTABLE=/usr/bin/python2.7 -DPYTHON_LIBRARY=/System/Library/Frameworks/Python.framework/Versions/2.7/lib/libpython2.7.dylib Furthermore, fixing bot configurations can be painful and often enough experimental changes are committed (e.g. https://github.com/llvm/llvm-zorg/commits/master/zorg/jenkins/jobs/jobs/lldb-cmake-standalone). Not sure if we want this in the LLDB history? I guess there's a reason why zorg is not part of the monorepo? :-) Also, I think fixing bot configurations typically goes hand in hand with changes on build bot scripts (which we don't have in the repo either) more than with changes on the codebase itself. I don't see it getting easier by moving the configurations to the codebase.. With all this considered, I wouldn't prefer to check in concrete caches for build bots in lldb. We could have them in zorg right? 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 zorg could then be handled the same way as downstream repos. Might that be a way forward? We may brainstorm a list of candidates or just handle them as they come up. ================ 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 "") + ---------------- Follow-up from: https://reviews.llvm.org/D61956?id=199645#inline-550727 > I believe the expected usage of this variable is to make it point to the > final resting place of the executables, ... It's been a pragmatic decision. Maybe we can improve this. The rationale was, that the default configuration should give the user something that works without touching caches or overriding parameters. In a previous sketch I used a real-world destination like: ``` set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr/) ``` But then `ninja install` would fail by default due to lack of permissions in the install destination. Actual release configurations tend to be more complex anyway and vendors will have their own downstream repos / caches for it. Thus, choosing a good default for developers sounded like a good way forward. What do you think? > Are you sure including {CMAKE_CURRENT_BINARY_DIR} here is a good idea? I > think llvm tries to avoid that generally, ... What exactly do you mean? Having absolute paths, or paths into the build-tree, or the `CMAKE_CURRENT_BINARY_DIR` specifically? I don't see problems with the two last points. Am I missing something? For the first: choosing an absolute path was for consistency with `LLDB_FRAMEWORK_INSTALL_DIR`. In the current build logic, they can both be absolute paths. Otherwise: * if the install prefix is relative, it will be appended to the path of the root build directory * if the framework install dir is relative, it will be appended to the install prefix > Then, if you want to copy the to-be-installed files into a staging area > first, you're expected to do that with "DESTDIR=whatever ninja install". [Clang cache scripts](https://github.com/llvm/llvm-project/blob/a8f88c38/clang/cmake/caches/Apple-stage1.cmake#L4) seem to accomplish it manually, which may look like this (but the default would again fail due to privileges): ``` if($ENV{DESTDIR}) set(CMAKE_INSTALL_PREFIX $ENV{DESTDIR}) set(LLDB_FRAMEWORK_INSTALL_DIR "../../SharedFrameworks" CACHE STRING "") else() set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr) set(LLDB_FRAMEWORK_INSTALL_DIR /Applications/Xcode.app/Contents/SharedFrameworks CACHE STRING "") endif() ``` Would you (and other reviewers) prefer this solution? ================ Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18 +set(CMAKE_BUILD_TYPE RelWithDebInfo) +set(LLVM_ENABLE_MODULES ON CACHE BOOL "") ---------------- Can / Should we add this? Here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61956/new/ https://reviews.llvm.org/D61956 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
