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
=
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
+++ tes
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 Window
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
__
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
@@
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 yo
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
> On Mon, Nov 6, 2017
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 chang
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 sub
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
https://reviews.llvm.
10 matches
Mail list logo
