Hi Mandy,


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

Best regards,
Andrej Golovnin

On Fri, Jan 13, 2017 at 6:13 AM, Mandy Chung <mandy.ch...@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.ch...@oracle.com> wrote:
>>> On Jan 12, 2017, at 7:08 AM, Alan Bateman <alan.bate...@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

Reply via email to