Fixed. Updated webrev for the record:
http://cr.openjdk.java.net/~sundar/8170289/webrev.03
I'm pushing this as you suggested..
Thanks,
-Sundar
On 19/12/16, 6:55 AM, Mandy Chung wrote:
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
