RFR: JDK-8230920 : jpackage problems when -input dir contains any files with "cfg" extension.
Alexander Matveev
alexander.matveev at oracle.com
Mon Sep 30 20:31:11 UTC 2019
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
>>>>>>>
>>>>>
>>>
>>
More information about the core-libs-dev
mailing list