thakis added inline comments.
================ Comment at: llvm/utils/gn/secondary/BUILD.gn:15 "//lld/test", + "//lldb", "//llvm/test", ---------------- pcc wrote: > thakis wrote: > > pcc wrote: > > > thakis wrote: > > > > Does this build fine on windows? > > > > > > > > Generally this only depends on the test targets which in turn depend on > > > > the binaries, so probably should only have the old/test dep in this > > > > file anyways. > > > You're right, this needs to avoid the dependency on lldb on Windows > > > targets. > > > > > > The problem with only depending on `//lldb/test` is that nothing else > > > refers to `//lldb`, so `ninja lldb` wouldn't also build `lldb-server`. > > > Maybe it would be better to add `lldb-server` to the `data_deps` of > > > `//lldb/tools/driver:lldb` instead then. > > I guess `//lldb/test` could depend on `//:lldb` instead of > > `//lldb/tools/driver`? > > > > We currently don't use data_deps anywhere else. (I have > > https://github.com/nico/llvm-project/commit/7246393c6bbc270044641415ffb0db93ffee3e29 > > on a branch, but uploads with static links take so long that it isn't > > really worth it. Maybe I should revisit that with > > `-fno-semantic-interposition`…) > And then `//:lldb` would depend on lldb and the various lldb-servers? I > suppose that could work. > > It seems like clang's existing dependency on e.g. clang-offload-bundler, > which is the sort of dependency we want here, is currently added via deps, > and I see that your change moves that to data_deps. For > executable->executable dependencies I suppose that deps means the same thing > as data_deps (test isolation aside) but to be consistent with what's in clang > maybe we should just stick to what I have here except with s/data_deps/deps/g. sgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109463/new/ https://reviews.llvm.org/D109463 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits