[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

Fri Oct 8 11:15:24 PDT 2021

d-millar added a comment.

Ah, OK, after more digging, I realize I have probably provided only half of what you would like for this commit.  My primary intention was to expose the SB API so I could make calls from Java into it, but for it to be, in some sense, a full-fledged member of the API, it would make sense to enable Java scripting from lldb.  I will attempt to fill out that side of the equation.  Cheers, D

________________________________

From: lldb-commits <lldb-commits-bounces at lists.llvm.org> on behalf of David Millar via lldb-commits <lldb-commits at lists.llvm.org>
Sent: Friday, October 8, 2021 1:22:07 PM
To: anoronha at apple.com; fallkrum at yahoo.com; kkleine at redhat.com; medismail.bennani at gmail.com; jonas at devlieghere.com; tedwood at quicinc.com; jmolenda at apple.com; syaghmour at apple.com; jingham at apple.com; vsk at apple.com; boris.ulasevich at gmail.com; lldb-commits at lists.llvm.org; h.imai.833 at nitech.jp; bruce.mitchener at gmail.com; david.spickett at linaro.org; quic_sourabhs at quicinc.com; djordje.todorovic at syrmia.com; serhiy.redko at gmail.com; Liburd1994 at outlook.com; Raphael Isemann
Subject: Re: [Lldb-commits] [PATCH] D111409 <https://reviews.llvm.org/D111409>: proposed support for Java interface to Scripting Bridge

I'm not sure I understand your testing strategy, in particular how it applies to the existing Lua and Python extensions.  I am looking at the files in lldb/unittests/ScriptInterpreter/Lua&Python.  Do you execute test from native Lua/Python environments or through C wrappers-only?  You mentioned a bot - is that code in the main repository?

________________________________

From: Raphael Isemann via Phabricator <reviews at reviews.llvm.org>
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoronha at apple.com; fallkrum at yahoo.com; kkleine at redhat.com; medismail.bennani at gmail.com; jonas at devlieghere.com; tedwood at quicinc.com; jmolenda at apple.com; syaghmour at apple.com; jingham at apple.com; vsk at apple.com; boris.ulasevich at gmail.com; lldb-commits at lists.llvm.org; h.imai.833 at nitech.jp; bruce.mitchener at gmail.com; david.spickett at linaro.org; quic_sourabhs at quicinc.com; djordje.todorovic at syrmia.com; serhiy.redko at gmail.com; Liburd1994 at outlook.com
Cc: mgorny at gentoo.org
Subject: [PATCH] D111409 <https://reviews.llvm.org/D111409>: proposed support for Java interface to Scripting Bridge

teemperor added a comment.

No problem, first time using Phabricator is always a bit confusing. You can just do a `git diff -U999999 > ~/java-patch.diff`, click the "Update Diff" button on the top right of this website and then select *just* this diff file that contains your changes. Phabricator will render the diff properly for you (-> it will hide all the diff context by default). There is need to attach a separate diff file or anything else (users can just download the diff you uploaded).

Regarding the tests: We would essentially just need some basic test that exercises the new API a bit so that we know this works. The test code itself will be straightforward, but we would need a nice way to (automatically) find the system JRE and then set it up to be able to run the test code.

In D111409#3051140 <https://reviews.llvm.org/D111409#3051140> https://reviews.llvm.org/D111409#3051140, @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes to learning new tricks.  Would you prefer I make a new submission with the -U999999 diff?   Also, am more than willing to help with the Java tests if that would be useful.
>
> ________________________________
>
> From: Raphael Isemann via Phabricator <reviews at reviews.llvm.org>
> Sent: Friday, October 8, 2021 10:46:50 AM
> To: David Millar; anoronha at apple.com; fallkrum at yahoo.com; kkleine at redhat.com; medismail.bennani at gmail.com; jonas at devlieghere.com; tedwood at quicinc.com; jmolenda at apple.com; syaghmour at apple.com; jingham at apple.com; vsk at apple.com; boris.ulasevich at gmail.com; lldb-commits at lists.llvm.org; h.imai.833 at nitech.jp; bruce.mitchener at gmail.com; david.spickett at linaro.org; quic_sourabhs at quicinc.com; djordje.todorovic at syrmia.com; serhiy.redko at gmail.com; Liburd1994 at outlook.com
> Cc: mgorny at gentoo.org
> Subject: [PATCH] D111409 <https://reviews.llvm.org/D111409> https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridge
>
> teemperor added a comment.
>
> In D111409#3051110 <https://reviews.llvm.org/D111409#3051110> https://reviews.llvm.org/D111409#3051110 https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
>
>> Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
>
> You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
>
> Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
>
> Comment at: lldb/bindings/java/CMakeLists.txt:3
> + * IP: Apache License 2.0 with LLVM Exceptions
> + */
>
> +add_custom_command(
> --------------------
>
> I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts.
>
> Comment at: lldb/source/API/CMakeLists.txt:84
>
>   SBTrace.cpp
>
> +  SBTraceOptions.cpp
>
>   SBType.cpp
>
> ----------------
>
> I think this is some conflict with one of the SBTrace patches.
>
> Repository:
>
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D111409/new/
>
> https://reviews.llvm.org/D111409

Repository:

  rLLDB LLDB

CHANGES SINCE LAST ACTION

  https://reviews.llvm.org/D111409/new/

https://reviews.llvm.org/D111409

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409