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
