Review Request JDK-8160286: jmod hash is creating unlinkable modules.

[Mandy Chung]

Not really need to be. I can take it out.
Mandy

> On Jan 13, 2017, at 1:43 AM, Andrej Golovnin <andrej.golovnin at gmail.com> wrote:
> 
> Hi Mandy,
> 
> src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java
> 
> 806         final Optional<String> moduleName;  // a specific module
> to record hashes
> 
> I thought that using Optional in a class field is considered a misuse
> of the API. The field hashesBuilder can also be null. But you don't
> wrap it into an Optional. So why it is needed to wrap moduleName into
> Optional?
>  
> Best regards,
> Andrej Golovnin
> 
> On Fri, Jan 13, 2017 at 6:13 AM, Mandy Chung <mandy.chung at oracle.com> wrote:
>> Updated webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8160286/webrev.01/
>> 
>> I did a further clean up and ModuleHashesBuilder now has one constructor.
>> Also updated the comment to avoid using the term “base”.  Due to the
>> refactoring, I moved the location of the tmp jmod file to be in the
>> tmp directory rather than the same directory of the jmod file;
>> otherwise, it would confuse the ModuleFinder with the tmp file.
>> 
>> I added a few more test cases that remind me that a packaged user
>> module can’t tie with a specific JDK build since a module requiring
>> a JDK module that would be a leaf node of the subgraph rather than
>> that module.  The `jar` tool locating JMOD files on the module path
>> allows to tie a modular JAR with a module requiring it but packaged
>> as JMOD file (e.g. with native library).
>> 
>> Mandy
>> 
>>> On Jan 12, 2017, at 7:52 AM, Mandy Chung <mandy.chung at oracle.com> wrote:
>>> 
>>> 
>>>> On Jan 12, 2017, at 7:08 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>>> 
>>>> On 11/01/2017 23:47, Mandy Chung wrote:
>>>> 
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8160286/webrev.00/
>>>>> 
>>>>> jmod and jar -—hash-modules option to specify a pattern of modules
>>>>> to be hashed in the module M being created.  It records the modules
>>>>> that depend on M directly and indirectly.
>>>>> 
>>>> This looks quite good.  At some point then we'll need to move the tool support out of jdk.internal.module but is something for another day.
>>>> 
>>>> For ModuleHashesBuilder then it might be useful to put a comment on the constructors as it's not immediately obvious why both are needed. Also I wonder if we should use a term other than "base" for the modules that don't have references to other modules in the sub-graph (they are sort of leaf modules in the sub-graph).  A typo at L96 "in topological orders" -> "order”.
>>> 
>>> I’ll take a pass and update the comments.  I can see “base” can be confusing.
>>> 
>>>> One of the changes in this patch is that the `jar` tool will locate JMOD files on the module path. I assume this is to provide flexibility to those creating a modular JAR that want to tie it to a specific JDK build. I guess it's okay but I suspect will not be widely used.
>>> 
>>> Right and another case is packaged module with a native library (security providers). It will not be widey used.
>>> 
>>> Mandy
>>