JDevlieghere added a comment.

Hi David, this looks really comprehensive. As far as the code is concerned, it 
has all the parts that we'd want to support another scripting language in LLDB. 
That leaves us with the last remaining questions:

- Testing: having a bot that actually builds & tests this configuration.
- Maintainership: having a commitment from you (or someone else) to maintain 
this code.

Are you willing and able to sign up for those things?

PS: I installed Java and applied your patch on top-of-tree. Things didn't built 
because of the recent change to the plugin manager that now expects 
`StringRef`s instead of `ConstString`s. Please let me know if you've rebased 
the patch and I'd love to give it a try.



================
Comment at: lldb/cmake/modules/FindJavaAndSwig.cmake:13
+    find_package(Java 11.0)
+    find_package(JNI REQUIRED)
+    if(JAVA_FOUND AND SWIG_FOUND)
----------------
This can't be `REQUIRED` because that will fail the CMake configuration stage 
when Java isn't found which is not what you want when doing autodetection (the 
default). Making it required is handled at a higher level for `FindJavaAndSwig` 
as a whole. 


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111409/new/

https://reviews.llvm.org/D111409

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to