[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
This revision was automatically updated to reflect the committed changes. Closed by commit rG988ad4194848: [LLDB] Remove standalone build dep on llvm-strip (authored by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D68614?vs=224148=224160#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 Files: lldb/test/CMakeLists.txt Index: lldb/test/CMakeLists.txt === --- lldb/test/CMakeLists.txt +++ lldb/test/CMakeLists.txt @@ -58,9 +58,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + add_lldb_test_dependency(llvm-strip) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) else() Index: lldb/test/CMakeLists.txt === --- lldb/test/CMakeLists.txt +++ lldb/test/CMakeLists.txt @@ -58,9 +58,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + add_lldb_test_dependency(llvm-strip) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) else() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
gmittert updated this revision to Diff 224148. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 Files: test/CMakeLists.txt Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@ -58,9 +58,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + add_lldb_test_dependency(llvm-strip) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) else() Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@ -58,9 +58,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + add_lldb_test_dependency(llvm-strip) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) else() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
gmittert updated this revision to Diff 224147. gmittert added a comment. Updated/Rebased for the rename of lit->test Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 Files: test/CMakeLists.txt Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@ -58,9 +58,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + add_lldb_test_dependency(llvm-stirp) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) else() Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@ -58,9 +58,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + add_lldb_test_dependency(llvm-stirp) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) else() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
gmittert added a comment. Thanks! Can someone commit this for me? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
xiaobai accepted this revision. xiaobai added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
gmittert updated this revision to Diff 223903. gmittert added a comment. Alright, rebased and added a comment to it Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 Files: lit/CMakeLists.txt Index: lit/CMakeLists.txt === --- lit/CMakeLists.txt +++ lit/CMakeLists.txt @@ -56,9 +56,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + list(APPPEND LLDB_TEST_DEPS llvm-strip) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) endif() Index: lit/CMakeLists.txt === --- lit/CMakeLists.txt +++ lit/CMakeLists.txt @@ -56,9 +56,14 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +# Since llvm-strip is a symlink created by add_custom_target, it +# doesn't expose an export target when building standalone. +if(NOT LLDB_BUILT_STANDALONE) + list(APPPEND LLDB_TEST_DEPS llvm-strip) +endif() + if(TARGET lld) add_lldb_test_dependency(lld) endif() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
compnerd added inline comments. Comment at: lit/CMakeLists.txt:64 ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) JDevlieghere wrote: > xiaobai wrote: > > why not `if(TARGET llvm-strip)`? I think that expresses the intent more > > cleanly (and conforms to the existing pattern). > I actually ran into an issue with that today, we had the following code in > the top-level CMake list: > > ``` > if(TARGET dsymutil) > add_lldb_test_dependency(dsymutil) > endif() > ``` > Nevertheless, `ninja lldb-test-deps` didn't build dsymutil. > > ``` > $ /V/J/l/build-ra> ninja lldb-test-deps > [1/1] Python script sym-linking LLDB Python API > $ /V/J/l/build-ra> ninja dsymutil > [1/1] Linking CXX executable bin/dsymutil > ``` My slight preference for this is to actually have a good way to identify that this can be simplified once unified builds are the only supported build style. I suppose a comment along with the `if(TARGET)` check would work too. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
kwk added a comment. I think I'm guilty for adding `llvm-strip` to `LLDB_TEST_DEPS` I wasn't aware that it might cause problems. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
JDevlieghere added inline comments. Comment at: lit/CMakeLists.txt:64 ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) xiaobai wrote: > why not `if(TARGET llvm-strip)`? I think that expresses the intent more > cleanly (and conforms to the existing pattern). I actually ran into an issue with that today, we had the following code in the top-level CMake list: ``` if(TARGET dsymutil) add_lldb_test_dependency(dsymutil) endif() ``` Nevertheless, `ninja lldb-test-deps` didn't build dsymutil. ``` $ /V/J/l/build-ra> ninja lldb-test-deps [1/1] Python script sym-linking LLDB Python API $ /V/J/l/build-ra> ninja dsymutil [1/1] Linking CXX executable bin/dsymutil ``` Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
xiaobai added a reviewer: JDevlieghere. xiaobai added a comment. @JDevlieghere has been touching similar things today. You should coordinate with him on this change. Comment at: lit/CMakeLists.txt:64 ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) why not `if(TARGET llvm-strip)`? I think that expresses the intent more cleanly (and conforms to the existing pattern). Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip
gmittert created this revision. gmittert added reviewers: compnerd, kwk. Herald added subscribers: lldb-commits, JDevlieghere, mgorny. Herald added a project: LLDB. When building standalone, since llvm-strip is a symlink, it is created using add_custom_command/add_custom_target which cannot be exported, and thus cannot be depended on by lldb. Repository: rLLDB LLDB https://reviews.llvm.org/D68614 Files: lit/CMakeLists.txt Index: lit/CMakeLists.txt === --- lit/CMakeLists.txt +++ lit/CMakeLists.txt @@ -60,8 +60,10 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) +endif() if(TARGET lld) list(APPEND LLDB_TEST_DEPS lld) Index: lit/CMakeLists.txt === --- lit/CMakeLists.txt +++ lit/CMakeLists.txt @@ -60,8 +60,10 @@ llvm-mc llvm-objcopy llvm-readobj - llvm-strip ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) +endif() if(TARGET lld) list(APPEND LLDB_TEST_DEPS lld) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
