RFR: JDK-8230920 : jpackage problems when -input dir contains any files with "cfg" extension.

[Alexander Matveev]

Hi Andy,

webrev.04 looks good.

Thanks,
Alexander

On 9/30/2019 1:09 PM, Andy Herrick wrote:
> yes - I removed get, and the "instance" variable and just call load 
> (see: webrev.04).
>
> we shouldn't actually call it with different base dir arg, except 
> possibly in a test.
>
> /Andy
>
> On 9/30/2019 4:07 PM, Alexander Matveev wrote:
>> Looks good.
>>
>> Do we really planning to call AppImageFile.get() with different 
>> values? I think removing or fixing is still better to avoid any 
>> potential bugs.
>>
>> Thanks,
>> Alexander
>>
>> On 9/29/2019 1:06 PM, Alexey Semenyuk wrote:
>>> One thing that I just notice is that you introduced caching in 
>>> AppImageFile file. The caching doesn't take into consideration that 
>>> different values can be passed in AppImageFile.get() function. So if 
>>> the function would be called multiple times with different values of 
>>> `appImageDir` parameter the same AppImageFile instance will be 
>>> returned in all the calls. Unfortunately unit tests didn't catch 
>>> this. I suggest you either fix the caching logic or get rid of it in 
>>> AppImageFile class.
>>>
>>> - Alexey
>>>
>>> On 9/29/2019 10:37 AM, Andy Herrick wrote:
>>>> Yes  - I meant webrev.03
>>>>
>>>> Sorry
>>>>
>>>> /Andy
>>>>
>>>> On 9/29/19 10:23 AM, Alexey Semenyuk wrote:
>>>>> I guess it was a type in webrev adderss. Should be 
>>>>> http://cr.openjdk.java.net/~herrick/8230920/webrev.03/
>>>>>
>>>>> This webrev looks good.
>>>>>
>>>>> - Alexey
>>>>>
>>>>> On 9/29/2019 10:08 AM, Andy Herrick wrote:
>>>>>> Please review the revised jpackage fix for bug [1] at [3].
>>>>>>
>>>>>> This is a fix for the JDK-8200758-branch branch of the open 
>>>>>> sandbox repository (jpackage).
>>>>>>
>>>>>> The revised fix stores an xml file (.jpackage.xml) in the 
>>>>>> app-image so that jpackage commands using that app-image can 
>>>>>> determine some things previously available only when building the 
>>>>>> app-image, such as the name of the application and the name of 
>>>>>> additional launchers.
>>>>>>
>>>>>> Initially this is only used on windows, but should be later used 
>>>>>> on linux and possibly macOS as well (if additional data needed by 
>>>>>> dmg or pkg building are identified).
>>>>>>
>>>>>> The windows fix also ensures that shortcuts are created for all 
>>>>>> launchers (when shortcut hint option(s) are used).
>>>>>>
>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8230920
>>>>>>
>>>>>> [3] http://cr.openjdk.java.net/~herrick/8230920/webrev.01/
>>>>>>
>>>>>> /Andy
>>>>>>
>>>>>> On 9/24/2019 8:54 AM, Andy Herrick wrote:
>>>>>>> Please review the jpackage fix for bug [1] at [2].
>>>>>>>
>>>>>>> This is a fix for the JDK-8200758-branch branch of the open 
>>>>>>> sandbox repository (jpackage).
>>>>>>>
>>>>>>> This fix replaces the practice we were using on windows to 
>>>>>>> determine the application name, and name of additional launchers 
>>>>>>> (by looking for ".cfg" files in the app dir.
>>>>>>>
>>>>>>> Instead we now add a file ".jpackage.args" to the root of the 
>>>>>>> app-image, and record in that file all the arguments used to 
>>>>>>> create the app-image.  We later read that file to determine the 
>>>>>>> original app name and any additional launcher names.
>>>>>>>
>>>>>>> This change also fixes the shortcut creation on windows to 
>>>>>>> create shortcuts (if so directed) for all launchers.
>>>>>>>
>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8230920
>>>>>>>
>>>>>>> [2] http://cr.openjdk.java.net/~herrick/8230920/webrev.01/
>>>>>>>
>>>>>>> /Andy
>>>>>>>
>>>>>
>>>
>>