[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

This revision was automatically updated to reflect the committed changes. Closed by commit rL317501: Add a dependency from check-lldb on lld (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D39689 Files: lldb/trunk/test/CMakeLists.txt Index: lldb/trunk/test/CMakeLists.txt

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

sas updated this revision to Diff 121760. sas added a comment. Move check out of if (TARGET clang) block. https://reviews.llvm.org/D39689 Files: test/CMakeLists.txt Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

zturner accepted this revision. zturner added inline comments. Comment at: test/CMakeLists.txt:116-122 + if (CMAKE_SYSTEM_NAME MATCHES "Windows") +if (TARGET lld) + add_dependencies(check-lldb lld) +else () + message(WARNING "lld required to test LLDB on

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

clayborg accepted this revision. clayborg added a comment. Do we want to add an option to our build system to try LLD where it is supported? Doesn't need to be part of this patch, but it would be great to be able to try it out on ELF based systems. https://reviews.llvm.org/D39689

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

sas updated this revision to Diff 121750. sas added a comment. Check only on Windows. https://reviews.llvm.org/D39689 Files: test/CMakeLists.txt Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. lld is required for building windows inferiors, but that can't be the case elsewhere (I don't even have lld checked out). It's possible it gets somehow auto-selected by clang when

Re: [Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

Makes sense. I'll update the diff. On Mon, Nov 6, 2017 at 9:28 AM Zachary Turner wrote: > This is definitely required, but only on windows. I’d put it behind a > check for Windows, and I’d also add a check to print a warning/error if > (TARGET lld) returns false on windows >

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

sas added a comment. @davide: I agree, the extra dependency is annoying. FWIW, I was trying to run tests on Windows and nothing worked because lld wasn't built. Maybe the behavior is different on Windows and on other hosts? I just assumed this was the new default because I saw a couple of

Re: [Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

This is definitely required, but only on windows. I’d put it behind a check for Windows, and I’d also add a check to print a warning/error if (TARGET lld) returns false on windows On Mon, Nov 6, 2017 at 9:22 AM Davide Italiano via Phabricator < revi...@reviews.llvm.org> wrote: > davide added a

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

davide added a subscriber: zturner. davide added a comment. Not sure lld is the default? I think I always build with bfd (or gold). I'll check later today when I'm in the office. I'm not against this change per se, but adding another dependency seems annoying. cc: @zturner