RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Kevin Rushforth]

Hi Alexander,

I'll file cleanup issues (or add to existing bugs) for the rest.

Thanks.

-- Kevin



On 5/2/2019 6:33 PM, Alexander Matveev wrote:
> Hi Kevin,
>
> See below.
>
> On 5/2/2019 5:38 PM, Kevin Rushforth wrote:
>> Here are a few follow-on comments. As with my earlier comments, none 
>> of these need to be addressed prior to integration.
>>
>>
>> 1. I found a few more classes that do I/O and could benefit from 
>> using try-with-resources:
>>    IOUtils, LinuxAppImageBuilder, LinuxDebBundler, LinuxRpmBundler, 
>> MacAppImageBuilder, etc.
>>
>>
>> JLinkBundlerHelper.java:
>>
>> 2. JRE_MODULES_FILENAME and SERVER_JRE_MODULES_FILENAME are unused 
>> (obsolete) and should be removed.
>>
>>
>> VersionExtractor.java:
>>
>> 3. The isLessThan method only looks at MAJOR.MINOR so might not be 
>> flexible enough for some applications
> Currently it is being used for InnoSetup and Wixs and we only need to 
> compare major.minor, so should be fine for now.
>>
>>
>> LinuxAppBundler.java:
>>
>> 3. Several places where non-public (package-scope) API is exported 
>> publicly; these should all be package-scope itself or else 
>> BundlerParamInfo should be public
>>
>>
>>
>> LinuxAppImageBuilder.java:
>>
>> 3. createUtf8File is unused (I went looking because I was curious how 
>> and why we would use such a method). I see similarly-unused methods 
>> of the same name in the Mac and Windows AppImageBuilder classes.
>>
>>
>> MacAppImageBuilder.java:
>>
>> 4. Line 818: is the following still needed?
>>
>>                         || p.toString().contains(
>> "/Contents/MacOS/JavaAppletPlugin")
>>
>>
>> WindowsDefender.java + WindowsRegistry.java:
>>
>> 5. Is the check and warning for Windows Defender really needed? Have 
>> we seen problems as a result of it running while jpackage is building 
>> an app installer?
> Yes, it is still needed. Resource update fails sometimes and I was 
> able to reproduce this issue. However, not very often.
>
> Thanks,
> Alexander