Re: RFR 8170289: Re-examine entry point support in jlink
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=: {0}
- should this message include optional main class; something
like=[/]
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 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 wrote:
Hi,
Please review http://cr.openjdk.java.net/~sundar/8170289/webrev.01/ for
https://bugs.openjdk.java.net/browse/JDK-8170289
273 Optional 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
Review Request: JDK-8168836 Minor clean up on warning/error messages on --add-exports and --add-reads
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8168836/webrev.00/ This patch improves the warning/error message to include the option name, emit a warning if unknown module is specified with —-patch-module be consistent with the options. Mandy
Re: RFR 8170289: Re-examine entry point support in jlink
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
=: {0}
- should this message include optional main class; something like
=[/]
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
> 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 wrote:
>>>
>>> Hi,
>>>
>>> Please review http://cr.openjdk.java.net/~sundar/8170289/webrev.01/ for
>>> https://bugs.openjdk.java.net/browse/JDK-8170289
>>
>> 273 Optional 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
>>
>>
Re: RFR 8170289: Re-examine entry point support in jlink
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 wrote:
Hi,
Please review http://cr.openjdk.java.net/~sundar/8170289/webrev.01/ for
https://bugs.openjdk.java.net/browse/JDK-8170289
273 Optional 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
