labath added a comment.

In D61956#1504761 <https://reviews.llvm.org/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 zorg could then be handled the 
> same way as downstream repos.


Yeah, that's more or less what I had in mind. I didn't mean to include paths to 
various things, which are only valid on the single machine that the bot is 
running on, but more like if a bot is using some specific combo of 
LLVM_ENABLE_ASSERTS/LLVM_ENABLE_MODULES/LLVM_LINK_LLVM_DYLIB/... settings then 
these things might be helpful in figuring out why something is only failing 
there. Though I suppose these things are accessible through the buildbot UIs 
already, so it's already accessible in other ways. I think it still might be 
useful to have these configs accessible in a the repo for to make it easier to 
use them, but it's not a big deal either. The other llvm projects don't do that 
afaik, so it may be better to follow that example..

Having the buildbot config changes in the history is also an interesting 
question. On one hand, they do clutter the history, but on the other, they make 
it more obvious that something has changed -- if a bot mysteriously starts 
failing after one of my changes, i'd like to know that this could be due to a 
configuration change that happened at the same time -- this assumes that the 
configuration changes take effect instantly, and not after a bot reboot as is 
the case with buildbot (I'm not sure how greendragon stands on this front).



================
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 "")
+
----------------
sgraenitz wrote:
> 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?
> 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?

I don't think most developers actually run the "install" rule TBH. :) But if 
they do, I think we should encourage them to do the right thing, and run 
"DESTDIR=foo ninja install", which will work even with a real-world prefix. At 
least that is the right thing to do in the linuxy world -- on a mac I guess 
most people will not be able to install lldb to the system destination anyway, 
so I'm not sure what would be a sensible default.

> Would you (and other reviewers) prefer this solution?

I don't care that much about this TBH. I just wanted to explain the difference 
between the install prefix and destdir, because in my experience, a lot of 
people get those two mixed up. ccing  @mgorny, in case I'm saying something 
wrong, as he does a lot of building and installing..


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

Reply via email to