[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb closed this revision. hhb added a comment. I gave up on this. 😊 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ https://reviews.llvm.org/D69589 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

JDevlieghere added a comment. Ha, I looked at the date but didn't realize this was a review from fall last year :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ https://reviews.llvm.org/D69589 _

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

JDevlieghere added a subscriber: sgraenitz. JDevlieghere added a comment. In D69589#1769663 , @sgraenitz wrote: > I'd recommend to be careful when making a change of this size. There are a > lot of possible configurations. > If you split it up into a few

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

labath added inline comments. Comment at: lldb/CMakeLists.txt:114 + COMMAND ${CMAKE_COMMAND} -E make_directory ${ARG_DEST_DIR}) +foreach(src_file ${ARG_FILES}) + if(ARG_DEST_FILE_NAME) tatyana-krasnukha wrote: > This is quite off-topic, but it would

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

tatyana-krasnukha added inline comments. Comment at: lldb/CMakeLists.txt:114 + COMMAND ${CMAKE_COMMAND} -E make_directory ${ARG_DEST_DIR}) +foreach(src_file ${ARG_FILES}) + if(ARG_DEST_FILE_NAME) This is quite off-topic, but it would be great if thi

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

sgraenitz added a comment. I'd recommend to be careful when making a change of this size. There are a lot of possible configurations. If you split it up into a few smaller and more dedicated commits, you will make your life easier. Otherwise someone might just revert everything if this happens

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

aadsm added a comment. Yeah, weird. I wonder if this is something cmake does implicitly when the target is defined as `FRAMEWORK`: https://cmake.org/cmake/help/latest/prop_tgt/FRAMEWORK.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb added a comment. In D69589#1734818 , @aadsm wrote: > @hhb I just did, and get the same error as well :( > `ninja: error: 'bin/LLDB.framework/LLDB', needed by > 'bin/LLDB.framework/Resources/Python/lldb/_lldb.so', missing and no known > rule to make

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

aadsm added a comment. @hhb I just did, and get the same error as well :( `ninja: error: 'bin/LLDB.framework/LLDB', needed by 'bin/LLDB.framework/Resources/Python/lldb/_lldb.so', missing and no known rule to make it` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb added a comment. In D69589#1733567 , @aadsm wrote: > @hhb fwiw, I still get the same error with this diff (after applying it on > top of D69834 ). But D69834 > by itself works great!

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb updated this revision to Diff 227974. hhb added a comment. Fix some naming Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ https://reviews.llvm.org/D69589 Files: lldb/CMakeLists.txt lldb/scripts/CMakeLists.txt Index: lldb/scripts

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb updated this revision to Diff 227958. hhb added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ https://reviews.llvm.org/D69589 Files: lldb/CMakeLists.txt lldb/scripts/CMakeLists.txt Index: lldb/scripts/CMakeLis

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

aadsm added a comment. @hhb fwiw, I still get the same error with this diff (after applying it on top of D69834 ). But D69834 by itself works great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

aadsm accepted this revision. aadsm added a comment. This revision is now accepted and ready to land. This is great! it's so much better. Thank so much for looking into this and fixing it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ h

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb added a comment. Sent D69834 . Haven't get a chance to test but you can see the idea... Also please add lldb-python-scripts to your LLVM_DISTRIBUTION_COMPONENTS when testing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb added a comment. See D68370 . We probably need to do the same thing for darwin... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ https://reviews.llvm.org/D69589

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb added a comment. Alternatively we should not depends on install-liblldb to install python packages, but have a install command similar to what we did below for other platforms. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69589/new/ https://

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb added a comment. In D69589#1731782 , @aadsm wrote: > I fixed this locally by doing this (this fix doesn't really feel good but not > sure what's the best way to do it): I found it confusing too. So finish_swig should depends on liblldb, because at

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

aadsm requested changes to this revision. aadsm added a comment. This revision now requires changes to proceed. I'm sorry to reject but this diff breaks `install-distribution` on Darwin. Here's how to repro: $ mkdir -p ~/Projects/llvm-build/Release && cd ~/Projects/llvm-build/Release $ cmake

Re: [Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

I see your concern about source and target paths for Python files, but I also think it is valuable that it is really easy to find examples of using lldb with the Python SB API's - both with toy examples and with fully worked examples. I know people have found and copied from here to good effect

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks great to me. Maybe wait a while to see if anyone else has any thoughts on this? This not your fault, but the thing that bothers me about this build process (and which this patch ma

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb updated this revision to Diff 227204. hhb added a comment. When creating symlink, make the target depends on relative target path. So that if the target file doesn't exist, the build will fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6958

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb marked an inline comment as done. hhb added a comment. In D69589#1726812 , @labath wrote: > This looks reasonable to me. I'm just wondering, now that these are separate > targets, I guess that means they can be run in random order, right? Will that >

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

labath added reviewers: labath, mgorny, sgraenitz. labath added a comment. This looks reasonable to me. I'm just wondering, now that these are separate targets, I guess that means they can be run in random order, right? Will that cause any problems, for instance when creating a package and its s

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

hhb created this revision. Herald added subscribers: lldb-commits, mgorny. Herald added a project: LLDB. This makes all dependencies correct. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D69589 Files: lldb/CMakeLists.txt lldb/scripts/CMakeLists.txt Index: lldb/scripts/CM