282 throw new RuntimeException(module + " does not have
main class: " + mainClassName);
- Throwing IllegalArgumentException would probably be better.
101 err.launcher.value.format:launcher value should be of form
<command>=<module>: {0}
- should this message include optional main class; something like
<command>=<module>[/<main-class>]
This validation is done after the image is created. Can you file a JBS issue
to separate the launcher file creation and the validation from image creation?
That can be improved in the future.
Otherwise, looks okay. No need to send a new webrev.
Mandy
P.S. There is some issue to access cr.openjdk.java.net. I got a copy of
webrev.02 from Sundar to review.
> On Dec 18, 2016, at 7:29 AM, Sundararajan Athijegannathan
> <sundararajan.athijegannat...@oracle.com> wrote:
>
> Updated it: http://cr.openjdk.java.net/~sundar/8170289/webrev.02
>
> Thanks,
> -Sundar
>
> On 17/12/16, 12:33 AM, Mandy Chung wrote:
>>> On Dec 16, 2016, at 8:36 AM, Sundararajan
>>> Athijegannathan<sundararajan.athijegannat...@oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> Please review http://cr.openjdk.java.net/~sundar/8170289/webrev.01/ for
>>> https://bugs.openjdk.java.net/browse/JDK-8170289
>>
>> 273 Optional<String> mainClass =
>> ModuleDescriptor.read(stream).mainClass();
>> 274 if (mainClass.isPresent()) {
>> 275 mainClassName = mainClass.get();
>> 276 }
>>
>> This should set mainClassName only if the main class is not specified
>> in the -—launcher option. One may want to create launchers for
>> multiple entry points.
>>
>> I think it should validate if the main class is present in the image.
>> If not found, it should output an error.
>>
>> Something to be considered in a future release - the existing implementation
>> creates the launcher scripts as a special case in DefaultImageBuilder.
>> It seems cleaner to keep DefaultImageBuilder just for the image creation,
>> i.e. simply write out entries of the ResourcePool to the image.
>> The launchers could be added to the ResourcePool entries to the
>> corresponding module by one builtin plugin implementation.
>>
>> For this issue, keeping the change to minimal is good.
>>
>> Mandy
>>
>>
