pcc added inline comments.
================
Comment at: llvm/utils/gn/secondary/BUILD.gn:15
"//lld/test",
+ "//lldb",
"//llvm/test",
----------------
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.
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
