RFR: JDK-8251988: jpackage --runtime-image fails on mac when using JDK11 based runtime.

[alexander.matveev at oracle.com]

Hi Andy,

Looks good.

Thanks,
Alexander

On 8/24/20 4:53 PM, Alexey Semenyuk wrote:
> Looks good!
>
> - Alexey
>
> On 8/24/2020 6:23 PM, Andy Herrick wrote:
>>
>> On 8/24/2020 5:08 PM, Alexey Semenyuk wrote:
>>> Andy,
>>>
>>> I'd propose instead of checking if the app is bundled with proper 
>>> runtime in the test case, enforce the test case to always create 
>>> runtime for the app with JPackageCommand.ignoreDefaultRuntime(true) 
>>> call:
>>> ---
>>> JPackageCommand cmd = JPackageCommand.helloAppImage(
>>>   71 JavaAppDesc.parse("com.hello/com.hello.Hello"))
>>> ...
>>>   76                 .addArguments(...)
>>>                        .ignoreDefaultRuntime(true);
>> OK.
>>> ---
>>> With ignoreDefaultRuntime(true) call added there is no need to call 
>>> JPackageCommand.isFakeRuntime().
>>>
>>> Is com.hello/com/hello/Hello.java used at all? I'd guess after we 
>>> had refactor all package tests this source is not used any more. Can 
>>> it be removed yet?
>>
>> It looked like ArgumentTest was using it, but I removed it and all 
>> still run
>>
>> revised webrev: http://cr.openjdk.java.net/~herrick/8251988/webrev.04/
>>
>> /Andy
>>
>>
>>>
>>> - Alexey
>>>
>>> On 8/24/2020 4:50 PM, Andy Herrick wrote:
>>>> please review revised webrev at: 
>>>> http://cr.openjdk.java.net/~herrick/8251988/webrev.03/
>>>>
>>>> jpackage.app-version set in cfg file, and jpackage.app-path now set 
>>>> from native code.
>>>>
>>>> New testcase in BasicTest to check that they are set.
>>>>
>>>> /Andy
>>>>
>>>> On 8/20/2020 7:36 PM, Alexey Semenyuk wrote:
>>>>> Andy,
>>>>>
>>>>> I'd replace 'File.separator' with '/' to make value of 
>>>>> 'jpackage.app-path' property platform independent.
>>>>> It would be also good to have a unit test verifying values of the 
>>>>> new properties.
>>>>>
>>>>> - Alexey
>>>>>
>>>>> On 8/20/2020 5:44 PM, Andy Herrick wrote:
>>>>>> Please review the jpackage fix at [1] to issue [2].
>>>>>>
>>>>>> This fixes the problem loading libjli.dylib as well as providing 
>>>>>> some useful generic system properties.
>>>>>>
>>>>>> /Andy
>>>>>>
>>>>>>
>>>>>> [1] - http://cr.openjdk.java.net/~herrick/8251988/webrev.02/ 
>>>>>> <http://cr.openjdk.java.net/~herrick/8251988/webrev.01/>
>>>>>>
>>>>>> [2] - https://bugs.openjdk.java.net/browse/JDK-8251988
>>>>>>
>>>>>
>>>
>