RFR: JDK-8219536: Add Option for user defined jlink options

Alexey Semenyuk alexey.semenyuk at oracle.com
Wed Apr 29 21:01:15 UTC 2020


Looks good.

- Alexey

On 4/29/2020 2:36 PM, Andy Herrick wrote:
> I don't think I sent out webrev.5 [6] fixing Alexander's points below. 
> Please Review:
>
> [6] http://cr.openjdk.java.net/~herrick/8219536/webrev.05/index.html
>
> /Andy
>
> On 4/23/2020 7:59 PM, Alexander Matveev wrote:
>> Hi Andy,
>>
>> http://cr.openjdk.java.net/~herrick/8219536/webrev.04/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/Arguments.java.frames.html 
>>
>> 1) Copyright year needs to be updated. Other files also needs 
>> copyright year to be updated.
>> 2) Line 778: Not sure why it was moved to single line. I think it is 
>> better to revert it back.
>>
>> Otherwise looks fine.
>>
>> Thanks,
>> Alexander
>>
>> On 4/23/20 1:48 PM, Andy Herrick wrote:
>>> Please review updated webrev at [5] to address comments below from 
>>> Alexey.
>>>
>>> [5] http://cr.openjdk.java.net/~herrick/8219536/webrev.04
>>>
>>> /Andy
>>>
>>> On 4/23/2020 11:17 AM, Alexey Semenyuk wrote:
>>>> http://cr.openjdk.java.net/~herrick/8219536/webrev.03/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java.sdiff.html:731 
>>>> - 'launcherName' parameter of readRuntimeReleaseFile() function 
>>>> seems to be not used.
>>>>
>>>> http://cr.openjdk.java.net/~herrick/8219536/webrev.03/test/jdk/tools/jpackage/share/jdk/jpackage/tests/JLinkOptionsTest.java.html: 
>>>> copyright year should be 2020.
>>>>
>>>> http://cr.openjdk.java.net/~herrick/8219536/webrev.03/test/jdk/tools/jpackage/share/jdk/jpackage/tests/JLinkOptionsTest.java.html: 
>>>> 128 - There is no point to assert if 'cfgfile' is not null. It 
>>>> should be always non null for application packaging or 
>>>> readLaunherCfgFile.
>>>>
>>>> For testing if string contains some values, I'd suggest to use 
>>>> TKit.assertTextStream():
>>>> ---
>>>> List<String> mods = List.of(release.get(1));
>>>> if (required != null) {
>>>>     for (String s : required) {
>>>> TKit.assertTextStream(s).label("mods").apply(mods.stream());
>>>>     }
>>>> }
>>>>
>>>> if (prohibited != null) {
>>>>     for (String s : prohibited) {
>>>> TKit.assertTextStream(s).label("mods").negate().apply(mods.stream());
>>>>     }
>>>> }
>>>> ---
>>>> Will save you from maintaining explicit log messages.
>>>>
>>>> - Alexey
>>>>
>>>> On 4/23/2020 10:08 AM, Andy Herrick wrote:
>>>>> Please review webrev at [1] to address issue [2].
>>>>>
>>>>> This is the new feature to add the jpackage option --jlink-options 
>>>>> as specified in CSR at [3]
>>>>>
>>>>> /Andy
>>>>>
>>>>>
>>>>> [1] http://cr.openjdk.java.net/~herrick/8219536/webrev.03
>>>>>
>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8219536
>>>>>
>>>>> [3] https://bugs.openjdk.java.net/browse/JDK-8243272
>>>>>
>>>>
>>



More information about the core-libs-dev mailing list