Re: Review Request: JDK-8167558 Add new JMOD section for header files and man pages

[Alan Bateman]

On 18/10/2016 17:43, Mandy Chung wrote:

:
I agree we should do the validation early.  I think this should take the 
exclude plugins into consideration and whether there is a way to filter out 
duplicated resource files to allow the image be created.

I’m inclinded to keep the current jlink behavior and file a separate issue to 
follow this up.


Okay with me.

-Alan

Re: Review Request: JDK-8167558 Add new JMOD section for header files and man pages

[Mandy Chung]

> On Oct 18, 2016, at 9:29 AM, Alan Bateman  wrote:
> 
> 
> 
> On 17/10/2016 19:24, Mandy Chung wrote:
>> Updated webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.01/
>> 
>> This updates the exclude-jmod-section plugin not to filter specific modules. 
>>  Also clean up DefaultImageBuilder further and improve the exception message 
>> when it detects duplicated resource entries.
>> 
> This looks good, the only thing is storeFiles where I assume it could fail 
> early when there are duplicates rather than writing them and dealing with 
> FileAlreadyExistsException.

I agree we should do the validation early.  I think this should take the 
exclude plugins into consideration and whether there is a way to filter out 
duplicated resource files to allow the image be created.

I’m inclinded to keep the current jlink behavior and file a separate issue to 
follow this up.

Mandy

Re: Review Request: JDK-8167558 Add new JMOD section for header files and man pages

[Mandy Chung]

Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.01/

This updates the exclude-jmod-section plugin not to filter specific modules.  
Also clean up DefaultImageBuilder further and improve the exception message 
when it detects duplicated resource entries.

Mandy

> On Oct 14, 2016, at 8:28 AM, Mandy Chung  wrote:
> 
> 
>> On Oct 14, 2016, at 5:55 AM, Alan Bateman  wrote:
>> 
>> On 13/10/2016 03:22, Mandy Chung wrote:
>> 
>>> Webrev:
>>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.00/
>>> 
>>> Header files and man pages are currently copied to the image. The header 
>>> files are modularized and in the following directory:
>>>   src/$MODULE/share/native/include
>>> 
>>> The man page for the corresponding tool should also be modularized.
>>> 
>>> This patch proposes to add a JMOD section for include header files and one 
>>> for man pages such that they should be packaged in a JMOD file of the 
>>> module they belong to.
>> The changes to add the new section looks good, as does the options for the 
>> jmod tool.
>> 
>> For jlink then --no-man-pages and --no-header-files look good (as 
>> demonstrated by how they are used in Images.gmk to create the JRE). I'm less 
>> sure about the exclude-jmod-section plugin needing the flexibility to 
>> exclude the resources in the man page or headers section for specific 
>> modules. I would be tempted to leave that out unless it is really needed.
> 
> I think it should be rare to exclude resources in the man page or headers 
> section for specific modules.   On the other hand, —-exclude-files can 
> exclude resources and that should be used in case of excluding any specific 
> resources.  I will leave that out.
> 
> 
> 
> Mandy
> 

Re: Review Request: JDK-8167558 Add new JMOD section for header files and man pages

[Mandy Chung]

> On Oct 14, 2016, at 5:55 AM, Alan Bateman  wrote:
> 
> On 13/10/2016 03:22, Mandy Chung wrote:
> 
>> Webrev:
>>http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.00/
>> 
>> Header files and man pages are currently copied to the image. The header 
>> files are modularized and in the following directory:
>>src/$MODULE/share/native/include
>> 
>> The man page for the corresponding tool should also be modularized.
>> 
>> This patch proposes to add a JMOD section for include header files and one 
>> for man pages such that they should be packaged in a JMOD file of the module 
>> they belong to.
> The changes to add the new section looks good, as does the options for the 
> jmod tool.
> 
> For jlink then --no-man-pages and --no-header-files look good (as 
> demonstrated by how they are used in Images.gmk to create the JRE). I'm less 
> sure about the exclude-jmod-section plugin needing the flexibility to exclude 
> the resources in the man page or headers section for specific modules. I 
> would be tempted to leave that out unless it is really needed.

I think it should be rare to exclude resources in the man page or headers 
section for specific modules.   On the other hand, —-exclude-files can exclude 
resources and that should be used in case of excluding any specific resources.  
I will leave that out.



Mandy

Re: Review Request: JDK-8167558 Add new JMOD section for header files and man pages

[Alan Bateman]

On 13/10/2016 03:22, Mandy Chung wrote:


Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.00/

Header files and man pages are currently copied to the image. The header files 
are modularized and in the following directory:
src/$MODULE/share/native/include

The man page for the corresponding tool should also be modularized.

This patch proposes to add a JMOD section for include header files and one for 
man pages such that they should be packaged in a JMOD file of the module they 
belong to.
The changes to add the new section looks good, as does the options for 
the jmod tool.


For jlink then --no-man-pages and --no-header-files look good (as 
demonstrated by how they are used in Images.gmk to create the JRE). I'm 
less sure about the exclude-jmod-section plugin needing the flexibility 
to exclude the resources in the man page or headers section for specific 
modules. I would be tempted to leave that out unless it is really needed.


Implementation-wise then I don't think I have any comments. As 
alternative for splitting the option value is 
Pattern.compile(",").splitAsStream(mods).


-Alan