> On Nov 28, 2018, at 12:46 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
>
> On 27/11/2018 23:05, Henry Jen wrote:
>> Hi,
>>
>> Please review a follow up webrev[1] based on Priyanka’s patch, it simply
>> added a test case for Mac only that will link with libjli.
>> Note that, to use invoke API, one should probably link with libjvm, which
>> works for all supported platforms, not just Mac.
>>
>> Cheers,
>> Henry
>>
>> [1] http://cr.openjdk.java.net/~henryjen/jdk12/8213362.0/webrev/
>>
> The changes to java_md_macosx.m looks okay although the issue as to how
> Eclipse runs into this have not been established. If Eclipse dlopen's libjvm
> then it should have no issue locating and calling JNI's CreateJavaVM. It may
> be that Eclipse is using libjli directly but it shouldn't do that because
> it's not a documented/supported interface. It might be that it's using the
> CFBundleExecutable key in Info.plist. Henry, Priyanka and I discussed this a
> bit and were not able to able to establish what it is doing.
>
> On the test: it's clear whether you've moved or copied
> test/hotspot/jtreg/runtime/jni/CalleeSavedRegisters/FPRegs.java. The webrev
> suggests you've moved it but it's doesn't handle hg copy. Either way, it
> needs clean up, e.g. it shouldn't need @modules java.base/jdk.internal.misc,
> the really long lines make it difficult to look at side-by-side changes, why
> does it check for Windows when the @requires means it runs on Mac only. I
> assume we can find a better name for exeJNILauncher.c. It will also need a
> copyright header.
>
It’s hg copy. I’ll clean up the module and windows line, also add a copyright
for exeJNILauncher.c. The exe is prefix for build, if it’s JNILauncher not
desired, I am open for suggestions.
Cheers,
Henry
